[PATCH] i2c: imx-lpi2c: Fix clock count when probe defers

[Carlos Song]

> -----Original Message-----
> From: Andi Shyti <andi.shyti at kernel.org>
> Sent: Tuesday, April 29, 2025 3:17 AM
> To: Carlos Song <carlos.song at nxp.com>
> Cc: Aisheng Dong <aisheng.dong at nxp.com>; Frank Li <frank.li at nxp.com>;
> shawnguo at kernel.org; s.hauer at pengutronix.de; kernel at pengutronix.de;
> festevam at gmail.com; linux-i2c at vger.kernel.org; imx at lists.linux.dev;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: [EXT] Re: [PATCH] i2c: imx-lpi2c: Fix clock count when probe defers
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> Hi Carlos,
> 
> > rpm_disable:
> >       pm_runtime_put(&pdev->dev);
> >       pm_runtime_disable(&pdev->dev);
> >       pm_runtime_dont_use_autosuspend(&pdev->dev);
> >
> > pm_runtime_put() may not work really sometimes.
> >
> > [    3.203715] imx-lpi2c 42530000.i2c: defer probe
> > [    3.208324] imx-lpi2c 42530000.i2c: lpi2c_runtime_suspend
> > [    3.214801] imx-lpi2c 42540000.i2c: defer probe
> > ---> (No lpi2c_runtime_suspend callback(pm_runtime_put() is not really
> > ---> wrok))
> > [    3.220672] imx-lpi2c 426c0000.i2c: defer probe
> > [    3.225248] imx-lpi2c 426c0000.i2c: lpi2c_runtime_suspend
> >
> > After apply this change:
> > rpm_disable:
> >       pm_runtime_dont_use_autosuspend(&pdev->dev);
> >       pm_runtime_put_sync(&pdev->dev);
> >       pm_runtime_disable(&pdev->dev);
> >
> > all issues gone.
> > [    3.093025] imx-lpi2c 42530000.i2c: defer probe
> > [    3.097592] imx-lpi2c 42530000.i2c: lpi2c_runtime_suspend
> > [    3.104281] imx-lpi2c 42540000.i2c: defer probe
> > [    3.108858] imx-lpi2c 42540000.i2c: lpi2c_runtime_suspend
> > [    3.115278] imx-lpi2c 426c0000.i2c: defer probe
> > [    3.119818] imx-lpi2c 426c0000.i2c: lpi2c_runtime_suspend
> 
> Thanks for the explanation. If we lose the count here, I think we also need for
> the Fixes tag, do you agree? Should we add:
> 
> Fixes: 13d6eb20fc79 ("i2c: imx-lpi2c: add runtime pm support")
> Cc: <stable at vger.kernel.org> # v4.16+
> 
> as well?
> 
> ...

Hi, Andi

Yes. You are right We should add it.

> 
> > > > Signed-off-by: Carlos Song <carlos.song at nxp.com>
> > > > Signed-off-by: Clark Wang <xiaoning.wang at nxp.com>
> > > > Signed-off-by: Jun Li <jun.li at nxp.com>
> > > > Signed-off-by: Haibo Chen <haibo.chen at nxp.com>
> > >
> > > Carlo's SoB should be at the end of the chain. Should be nice to
> > > know what these are, though, are they co-developed-by? tested-by?
> > > Why so many SoB's?
> > >
> >
> > This patch author is not me and they meet similar issue at some
> > boards(Vague history), now I meet this issue at new SOC again. I think this local
> patch is helpful and looks reasonable.
> > So I send the patch to community adding my SoB.
> 
> So Clark has authored the patch and you have sent it. And your SoB makes sense
> (even though, your SoB shoud be placed at the end, because you sent the
> patch).
> 
> Juan and Haibo have tested it? Reported it?
> 

They are local reviewer. So I change their Reviewed-by: to Signed-off-by:.
Simply you can remove Jun and Haibo tags only keep two tags:

Signed-off-by: Carlos Song <carlos.song at nxp.com>
Signed-off-by: Clark Wang <xiaoning.wang at nxp.com>

Thank you!

> A practical rule of thumb for tags is that they are placed in chronological order,
> e.g.:
> 
>  Reported-by: <reporter of the bug>
>  Fixes: <commit that introduced the bug>
>  Closes: <link where the bug has been reported>
>  Co-developed-by: <co-developer>
>  Signed-off-by: <co-developer>
>  Signed-off-by: <developer>
>  Cc: <people who should be aware of the patch>
>  Tested-by: <tester>
>  Reviewed-by: <reviewer>
>  Acked-by: <someone who agrees>
>  Signed-off-by: <patch sender>
>  Link: <lore link>
>  Signed-off-by: <maintainer who applies the patch>
> 
> If you notice, more or less things are in chronological order. I hope this makes it
> a bit clearer.
> 
> Andi