[Linux-stable-mirror] [PATCH] net: dsa: lan9303: correctly check return value of devm_gpiod_get_optional
Function devm_gpiod_get_optional() returns an ERR_PTR on failure. Its return value should not be validated by a NULL check. Instead, use IS_ERR.
Signed-off-by: Pan Bian bianpan2016@163.com --- drivers/net/dsa/lan9303-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index b471413..6d3fc8f 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -828,7 +828,7 @@ static void lan9303_probe_reset_gpio(struct lan9303 *chip, chip->reset_gpio = devm_gpiod_get_optional(chip->dev, "reset", GPIOD_OUT_LOW);
- if (!chip->reset_gpio) { + if (IS_ERR(chip->reset_gpio)) { dev_dbg(chip->dev, "No reset GPIO defined\n"); return; }
On Sun, Nov 12, 2017 at 11:38:09PM +0800, Pan Bian wrote:
Function devm_gpiod_get_optional() returns an ERR_PTR on failure. Its return value should not be validated by a NULL check. Instead, use IS_ERR.
Signed-off-by: Pan Bian bianpan2016@163.com
Reviewed-by: Andrew Lunn andrew@lunn.ch
Andrew
On 12/11/2017 23:38, Pan Bian wrote:
Function devm_gpiod_get_optional() returns an ERR_PTR on failure. Its return value should not be validated by a NULL check. Instead, use IS_ERR.
Signed-off-by: Pan Bian bianpan2016@163.com
drivers/net/dsa/lan9303-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index b471413..6d3fc8f 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -828,7 +828,7 @@ static void lan9303_probe_reset_gpio(struct lan9303 *chip, chip->reset_gpio = devm_gpiod_get_optional(chip->dev, "reset", GPIOD_OUT_LOW);
- if (!chip->reset_gpio) {
- if (IS_ERR(chip->reset_gpio)) { dev_dbg(chip->dev, "No reset GPIO defined\n"); return; }
Should not an error actually report the error and error out (ie fail probe). But a null is the optional return and ok. (ie when -ENOENT return from sub gpiod_get call).
IS_ERR should be a separate condition check I think.
related lan9303_handle_reset() always returns 0.
lan9303_probe checks lan9303_handle_reset() return value.
Probably should be checking lan9303_probe_reset_gpio() instead.
On Mon, Nov 13, 2017 at 12:08:49PM +0800, Phil Reid wrote:
On 12/11/2017 23:38, Pan Bian wrote:
Function devm_gpiod_get_optional() returns an ERR_PTR on failure. Its return value should not be validated by a NULL check. Instead, use IS_ERR.
Signed-off-by: Pan Bian bianpan2016@163.com
drivers/net/dsa/lan9303-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index b471413..6d3fc8f 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -828,7 +828,7 @@ static void lan9303_probe_reset_gpio(struct lan9303 *chip, chip->reset_gpio = devm_gpiod_get_optional(chip->dev, "reset", GPIOD_OUT_LOW);
- if (!chip->reset_gpio) {
- if (IS_ERR(chip->reset_gpio)) { dev_dbg(chip->dev, "No reset GPIO defined\n"); return; }
Should not an error actually report the error and error out (ie fail probe). But a null is the optional return and ok. (ie when -ENOENT return from sub gpiod_get call).
IS_ERR should be a separate condition check I think.
Hi Phil
Yes, you are right. In particular, -EPROBE_DEFFER should be propagated up and cause the probe to fail and be called later.
Care to submit a patch?
Andrew
From: Pan Bian bianpan2016@163.com Date: Sun, 12 Nov 2017 23:38:09 +0800
Function devm_gpiod_get_optional() returns an ERR_PTR on failure. Its return value should not be validated by a NULL check. Instead, use IS_ERR.
Signed-off-by: Pan Bian bianpan2016@163.com
Applied.
linux-stable-mirror@lists.linaro.org
-
Andrew Lunn -
David Miller -
Pan Bian -
Phil Reid