[PATCH 0/4] i2c: devm_pinctrl_get() usage fixes

[PATCH 0/4] i2c: devm_pinctrl_get() usage fixes

[Yann Sionneau]

Most of the patches just remove the check for NULL for devm_pinctrl_get()
return value since it cannot return NULL.

One patch also sets back rinfo->pinctrl to NULL when devm_pinctrl_get()
returns an error instead of a valid pointer and the driver does not bail
out.
This last change prevents core i2c code to dereference invalid pointer
because it checks for rinfo->pinctrl validity with "if (rinfo->pinctrl)"
before dereferencing it.

Yann Sionneau (4):
  i2c: mv64xxx: devm_pinctrl_get() cannot return NULL
  i2c: at91-master: devm_pinctrl_get() cannot return NULL
  i2c: i2c-cadence: Reset pinctrl to NULL in case devm_pinctrl_get()
    fails
  i2c: imx: devm_pinctrl_get() cannot return NULL

 drivers/i2c/busses/i2c-at91-master.c | 2 +-
 drivers/i2c/busses/i2c-cadence.c     | 1 +
 drivers/i2c/busses/i2c-imx.c         | 2 +-
 drivers/i2c/busses/i2c-mv64xxx.c     | 2 --
 4 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/4] i2c: mv64xxx: devm_pinctrl_get() cannot return NULL

[Yann Sionneau]

Remove unnecessary check against NULL for devm_pinctrl_get() return value.

Signed-off-by: Yann Sionneau <yann@sionneau.net>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index fd8403b07fa6..0c4a0110978f 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -947,8 +947,6 @@ static int mv64xxx_i2c_init_recovery_info(struct mv64xxx_i2c_data *drv_data,
 			return -EPROBE_DEFER;
 		dev_info(dev, "can't get pinctrl, bus recovery not supported\n");
 		return PTR_ERR(rinfo->pinctrl);
-	} else if (!rinfo->pinctrl) {
-		return -ENODEV;
 	}
 
 	drv_data->adapter.bus_recovery_info = rinfo;
-- 
2.34.1

[PATCH 2/4] i2c: at91-master: devm_pinctrl_get() cannot return NULL

[Yann Sionneau]

Remove unnecessary check against NULL for devm_pinctrl_get() return value.

Signed-off-by: Yann Sionneau <yann@sionneau.net>
---
 drivers/i2c/busses/i2c-at91-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index c0c35785a0dc..b598d2439eb1 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -832,7 +832,7 @@ 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 (IS_ERR(rinfo->pinctrl)) {
 		dev_info(dev->dev, "can't get pinctrl, bus recovery not supported\n");
 		return PTR_ERR(rinfo->pinctrl);
 	}
-- 
2.34.1

[PATCH 3/4] i2c: i2c-cadence: Reset pinctrl to NULL in case devm_pinctrl_get() fails

[Yann Sionneau]

The core i2c code generally checks if pinctrl has been properly initialized
by just doing 'if (p) some_pinctrl_function(p)'.
In case devm_pinctrl_get() fails, pinctrl variable contains an error
instead of a valid pointer.
To prevent core i2c code to crash, reset back pinctrl pointer to NULL when
this happens.

Signed-off-by: Yann Sionneau <yann@sionneau.net>
---
 drivers/i2c/busses/i2c-cadence.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-cadence.c b/drivers/i2c/busses/i2c-cadence.c
index 9849f4502570..aa9ae0755674 100644
--- a/drivers/i2c/busses/i2c-cadence.c
+++ b/drivers/i2c/busses/i2c-cadence.c
@@ -1299,6 +1299,7 @@ static int cdns_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(id->rinfo.pinctrl)) {
 		int err = PTR_ERR(id->rinfo.pinctrl);
 
+		id->rinfo.pinctrl = NULL;
 		dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
 		if (err != -ENODEV)
 			return err;
-- 
2.34.1

[PATCH 4/4] i2c: imx: devm_pinctrl_get() cannot return NULL

[Yann Sionneau]

Remove unnecessary check against NULL for devm_pinctrl_get() return value.

Signed-off-by: Yann Sionneau <yann@sionneau.net>
---
 drivers/i2c/busses/i2c-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 65128a73e8a3..502276e8ded5 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1389,7 +1389,7 @@ 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 (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

Re: [PATCH 4/4] i2c: imx: devm_pinctrl_get() cannot return NULL

[Oleksij Rempel]

On Wed, Aug 16, 2023 at 10:04:10PM +0200, Yann Sionneau wrote:
> Remove unnecessary check against NULL for devm_pinctrl_get() return value.
> 
> Signed-off-by: Yann Sionneau <yann@sionneau.net>
> ---
>  drivers/i2c/busses/i2c-imx.c | 2 +-

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

Thank you!

>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 65128a73e8a3..502276e8ded5 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1389,7 +1389,7 @@ 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 (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
> 
> 

-- 
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 |

Re: [PATCH 1/4] i2c: mv64xxx: devm_pinctrl_get() cannot return NULL

[Wolfram Sang]

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

On Wed, Aug 16, 2023 at 10:04:07PM +0200, Yann Sionneau wrote:
> Remove unnecessary check against NULL for devm_pinctrl_get() return value.
> 
> Signed-off-by: Yann Sionneau <yann@sionneau.net>

As discussed elsewhere, it can be NULL when pinctrl is not selected.


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

Re: [PATCH 2/4] i2c: at91-master: devm_pinctrl_get() cannot return NULL

[Hari.PrasathGE]

Thanks for your patch.Looks good to me.

Acked-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com>

On 17/08/23 1:34 am, Yann Sionneau wrote:
> [You don't often get email from yann@sionneau.net. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Remove unnecessary check against NULL for devm_pinctrl_get() return value.
> 
> Signed-off-by: Yann Sionneau <yann@sionneau.net>
> ---
>   drivers/i2c/busses/i2c-at91-master.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> index c0c35785a0dc..b598d2439eb1 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -832,7 +832,7 @@ 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 (IS_ERR(rinfo->pinctrl)) {
>                  dev_info(dev->dev, "can't get pinctrl, bus recovery not supported\n");
>                  return PTR_ERR(rinfo->pinctrl);
>          }
> --
> 2.34.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel