Re: [PATCH] i2c: rk3x: Disable the controller between transfers

[David Wu]

Hi Alexey,

There's no deterministic way to reproduce this, looping suspend/resume 
seems to be able to trigger it eventually. The problem case is IEN != 0 
while pclk is already disabled: the irq handler doesn't re-enable pclk, 
so it can't actually clear the interrupt, and you get "irq in 
STATE_IDLE" printing forever. So the extra IEN write here is just a 
belt-and-suspenders safeguard.

在 2026/10/20 08:48, Shawn Lin 写道:
> Hi
>
> 在 2026/10/20 星期二 0:50, Alexey Charkov 写道:
> > Hi Shawn,
> >
> > On Fri, Oct 16, 2026 at 4:33 AM Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> 
> > wrote:
> > > Cc: + David
> > >
> > > 在 2026/10/15 星期四 22:00, Alexey Charkov 写道:
> > > > The driver leaves the controller enabled once a transfer completes, and
> > > > an enabled controller keeps tracking the bus. If the lines collapse 
> > > > under
> > > > it while idle, as happens when a target's supply is switched off while
> > > > its bus pull-ups stay powered, its state machine wedges driving SDA 
> > > > low.
> > > > Every later transfer then times out, since the timeout path only 
> > > > requests
> > > > a STOP and never disables the controller, and the stuck state even
> > > > survives a kexec.
> > > >
> > > > Clear the enable bit once each transfer is done, successful or not, so
> > > > that the controller does not watch an idle bus and starts every 
> > > > transfer
> > > > from a clean state.
> > >
> > > I checked the vendor kernel tree and saw another case that you perhaps
> > > need to fold something similar below into your patch, along with your
> > > disabling controller?
> > >
> > >       In some case，log like this:
> > >
> > >       [   12.393926] rk3x-i2c ff150000.i2c: irq in STATE_IDLE, ipd = 
> > > 0x51
> > >       [   12.416592] rk3x-i2c ff150000.i2c: irq in STATE_IDLE, ipd = 
> > > 0x51
> > >
> > >       The i2c clock is disabled, so the pending irq clean is not
> > >       worked. Disable the interrupt after the i2c jobs were done,
> > >       the error log would not happen.
> > >
> > >
> > > /* Disable i2c all irqs */
> > > #define IEN_ALL_DISABLE   0
> > >
> > > i2c_writel(i2c, IEN_ALL_DISABLE, REG_IEN);i2c_writel(i2c,
> > > IEN_ALL_DISABLE, REG_IEN);
> >
> > Thanks for looking into this! Do you happen to have a reproducer or
> > any additional info on how this state gets triggered? I haven't run
> > into it in my setups - did I just get lucky? :)
>
> Unfortunately, I don't. That came from what I found when checking
> the vendor kernel commit to see how we handled your case.
>
> So I have looped David who made the internal submision for the same case
> as yours 8 years ago and hope him could have a comment here.
>
> > I see the same write of 0 to REG_IEN just above my changed lines for
> > the case if (time_left == 0), which makes sense because a
> > too-long-running transfer might eventually complete and trigger the
> > interrupt with the controller disabled, unless the interrupt is
> > disabled too. For other cases I'd be interested to know what can raise
> > the interrupt.
> >
> > Best regards,
> > Alexey