gpio: Fix return value mismatch of function gpiod_get_from_of_node()
Subsystems (5)
PATCH 1/1 gpio: Fix return value mismatch of function gpiod_get_from_of_node()
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.
--- 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(-)
drivers/gpio/gpiolib.c +1 -5
@@ -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; }
drivers/regulator/da9211-regulator.c +2 -0
@@ -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++; }
drivers/regulator/s2mps11.c +3 -1
@@ -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 ;
drivers/regulator/s5m8767.c +3 -1
@@ -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;
drivers/regulator/tps65090-regulator.c +4 -3
@@ -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,
Signature · 1 line
2.21.0
Quoted 85 lines · Waibel Georg
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 particulardevm_fwnode_get_index_gpiod_from_child()expects-ENOENT. Currently it gets a NULL, which breaks the loop that tries allgpio_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-ENOENTinstead 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 indevm_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 <[email protected]>--- 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(-)drivers/gpio/gpiolib.c +1
-5@@ -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; }drivers/regulator/da9211-regulator.c +2
-0@@ -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++; }drivers/regulator/s2mps11.c +3
-1@@ -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 ;drivers/regulator/s5m8767.c +0
-0
drivers/regulator/s5m8767.c static int s5m8767_pmic_dt_parse_pdata(struct platform_device *pdev, · line 581
Quoted 9 lines
drivers/regulator/s5m8767.c +3
-1@@ -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:
The s2mps11 code brought a bug to my attention so you might rebase on top of it.
Best regards,
Krzysztof
PATCH v2 1/1 gpio: Fix return value mismatch of function gpiod_get_from_of_node()
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.
---
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(-)
drivers/gpio/gpiolib.c +1 -5
@@ -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; }
drivers/regulator/da9211-regulator.c +2 -0
@@ -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++; }
drivers/regulator/s2mps11.c +3 -1
@@ -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 ;
drivers/regulator/s5m8767.c +3 -1
@@ -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;
drivers/regulator/tps65090-regulator.c +4 -3
@@ -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,
Signature · 1 line
2.7.4
Quoted 24 lines · Waibel Georg
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 particulardevm_fwnode_get_index_gpiod_from_child()expects-ENOENT. Currently it gets a NULL, which breaks the loop that tries allgpio_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-ENOENTinstead 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 indevm_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 <[email protected]>---
V2: Rebased on top of [PATCH] regulator: s2mps11: Fix
ERR_PTRdereference on GPIO lookup failure
Quoted 7 lines
--- 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:
Best regards,
Krzysztof
Quoted 90 lines
drivers/gpio/gpiolib.c +1
-5@@ -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; }drivers/regulator/da9211-regulator.c +2
-0@@ -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++; }drivers/regulator/s2mps11.c +3
-1@@ -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 ;drivers/regulator/s5m8767.c +3
-1@@ -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;drivers/regulator/tps65090-regulator.c +4
-4@@ -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
Hi Georg,
first: good catch! Sorry for breaking this, and kudos for fixing it!
Quoted 18 lines · Waibel Georg
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 particulardevm_fwnode_get_index_gpiod_from_child()expects-ENOENT. Currently it gets a NULL, which breaks the loop that tries allgpio_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-ENOENTinstead 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 indevm_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.
Quoted 4 lines
Signed-off-by Georg Waibel <[email protected]>---
V2: Rebased on top of [PATCH] regulator: s2mps11: Fix
ERR_PTRdereference on GPIO lookup failure
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
Quoted 7 lines · Waibel Georg
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 particulardevm_fwnode_get_index_gpiod_from_child()expects-ENOENT. Currently it gets a NULL, which breaks the loop that tries allgpio_suffixes. The result is that a gpio property is not found, even though it is there.