[PATCH 0/2] i2c: omap: 2 Fixes

[PATCH 0/2] i2c: omap: 2 Fixes

[Christophe JAILLET]

This small serie is a follow-up of [1].

For an unkwown reason I don't feel really confident with these patches.
Maybe because one is really old and the other one is related to
pm, which ordering is sometimes tricky (from my PoV at least).

They are compile tested only.

So review with care ;-)

[1]: https://lore.kernel.org/all/vhhxtsspywvuzkfgbn52hysghd6tdxhk32wv3wcnlqwhskto3f@h2bbhek3s4s3/

Christophe JAILLET (2):
  i2c: omap: Handle omap_i2c_init() errors in omap_i2c_probe()
  i2c: omap: Fix an error handling path in omap_i2c_probe()

 drivers/i2c/busses/i2c-omap.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.50.0

[PATCH 1/2] i2c: omap: Handle omap_i2c_init() errors in omap_i2c_probe()

[Christophe JAILLET]

omap_i2c_init() can fail. Handle this error in omap_i2c_probe().

Fixes: 010d442c4a29 ("i2c: New bus driver for TI OMAP boards")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
---
 drivers/i2c/busses/i2c-omap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 8b01df3cc8e9..485313d872e5 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1472,7 +1472,11 @@ omap_i2c_probe(struct platform_device *pdev)
 	}
 
 	/* reset ASAP, clearing any IRQs */
-	omap_i2c_init(omap);
+	r = omap_i2c_init(omap);
+	if (r) {
+		dev_err(omap->dev, "failure to initialize i2c: %d\n", r);
+		goto err_mux_state_deselect;
+	}
 
 	if (omap->rev < OMAP_I2C_OMAP1_REV_2)
 		r = devm_request_irq(&pdev->dev, omap->irq, omap_i2c_omap1_isr,
@@ -1515,6 +1519,7 @@ omap_i2c_probe(struct platform_device *pdev)
 
 err_unuse_clocks:
 	omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
+err_mux_state_deselect:
 	if (omap->mux_state)
 		mux_state_deselect(omap->mux_state);
 err_put_pm:
-- 
2.50.0

[PATCH 2/2] i2c: omap: Fix an error handling path in omap_i2c_probe()

[Christophe JAILLET]

If an error occurs after pm_runtime_use_autosuspend(), a corresponding
pm_runtime_dont_use_autosuspend() should be called.

In case of error in pm_runtime_resume_and_get(), it is not the case because
the error handling path is wrongly ordered.
Fix it.

Fixes: 780f62974125 ("i2c: omap: fix reference leak when pm_runtime_get_sync fails")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
---
 drivers/i2c/busses/i2c-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 485313d872e5..ef1193e0e62d 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1523,9 +1523,9 @@ omap_i2c_probe(struct platform_device *pdev)
 	if (omap->mux_state)
 		mux_state_deselect(omap->mux_state);
 err_put_pm:
-	pm_runtime_dont_use_autosuspend(omap->dev);
 	pm_runtime_put_sync(omap->dev);
 err_disable_pm:
+	pm_runtime_dont_use_autosuspend(omap->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return r;
-- 
2.50.0

Re: [PATCH 0/2] i2c: omap: 2 Fixes

[Andi Shyti]

Hi Christophe,

> Christophe JAILLET (2):
>   i2c: omap: Handle omap_i2c_init() errors in omap_i2c_probe()
>   i2c: omap: Fix an error handling path in omap_i2c_probe()

they both make sense to me. It would be nice to have a comment on
the first patch from the Omap guys who have been very silent
whenever I asked for help on reviews.

I will apply them and if any concerns comes, we have time until
the pull request to revert.

Thanks,
Andi

Re: [PATCH 1/2] i2c: omap: Handle omap_i2c_init() errors in omap_i2c_probe()

[Aaro Koskinen]

Hi,

On Sat, Jul 05, 2025 at 09:57:37AM +0200, Christophe JAILLET wrote:
> omap_i2c_init() can fail. Handle this error in omap_i2c_probe().
> 
> Fixes: 010d442c4a29 ("i2c: New bus driver for TI OMAP boards")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only.
> ---
>  drivers/i2c/busses/i2c-omap.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 8b01df3cc8e9..485313d872e5 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1472,7 +1472,11 @@ omap_i2c_probe(struct platform_device *pdev)
>  	}
>  
>  	/* reset ASAP, clearing any IRQs */
> -	omap_i2c_init(omap);
> +	r = omap_i2c_init(omap);
> +	if (r) {
> +		dev_err(omap->dev, "failure to initialize i2c: %d\n", r);

Error paths in omap_i2c_init already print a message and error code,
so this is log is redundant.

A.

> +		goto err_mux_state_deselect;
> +	}
>  
>  	if (omap->rev < OMAP_I2C_OMAP1_REV_2)
>  		r = devm_request_irq(&pdev->dev, omap->irq, omap_i2c_omap1_isr,
> @@ -1515,6 +1519,7 @@ omap_i2c_probe(struct platform_device *pdev)
>  
>  err_unuse_clocks:
>  	omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
> +err_mux_state_deselect:
>  	if (omap->mux_state)
>  		mux_state_deselect(omap->mux_state);
>  err_put_pm:
> -- 
> 2.50.0
> 
> 

Re: [PATCH 1/2] i2c: omap: Handle omap_i2c_init() errors in omap_i2c_probe()

[Andi Shyti]

Hi Aaro,

> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index 8b01df3cc8e9..485313d872e5 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -1472,7 +1472,11 @@ omap_i2c_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	/* reset ASAP, clearing any IRQs */
> > -	omap_i2c_init(omap);
> > +	r = omap_i2c_init(omap);
> > +	if (r) {
> > +		dev_err(omap->dev, "failure to initialize i2c: %d\n", r);
> 
> Error paths in omap_i2c_init already print a message and error code,
> so this is log is redundant.

Good point! I will take care of it, no need to send a v2.

Thanks,
Andi

Re: [PATCH 2/2] i2c: omap: Fix an error handling path in omap_i2c_probe()

[Andreas Kemnade]

Am Sat,  5 Jul 2025 09:57:38 +0200
schrieb Christophe JAILLET <christophe.jaillet@wanadoo.fr>:

> If an error occurs after pm_runtime_use_autosuspend(), a corresponding
> pm_runtime_dont_use_autosuspend() should be called.
> 
> In case of error in pm_runtime_resume_and_get(), it is not the case because
> the error handling path is wrongly ordered.
> Fix it.
> 
> Fixes: 780f62974125 ("i2c: omap: fix reference leak when pm_runtime_get_sync fails")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: Andreas Kemnade <andreas@kemnade.info>

> ---
> Compile tested only.
> ---
>  drivers/i2c/busses/i2c-omap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 485313d872e5..ef1193e0e62d 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1523,9 +1523,9 @@ omap_i2c_probe(struct platform_device *pdev)
>  	if (omap->mux_state)
>  		mux_state_deselect(omap->mux_state);
>  err_put_pm:
> -	pm_runtime_dont_use_autosuspend(omap->dev);
>  	pm_runtime_put_sync(omap->dev);
>  err_disable_pm:
> +	pm_runtime_dont_use_autosuspend(omap->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
>  	return r;

Re: [PATCH 1/2] i2c: omap: Handle omap_i2c_init() errors in omap_i2c_probe()

[Andreas Kemnade]

Am Wed, 9 Jul 2025 21:25:28 +0300
schrieb Aaro Koskinen <aaro.koskinen@iki.fi>:

> Hi,
> 
> On Sat, Jul 05, 2025 at 09:57:37AM +0200, Christophe JAILLET wrote:
> > omap_i2c_init() can fail. Handle this error in omap_i2c_probe().
> > 
> > Fixes: 010d442c4a29 ("i2c: New bus driver for TI OMAP boards")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > Compile tested only.
> > ---
> >  drivers/i2c/busses/i2c-omap.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index 8b01df3cc8e9..485313d872e5 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -1472,7 +1472,11 @@ omap_i2c_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	/* reset ASAP, clearing any IRQs */
> > -	omap_i2c_init(omap);
> > +	r = omap_i2c_init(omap);
> > +	if (r) {
> > +		dev_err(omap->dev, "failure to initialize i2c: %d\n", r);  
> 
> Error paths in omap_i2c_init already print a message and error code,
> so this is log is redundant.
> 
And I have never seen these in normal operation, so adding that error
check should be safe.

Regards,
Andreas