diff mbox series

[1/1] i2c: imx: Make SDA actually optional for bus recovering

Message ID 20231130095751.65773-1-alexander.stein@ew.tq-group.com
State Accepted
Delegated to: Andi Shyti
Headers show
Series [1/1] i2c: imx: Make SDA actually optional for bus recovering | expand

Commit Message

Alexander Stein Nov. 30, 2023, 9:57 a.m. UTC
Both i2c_generic_scl_recovery() and the debug output indicate that SDA is
purely optional for bus recovery. But devm_gpiod_get() never returns NULL
making it mandatory. Fix this my calling devm_gpiod_get_optional instead.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
 drivers/i2c/busses/i2c-imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Uwe Kleine-K├Ânig Nov. 30, 2023, 3:51 p.m. UTC | #1
On Thu, Nov 30, 2023 at 10:57:51AM +0100, Alexander Stein wrote:
> Both i2c_generic_scl_recovery() and the debug output indicate that SDA is
> purely optional for bus recovery. But devm_gpiod_get() never returns NULL
> making it mandatory. Fix this my calling devm_gpiod_get_optional instead.

s/my/by/

Best regards
Uwe
Andi Shyti Dec. 3, 2023, 10:04 p.m. UTC | #2
Hi,

On Thu, Nov 30, 2023 at 10:57:51AM +0100, Alexander Stein wrote:
> Both i2c_generic_scl_recovery() and the debug output indicate that SDA is
> purely optional for bus recovery. But devm_gpiod_get() never returns NULL
> making it mandatory. Fix this my calling devm_gpiod_get_optional instead.
                               ^^^^
as Uwe pointed out, /my/by/.

I think Wolfram can fix this before taking it.

> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

otherwise:

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

Andi
Alexander Stein Dec. 4, 2023, 8:35 a.m. UTC | #3
Hi,

Am Sonntag, 3. Dezember 2023, 23:04:44 CET schrieb Andi Shyti:
> Hi,
> 
> On Thu, Nov 30, 2023 at 10:57:51AM +0100, Alexander Stein wrote:
> > Both i2c_generic_scl_recovery() and the debug output indicate that SDA is
> > purely optional for bus recovery. But devm_gpiod_get() never returns NULL
> > making it mandatory. Fix this my calling devm_gpiod_get_optional instead.
> 
>                                ^^^^
> as Uwe pointed out, /my/by/.
> 
> I think Wolfram can fix this before taking it.
> 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> otherwise:
> 
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org>

Thanks. Let me know if I should send a v2.

Best regards,
Alexander
Oleksij Rempel Dec. 4, 2023, 11:04 a.m. UTC | #4
On Thu, Nov 30, 2023 at 10:57:51AM +0100, Alexander Stein wrote:
> Both i2c_generic_scl_recovery() and the debug output indicate that SDA is
> purely optional for bus recovery. But devm_gpiod_get() never returns NULL
> making it mandatory. Fix this my calling devm_gpiod_get_optional instead.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

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

> ---
>  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 1775a79aeba2a..88a053987403c 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1401,7 +1401,7 @@ static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
>  			PINCTRL_STATE_DEFAULT);
>  	i2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(i2c_imx->pinctrl,
>  			"gpio");
> -	rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
> +	rinfo->sda_gpiod = devm_gpiod_get_optional(&pdev->dev, "sda", GPIOD_IN);
>  	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
>  
>  	if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER ||
> -- 
> 2.34.1
> 
>
Wolfram Sang Dec. 19, 2023, 5:17 p.m. UTC | #5
On Mon, Dec 04, 2023 at 09:35:00AM +0100, Alexander Stein wrote:
> Hi,
> 
> Am Sonntag, 3. Dezember 2023, 23:04:44 CET schrieb Andi Shyti:
> > Hi,
> > 
> > On Thu, Nov 30, 2023 at 10:57:51AM +0100, Alexander Stein wrote:
> > > Both i2c_generic_scl_recovery() and the debug output indicate that SDA is
> > > purely optional for bus recovery. But devm_gpiod_get() never returns NULL
> > > making it mandatory. Fix this my calling devm_gpiod_get_optional instead.
> > 
> >                                ^^^^
> > as Uwe pointed out, /my/by/.
> > 
> > I think Wolfram can fix this before taking it.
> > 
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > 
> > otherwise:
> > 
> > Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
> 
> Thanks. Let me know if I should send a v2.

I fixed it up and applied to for-next, thanks!
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 1775a79aeba2a..88a053987403c 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1401,7 +1401,7 @@  static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
 			PINCTRL_STATE_DEFAULT);
 	i2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(i2c_imx->pinctrl,
 			"gpio");
-	rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_IN);
+	rinfo->sda_gpiod = devm_gpiod_get_optional(&pdev->dev, "sda", GPIOD_IN);
 	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_OUT_HIGH_OPEN_DRAIN);
 
 	if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER ||