[PATCH 0/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

[PATCH 0/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

[Boris Brezillon]

Hello Linus,

I send these 2 patches separately from the Atmel NAND driver rework
since these are self-contained and do not have any external
dependencies.

Feel free to apply them in your tree if you are happy with this
version and once other subsystem maintainers have acked patch 1.

Thanks,

Boris

---
Changes since v1:
- Rename the devm_get_gpiod_from_child() helper into
  devm_fwnode_get_gpiod_from_child()
- Define as a static inline wrapper around
  devm_fwnode_get_gpiod_from_child()

Boris Brezillon (2):
  gpio: Rename devm_get_gpiod_from_child()
  gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

 drivers/gpio/devres.c                     | 14 ++++++++------
 drivers/gpio/gpiolib.c                    |  9 +++++----
 drivers/input/keyboard/gpio_keys.c        |  3 ++-
 drivers/input/keyboard/gpio_keys_polled.c |  5 +++--
 drivers/leds/leds-gpio.c                  |  2 +-
 drivers/video/fbdev/amba-clcd-nomadik.c   |  8 ++++----
 include/linux/gpio/consumer.h             | 23 ++++++++++++++++-------
 7 files changed, 39 insertions(+), 25 deletions(-)

-- 
2.7.4

[PATCH 0/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

[Boris Brezillon]

Hello Linus,

I send these 2 patches separately from the Atmel NAND driver rework
since these are self-contained and do not have any external
dependencies.

Feel free to apply them in your tree if you are happy with this
version and once other subsystem maintainers have acked patch 1.

Thanks,

Boris

---
Changes since v1:
- Rename the devm_get_gpiod_from_child() helper into
  devm_fwnode_get_gpiod_from_child()
- Define as a static inline wrapper around
  devm_fwnode_get_gpiod_from_child()

Boris Brezillon (2):
  gpio: Rename devm_get_gpiod_from_child()
  gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

 drivers/gpio/devres.c                     | 14 ++++++++------
 drivers/gpio/gpiolib.c                    |  9 +++++----
 drivers/input/keyboard/gpio_keys.c        |  3 ++-
 drivers/input/keyboard/gpio_keys_polled.c |  5 +++--
 drivers/leds/leds-gpio.c                  |  2 +-
 drivers/video/fbdev/amba-clcd-nomadik.c   |  8 ++++----
 include/linux/gpio/consumer.h             | 23 ++++++++++++++++-------
 7 files changed, 39 insertions(+), 25 deletions(-)

-- 
2.7.4

[PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

Rename devm_get_gpiod_from_child() into
devm_fwnode_get_gpiod_from_child() to reflect the fact that this
function is operating on a fwnode object.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpio/devres.c                     | 11 ++++++-----
 drivers/input/keyboard/gpio_keys.c        |  3 ++-
 drivers/input/keyboard/gpio_keys_polled.c |  5 +++--
 drivers/leds/leds-gpio.c                  |  2 +-
 drivers/video/fbdev/amba-clcd-nomadik.c   |  8 ++++----
 include/linux/gpio/consumer.h             |  8 ++++----
 6 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index b760cbbb41d8..dfbbd92d21b6 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -123,7 +123,8 @@ struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev,
 EXPORT_SYMBOL(devm_gpiod_get_index);
 
 /**
- * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
+ * devm_fwnode_get_gpiod_from_child - get a GPIO descriptor from a device's
+ *				      child node
  * @dev:	GPIO consumer
  * @con_id:	function within the GPIO consumer
  * @child:	firmware node (child of @dev)
@@ -131,9 +132,9 @@ EXPORT_SYMBOL(devm_gpiod_get_index);
  * GPIO descriptors returned from this function are automatically disposed on
  * driver detach.
  */
-struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
-					    const char *con_id,
-					    struct fwnode_handle *child)
+struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
+						   const char *con_id,
+						   struct fwnode_handle *child)
 {
 	static const char * const suffixes[] = { "gpios", "gpio" };
 	char prop_name[32]; /* 32 is max size of property name */
@@ -168,7 +169,7 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
 
 	return desc;
 }
-EXPORT_SYMBOL(devm_get_gpiod_from_child);
+EXPORT_SYMBOL(devm_fwnode_get_gpiod_from_child);
 
 /**
  * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 582462d0af75..ef6813c1f759 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -481,7 +481,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	spin_lock_init(&bdata->lock);
 
 	if (child) {
-		bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child);
+		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
+								child);
 		if (IS_ERR(bdata->gpiod)) {
 			error = PTR_ERR(bdata->gpiod);
 			if (error = -ENOENT) {
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index bed4f2086158..c0c9f2133ecd 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -303,8 +303,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 				return -EINVAL;
 			}
 
-			bdata->gpiod = devm_get_gpiod_from_child(dev, NULL,
-								 child);
+			bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev,
+									NULL,
+									child);
 			if (IS_ERR(bdata->gpiod)) {
 				error = PTR_ERR(bdata->gpiod);
 				if (error != -EPROBE_DEFER)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index d400dcaf4d29..c0ef838fc993 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -174,7 +174,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		const char *state = NULL;
 		struct device_node *np = to_of_node(child);
 
-		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
+		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child);
 		if (IS_ERR(led.gpiod)) {
 			fwnode_handle_put(child);
 			return ERR_CAST(led.gpiod);
diff --git a/drivers/video/fbdev/amba-clcd-nomadik.c b/drivers/video/fbdev/amba-clcd-nomadik.c
index 0c06fcaaa6e8..a4c58c650f8c 100644
--- a/drivers/video/fbdev/amba-clcd-nomadik.c
+++ b/drivers/video/fbdev/amba-clcd-nomadik.c
@@ -184,7 +184,7 @@ static void tpg110_init(struct device *dev, struct device_node *np,
 {
 	dev_info(dev, "TPG110 display init\n");
 
-	grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode);
+	grestb = devm_fwnode_get_gpiod_from_child(dev, "grestb", &np->fwnode);
 	if (IS_ERR(grestb)) {
 		dev_err(dev, "no GRESTB GPIO\n");
 		return;
@@ -192,19 +192,19 @@ static void tpg110_init(struct device *dev, struct device_node *np,
 	/* This asserts the GRESTB signal, putting the display into reset */
 	gpiod_direction_output(grestb, 1);
 
-	scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode);
+	scen = devm_fwnode_get_gpiod_from_child(dev, "scen", &np->fwnode);
 	if (IS_ERR(scen)) {
 		dev_err(dev, "no SCEN GPIO\n");
 		return;
 	}
 	gpiod_direction_output(scen, 0);
-	scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode);
+	scl = devm_fwnode_get_gpiod_from_child(dev, "scl", &np->fwnode);
 	if (IS_ERR(scl)) {
 		dev_err(dev, "no SCL GPIO\n");
 		return;
 	}
 	gpiod_direction_output(scl, 0);
-	sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode);
+	sda = devm_fwnode_get_gpiod_from_child(dev, "sda", &np->fwnode);
 	if (IS_ERR(sda)) {
 		dev_err(dev, "no SDA GPIO\n");
 		return;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index fb0fde686cb1..2ce4bc164735 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -136,9 +136,9 @@ struct fwnode_handle;
 
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 					 const char *propname);
-struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
-					    const char *con_id,
-					    struct fwnode_handle *child);
+struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
+						const char *con_id,
+						struct fwnode_handle *child);
 #else /* CONFIG_GPIOLIB */
 
 static inline int gpiod_count(struct device *dev, const char *con_id)
@@ -417,7 +417,7 @@ static inline struct gpio_desc *fwnode_get_named_gpiod(
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct gpio_desc *devm_get_gpiod_from_child(
+static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(
 	struct device *dev, const char *con_id, struct fwnode_handle *child)
 {
 	return ERR_PTR(-ENOSYS);
-- 
2.7.4

[PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

Rename devm_get_gpiod_from_child() into
devm_fwnode_get_gpiod_from_child() to reflect the fact that this
function is operating on a fwnode object.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpio/devres.c                     | 11 ++++++-----
 drivers/input/keyboard/gpio_keys.c        |  3 ++-
 drivers/input/keyboard/gpio_keys_polled.c |  5 +++--
 drivers/leds/leds-gpio.c                  |  2 +-
 drivers/video/fbdev/amba-clcd-nomadik.c   |  8 ++++----
 include/linux/gpio/consumer.h             |  8 ++++----
 6 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index b760cbbb41d8..dfbbd92d21b6 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -123,7 +123,8 @@ struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev,
 EXPORT_SYMBOL(devm_gpiod_get_index);
 
 /**
- * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
+ * devm_fwnode_get_gpiod_from_child - get a GPIO descriptor from a device's
+ *				      child node
  * @dev:	GPIO consumer
  * @con_id:	function within the GPIO consumer
  * @child:	firmware node (child of @dev)
@@ -131,9 +132,9 @@ EXPORT_SYMBOL(devm_gpiod_get_index);
  * GPIO descriptors returned from this function are automatically disposed on
  * driver detach.
  */
-struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
-					    const char *con_id,
-					    struct fwnode_handle *child)
+struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
+						   const char *con_id,
+						   struct fwnode_handle *child)
 {
 	static const char * const suffixes[] = { "gpios", "gpio" };
 	char prop_name[32]; /* 32 is max size of property name */
@@ -168,7 +169,7 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
 
 	return desc;
 }
-EXPORT_SYMBOL(devm_get_gpiod_from_child);
+EXPORT_SYMBOL(devm_fwnode_get_gpiod_from_child);
 
 /**
  * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 582462d0af75..ef6813c1f759 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -481,7 +481,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	spin_lock_init(&bdata->lock);
 
 	if (child) {
-		bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child);
+		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
+								child);
 		if (IS_ERR(bdata->gpiod)) {
 			error = PTR_ERR(bdata->gpiod);
 			if (error == -ENOENT) {
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index bed4f2086158..c0c9f2133ecd 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -303,8 +303,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 				return -EINVAL;
 			}
 
-			bdata->gpiod = devm_get_gpiod_from_child(dev, NULL,
-								 child);
+			bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev,
+									NULL,
+									child);
 			if (IS_ERR(bdata->gpiod)) {
 				error = PTR_ERR(bdata->gpiod);
 				if (error != -EPROBE_DEFER)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index d400dcaf4d29..c0ef838fc993 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -174,7 +174,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 		const char *state = NULL;
 		struct device_node *np = to_of_node(child);
 
-		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
+		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child);
 		if (IS_ERR(led.gpiod)) {
 			fwnode_handle_put(child);
 			return ERR_CAST(led.gpiod);
diff --git a/drivers/video/fbdev/amba-clcd-nomadik.c b/drivers/video/fbdev/amba-clcd-nomadik.c
index 0c06fcaaa6e8..a4c58c650f8c 100644
--- a/drivers/video/fbdev/amba-clcd-nomadik.c
+++ b/drivers/video/fbdev/amba-clcd-nomadik.c
@@ -184,7 +184,7 @@ static void tpg110_init(struct device *dev, struct device_node *np,
 {
 	dev_info(dev, "TPG110 display init\n");
 
-	grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode);
+	grestb = devm_fwnode_get_gpiod_from_child(dev, "grestb", &np->fwnode);
 	if (IS_ERR(grestb)) {
 		dev_err(dev, "no GRESTB GPIO\n");
 		return;
@@ -192,19 +192,19 @@ static void tpg110_init(struct device *dev, struct device_node *np,
 	/* This asserts the GRESTB signal, putting the display into reset */
 	gpiod_direction_output(grestb, 1);
 
-	scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode);
+	scen = devm_fwnode_get_gpiod_from_child(dev, "scen", &np->fwnode);
 	if (IS_ERR(scen)) {
 		dev_err(dev, "no SCEN GPIO\n");
 		return;
 	}
 	gpiod_direction_output(scen, 0);
-	scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode);
+	scl = devm_fwnode_get_gpiod_from_child(dev, "scl", &np->fwnode);
 	if (IS_ERR(scl)) {
 		dev_err(dev, "no SCL GPIO\n");
 		return;
 	}
 	gpiod_direction_output(scl, 0);
-	sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode);
+	sda = devm_fwnode_get_gpiod_from_child(dev, "sda", &np->fwnode);
 	if (IS_ERR(sda)) {
 		dev_err(dev, "no SDA GPIO\n");
 		return;
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index fb0fde686cb1..2ce4bc164735 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -136,9 +136,9 @@ struct fwnode_handle;
 
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 					 const char *propname);
-struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
-					    const char *con_id,
-					    struct fwnode_handle *child);
+struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
+						const char *con_id,
+						struct fwnode_handle *child);
 #else /* CONFIG_GPIOLIB */
 
 static inline int gpiod_count(struct device *dev, const char *con_id)
@@ -417,7 +417,7 @@ static inline struct gpio_desc *fwnode_get_named_gpiod(
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct gpio_desc *devm_get_gpiod_from_child(
+static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(
 	struct device *dev, const char *con_id, struct fwnode_handle *child)
 {
 	return ERR_PTR(-ENOSYS);
-- 
2.7.4

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Jacek Anaszewski]

On 01/30/2017 04:41 PM, Boris Brezillon wrote:
> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/gpio/devres.c                     | 11 ++++++-----
>  drivers/input/keyboard/gpio_keys.c        |  3 ++-
>  drivers/input/keyboard/gpio_keys_polled.c |  5 +++--
>  drivers/leds/leds-gpio.c                  |  2 +-
>  drivers/video/fbdev/amba-clcd-nomadik.c   |  8 ++++----
>  include/linux/gpio/consumer.h             |  8 ++++----
>  6 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
> index b760cbbb41d8..dfbbd92d21b6 100644
> --- a/drivers/gpio/devres.c
> +++ b/drivers/gpio/devres.c
> @@ -123,7 +123,8 @@ struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev,
>  EXPORT_SYMBOL(devm_gpiod_get_index);
>  
>  /**
> - * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
> + * devm_fwnode_get_gpiod_from_child - get a GPIO descriptor from a device's
> + *				      child node
>   * @dev:	GPIO consumer
>   * @con_id:	function within the GPIO consumer
>   * @child:	firmware node (child of @dev)
> @@ -131,9 +132,9 @@ EXPORT_SYMBOL(devm_gpiod_get_index);
>   * GPIO descriptors returned from this function are automatically disposed on
>   * driver detach.
>   */
> -struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
> -					    const char *con_id,
> -					    struct fwnode_handle *child)
> +struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
> +						   const char *con_id,
> +						   struct fwnode_handle *child)
>  {
>  	static const char * const suffixes[] = { "gpios", "gpio" };
>  	char prop_name[32]; /* 32 is max size of property name */
> @@ -168,7 +169,7 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
>  
>  	return desc;
>  }
> -EXPORT_SYMBOL(devm_get_gpiod_from_child);
> +EXPORT_SYMBOL(devm_fwnode_get_gpiod_from_child);
>  
>  /**
>   * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 582462d0af75..ef6813c1f759 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -481,7 +481,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  	spin_lock_init(&bdata->lock);
>  
>  	if (child) {
> -		bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child);
> +		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
> +								child);
>  		if (IS_ERR(bdata->gpiod)) {
>  			error = PTR_ERR(bdata->gpiod);
>  			if (error = -ENOENT) {
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index bed4f2086158..c0c9f2133ecd 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -303,8 +303,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
>  				return -EINVAL;
>  			}
>  
> -			bdata->gpiod = devm_get_gpiod_from_child(dev, NULL,
> -								 child);
> +			bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev,
> +									NULL,
> +									child);
>  			if (IS_ERR(bdata->gpiod)) {
>  				error = PTR_ERR(bdata->gpiod);
>  				if (error != -EPROBE_DEFER)
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index d400dcaf4d29..c0ef838fc993 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -174,7 +174,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>  		const char *state = NULL;
>  		struct device_node *np = to_of_node(child);
>  
> -		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
> +		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child);
>  		if (IS_ERR(led.gpiod)) {
>  			fwnode_handle_put(child);
>  			return ERR_CAST(led.gpiod);

For leds-gpio:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

> diff --git a/drivers/video/fbdev/amba-clcd-nomadik.c b/drivers/video/fbdev/amba-clcd-nomadik.c
> index 0c06fcaaa6e8..a4c58c650f8c 100644
> --- a/drivers/video/fbdev/amba-clcd-nomadik.c
> +++ b/drivers/video/fbdev/amba-clcd-nomadik.c
> @@ -184,7 +184,7 @@ static void tpg110_init(struct device *dev, struct device_node *np,
>  {
>  	dev_info(dev, "TPG110 display init\n");
>  
> -	grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode);
> +	grestb = devm_fwnode_get_gpiod_from_child(dev, "grestb", &np->fwnode);
>  	if (IS_ERR(grestb)) {
>  		dev_err(dev, "no GRESTB GPIO\n");
>  		return;
> @@ -192,19 +192,19 @@ static void tpg110_init(struct device *dev, struct device_node *np,
>  	/* This asserts the GRESTB signal, putting the display into reset */
>  	gpiod_direction_output(grestb, 1);
>  
> -	scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode);
> +	scen = devm_fwnode_get_gpiod_from_child(dev, "scen", &np->fwnode);
>  	if (IS_ERR(scen)) {
>  		dev_err(dev, "no SCEN GPIO\n");
>  		return;
>  	}
>  	gpiod_direction_output(scen, 0);
> -	scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode);
> +	scl = devm_fwnode_get_gpiod_from_child(dev, "scl", &np->fwnode);
>  	if (IS_ERR(scl)) {
>  		dev_err(dev, "no SCL GPIO\n");
>  		return;
>  	}
>  	gpiod_direction_output(scl, 0);
> -	sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode);
> +	sda = devm_fwnode_get_gpiod_from_child(dev, "sda", &np->fwnode);
>  	if (IS_ERR(sda)) {
>  		dev_err(dev, "no SDA GPIO\n");
>  		return;
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index fb0fde686cb1..2ce4bc164735 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -136,9 +136,9 @@ struct fwnode_handle;
>  
>  struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
>  					 const char *propname);
> -struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
> -					    const char *con_id,
> -					    struct fwnode_handle *child);
> +struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
> +						const char *con_id,
> +						struct fwnode_handle *child);
>  #else /* CONFIG_GPIOLIB */
>  
>  static inline int gpiod_count(struct device *dev, const char *con_id)
> @@ -417,7 +417,7 @@ static inline struct gpio_desc *fwnode_get_named_gpiod(
>  	return ERR_PTR(-ENOSYS);
>  }
>  
> -static inline struct gpio_desc *devm_get_gpiod_from_child(
> +static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(
>  	struct device *dev, const char *con_id, struct fwnode_handle *child)
>  {
>  	return ERR_PTR(-ENOSYS);
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Jacek Anaszewski]

On 01/30/2017 04:41 PM, Boris Brezillon wrote:
> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/gpio/devres.c                     | 11 ++++++-----
>  drivers/input/keyboard/gpio_keys.c        |  3 ++-
>  drivers/input/keyboard/gpio_keys_polled.c |  5 +++--
>  drivers/leds/leds-gpio.c                  |  2 +-
>  drivers/video/fbdev/amba-clcd-nomadik.c   |  8 ++++----
>  include/linux/gpio/consumer.h             |  8 ++++----
>  6 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
> index b760cbbb41d8..dfbbd92d21b6 100644
> --- a/drivers/gpio/devres.c
> +++ b/drivers/gpio/devres.c
> @@ -123,7 +123,8 @@ struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev,
>  EXPORT_SYMBOL(devm_gpiod_get_index);
>  
>  /**
> - * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
> + * devm_fwnode_get_gpiod_from_child - get a GPIO descriptor from a device's
> + *				      child node
>   * @dev:	GPIO consumer
>   * @con_id:	function within the GPIO consumer
>   * @child:	firmware node (child of @dev)
> @@ -131,9 +132,9 @@ EXPORT_SYMBOL(devm_gpiod_get_index);
>   * GPIO descriptors returned from this function are automatically disposed on
>   * driver detach.
>   */
> -struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
> -					    const char *con_id,
> -					    struct fwnode_handle *child)
> +struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
> +						   const char *con_id,
> +						   struct fwnode_handle *child)
>  {
>  	static const char * const suffixes[] = { "gpios", "gpio" };
>  	char prop_name[32]; /* 32 is max size of property name */
> @@ -168,7 +169,7 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
>  
>  	return desc;
>  }
> -EXPORT_SYMBOL(devm_get_gpiod_from_child);
> +EXPORT_SYMBOL(devm_fwnode_get_gpiod_from_child);
>  
>  /**
>   * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 582462d0af75..ef6813c1f759 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -481,7 +481,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>  	spin_lock_init(&bdata->lock);
>  
>  	if (child) {
> -		bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child);
> +		bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
> +								child);
>  		if (IS_ERR(bdata->gpiod)) {
>  			error = PTR_ERR(bdata->gpiod);
>  			if (error == -ENOENT) {
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index bed4f2086158..c0c9f2133ecd 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -303,8 +303,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
>  				return -EINVAL;
>  			}
>  
> -			bdata->gpiod = devm_get_gpiod_from_child(dev, NULL,
> -								 child);
> +			bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev,
> +									NULL,
> +									child);
>  			if (IS_ERR(bdata->gpiod)) {
>  				error = PTR_ERR(bdata->gpiod);
>  				if (error != -EPROBE_DEFER)
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index d400dcaf4d29..c0ef838fc993 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -174,7 +174,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>  		const char *state = NULL;
>  		struct device_node *np = to_of_node(child);
>  
> -		led.gpiod = devm_get_gpiod_from_child(dev, NULL, child);
> +		led.gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child);
>  		if (IS_ERR(led.gpiod)) {
>  			fwnode_handle_put(child);
>  			return ERR_CAST(led.gpiod);

For leds-gpio:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

> diff --git a/drivers/video/fbdev/amba-clcd-nomadik.c b/drivers/video/fbdev/amba-clcd-nomadik.c
> index 0c06fcaaa6e8..a4c58c650f8c 100644
> --- a/drivers/video/fbdev/amba-clcd-nomadik.c
> +++ b/drivers/video/fbdev/amba-clcd-nomadik.c
> @@ -184,7 +184,7 @@ static void tpg110_init(struct device *dev, struct device_node *np,
>  {
>  	dev_info(dev, "TPG110 display init\n");
>  
> -	grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode);
> +	grestb = devm_fwnode_get_gpiod_from_child(dev, "grestb", &np->fwnode);
>  	if (IS_ERR(grestb)) {
>  		dev_err(dev, "no GRESTB GPIO\n");
>  		return;
> @@ -192,19 +192,19 @@ static void tpg110_init(struct device *dev, struct device_node *np,
>  	/* This asserts the GRESTB signal, putting the display into reset */
>  	gpiod_direction_output(grestb, 1);
>  
> -	scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode);
> +	scen = devm_fwnode_get_gpiod_from_child(dev, "scen", &np->fwnode);
>  	if (IS_ERR(scen)) {
>  		dev_err(dev, "no SCEN GPIO\n");
>  		return;
>  	}
>  	gpiod_direction_output(scen, 0);
> -	scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode);
> +	scl = devm_fwnode_get_gpiod_from_child(dev, "scl", &np->fwnode);
>  	if (IS_ERR(scl)) {
>  		dev_err(dev, "no SCL GPIO\n");
>  		return;
>  	}
>  	gpiod_direction_output(scl, 0);
> -	sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode);
> +	sda = devm_fwnode_get_gpiod_from_child(dev, "sda", &np->fwnode);
>  	if (IS_ERR(sda)) {
>  		dev_err(dev, "no SDA GPIO\n");
>  		return;
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index fb0fde686cb1..2ce4bc164735 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -136,9 +136,9 @@ struct fwnode_handle;
>  
>  struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
>  					 const char *propname);
> -struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
> -					    const char *con_id,
> -					    struct fwnode_handle *child);
> +struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
> +						const char *con_id,
> +						struct fwnode_handle *child);
>  #else /* CONFIG_GPIOLIB */
>  
>  static inline int gpiod_count(struct device *dev, const char *con_id)
> @@ -417,7 +417,7 @@ static inline struct gpio_desc *fwnode_get_named_gpiod(
>  	return ERR_PTR(-ENOSYS);
>  }
>  
> -static inline struct gpio_desc *devm_get_gpiod_from_child(
> +static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(
>  	struct device *dev, const char *con_id, struct fwnode_handle *child)
>  {
>  	return ERR_PTR(-ENOSYS);
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.

I believe this is completely pointless rename. Are you planning on
adding devm_of_get_gpiod_from_child()? Or
devm_acpt_get_gpiod_from_child()? (I sure hope not).

Also, on what object? Does it take fwnode as first argument? Or maybe we
should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
know types of all arguments?

Please, no.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.

I believe this is completely pointless rename. Are you planning on
adding devm_of_get_gpiod_from_child()? Or
devm_acpt_get_gpiod_from_child()? (I sure hope not).

Also, on what object? Does it take fwnode as first argument? Or maybe we
should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
know types of all arguments?

Please, no.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Mon, 30 Jan 2017 17:06:07 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> > Rename devm_get_gpiod_from_child() into
> > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > function is operating on a fwnode object.  
> 
> I believe this is completely pointless rename. Are you planning on
> adding devm_of_get_gpiod_from_child()? Or
> devm_acpt_get_gpiod_from_child()? (I sure hope not).

Of course not.

> 
> Also, on what object? Does it take fwnode as first argument? Or maybe we
> should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> know types of all arguments?

Linus suggested to rename this function [1]. I personally don't care
much about the name, though I agree with Linus that names should be
consistent and descriptive. Moreover, he's the maintainer, and I tend
to follow maintainers suggestion when I contribute to a specific
subsystem.

IIUC, you're concerned about the length of this function name. If I had
to drop something it would be the _from_child() suffix, because the
function is not even checking that the child parameter is actually a
direct child (or a descendant) of device->fwnode. Also, if we want to
be consistent with the rest of the GPIO API, we could rename it
devm_gpiod_get_from_fwnode() (with the function in added in patch 2
renamed into devm_gpiod_get_from_fwnode()).

Linus, what do you think?

One last thing, I don't want to start a discussion where we're
bikeshedding on a function name instead of focusing on the
functionality, so if it turns into this kind of discussion I'll
probably implement devm_fwnode_get_gpiod_from_child() directly in the
atmel NAND driver and wait for an agreement before switching to the
official version.

Regards,

Boris

[1]https://www.spinics.net/lists/arm-kernel/msg558986.html

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Mon, 30 Jan 2017 17:06:07 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> > Rename devm_get_gpiod_from_child() into
> > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > function is operating on a fwnode object.  
> 
> I believe this is completely pointless rename. Are you planning on
> adding devm_of_get_gpiod_from_child()? Or
> devm_acpt_get_gpiod_from_child()? (I sure hope not).

Of course not.

> 
> Also, on what object? Does it take fwnode as first argument? Or maybe we
> should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> know types of all arguments?

Linus suggested to rename this function [1]. I personally don't care
much about the name, though I agree with Linus that names should be
consistent and descriptive. Moreover, he's the maintainer, and I tend
to follow maintainers suggestion when I contribute to a specific
subsystem.

IIUC, you're concerned about the length of this function name. If I had
to drop something it would be the _from_child() suffix, because the
function is not even checking that the child parameter is actually a
direct child (or a descendant) of device->fwnode. Also, if we want to
be consistent with the rest of the GPIO API, we could rename it
devm_gpiod_get_from_fwnode() (with the function in added in patch 2
renamed into devm_gpiod_get_from_fwnode()).

Linus, what do you think?

One last thing, I don't want to start a discussion where we're
bikeshedding on a function name instead of focusing on the
functionality, so if it turns into this kind of discussion I'll
probably implement devm_fwnode_get_gpiod_from_child() directly in the
atmel NAND driver and wait for an agreement before switching to the
official version.

Regards,

Boris

[1]https://www.spinics.net/lists/arm-kernel/msg558986.html

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:
> On Mon, 30 Jan 2017 17:06:07 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> > > Rename devm_get_gpiod_from_child() into
> > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > function is operating on a fwnode object.  
> > 
> > I believe this is completely pointless rename. Are you planning on
> > adding devm_of_get_gpiod_from_child()? Or
> > devm_acpt_get_gpiod_from_child()? (I sure hope not).
> 
> Of course not.
> 
> > 
> > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > know types of all arguments?
> 
> Linus suggested to rename this function [1]. I personally don't care
> much about the name, though I agree with Linus that names should be
> consistent and descriptive. Moreover, he's the maintainer, and I tend
> to follow maintainers suggestion when I contribute to a specific
> subsystem.

OK, I did not know that that was Linus' request, my objection still
stands.

> 
> IIUC, you're concerned about the length of this function name. If I had
> to drop something it would be the _from_child() suffix, because the
> function is not even checking that the child parameter is actually a
> direct child (or a descendant) of device->fwnode.

OK, that sounds better. Actually, we already have
fwnode_get_named_gpiod(), unfortunately it does not do suffixes
permutations. There are also no users, except
devm_get_gpiod_from_child(). So I would:

- rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
- made new fwnode_get_named_gpiod() that did suffix permutation and
  called __fwnode_get_named_gpiod() (or pulled its implementation
  inline)
- renamed devm_get_gpiod_from_child() ->
  devm_fwnode_get_named_gpiod(dev, fwnode, con_id)
  and called fwnode_get_named_gpiod().

This would indeed match the pattern with other fwnode/property handling
APIs.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:
> On Mon, 30 Jan 2017 17:06:07 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> > > Rename devm_get_gpiod_from_child() into
> > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > function is operating on a fwnode object.  
> > 
> > I believe this is completely pointless rename. Are you planning on
> > adding devm_of_get_gpiod_from_child()? Or
> > devm_acpt_get_gpiod_from_child()? (I sure hope not).
> 
> Of course not.
> 
> > 
> > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > know types of all arguments?
> 
> Linus suggested to rename this function [1]. I personally don't care
> much about the name, though I agree with Linus that names should be
> consistent and descriptive. Moreover, he's the maintainer, and I tend
> to follow maintainers suggestion when I contribute to a specific
> subsystem.

OK, I did not know that that was Linus' request, my objection still
stands.

> 
> IIUC, you're concerned about the length of this function name. If I had
> to drop something it would be the _from_child() suffix, because the
> function is not even checking that the child parameter is actually a
> direct child (or a descendant) of device->fwnode.

OK, that sounds better. Actually, we already have
fwnode_get_named_gpiod(), unfortunately it does not do suffixes
permutations. There are also no users, except
devm_get_gpiod_from_child(). So I would:

- rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
- made new fwnode_get_named_gpiod() that did suffix permutation and
  called __fwnode_get_named_gpiod() (or pulled its implementation
  inline)
- renamed devm_get_gpiod_from_child() ->
  devm_fwnode_get_named_gpiod(dev, fwnode, con_id)
  and called fwnode_get_named_gpiod().

This would indeed match the pattern with other fwnode/property handling
APIs.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Tue, 31 Jan 2017 00:44:47 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:
> > On Mon, 30 Jan 2017 17:06:07 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >   
> > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:  
> > > > Rename devm_get_gpiod_from_child() into
> > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > function is operating on a fwnode object.    
> > > 
> > > I believe this is completely pointless rename. Are you planning on
> > > adding devm_of_get_gpiod_from_child()? Or
> > > devm_acpt_get_gpiod_from_child()? (I sure hope not).  
> > 
> > Of course not.
> >   
> > > 
> > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > know types of all arguments?  
> > 
> > Linus suggested to rename this function [1]. I personally don't care
> > much about the name, though I agree with Linus that names should be
> > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > to follow maintainers suggestion when I contribute to a specific
> > subsystem.  
> 
> OK, I did not know that that was Linus' request, my objection still
> stands.
> 
> > 
> > IIUC, you're concerned about the length of this function name. If I had
> > to drop something it would be the _from_child() suffix, because the
> > function is not even checking that the child parameter is actually a
> > direct child (or a descendant) of device->fwnode.  
> 
> OK, that sounds better. Actually, we already have
> fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> permutations. There are also no users, except
> devm_get_gpiod_from_child(). So I would:
> 
> - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> - made new fwnode_get_named_gpiod() that did suffix permutation and
>   called __fwnode_get_named_gpiod() (or pulled its implementation
>   inline)

Sorry but I don't follow you. Why do you need
__fwnode_get_named_gpiod(), and what is the suffix permutation you're
mentioning here?

> - renamed devm_get_gpiod_from_child() ->
>   devm_fwnode_get_named_gpiod(dev, fwnode, con_id)
>   and called fwnode_get_named_gpiod().

Okay. I'm fine with this name, let's see what Linus says.

> 
> This would indeed match the pattern with other fwnode/property handling
> APIs.
> 
> Thanks.
> 

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Tue, 31 Jan 2017 00:44:47 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:
> > On Mon, 30 Jan 2017 17:06:07 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >   
> > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:  
> > > > Rename devm_get_gpiod_from_child() into
> > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > function is operating on a fwnode object.    
> > > 
> > > I believe this is completely pointless rename. Are you planning on
> > > adding devm_of_get_gpiod_from_child()? Or
> > > devm_acpt_get_gpiod_from_child()? (I sure hope not).  
> > 
> > Of course not.
> >   
> > > 
> > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > know types of all arguments?  
> > 
> > Linus suggested to rename this function [1]. I personally don't care
> > much about the name, though I agree with Linus that names should be
> > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > to follow maintainers suggestion when I contribute to a specific
> > subsystem.  
> 
> OK, I did not know that that was Linus' request, my objection still
> stands.
> 
> > 
> > IIUC, you're concerned about the length of this function name. If I had
> > to drop something it would be the _from_child() suffix, because the
> > function is not even checking that the child parameter is actually a
> > direct child (or a descendant) of device->fwnode.  
> 
> OK, that sounds better. Actually, we already have
> fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> permutations. There are also no users, except
> devm_get_gpiod_from_child(). So I would:
> 
> - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> - made new fwnode_get_named_gpiod() that did suffix permutation and
>   called __fwnode_get_named_gpiod() (or pulled its implementation
>   inline)

Sorry but I don't follow you. Why do you need
__fwnode_get_named_gpiod(), and what is the suffix permutation you're
mentioning here?

> - renamed devm_get_gpiod_from_child() ->
>   devm_fwnode_get_named_gpiod(dev, fwnode, con_id)
>   and called fwnode_get_named_gpiod().

Okay. I'm fine with this name, let's see what Linus says.

> 
> This would indeed match the pattern with other fwnode/property handling
> APIs.
> 
> Thanks.
> 

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:
> On Tue, 31 Jan 2017 00:44:47 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:
> > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >   
> > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:  
> > > > > Rename devm_get_gpiod_from_child() into
> > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > function is operating on a fwnode object.    
> > > > 
> > > > I believe this is completely pointless rename. Are you planning on
> > > > adding devm_of_get_gpiod_from_child()? Or
> > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).  
> > > 
> > > Of course not.
> > >   
> > > > 
> > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > know types of all arguments?  
> > > 
> > > Linus suggested to rename this function [1]. I personally don't care
> > > much about the name, though I agree with Linus that names should be
> > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > to follow maintainers suggestion when I contribute to a specific
> > > subsystem.  
> > 
> > OK, I did not know that that was Linus' request, my objection still
> > stands.
> > 
> > > 
> > > IIUC, you're concerned about the length of this function name. If I had
> > > to drop something it would be the _from_child() suffix, because the
> > > function is not even checking that the child parameter is actually a
> > > direct child (or a descendant) of device->fwnode.  
> > 
> > OK, that sounds better. Actually, we already have
> > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > permutations. There are also no users, except
> > devm_get_gpiod_from_child(). So I would:
> > 
> > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > - made new fwnode_get_named_gpiod() that did suffix permutation and
> >   called __fwnode_get_named_gpiod() (or pulled its implementation
> >   inline)
> 
> Sorry but I don't follow you. Why do you need
> __fwnode_get_named_gpiod(),

You do not need it, it will just reduce size of the patch if you use
it. I'd be perfectly fine not with having it and have everything in
fwnode_get_named_gpiod().

> and what is the suffix permutation you're
> mentioning here?

devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
property name literally.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:
> On Tue, 31 Jan 2017 00:44:47 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:
> > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >   
> > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:  
> > > > > Rename devm_get_gpiod_from_child() into
> > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > function is operating on a fwnode object.    
> > > > 
> > > > I believe this is completely pointless rename. Are you planning on
> > > > adding devm_of_get_gpiod_from_child()? Or
> > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).  
> > > 
> > > Of course not.
> > >   
> > > > 
> > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > know types of all arguments?  
> > > 
> > > Linus suggested to rename this function [1]. I personally don't care
> > > much about the name, though I agree with Linus that names should be
> > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > to follow maintainers suggestion when I contribute to a specific
> > > subsystem.  
> > 
> > OK, I did not know that that was Linus' request, my objection still
> > stands.
> > 
> > > 
> > > IIUC, you're concerned about the length of this function name. If I had
> > > to drop something it would be the _from_child() suffix, because the
> > > function is not even checking that the child parameter is actually a
> > > direct child (or a descendant) of device->fwnode.  
> > 
> > OK, that sounds better. Actually, we already have
> > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > permutations. There are also no users, except
> > devm_get_gpiod_from_child(). So I would:
> > 
> > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > - made new fwnode_get_named_gpiod() that did suffix permutation and
> >   called __fwnode_get_named_gpiod() (or pulled its implementation
> >   inline)
> 
> Sorry but I don't follow you. Why do you need
> __fwnode_get_named_gpiod(),

You do not need it, it will just reduce size of the patch if you use
it. I'd be perfectly fine not with having it and have everything in
fwnode_get_named_gpiod().

> and what is the suffix permutation you're
> mentioning here?

devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
property name literally.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Tue, 31 Jan 2017 01:11:55 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:
> > On Tue, 31 Jan 2017 00:44:47 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >   
> > > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:  
> > > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > >     
> > > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:    
> > > > > > Rename devm_get_gpiod_from_child() into
> > > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > > function is operating on a fwnode object.      
> > > > > 
> > > > > I believe this is completely pointless rename. Are you planning on
> > > > > adding devm_of_get_gpiod_from_child()? Or
> > > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).    
> > > > 
> > > > Of course not.
> > > >     
> > > > > 
> > > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > > know types of all arguments?    
> > > > 
> > > > Linus suggested to rename this function [1]. I personally don't care
> > > > much about the name, though I agree with Linus that names should be
> > > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > > to follow maintainers suggestion when I contribute to a specific
> > > > subsystem.    
> > > 
> > > OK, I did not know that that was Linus' request, my objection still
> > > stands.
> > >   
> > > > 
> > > > IIUC, you're concerned about the length of this function name. If I had
> > > > to drop something it would be the _from_child() suffix, because the
> > > > function is not even checking that the child parameter is actually a
> > > > direct child (or a descendant) of device->fwnode.    
> > > 
> > > OK, that sounds better. Actually, we already have
> > > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > > permutations. There are also no users, except
> > > devm_get_gpiod_from_child(). So I would:
> > > 
> > > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > > - made new fwnode_get_named_gpiod() that did suffix permutation and
> > >   called __fwnode_get_named_gpiod() (or pulled its implementation
> > >   inline)  
> > 
> > Sorry but I don't follow you. Why do you need
> > __fwnode_get_named_gpiod(),  
> 
> You do not need it, it will just reduce size of the patch if you use
> it. I'd be perfectly fine not with having it and have everything in
> fwnode_get_named_gpiod().

Okay.

> 
> > and what is the suffix permutation you're
> > mentioning here?  
> 
> devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
> to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
> property name literally.

fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
be weird/inconsistent to have the con_id suffixing logic moved in the
fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
sure it is).

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Tue, 31 Jan 2017 01:11:55 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:
> > On Tue, 31 Jan 2017 00:44:47 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >   
> > > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:  
> > > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > >     
> > > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:    
> > > > > > Rename devm_get_gpiod_from_child() into
> > > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > > function is operating on a fwnode object.      
> > > > > 
> > > > > I believe this is completely pointless rename. Are you planning on
> > > > > adding devm_of_get_gpiod_from_child()? Or
> > > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).    
> > > > 
> > > > Of course not.
> > > >     
> > > > > 
> > > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > > know types of all arguments?    
> > > > 
> > > > Linus suggested to rename this function [1]. I personally don't care
> > > > much about the name, though I agree with Linus that names should be
> > > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > > to follow maintainers suggestion when I contribute to a specific
> > > > subsystem.    
> > > 
> > > OK, I did not know that that was Linus' request, my objection still
> > > stands.
> > >   
> > > > 
> > > > IIUC, you're concerned about the length of this function name. If I had
> > > > to drop something it would be the _from_child() suffix, because the
> > > > function is not even checking that the child parameter is actually a
> > > > direct child (or a descendant) of device->fwnode.    
> > > 
> > > OK, that sounds better. Actually, we already have
> > > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > > permutations. There are also no users, except
> > > devm_get_gpiod_from_child(). So I would:
> > > 
> > > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > > - made new fwnode_get_named_gpiod() that did suffix permutation and
> > >   called __fwnode_get_named_gpiod() (or pulled its implementation
> > >   inline)  
> > 
> > Sorry but I don't follow you. Why do you need
> > __fwnode_get_named_gpiod(),  
> 
> You do not need it, it will just reduce size of the patch if you use
> it. I'd be perfectly fine not with having it and have everything in
> fwnode_get_named_gpiod().

Okay.

> 
> > and what is the suffix permutation you're
> > mentioning here?  
> 
> devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
> to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
> property name literally.

fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
be weird/inconsistent to have the con_id suffixing logic moved in the
fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
sure it is).

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Tue, Jan 31, 2017 at 10:24:24AM +0100, Boris Brezillon wrote:
> On Tue, 31 Jan 2017 01:11:55 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:
> > > On Tue, 31 Jan 2017 00:44:47 -0800
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >   
> > > > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:  
> > > > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > >     
> > > > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:    
> > > > > > > Rename devm_get_gpiod_from_child() into
> > > > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > > > function is operating on a fwnode object.      
> > > > > > 
> > > > > > I believe this is completely pointless rename. Are you planning on
> > > > > > adding devm_of_get_gpiod_from_child()? Or
> > > > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).    
> > > > > 
> > > > > Of course not.
> > > > >     
> > > > > > 
> > > > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > > > know types of all arguments?    
> > > > > 
> > > > > Linus suggested to rename this function [1]. I personally don't care
> > > > > much about the name, though I agree with Linus that names should be
> > > > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > > > to follow maintainers suggestion when I contribute to a specific
> > > > > subsystem.    
> > > > 
> > > > OK, I did not know that that was Linus' request, my objection still
> > > > stands.
> > > >   
> > > > > 
> > > > > IIUC, you're concerned about the length of this function name. If I had
> > > > > to drop something it would be the _from_child() suffix, because the
> > > > > function is not even checking that the child parameter is actually a
> > > > > direct child (or a descendant) of device->fwnode.    
> > > > 
> > > > OK, that sounds better. Actually, we already have
> > > > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > > > permutations. There are also no users, except
> > > > devm_get_gpiod_from_child(). So I would:
> > > > 
> > > > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > > > - made new fwnode_get_named_gpiod() that did suffix permutation and
> > > >   called __fwnode_get_named_gpiod() (or pulled its implementation
> > > >   inline)  
> > > 
> > > Sorry but I don't follow you. Why do you need
> > > __fwnode_get_named_gpiod(),  
> > 
> > You do not need it, it will just reduce size of the patch if you use
> > it. I'd be perfectly fine not with having it and have everything in
> > fwnode_get_named_gpiod().
> 
> Okay.
> 
> > 
> > > and what is the suffix permutation you're
> > > mentioning here?  
> > 
> > devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
> > to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
> > property name literally.
> 
> fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
> acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
> be weird/inconsistent to have the con_id suffixing logic moved in the
> fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
> sure it is).

Hmm, yeah, I agree, that would be weird. Then let's leave
devm_get_gpiod_from_child() as is ;)

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Tue, Jan 31, 2017 at 10:24:24AM +0100, Boris Brezillon wrote:
> On Tue, 31 Jan 2017 01:11:55 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:
> > > On Tue, 31 Jan 2017 00:44:47 -0800
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >   
> > > > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:  
> > > > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > >     
> > > > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:    
> > > > > > > Rename devm_get_gpiod_from_child() into
> > > > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > > > function is operating on a fwnode object.      
> > > > > > 
> > > > > > I believe this is completely pointless rename. Are you planning on
> > > > > > adding devm_of_get_gpiod_from_child()? Or
> > > > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).    
> > > > > 
> > > > > Of course not.
> > > > >     
> > > > > > 
> > > > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > > > know types of all arguments?    
> > > > > 
> > > > > Linus suggested to rename this function [1]. I personally don't care
> > > > > much about the name, though I agree with Linus that names should be
> > > > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > > > to follow maintainers suggestion when I contribute to a specific
> > > > > subsystem.    
> > > > 
> > > > OK, I did not know that that was Linus' request, my objection still
> > > > stands.
> > > >   
> > > > > 
> > > > > IIUC, you're concerned about the length of this function name. If I had
> > > > > to drop something it would be the _from_child() suffix, because the
> > > > > function is not even checking that the child parameter is actually a
> > > > > direct child (or a descendant) of device->fwnode.    
> > > > 
> > > > OK, that sounds better. Actually, we already have
> > > > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > > > permutations. There are also no users, except
> > > > devm_get_gpiod_from_child(). So I would:
> > > > 
> > > > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > > > - made new fwnode_get_named_gpiod() that did suffix permutation and
> > > >   called __fwnode_get_named_gpiod() (or pulled its implementation
> > > >   inline)  
> > > 
> > > Sorry but I don't follow you. Why do you need
> > > __fwnode_get_named_gpiod(),  
> > 
> > You do not need it, it will just reduce size of the patch if you use
> > it. I'd be perfectly fine not with having it and have everything in
> > fwnode_get_named_gpiod().
> 
> Okay.
> 
> > 
> > > and what is the suffix permutation you're
> > > mentioning here?  
> > 
> > devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
> > to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
> > property name literally.
> 
> fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
> acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
> be weird/inconsistent to have the con_id suffixing logic moved in the
> fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
> sure it is).

Hmm, yeah, I agree, that would be weird. Then let's leave
devm_get_gpiod_from_child() as is ;)

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Tue, 31 Jan 2017 10:39:36 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Tue, Jan 31, 2017 at 10:24:24AM +0100, Boris Brezillon wrote:
> > On Tue, 31 Jan 2017 01:11:55 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >   
> > > On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:  
> > > > On Tue, 31 Jan 2017 00:44:47 -0800
> > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > >     
> > > > > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:    
> > > > > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > > >       
> > > > > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:      
> > > > > > > > Rename devm_get_gpiod_from_child() into
> > > > > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > > > > function is operating on a fwnode object.        
> > > > > > > 
> > > > > > > I believe this is completely pointless rename. Are you planning on
> > > > > > > adding devm_of_get_gpiod_from_child()? Or
> > > > > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).      
> > > > > > 
> > > > > > Of course not.
> > > > > >       
> > > > > > > 
> > > > > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > > > > know types of all arguments?      
> > > > > > 
> > > > > > Linus suggested to rename this function [1]. I personally don't care
> > > > > > much about the name, though I agree with Linus that names should be
> > > > > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > > > > to follow maintainers suggestion when I contribute to a specific
> > > > > > subsystem.      
> > > > > 
> > > > > OK, I did not know that that was Linus' request, my objection still
> > > > > stands.
> > > > >     
> > > > > > 
> > > > > > IIUC, you're concerned about the length of this function name. If I had
> > > > > > to drop something it would be the _from_child() suffix, because the
> > > > > > function is not even checking that the child parameter is actually a
> > > > > > direct child (or a descendant) of device->fwnode.      
> > > > > 
> > > > > OK, that sounds better. Actually, we already have
> > > > > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > > > > permutations. There are also no users, except
> > > > > devm_get_gpiod_from_child(). So I would:
> > > > > 
> > > > > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > > > > - made new fwnode_get_named_gpiod() that did suffix permutation and
> > > > >   called __fwnode_get_named_gpiod() (or pulled its implementation
> > > > >   inline)    
> > > > 
> > > > Sorry but I don't follow you. Why do you need
> > > > __fwnode_get_named_gpiod(),    
> > > 
> > > You do not need it, it will just reduce size of the patch if you use
> > > it. I'd be perfectly fine not with having it and have everything in
> > > fwnode_get_named_gpiod().  
> > 
> > Okay.
> >   
> > >   
> > > > and what is the suffix permutation you're
> > > > mentioning here?    
> > > 
> > > devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
> > > to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
> > > property name literally.  
> > 
> > fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
> > acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
> > be weird/inconsistent to have the con_id suffixing logic moved in the
> > fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
> > sure it is).  
> 
> Hmm, yeah, I agree, that would be weird. Then let's leave
> devm_get_gpiod_from_child() as is ;)

Changing the internal implementation has never been the goal of this
patch. As explained in the commit log, I'm just renaming the function
to make it consistent with other fwnode functions (as suggested by
Linus).
What's happening here is exactly the kind of discussion I wanted to
avoid, and the reason I decided to not change the
devm_get_gpiod_from_child() prototype/name in the first place.

Linus, is this something you really care about? If that's the case, can
you step in?

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Tue, 31 Jan 2017 10:39:36 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Tue, Jan 31, 2017 at 10:24:24AM +0100, Boris Brezillon wrote:
> > On Tue, 31 Jan 2017 01:11:55 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >   
> > > On Tue, Jan 31, 2017 at 10:07:21AM +0100, Boris Brezillon wrote:  
> > > > On Tue, 31 Jan 2017 00:44:47 -0800
> > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > >     
> > > > > On Tue, Jan 31, 2017 at 09:04:32AM +0100, Boris Brezillon wrote:    
> > > > > > On Mon, 30 Jan 2017 17:06:07 -0800
> > > > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > > >       
> > > > > > > On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:      
> > > > > > > > Rename devm_get_gpiod_from_child() into
> > > > > > > > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > > > > > > > function is operating on a fwnode object.        
> > > > > > > 
> > > > > > > I believe this is completely pointless rename. Are you planning on
> > > > > > > adding devm_of_get_gpiod_from_child()? Or
> > > > > > > devm_acpt_get_gpiod_from_child()? (I sure hope not).      
> > > > > > 
> > > > > > Of course not.
> > > > > >       
> > > > > > > 
> > > > > > > Also, on what object? Does it take fwnode as first argument? Or maybe we
> > > > > > > should call it devm_dev_const_charp_fwnode_get_gpiod_from_child() so we
> > > > > > > know types of all arguments?      
> > > > > > 
> > > > > > Linus suggested to rename this function [1]. I personally don't care
> > > > > > much about the name, though I agree with Linus that names should be
> > > > > > consistent and descriptive. Moreover, he's the maintainer, and I tend
> > > > > > to follow maintainers suggestion when I contribute to a specific
> > > > > > subsystem.      
> > > > > 
> > > > > OK, I did not know that that was Linus' request, my objection still
> > > > > stands.
> > > > >     
> > > > > > 
> > > > > > IIUC, you're concerned about the length of this function name. If I had
> > > > > > to drop something it would be the _from_child() suffix, because the
> > > > > > function is not even checking that the child parameter is actually a
> > > > > > direct child (or a descendant) of device->fwnode.      
> > > > > 
> > > > > OK, that sounds better. Actually, we already have
> > > > > fwnode_get_named_gpiod(), unfortunately it does not do suffixes
> > > > > permutations. There are also no users, except
> > > > > devm_get_gpiod_from_child(). So I would:
> > > > > 
> > > > > - rename fwnode_get_named_gpiod() -> static __fwnode_get_named_gpiod()
> > > > > - made new fwnode_get_named_gpiod() that did suffix permutation and
> > > > >   called __fwnode_get_named_gpiod() (or pulled its implementation
> > > > >   inline)    
> > > > 
> > > > Sorry but I don't follow you. Why do you need
> > > > __fwnode_get_named_gpiod(),    
> > > 
> > > You do not need it, it will just reduce size of the patch if you use
> > > it. I'd be perfectly fine not with having it and have everything in
> > > fwnode_get_named_gpiod().  
> > 
> > Okay.
> >   
> > >   
> > > > and what is the suffix permutation you're
> > > > mentioning here?    
> > > 
> > > devm_get_gpiod_from_child() tries to apply "-gpio" and "-gpios" suffixes
> > > to the supplied GPIO ID while current fwnode_get_named_gpiod() takes
> > > property name literally.  
> > 
> > fwnode_get_named_gpiod() just mimics what of_get_named_gpiod_flags(),
> > acpi_node_get_gpiod(), of_find_gpio() and acpi_find_gpio() do. It would
> > be weird/inconsistent to have the con_id suffixing logic moved in the
> > fwnode_get_named_gpiod() (if that's what you're suggesting, but I'm not
> > sure it is).  
> 
> Hmm, yeah, I agree, that would be weird. Then let's leave
> devm_get_gpiod_from_child() as is ;)

Changing the internal implementation has never been the goal of this
patch. As explained in the commit log, I'm just renaming the function
to make it consistent with other fwnode functions (as suggested by
Linus).
What's happening here is exactly the kind of discussion I wanted to
avoid, and the reason I decided to not change the
devm_get_gpiod_from_child() prototype/name in the first place.

Linus, is this something you really care about? If that's the case, can
you step in?

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Linus Walleij]

On Tue, Jan 31, 2017 at 8:42 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 31 Jan 2017 10:39:36 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

>> Hmm, yeah, I agree, that would be weird. Then let's leave
>> devm_get_gpiod_from_child() as is ;)
>
> Changing the internal implementation has never been the goal of this
> patch. As explained in the commit log, I'm just renaming the function
> to make it consistent with other fwnode functions (as suggested by
> Linus).
> What's happening here is exactly the kind of discussion I wanted to
> avoid, and the reason I decided to not change the
> devm_get_gpiod_from_child() prototype/name in the first place.
>
> Linus, is this something you really care about? If that's the case, can
> you step in?

I can only throw up my hands... The way I percieved it, a new function
was added, but I guess it could be that the diffstat was so
convoluted in the other patch (by the way that diff sometimes give
very confusing stuff unless you use the right fuzz) so I misunderstood
some other renaming as introducing a new function.

Please drop the patch if it is controversial.

The name of the function *is* confusing though but maybe it's not
the biggest problem in the world.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Linus Walleij]

On Tue, Jan 31, 2017 at 8:42 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Tue, 31 Jan 2017 10:39:36 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

>> Hmm, yeah, I agree, that would be weird. Then let's leave
>> devm_get_gpiod_from_child() as is ;)
>
> Changing the internal implementation has never been the goal of this
> patch. As explained in the commit log, I'm just renaming the function
> to make it consistent with other fwnode functions (as suggested by
> Linus).
> What's happening here is exactly the kind of discussion I wanted to
> avoid, and the reason I decided to not change the
> devm_get_gpiod_from_child() prototype/name in the first place.
>
> Linus, is this something you really care about? If that's the case, can
> you step in?

I can only throw up my hands... The way I percieved it, a new function
was added, but I guess it could be that the diffstat was so
convoluted in the other patch (by the way that diff sometimes give
very confusing stuff unless you use the right fuzz) so I misunderstood
some other renaming as introducing a new function.

Please drop the patch if it is controversial.

The name of the function *is* confusing though but maybe it's not
the biggest problem in the world.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

Hi Linus,

On Wed, 1 Feb 2017 14:05:43 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Tue, Jan 31, 2017 at 8:42 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Tue, 31 Jan 2017 10:39:36 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:  
> 
> >> Hmm, yeah, I agree, that would be weird. Then let's leave
> >> devm_get_gpiod_from_child() as is ;)  
> >
> > Changing the internal implementation has never been the goal of this
> > patch. As explained in the commit log, I'm just renaming the function
> > to make it consistent with other fwnode functions (as suggested by
> > Linus).
> > What's happening here is exactly the kind of discussion I wanted to
> > avoid, and the reason I decided to not change the
> > devm_get_gpiod_from_child() prototype/name in the first place.
> >
> > Linus, is this something you really care about? If that's the case, can
> > you step in?  
> 
> I can only throw up my hands...

Sorry for forcing your hand like this, but this is the kind of
discussion I'm not comfortable with (when I need to argue on something
I'm not completely convinced of, or I don't have opinion on).

> The way I percieved it, a new function
> was added, but I guess it could be that the diffstat was so
> convoluted in the other patch (by the way that diff sometimes give
> very confusing stuff unless you use the right fuzz) so I misunderstood
> some other renaming as introducing a new function.

Indeed, a new function is added (see patch 2), and this new function is
taking an additional 'index' parameter. If that's a problem, I can also
change the prototype of devm_get_gpiod_from_child() and patch all
existing users of this function, but I fear we'll end up with pretty
much the same discussion :-/.

> 
> Please drop the patch if it is controversial.
> 
> The name of the function *is* confusing though but maybe it's not
> the biggest problem in the world.

I can still name the new function as you suggested
(devm_fwnode_get_index_gpiod_from_child()), and keep the existing one
unchanged if you want.

Just let me know what you prefer.

Thanks,

Boris

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

Hi Linus,

On Wed, 1 Feb 2017 14:05:43 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Tue, Jan 31, 2017 at 8:42 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Tue, 31 Jan 2017 10:39:36 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:  
> 
> >> Hmm, yeah, I agree, that would be weird. Then let's leave
> >> devm_get_gpiod_from_child() as is ;)  
> >
> > Changing the internal implementation has never been the goal of this
> > patch. As explained in the commit log, I'm just renaming the function
> > to make it consistent with other fwnode functions (as suggested by
> > Linus).
> > What's happening here is exactly the kind of discussion I wanted to
> > avoid, and the reason I decided to not change the
> > devm_get_gpiod_from_child() prototype/name in the first place.
> >
> > Linus, is this something you really care about? If that's the case, can
> > you step in?  
> 
> I can only throw up my hands...

Sorry for forcing your hand like this, but this is the kind of
discussion I'm not comfortable with (when I need to argue on something
I'm not completely convinced of, or I don't have opinion on).

> The way I percieved it, a new function
> was added, but I guess it could be that the diffstat was so
> convoluted in the other patch (by the way that diff sometimes give
> very confusing stuff unless you use the right fuzz) so I misunderstood
> some other renaming as introducing a new function.

Indeed, a new function is added (see patch 2), and this new function is
taking an additional 'index' parameter. If that's a problem, I can also
change the prototype of devm_get_gpiod_from_child() and patch all
existing users of this function, but I fear we'll end up with pretty
much the same discussion :-/.

> 
> Please drop the patch if it is controversial.
> 
> The name of the function *is* confusing though but maybe it's not
> the biggest problem in the world.

I can still name the new function as you suggested
(devm_fwnode_get_index_gpiod_from_child()), and keep the existing one
unchanged if you want.

Just let me know what you prefer.

Thanks,

Boris

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Linus Walleij]

On Wed, Feb 1, 2017 at 2:22 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Wed, 1 Feb 2017 14:05:43 +0100
> Linus Walleij <linus.walleij@linaro.org> wrote:

>> > Linus, is this something you really care about? If that's the case, can
>> > you step in?
>>
>> I can only throw up my hands...
>
> Sorry for forcing your hand like this, but this is the kind of
> discussion I'm not comfortable with (when I need to argue on something
> I'm not completely convinced of, or I don't have opinion on).

Sorry, I'm just too stressed by all patches. I now read back on the
context below.

>> The way I percieved it, a new function
>> was added, but I guess it could be that the diffstat was so
>> convoluted in the other patch (by the way that diff sometimes give
>> very confusing stuff unless you use the right fuzz) so I misunderstood
>> some other renaming as introducing a new function.
>
> Indeed, a new function is added (see patch 2), and this new function is
> taking an additional 'index' parameter. If that's a problem, I can also
> change the prototype of devm_get_gpiod_from_child() and patch all
> existing users of this function, but I fear we'll end up with pretty
> much the same discussion :-/.

Yeah.

>> Please drop the patch if it is controversial.
>>
>> The name of the function *is* confusing though but maybe it's not
>> the biggest problem in the world.
>
> I can still name the new function as you suggested
> (devm_fwnode_get_index_gpiod_from_child()), and keep the existing one
> unchanged if you want.

But that is just insane. Then it is just better to apply this and the
other patch making the situation manageable.

This is a good time to do it too since I'm anyways patching around
in all the consumers this merge window.

Dmitry: is this such a big deal to you?

commit 40b7318319281b1bdec804f6435f26cadd329c13
"gpio: Support for unified device properties interface"

by Mika Westerberg introduced

fwnode_get_named_gpiod()
devm_get_gpiod_from_child()

Both are taking a fwnode as argument and the naming is as
inconsistent as it can be.

Some more churn should be expected as a side
effect of naming this function wrong in the first place.
The fwnode API change was on fast-forward and mistakes
were made, also by me, mea culpa.

When I write kernel code, I usually intuitively look for a function doing
what I want, this naming is unintuitive, and it has confused me so
it will confuse others.

Can I please apply these two patches?

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Linus Walleij]

On Wed, Feb 1, 2017 at 2:22 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Wed, 1 Feb 2017 14:05:43 +0100
> Linus Walleij <linus.walleij@linaro.org> wrote:

>> > Linus, is this something you really care about? If that's the case, can
>> > you step in?
>>
>> I can only throw up my hands...
>
> Sorry for forcing your hand like this, but this is the kind of
> discussion I'm not comfortable with (when I need to argue on something
> I'm not completely convinced of, or I don't have opinion on).

Sorry, I'm just too stressed by all patches. I now read back on the
context below.

>> The way I percieved it, a new function
>> was added, but I guess it could be that the diffstat was so
>> convoluted in the other patch (by the way that diff sometimes give
>> very confusing stuff unless you use the right fuzz) so I misunderstood
>> some other renaming as introducing a new function.
>
> Indeed, a new function is added (see patch 2), and this new function is
> taking an additional 'index' parameter. If that's a problem, I can also
> change the prototype of devm_get_gpiod_from_child() and patch all
> existing users of this function, but I fear we'll end up with pretty
> much the same discussion :-/.

Yeah.

>> Please drop the patch if it is controversial.
>>
>> The name of the function *is* confusing though but maybe it's not
>> the biggest problem in the world.
>
> I can still name the new function as you suggested
> (devm_fwnode_get_index_gpiod_from_child()), and keep the existing one
> unchanged if you want.

But that is just insane. Then it is just better to apply this and the
other patch making the situation manageable.

This is a good time to do it too since I'm anyways patching around
in all the consumers this merge window.

Dmitry: is this such a big deal to you?

commit 40b7318319281b1bdec804f6435f26cadd329c13
"gpio: Support for unified device properties interface"

by Mika Westerberg introduced

fwnode_get_named_gpiod()
devm_get_gpiod_from_child()

Both are taking a fwnode as argument and the naming is as
inconsistent as it can be.

Some more churn should be expected as a side
effect of naming this function wrong in the first place.
The fwnode API change was on fast-forward and mistakes
were made, also by me, mea culpa.

When I write kernel code, I usually intuitively look for a function doing
what I want, this naming is unintuitive, and it has confused me so
it will confuse others.

Can I please apply these two patches?

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Wed, Feb 01, 2017 at 03:51:06PM +0100, Linus Walleij wrote:
> On Wed, Feb 1, 2017 at 2:22 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Wed, 1 Feb 2017 14:05:43 +0100
> > Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> >> > Linus, is this something you really care about? If that's the case, can
> >> > you step in?
> >>
> >> I can only throw up my hands...
> >
> > Sorry for forcing your hand like this, but this is the kind of
> > discussion I'm not comfortable with (when I need to argue on something
> > I'm not completely convinced of, or I don't have opinion on).
> 
> Sorry, I'm just too stressed by all patches. I now read back on the
> context below.
> 
> >> The way I percieved it, a new function
> >> was added, but I guess it could be that the diffstat was so
> >> convoluted in the other patch (by the way that diff sometimes give
> >> very confusing stuff unless you use the right fuzz) so I misunderstood
> >> some other renaming as introducing a new function.
> >
> > Indeed, a new function is added (see patch 2), and this new function is
> > taking an additional 'index' parameter. If that's a problem, I can also
> > change the prototype of devm_get_gpiod_from_child() and patch all
> > existing users of this function, but I fear we'll end up with pretty
> > much the same discussion :-/.
> 
> Yeah.
> 
> >> Please drop the patch if it is controversial.
> >>
> >> The name of the function *is* confusing though but maybe it's not
> >> the biggest problem in the world.
> >
> > I can still name the new function as you suggested
> > (devm_fwnode_get_index_gpiod_from_child()), and keep the existing one
> > unchanged if you want.
> 
> But that is just insane. Then it is just better to apply this and the
> other patch making the situation manageable.
> 
> This is a good time to do it too since I'm anyways patching around
> in all the consumers this merge window.
> 
> Dmitry: is this such a big deal to you?

No, not really. But sometimes it is soooo hard to pass on some
bikeshedding opportunity ;)

> 
> commit 40b7318319281b1bdec804f6435f26cadd329c13
> "gpio: Support for unified device properties interface"
> 
> by Mika Westerberg introduced
> 
> fwnode_get_named_gpiod()
> devm_get_gpiod_from_child()
> 
> Both are taking a fwnode as argument and the naming is as
> inconsistent as it can be.
> 
> Some more churn should be expected as a side
> effect of naming this function wrong in the first place.
> The fwnode API change was on fast-forward and mistakes
> were made, also by me, mea culpa.
> 
> When I write kernel code, I usually intuitively look for a function doing
> what I want, this naming is unintuitive, and it has confused me so
> it will confuse others.
> 
> Can I please apply these two patches?

You have my ack for the input bits.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Wed, Feb 01, 2017 at 03:51:06PM +0100, Linus Walleij wrote:
> On Wed, Feb 1, 2017 at 2:22 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Wed, 1 Feb 2017 14:05:43 +0100
> > Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> >> > Linus, is this something you really care about? If that's the case, can
> >> > you step in?
> >>
> >> I can only throw up my hands...
> >
> > Sorry for forcing your hand like this, but this is the kind of
> > discussion I'm not comfortable with (when I need to argue on something
> > I'm not completely convinced of, or I don't have opinion on).
> 
> Sorry, I'm just too stressed by all patches. I now read back on the
> context below.
> 
> >> The way I percieved it, a new function
> >> was added, but I guess it could be that the diffstat was so
> >> convoluted in the other patch (by the way that diff sometimes give
> >> very confusing stuff unless you use the right fuzz) so I misunderstood
> >> some other renaming as introducing a new function.
> >
> > Indeed, a new function is added (see patch 2), and this new function is
> > taking an additional 'index' parameter. If that's a problem, I can also
> > change the prototype of devm_get_gpiod_from_child() and patch all
> > existing users of this function, but I fear we'll end up with pretty
> > much the same discussion :-/.
> 
> Yeah.
> 
> >> Please drop the patch if it is controversial.
> >>
> >> The name of the function *is* confusing though but maybe it's not
> >> the biggest problem in the world.
> >
> > I can still name the new function as you suggested
> > (devm_fwnode_get_index_gpiod_from_child()), and keep the existing one
> > unchanged if you want.
> 
> But that is just insane. Then it is just better to apply this and the
> other patch making the situation manageable.
> 
> This is a good time to do it too since I'm anyways patching around
> in all the consumers this merge window.
> 
> Dmitry: is this such a big deal to you?

No, not really. But sometimes it is soooo hard to pass on some
bikeshedding opportunity ;)

> 
> commit 40b7318319281b1bdec804f6435f26cadd329c13
> "gpio: Support for unified device properties interface"
> 
> by Mika Westerberg introduced
> 
> fwnode_get_named_gpiod()
> devm_get_gpiod_from_child()
> 
> Both are taking a fwnode as argument and the naming is as
> inconsistent as it can be.
> 
> Some more churn should be expected as a side
> effect of naming this function wrong in the first place.
> The fwnode API change was on fast-forward and mistakes
> were made, also by me, mea culpa.
> 
> When I write kernel code, I usually intuitively look for a function doing
> what I want, this naming is unintuitive, and it has confused me so
> it will confuse others.
> 
> Can I please apply these two patches?

You have my ack for the input bits.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Mika Westerberg]

On Wed, Feb 01, 2017 at 03:51:06PM +0100, Linus Walleij wrote:
> fwnode_get_named_gpiod()
> devm_get_gpiod_from_child()
> 
> Both are taking a fwnode as argument and the naming is as
> inconsistent as it can be.
> 
> Some more churn should be expected as a side
> effect of naming this function wrong in the first place.
> The fwnode API change was on fast-forward and mistakes
> were made, also by me, mea culpa.

The name fwnode_get_named_gpiod() tries to follow of_get_named_gpio() so
that we can easily convert a driver to use fwnodes instead. The other
function is named so because we look child fwnodes under a device. For
that, yes certainly we could have invented a better name ;-)

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Mika Westerberg]

On Wed, Feb 01, 2017 at 03:51:06PM +0100, Linus Walleij wrote:
> fwnode_get_named_gpiod()
> devm_get_gpiod_from_child()
> 
> Both are taking a fwnode as argument and the naming is as
> inconsistent as it can be.
> 
> Some more churn should be expected as a side
> effect of naming this function wrong in the first place.
> The fwnode API change was on fast-forward and mistakes
> were made, also by me, mea culpa.

The name fwnode_get_named_gpiod() tries to follow of_get_named_gpio() so
that we can easily convert a driver to use fwnodes instead. The other
function is named so because we look child fwnodes under a device. For
that, yes certainly we could have invented a better name ;-)

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/gpio/devres.c                     | 11 ++++++-----
>  drivers/input/keyboard/gpio_keys.c        |  3 ++-
>  drivers/input/keyboard/gpio_keys_polled.c |  5 +++--

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

for input bits.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Dmitry Torokhov]

On Mon, Jan 30, 2017 at 04:41:48PM +0100, Boris Brezillon wrote:
> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  drivers/gpio/devres.c                     | 11 ++++++-----
>  drivers/input/keyboard/gpio_keys.c        |  3 ++-
>  drivers/input/keyboard/gpio_keys_polled.c |  5 +++--

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

for input bits.

-- 
Dmitry

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Linus Walleij]

On Mon, Jan 30, 2017 at 4:41 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:

> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

All right! So we settled we're gonna merge this and the other patch and
we have the necessary ACKs.

This is unfortunately not applying because I have a bunch of other patches
touching the same files (that's why I think it's a good opportunity to
rename it now, as mentioned IIRC).

Could you rebase this on the following branch:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-gpiod-flags

Then I will apply it on top of that immutable branch and pull it into the
devel branch of gpio so that we get it in and also have an immutable
branch with all changes if some other subsystem maintainer needs it
because of clashes.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Linus Walleij]

On Mon, Jan 30, 2017 at 4:41 PM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:

> Rename devm_get_gpiod_from_child() into
> devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> function is operating on a fwnode object.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

All right! So we settled we're gonna merge this and the other patch and
we have the necessary ACKs.

This is unfortunately not applying because I have a bunch of other patches
touching the same files (that's why I think it's a good opportunity to
rename it now, as mentioned IIRC).

Could you rebase this on the following branch:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-gpiod-flags

Then I will apply it on top of that immutable branch and pull it into the
devel branch of gpio so that we get it in and also have an immutable
branch with all changes if some other subsystem maintainer needs it
because of clashes.

Yours,
Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Thu, 2 Feb 2017 11:53:17 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Mon, Jan 30, 2017 at 4:41 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> 
> > Rename devm_get_gpiod_from_child() into
> > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > function is operating on a fwnode object.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
> 
> All right! So we settled we're gonna merge this and the other patch and
> we have the necessary ACKs.
> 
> This is unfortunately not applying because I have a bunch of other patches
> touching the same files (that's why I think it's a good opportunity to
> rename it now, as mentioned IIRC).
> 
> Could you rebase this on the following branch:
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-gpiod-flags

While rebasing I noticed that you forgot to add the label parameter to
the kernel doc header in commit b2987d7438e0 ("gpio: Pass GPIO label
down to gpiod_request").

> 
> Then I will apply it on top of that immutable branch and pull it into the
> devel branch of gpio so that we get it in and also have an immutable
> branch with all changes if some other subsystem maintainer needs it
> because of clashes.
> 
> Yours,
> Linus Walleij

Re: [PATCH 1/2] gpio: Rename devm_get_gpiod_from_child()

[Boris Brezillon]

On Thu, 2 Feb 2017 11:53:17 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> On Mon, Jan 30, 2017 at 4:41 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> 
> > Rename devm_get_gpiod_from_child() into
> > devm_fwnode_get_gpiod_from_child() to reflect the fact that this
> > function is operating on a fwnode object.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>  
> 
> All right! So we settled we're gonna merge this and the other patch and
> we have the necessary ACKs.
> 
> This is unfortunately not applying because I have a bunch of other patches
> touching the same files (that's why I think it's a good opportunity to
> rename it now, as mentioned IIRC).
> 
> Could you rebase this on the following branch:
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-gpiod-flags

While rebasing I noticed that you forgot to add the label parameter to
the kernel doc header in commit b2987d7438e0 ("gpio: Pass GPIO label
down to gpiod_request").

> 
> Then I will apply it on top of that immutable branch and pull it into the
> devel branch of gpio so that we get it in and also have an immutable
> branch with all changes if some other subsystem maintainer needs it
> because of clashes.
> 
> Yours,
> Linus Walleij

[PATCH 2/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

[Boris Brezillon]

devm_fwnode_get_gpiod_from_child() currently allows GPIO users to
request a GPIO that is defined in a child fwnode instead of directly in
the device fwnode.
Extend this API by adding the devm_fwnode_get_index_gpiod_from_child()
helper which does the same except you can also specify an index in case
the 'xx-gpios' property describe several GPIOs.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpio/devres.c         | 15 ++++++++-------
 drivers/gpio/gpiolib.c        |  9 +++++----
 include/linux/gpio/consumer.h | 21 +++++++++++++++------
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index dfbbd92d21b6..be5f947570db 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -123,18 +123,19 @@ struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev,
 EXPORT_SYMBOL(devm_gpiod_get_index);
 
 /**
- * devm_fwnode_get_gpiod_from_child - get a GPIO descriptor from a device's
- *				      child node
+ * devm_fwnode_get_index_gpiod_from_child - get a GPIO descriptor from a
+ *					    device's child node
  * @dev:	GPIO consumer
  * @con_id:	function within the GPIO consumer
+ * @index:	index of the GPIO to obtain in the consumer
  * @child:	firmware node (child of @dev)
  *
  * GPIO descriptors returned from this function are automatically disposed on
  * driver detach.
  */
-struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
-						   const char *con_id,
-						   struct fwnode_handle *child)
+struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
+						const char *con_id, int index,
+						struct fwnode_handle *child)
 {
 	static const char * const suffixes[] = { "gpios", "gpio" };
 	char prop_name[32]; /* 32 is max size of property name */
@@ -155,7 +156,7 @@ struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
 			snprintf(prop_name, sizeof(prop_name), "%s",
 							       suffixes[i]);
 
-		desc = fwnode_get_named_gpiod(child, prop_name);
+		desc = fwnode_get_named_gpiod(child, prop_name, index);
 		if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT))
 			break;
 	}
@@ -169,7 +170,7 @@ struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
 
 	return desc;
 }
-EXPORT_SYMBOL(devm_fwnode_get_gpiod_from_child);
+EXPORT_SYMBOL(devm_fwnode_get_index_gpiod_from_child);
 
 /**
  * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 86bf3b84ada5..9b7a7734e742 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3309,6 +3309,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
  * fwnode_get_named_gpiod - obtain a GPIO from firmware node
  * @fwnode:	handle of the firmware node
  * @propname:	name of the firmware property representing the GPIO
+ * @index:	index of the GPIO to obtain in the consumer
  *
  * This function can be used for drivers that get their configuration
  * from firmware.
@@ -3320,7 +3321,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
  * In case of error an ERR_PTR() is returned.
  */
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-					 const char *propname)
+					 const char *propname, int index)
 {
 	struct gpio_desc *desc = ERR_PTR(-ENODEV);
 	bool active_low = false;
@@ -3333,8 +3334,8 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 	if (is_of_node(fwnode)) {
 		enum of_gpio_flags flags;
 
-		desc = of_get_named_gpiod_flags(to_of_node(fwnode), propname, 0,
-						&flags);
+		desc = of_get_named_gpiod_flags(to_of_node(fwnode), propname,
+						index, &flags);
 		if (!IS_ERR(desc)) {
 			active_low = flags & OF_GPIO_ACTIVE_LOW;
 			single_ended = flags & OF_GPIO_SINGLE_ENDED;
@@ -3342,7 +3343,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 	} else if (is_acpi_node(fwnode)) {
 		struct acpi_gpio_info info;
 
-		desc = acpi_node_get_gpiod(fwnode, propname, 0, &info);
+		desc = acpi_node_get_gpiod(fwnode, propname, index, &info);
 		if (!IS_ERR(desc))
 			active_low = info.polarity = GPIO_ACTIVE_LOW;
 	}
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 2ce4bc164735..5d1125198fd7 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -135,10 +135,11 @@ int desc_to_gpio(const struct gpio_desc *desc);
 struct fwnode_handle;
 
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-					 const char *propname);
-struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
-						const char *con_id,
+					 const char *propname, int index);
+struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
+						const char *con_id, int index,
 						struct fwnode_handle *child);
+
 #else /* CONFIG_GPIOLIB */
 
 static inline int gpiod_count(struct device *dev, const char *con_id)
@@ -412,19 +413,27 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
 struct fwnode_handle;
 
 static inline struct gpio_desc *fwnode_get_named_gpiod(
-	struct fwnode_handle *fwnode, const char *propname)
+	struct fwnode_handle *fwnode, const char *propname, int index)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(
-	struct device *dev, const char *con_id, struct fwnode_handle *child)
+static inline struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(
+	struct device *dev, const char *con_id, int index,
+	struct fwnode_handle *child)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
 #endif /* CONFIG_GPIOLIB */
 
+static inline struct gpio_desc *
+devm_fwnode_get_gpiod_from_child(struct device *dev, const char *con_id,
+				 struct fwnode_handle *child)
+{
+	return devm_fwnode_get_index_gpiod_from_child(dev, con_id, 0, child);
+}
+
 #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
 
 int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
-- 
2.7.4

[PATCH 2/2] gpio: Add the devm_fwnode_get_index_gpiod_from_child() helper

[Boris Brezillon]

devm_fwnode_get_gpiod_from_child() currently allows GPIO users to
request a GPIO that is defined in a child fwnode instead of directly in
the device fwnode.
Extend this API by adding the devm_fwnode_get_index_gpiod_from_child()
helper which does the same except you can also specify an index in case
the 'xx-gpios' property describe several GPIOs.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/gpio/devres.c         | 15 ++++++++-------
 drivers/gpio/gpiolib.c        |  9 +++++----
 include/linux/gpio/consumer.h | 21 +++++++++++++++------
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c
index dfbbd92d21b6..be5f947570db 100644
--- a/drivers/gpio/devres.c
+++ b/drivers/gpio/devres.c
@@ -123,18 +123,19 @@ struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev,
 EXPORT_SYMBOL(devm_gpiod_get_index);
 
 /**
- * devm_fwnode_get_gpiod_from_child - get a GPIO descriptor from a device's
- *				      child node
+ * devm_fwnode_get_index_gpiod_from_child - get a GPIO descriptor from a
+ *					    device's child node
  * @dev:	GPIO consumer
  * @con_id:	function within the GPIO consumer
+ * @index:	index of the GPIO to obtain in the consumer
  * @child:	firmware node (child of @dev)
  *
  * GPIO descriptors returned from this function are automatically disposed on
  * driver detach.
  */
-struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
-						   const char *con_id,
-						   struct fwnode_handle *child)
+struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
+						const char *con_id, int index,
+						struct fwnode_handle *child)
 {
 	static const char * const suffixes[] = { "gpios", "gpio" };
 	char prop_name[32]; /* 32 is max size of property name */
@@ -155,7 +156,7 @@ struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
 			snprintf(prop_name, sizeof(prop_name), "%s",
 							       suffixes[i]);
 
-		desc = fwnode_get_named_gpiod(child, prop_name);
+		desc = fwnode_get_named_gpiod(child, prop_name, index);
 		if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT))
 			break;
 	}
@@ -169,7 +170,7 @@ struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
 
 	return desc;
 }
-EXPORT_SYMBOL(devm_fwnode_get_gpiod_from_child);
+EXPORT_SYMBOL(devm_fwnode_get_index_gpiod_from_child);
 
 /**
  * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 86bf3b84ada5..9b7a7734e742 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3309,6 +3309,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
  * fwnode_get_named_gpiod - obtain a GPIO from firmware node
  * @fwnode:	handle of the firmware node
  * @propname:	name of the firmware property representing the GPIO
+ * @index:	index of the GPIO to obtain in the consumer
  *
  * This function can be used for drivers that get their configuration
  * from firmware.
@@ -3320,7 +3321,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
  * In case of error an ERR_PTR() is returned.
  */
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-					 const char *propname)
+					 const char *propname, int index)
 {
 	struct gpio_desc *desc = ERR_PTR(-ENODEV);
 	bool active_low = false;
@@ -3333,8 +3334,8 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 	if (is_of_node(fwnode)) {
 		enum of_gpio_flags flags;
 
-		desc = of_get_named_gpiod_flags(to_of_node(fwnode), propname, 0,
-						&flags);
+		desc = of_get_named_gpiod_flags(to_of_node(fwnode), propname,
+						index, &flags);
 		if (!IS_ERR(desc)) {
 			active_low = flags & OF_GPIO_ACTIVE_LOW;
 			single_ended = flags & OF_GPIO_SINGLE_ENDED;
@@ -3342,7 +3343,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 	} else if (is_acpi_node(fwnode)) {
 		struct acpi_gpio_info info;
 
-		desc = acpi_node_get_gpiod(fwnode, propname, 0, &info);
+		desc = acpi_node_get_gpiod(fwnode, propname, index, &info);
 		if (!IS_ERR(desc))
 			active_low = info.polarity == GPIO_ACTIVE_LOW;
 	}
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 2ce4bc164735..5d1125198fd7 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -135,10 +135,11 @@ int desc_to_gpio(const struct gpio_desc *desc);
 struct fwnode_handle;
 
 struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
-					 const char *propname);
-struct gpio_desc *devm_fwnode_get_gpiod_from_child(struct device *dev,
-						const char *con_id,
+					 const char *propname, int index);
+struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(struct device *dev,
+						const char *con_id, int index,
 						struct fwnode_handle *child);
+
 #else /* CONFIG_GPIOLIB */
 
 static inline int gpiod_count(struct device *dev, const char *con_id)
@@ -412,19 +413,27 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
 struct fwnode_handle;
 
 static inline struct gpio_desc *fwnode_get_named_gpiod(
-	struct fwnode_handle *fwnode, const char *propname)
+	struct fwnode_handle *fwnode, const char *propname, int index)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline struct gpio_desc *devm_fwnode_get_gpiod_from_child(
-	struct device *dev, const char *con_id, struct fwnode_handle *child)
+static inline struct gpio_desc *devm_fwnode_get_index_gpiod_from_child(
+	struct device *dev, const char *con_id, int index,
+	struct fwnode_handle *child)
 {
 	return ERR_PTR(-ENOSYS);
 }
 
 #endif /* CONFIG_GPIOLIB */
 
+static inline struct gpio_desc *
+devm_fwnode_get_gpiod_from_child(struct device *dev, const char *con_id,
+				 struct fwnode_handle *child)
+{
+	return devm_fwnode_get_index_gpiod_from_child(dev, con_id, 0, child);
+}
+
 #if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_GPIO_SYSFS)
 
 int gpiod_export(struct gpio_desc *desc, bool direction_may_change);
-- 
2.7.4