[PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()

[PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()

[Jinjie Ruan]

If pinctrl is not available (thus devm_pinctrl_get() returns NULL) then
recovery can't work, because we can't switch the I2C pins between the
I2C controller and GPIO. So, it is quite correct to print
"can't get pinctrl, bus recovery not supported" because the I2C bus
can't be recovered without pinctrl.

The PTR_ERR() is also fine - because if pinctrl is not present and
returns NULL, we'll end up returning zero, which is exactly what we
want.

However, open code that with a more accurate message will be more explicit
for NULL case when CONFIG_PINCTRL is not defined.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
---
v3:
- Split into two if() statements instead of removing the NULL checks.
- Remove the fix tags.
- Update the commit title and message.
- Update to bring the author's name before.
v2:
- Remove NULL check instead of using IS_ERR_OR_NULL() to avoid leaving them behind.
- Update the commit title and message.
---
 drivers/i2c/busses/i2c-at91-master.c | 6 +++++-
 drivers/i2c/busses/i2c-imx.c         | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index 94cff1cd527e..d311981d3e60 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -831,7 +831,11 @@ static int at91_init_twi_recovery_gpio(struct platform_device *pdev,
 	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
 
 	rinfo->pinctrl = devm_pinctrl_get(&pdev->dev);
-	if (!rinfo->pinctrl || IS_ERR(rinfo->pinctrl)) {
+	if (!rinfo->pinctrl) {
+		dev_info(dev->dev, "pinctrl unavailable, bus recovery not supported\n");
+		return 0;
+	}
+	if (IS_ERR(rinfo->pinctrl)) {
 		dev_info(dev->dev, "can't get pinctrl, bus recovery not supported\n");
 		return PTR_ERR(rinfo->pinctrl);
 	}
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 10e89586ca72..1775a79aeba2 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1388,7 +1388,11 @@ static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
 	struct i2c_bus_recovery_info *rinfo = &i2c_imx->rinfo;
 
 	i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
-	if (!i2c_imx->pinctrl || IS_ERR(i2c_imx->pinctrl)) {
+	if (!i2c_imx->pinctrl) {
+		dev_info(&pdev->dev, "pinctrl unavailable, bus recovery not supported\n");
+		return 0;
+	}
+	if (IS_ERR(i2c_imx->pinctrl)) {
 		dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
 		return PTR_ERR(i2c_imx->pinctrl);
 	}
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()

[Oleksij Rempel]

On Mon, Aug 21, 2023 at 11:29:14AM +0800, Jinjie Ruan wrote:
> If pinctrl is not available (thus devm_pinctrl_get() returns NULL) then
> recovery can't work, because we can't switch the I2C pins between the
> I2C controller and GPIO. So, it is quite correct to print
> "can't get pinctrl, bus recovery not supported" because the I2C bus
> can't be recovered without pinctrl.
> 
> The PTR_ERR() is also fine - because if pinctrl is not present and
> returns NULL, we'll end up returning zero, which is exactly what we
> want.
> 
> However, open code that with a more accurate message will be more explicit
> for NULL case when CONFIG_PINCTRL is not defined.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
> ---
> v3:
> - Split into two if() statements instead of removing the NULL checks.
> - Remove the fix tags.
> - Update the commit title and message.
> - Update to bring the author's name before.
> v2:
> - Remove NULL check instead of using IS_ERR_OR_NULL() to avoid leaving them behind.
> - Update the commit title and message.
> ---
>  drivers/i2c/busses/i2c-at91-master.c | 6 +++++-
>  drivers/i2c/busses/i2c-imx.c         | 6 +++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)

Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>

for the i2c-imx.c

Thank you!

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()

[Andi Shyti]

Hi,

On Thu, Aug 24, 2023 at 10:34:53AM +0200, Oleksij Rempel wrote:
> On Mon, Aug 21, 2023 at 11:29:14AM +0800, Jinjie Ruan wrote:
> > If pinctrl is not available (thus devm_pinctrl_get() returns NULL) then
> > recovery can't work, because we can't switch the I2C pins between the
> > I2C controller and GPIO. So, it is quite correct to print
> > "can't get pinctrl, bus recovery not supported" because the I2C bus
> > can't be recovered without pinctrl.
> > 
> > The PTR_ERR() is also fine - because if pinctrl is not present and
> > returns NULL, we'll end up returning zero, which is exactly what we
> > want.
> > 
> > However, open code that with a more accurate message will be more explicit
> > for NULL case when CONFIG_PINCTRL is not defined.
> > 
> > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> > Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
> > ---
> > v3:
> > - Split into two if() statements instead of removing the NULL checks.
> > - Remove the fix tags.
> > - Update the commit title and message.
> > - Update to bring the author's name before.
> > v2:
> > - Remove NULL check instead of using IS_ERR_OR_NULL() to avoid leaving them behind.
> > - Update the commit title and message.
> > ---
> >  drivers/i2c/busses/i2c-at91-master.c | 6 +++++-
> >  drivers/i2c/busses/i2c-imx.c         | 6 +++++-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> for the i2c-imx.c

For some reason lore+lei failed to deliver this message to my
inbox :/

Anyway, looks good!

Reviewed-by: Andi Shyti <andi.shyti@kernel.org> 

Thank you, Jinjie for following up.
Andi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH -next v3] I2C: Make return value check more accurate and explicit for devm_pinctrl_get()

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 993 bytes --]

On Mon, Aug 21, 2023 at 11:29:14AM +0800, Jinjie Ruan wrote:
> If pinctrl is not available (thus devm_pinctrl_get() returns NULL) then
> recovery can't work, because we can't switch the I2C pins between the
> I2C controller and GPIO. So, it is quite correct to print
> "can't get pinctrl, bus recovery not supported" because the I2C bus
> can't be recovered without pinctrl.
> 
> The PTR_ERR() is also fine - because if pinctrl is not present and
> returns NULL, we'll end up returning zero, which is exactly what we
> want.
> 
> However, open code that with a more accurate message will be more explicit
> for NULL case when CONFIG_PINCTRL is not defined.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>

Applied to for-next, thanks!


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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel