[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 at rock-chips.com> 
>> 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
>>
> 
>