linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpio: Fix return value mismatch of function gpiod_get_from_of_node()
@ 2019-06-19  9:54 Waibel Georg
  2019-06-19 12:45 ` Krzysztof Kozlowski
  2019-06-21 12:06 ` [PATCH] " Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Waibel Georg @ 2019-06-19  9:54 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Support Opensource,
	Liam Girdwood, Mark Brown, Sangbeom Kim, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, linux-gpio, linux-kernel,
	linux-samsung-soc

In case the requested gpio property is not found in the device tree, some
callers of gpiod_get_from_of_node() expect a return value of NULL, others
expect -ENOENT.
In particular devm_fwnode_get_index_gpiod_from_child() expects -ENOENT.
Currently it gets a NULL, which breaks the loop that tries all
gpio_suffixes. The result is that a gpio property is not found, even
though it is there.

This patch changes gpiod_get_from_of_node() to return -ENOENT instead
of NULL when the requested gpio property is not found in the device
tree. Additionally it modifies all calling functions to properly
evaluate the return value.

Another approach would be to leave the return value of
gpiod_get_from_of_node() as is and fix the bug in
devm_fwnode_get_index_gpiod_from_child(). Other callers would still need
to be reworked. The effort would be the same as with the chosen solution.

Signed-off-by: Georg Waibel <georg.waibel@sensor-technik.de>
---
 drivers/gpio/gpiolib.c                 | 6 +-----
 drivers/regulator/da9211-regulator.c   | 2 ++
 drivers/regulator/s2mps11.c            | 4 +++-
 drivers/regulator/s5m8767.c            | 4 +++-
 drivers/regulator/tps65090-regulator.c | 7 ++++---
 5 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..be1d1d2f8aaa 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4244,8 +4244,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
  *
  * Returns:
  * On successful request the GPIO pin is configured in accordance with
- * provided @dflags. If the node does not have the requested GPIO
- * property, NULL is returned.
+ * provided @dflags.
  *
  * In case of error an ERR_PTR() is returned.
  */
@@ -4267,9 +4266,6 @@ struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
 					index, &flags);
 
 	if (!desc || IS_ERR(desc)) {
-		/* If it is not there, just return NULL */
-		if (PTR_ERR(desc) == -ENOENT)
-			return NULL;
 		return desc;
 	}
 
diff --git a/drivers/regulator/da9211-regulator.c b/drivers/regulator/da9211-regulator.c
index da37b4ccd834..0309823d2c72 100644
--- a/drivers/regulator/da9211-regulator.c
+++ b/drivers/regulator/da9211-regulator.c
@@ -289,6 +289,8 @@ static struct da9211_pdata *da9211_parse_regulators_dt(
 				  0,
 				  GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
 				  "da9211-enable");
+		if (IS_ERR(pdata->gpiod_ren[n]))
+			pdata->gpiod_ren[n] = NULL;
 		n++;
 	}
 
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 134c62db36c5..b518a81f75a3 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -821,7 +821,9 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
 				0,
 				GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
 				"s2mps11-regulator");
-		if (IS_ERR(gpio[reg])) {
+		if (PTR_ERR(gpio[reg]) == -ENOENT)
+			gpio[reg] = NULL;
+		else if (IS_ERR(gpio[reg])) {
 			dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
 				reg, rdata[reg].name);
 			continue;
diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index bb9d1a083299..6ca27e9d5ef7 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -574,7 +574,9 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 			0,
 			GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
 			"s5m8767");
-		if (IS_ERR(rdata->ext_control_gpiod))
+		if (PTR_ERR(rdata->ext_control_gpiod) == -ENOENT)
+			rdata->ext_control_gpiod = NULL;
+		else if (IS_ERR(rdata->ext_control_gpiod))
 			return PTR_ERR(rdata->ext_control_gpiod);
 
 		rdata->id = i;
diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index ca39b3d55123..10ea4b5a0f55 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -371,11 +371,12 @@ static struct tps65090_platform_data *tps65090_parse_dt_reg_data(
 								    "dcdc-ext-control-gpios", 0,
 								    gflags,
 								    "tps65090");
-			if (IS_ERR(rpdata->gpiod))
-				return ERR_CAST(rpdata->gpiod);
-			if (!rpdata->gpiod)
+			if (PTR_ERR(rpdata->gpiod) == -ENOENT) {
 				dev_err(&pdev->dev,
 					"could not find DCDC external control GPIO\n");
+				rpdata->gpiod = NULL;
+			} else if (IS_ERR(rpdata->gpiod))
+				return ERR_CAST(rpdata->gpiod);
 		}
 
 		if (of_property_read_u32(tps65090_matches[idx].of_node,
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: Fix return value mismatch of function gpiod_get_from_of_node()
  2019-06-19  9:54 [PATCH] gpio: Fix return value mismatch of function gpiod_get_from_of_node() Waibel Georg
@ 2019-06-19 12:45 ` Krzysztof Kozlowski
  2019-06-20 21:37   ` [PATCH V2] " Waibel Georg
  2019-06-21 12:06 ` [PATCH] " Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2019-06-19 12:45 UTC (permalink / raw)
  To: Waibel Georg
  Cc: Linus Walleij, Bartosz Golaszewski, Support Opensource,
	Liam Girdwood, Mark Brown, Sangbeom Kim,
	Bartlomiej Zolnierkiewicz, linux-gpio, linux-kernel,
	linux-samsung-soc

On Wed, 19 Jun 2019 at 11:56, Waibel Georg
<Georg.Waibel@sensor-technik.de> wrote:
>
> In case the requested gpio property is not found in the device tree, some
> callers of gpiod_get_from_of_node() expect a return value of NULL, others
> expect -ENOENT.
> In particular devm_fwnode_get_index_gpiod_from_child() expects -ENOENT.
> Currently it gets a NULL, which breaks the loop that tries all
> gpio_suffixes. The result is that a gpio property is not found, even
> though it is there.
>
> This patch changes gpiod_get_from_of_node() to return -ENOENT instead
> of NULL when the requested gpio property is not found in the device
> tree. Additionally it modifies all calling functions to properly
> evaluate the return value.
>
> Another approach would be to leave the return value of
> gpiod_get_from_of_node() as is and fix the bug in
> devm_fwnode_get_index_gpiod_from_child(). Other callers would still need
> to be reworked. The effort would be the same as with the chosen solution.
>
> Signed-off-by: Georg Waibel <georg.waibel@sensor-technik.de>
> ---
>  drivers/gpio/gpiolib.c                 | 6 +-----
>  drivers/regulator/da9211-regulator.c   | 2 ++
>  drivers/regulator/s2mps11.c            | 4 +++-
>  drivers/regulator/s5m8767.c            | 4 +++-
>  drivers/regulator/tps65090-regulator.c | 7 ++++---
>  5 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e013d417a936..be1d1d2f8aaa 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4244,8 +4244,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
>   *
>   * Returns:
>   * On successful request the GPIO pin is configured in accordance with
> - * provided @dflags. If the node does not have the requested GPIO
> - * property, NULL is returned.
> + * provided @dflags.
>   *
>   * In case of error an ERR_PTR() is returned.
>   */
> @@ -4267,9 +4266,6 @@ struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
>                                         index, &flags);
>
>         if (!desc || IS_ERR(desc)) {
> -               /* If it is not there, just return NULL */
> -               if (PTR_ERR(desc) == -ENOENT)
> -                       return NULL;
>                 return desc;
>         }
>
> diff --git a/drivers/regulator/da9211-regulator.c b/drivers/regulator/da9211-regulator.c
> index da37b4ccd834..0309823d2c72 100644
> --- a/drivers/regulator/da9211-regulator.c
> +++ b/drivers/regulator/da9211-regulator.c
> @@ -289,6 +289,8 @@ static struct da9211_pdata *da9211_parse_regulators_dt(
>                                   0,
>                                   GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>                                   "da9211-enable");
> +               if (IS_ERR(pdata->gpiod_ren[n]))
> +                       pdata->gpiod_ren[n] = NULL;
>                 n++;
>         }
>
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index 134c62db36c5..b518a81f75a3 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -821,7 +821,9 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
>                                 0,
>                                 GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>                                 "s2mps11-regulator");
> -               if (IS_ERR(gpio[reg])) {
> +               if (PTR_ERR(gpio[reg]) == -ENOENT)
> +                       gpio[reg] = NULL;
> +               else if (IS_ERR(gpio[reg])) {
>                         dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
>                                 reg, rdata[reg].name);
>                         continue;
> diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
> index bb9d1a083299..6ca27e9d5ef7 100644
> --- a/drivers/regulator/s5m8767.c
> +++ b/drivers/regulator/s5m8767.c
> @@ -574,7 +574,9 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
>                         0,
>                         GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>                         "s5m8767");
> -               if (IS_ERR(rdata->ext_control_gpiod))
> +               if (PTR_ERR(rdata->ext_control_gpiod) == -ENOENT)
> +                       rdata->ext_control_gpiod = NULL;
> +               else if (IS_ERR(rdata->ext_control_gpiod))
>                         return PTR_ERR(rdata->ext_control_gpiod);
>

For s2mps11 and s5m8767 bits:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

The s2mps11 code brought a bug to my attention so you might rebase on top of it.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2] gpio: Fix return value mismatch of function gpiod_get_from_of_node()
  2019-06-19 12:45 ` Krzysztof Kozlowski
@ 2019-06-20 21:37   ` Waibel Georg
  2019-06-21  6:07     ` Krzysztof Kozlowski
  2019-06-25 13:47     ` Linus Walleij
  0 siblings, 2 replies; 6+ messages in thread
From: Waibel Georg @ 2019-06-20 21:37 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linus Walleij, Bartosz Golaszewski, Support Opensource,
	Liam Girdwood, Mark Brown, Sangbeom Kim,
	Bartlomiej Zolnierkiewicz, linux-gpio, linux-kernel,
	linux-samsung-soc

In case the requested gpio property is not found in the device tree, some
callers of gpiod_get_from_of_node() expect a return value of NULL, others
expect -ENOENT.
In particular devm_fwnode_get_index_gpiod_from_child() expects -ENOENT.
Currently it gets a NULL, which breaks the loop that tries all
gpio_suffixes. The result is that a gpio property is not found, even
though it is there.

This patch changes gpiod_get_from_of_node() to return -ENOENT instead
of NULL when the requested gpio property is not found in the device
tree. Additionally it modifies all calling functions to properly
evaluate the return value.

Another approach would be to leave the return value of
gpiod_get_from_of_node() as is and fix the bug in
devm_fwnode_get_index_gpiod_from_child(). Other callers would still need
to be reworked. The effort would be the same as with the chosen solution.

Signed-off-by: Georg Waibel <georg.waibel@sensor-technik.de>
---

V2: Rebased on top of [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

---
 drivers/gpio/gpiolib.c                 | 6 +-----
 drivers/regulator/da9211-regulator.c   | 2 ++
 drivers/regulator/s2mps11.c            | 4 +++-
 drivers/regulator/s5m8767.c            | 4 +++-
 drivers/regulator/tps65090-regulator.c | 7 ++++---
 5 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d41..be1d1d2 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4244,8 +4244,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
  *
  * Returns:
  * On successful request the GPIO pin is configured in accordance with
- * provided @dflags. If the node does not have the requested GPIO
- * property, NULL is returned.
+ * provided @dflags.
  *
  * In case of error an ERR_PTR() is returned.
  */
@@ -4267,9 +4266,6 @@ struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
 					index, &flags);
 
 	if (!desc || IS_ERR(desc)) {
-		/* If it is not there, just return NULL */
-		if (PTR_ERR(desc) == -ENOENT)
-			return NULL;
 		return desc;
 	}
 
diff --git a/drivers/regulator/da9211-regulator.c b/drivers/regulator/da9211-regulator.c
index da37b4c..0309823 100644
--- a/drivers/regulator/da9211-regulator.c
+++ b/drivers/regulator/da9211-regulator.c
@@ -289,6 +289,8 @@ static struct da9211_pdata *da9211_parse_regulators_dt(
 				  0,
 				  GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
 				  "da9211-enable");
+		if (IS_ERR(pdata->gpiod_ren[n]))
+			pdata->gpiod_ren[n] = NULL;
 		n++;
 	}
 
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index af9bf10..209d1ff 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -821,7 +821,9 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
 				0,
 				GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
 				"s2mps11-regulator");
-		if (IS_ERR(gpio[reg])) {
+		if (PTR_ERR(gpio[reg]) == -ENOENT)
+			gpio[reg] = NULL;
+		else if (IS_ERR(gpio[reg])) {
 			dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
 				reg, rdata[reg].name);
 			gpio[reg] = NULL;
diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index bb9d1a0..6ca27e9 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -574,7 +574,9 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
 			0,
 			GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
 			"s5m8767");
-		if (IS_ERR(rdata->ext_control_gpiod))
+		if (PTR_ERR(rdata->ext_control_gpiod) == -ENOENT)
+			rdata->ext_control_gpiod = NULL;
+		else if (IS_ERR(rdata->ext_control_gpiod))
 			return PTR_ERR(rdata->ext_control_gpiod);
 
 		rdata->id = i;
diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index ca39b3d..10ea4b5 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -371,11 +371,12 @@ static struct tps65090_platform_data *tps65090_parse_dt_reg_data(
 								    "dcdc-ext-control-gpios", 0,
 								    gflags,
 								    "tps65090");
-			if (IS_ERR(rpdata->gpiod))
-				return ERR_CAST(rpdata->gpiod);
-			if (!rpdata->gpiod)
+			if (PTR_ERR(rpdata->gpiod) == -ENOENT) {
 				dev_err(&pdev->dev,
 					"could not find DCDC external control GPIO\n");
+				rpdata->gpiod = NULL;
+			} else if (IS_ERR(rpdata->gpiod))
+				return ERR_CAST(rpdata->gpiod);
 		}
 
 		if (of_property_read_u32(tps65090_matches[idx].of_node,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] gpio: Fix return value mismatch of function gpiod_get_from_of_node()
  2019-06-20 21:37   ` [PATCH V2] " Waibel Georg
@ 2019-06-21  6:07     ` Krzysztof Kozlowski
  2019-06-25 13:47     ` Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2019-06-21  6:07 UTC (permalink / raw)
  To: Waibel Georg
  Cc: Linus Walleij, Bartosz Golaszewski, Support Opensource,
	Liam Girdwood, Mark Brown, Sangbeom Kim,
	Bartlomiej Zolnierkiewicz, linux-gpio, linux-kernel,
	linux-samsung-soc

On Thu, 20 Jun 2019 at 23:37, Waibel Georg
<Georg.Waibel@sensor-technik.de> wrote:
>
> In case the requested gpio property is not found in the device tree, some
> callers of gpiod_get_from_of_node() expect a return value of NULL, others
> expect -ENOENT.
> In particular devm_fwnode_get_index_gpiod_from_child() expects -ENOENT.
> Currently it gets a NULL, which breaks the loop that tries all
> gpio_suffixes. The result is that a gpio property is not found, even
> though it is there.
>
> This patch changes gpiod_get_from_of_node() to return -ENOENT instead
> of NULL when the requested gpio property is not found in the device
> tree. Additionally it modifies all calling functions to properly
> evaluate the return value.
>
> Another approach would be to leave the return value of
> gpiod_get_from_of_node() as is and fix the bug in
> devm_fwnode_get_index_gpiod_from_child(). Other callers would still need
> to be reworked. The effort would be the same as with the chosen solution.
>
> Signed-off-by: Georg Waibel <georg.waibel@sensor-technik.de>
> ---
>
> V2: Rebased on top of [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure
>
> ---
>  drivers/gpio/gpiolib.c                 | 6 +-----
>  drivers/regulator/da9211-regulator.c   | 2 ++
>  drivers/regulator/s2mps11.c            | 4 +++-
>  drivers/regulator/s5m8767.c            | 4 +++-
>  drivers/regulator/tps65090-regulator.c | 7 ++++---
>  5 files changed, 13 insertions(+), 10 deletions(-)

You missed my tag from v1:

For s2mps11 and s5m8767 bits:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e013d41..be1d1d2 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4244,8 +4244,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index);
>   *
>   * Returns:
>   * On successful request the GPIO pin is configured in accordance with
> - * provided @dflags. If the node does not have the requested GPIO
> - * property, NULL is returned.
> + * provided @dflags.
>   *
>   * In case of error an ERR_PTR() is returned.
>   */
> @@ -4267,9 +4266,6 @@ struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
>                                         index, &flags);
>
>         if (!desc || IS_ERR(desc)) {
> -               /* If it is not there, just return NULL */
> -               if (PTR_ERR(desc) == -ENOENT)
> -                       return NULL;
>                 return desc;
>         }
>
> diff --git a/drivers/regulator/da9211-regulator.c b/drivers/regulator/da9211-regulator.c
> index da37b4c..0309823 100644
> --- a/drivers/regulator/da9211-regulator.c
> +++ b/drivers/regulator/da9211-regulator.c
> @@ -289,6 +289,8 @@ static struct da9211_pdata *da9211_parse_regulators_dt(
>                                   0,
>                                   GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>                                   "da9211-enable");
> +               if (IS_ERR(pdata->gpiod_ren[n]))
> +                       pdata->gpiod_ren[n] = NULL;
>                 n++;
>         }
>
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index af9bf10..209d1ff 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -821,7 +821,9 @@ static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
>                                 0,
>                                 GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>                                 "s2mps11-regulator");
> -               if (IS_ERR(gpio[reg])) {
> +               if (PTR_ERR(gpio[reg]) == -ENOENT)
> +                       gpio[reg] = NULL;
> +               else if (IS_ERR(gpio[reg])) {
>                         dev_err(&pdev->dev, "Failed to get control GPIO for %d/%s\n",
>                                 reg, rdata[reg].name);
>                         gpio[reg] = NULL;
> diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
> index bb9d1a0..6ca27e9 100644
> --- a/drivers/regulator/s5m8767.c
> +++ b/drivers/regulator/s5m8767.c
> @@ -574,7 +574,9 @@ static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev,
>                         0,
>                         GPIOD_OUT_HIGH | GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>                         "s5m8767");
> -               if (IS_ERR(rdata->ext_control_gpiod))
> +               if (PTR_ERR(rdata->ext_control_gpiod) == -ENOENT)
> +                       rdata->ext_control_gpiod = NULL;
> +               else if (IS_ERR(rdata->ext_control_gpiod))
>                         return PTR_ERR(rdata->ext_control_gpiod);
>
>                 rdata->id = i;
> diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
> index ca39b3d..10ea4b5 100644
> --- a/drivers/regulator/tps65090-regulator.c
> +++ b/drivers/regulator/tps65090-regulator.c
> @@ -371,11 +371,12 @@ static struct tps65090_platform_data *tps65090_parse_dt_reg_data(
>                                                                     "dcdc-ext-control-gpios", 0,
>                                                                     gflags,
>                                                                     "tps65090");
> -                       if (IS_ERR(rpdata->gpiod))
> -                               return ERR_CAST(rpdata->gpiod);
> -                       if (!rpdata->gpiod)
> +                       if (PTR_ERR(rpdata->gpiod) == -ENOENT) {
>                                 dev_err(&pdev->dev,
>                                         "could not find DCDC external control GPIO\n");
> +                               rpdata->gpiod = NULL;
> +                       } else if (IS_ERR(rpdata->gpiod))
> +                               return ERR_CAST(rpdata->gpiod);
>                 }
>
>                 if (of_property_read_u32(tps65090_matches[idx].of_node,
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gpio: Fix return value mismatch of function gpiod_get_from_of_node()
  2019-06-19  9:54 [PATCH] gpio: Fix return value mismatch of function gpiod_get_from_of_node() Waibel Georg
  2019-06-19 12:45 ` Krzysztof Kozlowski
@ 2019-06-21 12:06 ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2019-06-21 12:06 UTC (permalink / raw)
  To: Waibel Georg
  Cc: Linus Walleij, Bartosz Golaszewski, Support Opensource,
	Liam Girdwood, Sangbeom Kim, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, linux-gpio, linux-kernel,
	linux-samsung-soc

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Wed, Jun 19, 2019 at 09:54:48AM +0000, Waibel Georg wrote:
> In case the requested gpio property is not found in the device tree, some
> callers of gpiod_get_from_of_node() expect a return value of NULL, others
> expect -ENOENT.
> In particular devm_fwnode_get_index_gpiod_from_child() expects -ENOENT.
> Currently it gets a NULL, which breaks the loop that tries all
> gpio_suffixes. The result is that a gpio property is not found, even
> though it is there.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] gpio: Fix return value mismatch of function gpiod_get_from_of_node()
  2019-06-20 21:37   ` [PATCH V2] " Waibel Georg
  2019-06-21  6:07     ` Krzysztof Kozlowski
@ 2019-06-25 13:47     ` Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-06-25 13:47 UTC (permalink / raw)
  To: Waibel Georg
  Cc: Krzysztof Kozlowski, Bartosz Golaszewski, Support Opensource,
	Liam Girdwood, Mark Brown, Sangbeom Kim,
	Bartlomiej Zolnierkiewicz, linux-gpio, linux-kernel,
	linux-samsung-soc

Hi Georg,

first: good catch! Sorry for breaking this, and kudos for fixing it!

On Thu, Jun 20, 2019 at 11:37 PM Waibel Georg
<Georg.Waibel@sensor-technik.de> wrote:

> In case the requested gpio property is not found in the device tree, some
> callers of gpiod_get_from_of_node() expect a return value of NULL, others
> expect -ENOENT.
> In particular devm_fwnode_get_index_gpiod_from_child() expects -ENOENT.
> Currently it gets a NULL, which breaks the loop that tries all
> gpio_suffixes. The result is that a gpio property is not found, even
> though it is there.
>
> This patch changes gpiod_get_from_of_node() to return -ENOENT instead
> of NULL when the requested gpio property is not found in the device
> tree. Additionally it modifies all calling functions to properly
> evaluate the return value.
>
> Another approach would be to leave the return value of
> gpiod_get_from_of_node() as is and fix the bug in
> devm_fwnode_get_index_gpiod_from_child(). Other callers would still need
> to be reworked. The effort would be the same as with the chosen solution.
>
> Signed-off-by: Georg Waibel <georg.waibel@sensor-technik.de>
> ---
>
> V2: Rebased on top of [PATCH] regulator: s2mps11: Fix ERR_PTR dereference on GPIO lookup failure

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Mark: as most of the changed lines are in the regulator tree,
would you please pick this patch up?

(Else tell me and I will take care of it.)

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-25 13:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19  9:54 [PATCH] gpio: Fix return value mismatch of function gpiod_get_from_of_node() Waibel Georg
2019-06-19 12:45 ` Krzysztof Kozlowski
2019-06-20 21:37   ` [PATCH V2] " Waibel Georg
2019-06-21  6:07     ` Krzysztof Kozlowski
2019-06-25 13:47     ` Linus Walleij
2019-06-21 12:06 ` [PATCH] " Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).