gpiolib: rename gpiod_set_array to gpiod_set_array_value

PATCH RESEND In review Rojhalat Ibrahim 2 participants 5 messages Jun 1, 2015 – Jun 8, 2015

Subsystems (4)

Network drivers GPIO TTY layer and serial drivers Ethernet PHY

lore

Share & tools

lore

Rojhalat Ibrahim

author

2015-06-01 11:41 UTC #0 lore raw

PATCH RESEND 1/1 gpiolib: rename gpiod_set_array to gpiod_set_array_value

There have been concerns that the function names gpiod_set_array() and gpiod_get_array() might be confusing to users. One might expect gpiod_get_array() to return array values, while it is actually the array counterpart of gpiod_get(). To be consistent with the single descriptor API gpiod_set_array() is renamed to gpiod_set_array_value().

Signed-off-by Rojhalat Ibrahim <[email protected]>

Acked-by Alexandre Courbot <[email protected]>

---
 drivers/gpio/gpiolib.c                 |   54 +++++++++++++++++----------------
 drivers/net/phy/mdio-mux-gpio.c        |    3 +
 drivers/tty/serial/serial_mctrl_gpio.c |    2 -
 include/linux/gpio/consumer.h          |   37 +++++++++++-----------
 4 files changed, 51 insertions(+), 45 deletions(-)

drivers/gpio/gpiolib.c +29 ~~-25~~

index 2ce3df3..37fe323 100644
@@ -1309,10 +1309,10 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
 	}
 }
 
-static void gpiod_set_array_priv(bool raw, bool can_sleep,
-				 unsigned int array_size,
-				 struct gpio_desc **desc_array,
-				 int *value_array)
+static void gpiod_set_array_value_priv(bool raw, bool can_sleep,
+				       unsigned int array_size,
+				       struct gpio_desc **desc_array,
+				       int *value_array)
 {
 	int i = 0;

@@ -1405,7 +1405,7 @@ void gpiod_set_value(struct gpio_desc *desc, int value)
 EXPORT_SYMBOL_GPL(gpiod_set_value);
 
 /**
- * gpiod_set_raw_array() - assign values to an array of GPIOs
+ * gpiod_set_raw_array_value() - assign values to an array of GPIOs
  * @array_size: number of elements in the descriptor / value arrays
  * @desc_array: array of GPIO descriptors whose values will be assigned
  * @value_array: array of values to assign

@@ -1416,17 +1416,18 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
  */
-void gpiod_set_raw_array(unsigned int array_size,
+void gpiod_set_raw_array_value(unsigned int array_size,
 			 struct gpio_desc **desc_array, int *value_array)
 {
 	if (!desc_array)
 		return;
-	gpiod_set_array_priv(true, false, array_size, desc_array, value_array);
+	gpiod_set_array_value_priv(true, false, array_size, desc_array,
+				   value_array);
 }
-EXPORT_SYMBOL_GPL(gpiod_set_raw_array);
+EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
 
 /**
- * gpiod_set_array() - assign values to an array of GPIOs
+ * gpiod_set_array_value() - assign values to an array of GPIOs
  * @array_size: number of elements in the descriptor / value arrays
  * @desc_array: array of GPIO descriptors whose values will be assigned
  * @value_array: array of values to assign

@@ -1437,14 +1438,15 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array);
  * This function should be called from contexts where we cannot sleep, and will
  * complain if the GPIO chip functions potentially sleep.
  */
-void gpiod_set_array(unsigned int array_size,
-		     struct gpio_desc **desc_array, int *value_array)
+void gpiod_set_array_value(unsigned int array_size,
+			   struct gpio_desc **desc_array, int *value_array)
 {
 	if (!desc_array)
 		return;
-	gpiod_set_array_priv(false, false, array_size, desc_array, value_array);
+	gpiod_set_array_value_priv(false, false, array_size, desc_array,
+				   value_array);
 }
-EXPORT_SYMBOL_GPL(gpiod_set_array);
+EXPORT_SYMBOL_GPL(gpiod_set_array_value);
 
 /**
  * gpiod_cansleep() - report whether gpio value access may sleep

@@ -1606,7 +1608,7 @@ void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
 
 /**
- * gpiod_set_raw_array_cansleep() - assign values to an array of GPIOs
+ * gpiod_set_raw_array_value_cansleep() - assign values to an array of GPIOs
  * @array_size: number of elements in the descriptor / value arrays
  * @desc_array: array of GPIO descriptors whose values will be assigned
  * @value_array: array of values to assign

@@ -1616,19 +1618,20 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
  *
  * This function is to be called from contexts that can sleep.
  */
-void gpiod_set_raw_array_cansleep(unsigned int array_size,
-				  struct gpio_desc **desc_array,
-				  int *value_array)
+void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
+					struct gpio_desc **desc_array,
+					int *value_array)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return;
-	gpiod_set_array_priv(true, true, array_size, desc_array, value_array);
+	gpiod_set_array_value_priv(true, true, array_size, desc_array,
+				   value_array);
 }
-EXPORT_SYMBOL_GPL(gpiod_set_raw_array_cansleep);
+EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
 
 /**
- * gpiod_set_array_cansleep() - assign values to an array of GPIOs
+ * gpiod_set_array_value_cansleep() - assign values to an array of GPIOs
  * @array_size: number of elements in the descriptor / value arrays
  * @desc_array: array of GPIO descriptors whose values will be assigned
  * @value_array: array of values to assign

@@ -1638,16 +1641,17 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_cansleep);
  *
  * This function is to be called from contexts that can sleep.
  */
-void gpiod_set_array_cansleep(unsigned int array_size,
-			      struct gpio_desc **desc_array,
-			      int *value_array)
+void gpiod_set_array_value_cansleep(unsigned int array_size,
+				    struct gpio_desc **desc_array,
+				    int *value_array)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return;
-	gpiod_set_array_priv(false, true, array_size, desc_array, value_array);
+	gpiod_set_array_value_priv(false, true, array_size, desc_array,
+				   value_array);
 }
-EXPORT_SYMBOL_GPL(gpiod_set_array_cansleep);
+EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
 
 /**
  * gpiod_add_lookup_table() - register GPIO device consumers

drivers/net/phy/mdio-mux-gpio.c +2 -1

index 66edd99..7ddb1ab 100644
@@ -35,7 +35,8 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
 	for (n = 0; n < s->gpios->ndescs; n++)
 		values[n] = (desired_child >> n) & 1;
 
-	gpiod_set_array_cansleep(s->gpios->ndescs, s->gpios->desc, values);
+	gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
+				       values);
 
 	return 0;
 }

drivers/tty/serial/serial_mctrl_gpio.c +1 -1

index 0ec756c..d7b846d 100644
@@ -55,7 +55,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
 			value_array[count] = !!(mctrl & mctrl_gpios_desc[i].mctrl);
 			count++;
 		}
-	gpiod_set_array(count, desc_array, value_array);
+	gpiod_set_array_value(count, desc_array, value_array);
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_set);

include/linux/gpio/consumer.h +19 ~~-18~~

index 09a7fb0..fd09816 100644
@@ -100,24 +100,25 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
 /* Value get/set from non-sleeping context */
 int gpiod_get_value(const struct gpio_desc *desc);
 void gpiod_set_value(struct gpio_desc *desc, int value);
-void gpiod_set_array(unsigned int array_size,
-		     struct gpio_desc **desc_array, int *value_array);
+void gpiod_set_array_value(unsigned int array_size,
+			   struct gpio_desc **desc_array, int *value_array);
 int gpiod_get_raw_value(const struct gpio_desc *desc);
 void gpiod_set_raw_value(struct gpio_desc *desc, int value);
-void gpiod_set_raw_array(unsigned int array_size,
-			 struct gpio_desc **desc_array, int *value_array);
+void gpiod_set_raw_array_value(unsigned int array_size,
+			       struct gpio_desc **desc_array,
+			       int *value_array);
 
 /* Value get/set from sleeping context */
 int gpiod_get_value_cansleep(const struct gpio_desc *desc);
 void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
-void gpiod_set_array_cansleep(unsigned int array_size,
-			      struct gpio_desc **desc_array,
-			      int *value_array);
+void gpiod_set_array_value_cansleep(unsigned int array_size,
+				    struct gpio_desc **desc_array,
+				    int *value_array);
 int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
 void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
-void gpiod_set_raw_array_cansleep(unsigned int array_size,
-				  struct gpio_desc **desc_array,
-				  int *value_array);
+void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
+					struct gpio_desc **desc_array,
+					int *value_array);
 
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);

@@ -304,9 +305,9 @@ static inline void gpiod_set_value(struct gpio_desc *desc, int value)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
-static inline void gpiod_set_array(unsigned int array_size,
-				   struct gpio_desc **desc_array,
-				   int *value_array)
+static inline void gpiod_set_array_value(unsigned int array_size,
+					 struct gpio_desc **desc_array,
+					 int *value_array)
 {
 	/* GPIO can never have been requested */
 	WARN_ON(1);

@@ -322,9 +323,9 @@ static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
-static inline void gpiod_set_raw_array(unsigned int array_size,
-				       struct gpio_desc **desc_array,
-				       int *value_array)
+static inline void gpiod_set_raw_array_value(unsigned int array_size,
+					     struct gpio_desc **desc_array,
+					     int *value_array)
 {
 	/* GPIO can never have been requested */
 	WARN_ON(1);

@@ -341,7 +342,7 @@ static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
-static inline void gpiod_set_array_cansleep(unsigned int array_size,
+static inline void gpiod_set_array_value_cansleep(unsigned int array_size,
 					    struct gpio_desc **desc_array,
 					    int *value_array)
 {

@@ -360,7 +361,7 @@ static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
 	/* GPIO can never have been requested */
 	WARN_ON(1);
 }
-static inline void gpiod_set_raw_array_cansleep(unsigned int array_size,
+static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 						struct gpio_desc **desc_array,
 						int *value_array)
 {

--
2.3.6

Alexandre Courbot

2015-06-08 05:37 UTC #1 lore raw

Quoted 5 lines · Rojhalat Ibrahim

There have been concerns that the function names gpiod_set_array() and gpiod_get_array() might be confusing to users. One might expect gpiod_get_array() to return array values, while it is actually the array counterpart of gpiod_get(). To be consistent with the single descriptor API gpiod_set_array() is renamed to gpiod_set_array_value().

Linus, if you are ok with this change I suggest we merge it early in order to avoid conflicts as more people start using these APIs! :)

Alexandre Courbot

2015-06-08 06:47 UTC #2 lore raw

Quoted 9 lines · Alexandre Courbot

Quoted 5 lines · Rojhalat Ibrahim
There have been concerns that the function names gpiod_set_array() and gpiod_get_array() might be confusing to users. One might expect gpiod_get_array() to return array values, while it is actually the array counterpart of gpiod_get(). To be consistent with the single descriptor API gpiod_set_array() is renamed to gpiod_set_array_value().
Linus, if you are ok with this change I suggest we merge it early in order to avoid conflicts as more people start using these APIs! :)

Actually, Rojhalat: could you define temporary macros to ease the transition? Something like

#define gpiod_set_raw_array gpiod_set_raw_array_value

We would then take them out around 4.2, once all consumers are converted.

Rojhalat Ibrahim

author

2015-06-08 07:17 UTC #3 lore raw

Quoted 11 lines · On Monday 08 June 2015 15:46:44 Alexandre Courbot wrote:

Quoted 9 lines · Alexandre Courbot
Quoted 5 lines · Rojhalat Ibrahim
There have been concerns that the function names gpiod_set_array() and gpiod_get_array() might be confusing to users. One might expect gpiod_get_array() to return array values, while it is actually the array counterpart of gpiod_get(). To be consistent with the single descriptor API gpiod_set_array() is renamed to gpiod_set_array_value().
Linus, if you are ok with this change I suggest we merge it early in order to avoid conflicts as more people start using these APIs! :)

Quoted 7 lines

Actually, Rojhalat: could you define temporary macros to ease the transition? Something like
#define gpiod_set_raw_array gpiod_set_raw_array_value
We would then take them out around 4.2, once all consumers are converted.

Hi Alexandre,

Linus already applied the patch. (He replied to my original RFC mail.)

I am not sure those temporary macros are justified. Do you really think there are that many out-of-tree consumers? And if so, how would we know when all of them have converted to the new interface? After all with those macros around, they might not even notice they are using a deprecated interface.

Rojhalat

Alexandre Courbot

2015-06-08 07:20 UTC #4 lore raw

Quoted 15 lines · Rojhalat Ibrahim

Quoted 14 lines · On Monday 08 June 2015 15:46:44 Alexandre Courbot wrote:
Quoted 9 lines · Alexandre Courbot
Quoted 5 lines · Rojhalat Ibrahim
There have been concerns that the function names gpiod_set_array() and gpiod_get_array() might be confusing to users. One might expect gpiod_get_array() to return array values, while it is actually the array counterpart of gpiod_get(). To be consistent with the single descriptor API gpiod_set_array() is renamed to gpiod_set_array_value().
Linus, if you are ok with this change I suggest we merge it early in order to avoid conflicts as more people start using these APIs! :)
Actually, Rojhalat: could you define temporary macros to ease the transition? Something like

Quoted 8 lines

Quoted 4 lines
#define gpiod_set_raw_array gpiod_set_raw_array_value
We would then take them out around 4.2, once all consumers are converted.
Hi Alexandre,
Linus already applied the patch. (He replied to my original RFC mail.)

Ah, that's perfect then. Sorry for the noise.

Quoted 5 lines

I am not sure those temporary macros are justified. Do you really think there are that many out-of-tree consumers? And if so, how would we know when all of them have converted to the new interface? After all with those macros around, they might not even notice they are using a deprecated interface.

My intention was to avoid in-tree breakage with linux-next (we do not worry about out-of-tree consumers since they can easily update their code. And they are out-of-tree anyway). But if the patch has already been merged and is working, then I have no concern at all.

Thanks for keeping up with this!