Re: [PATCH v2] i2c: omap: fix IRQ storms

Andi Shyti
2025-03-11T22:26:01Z

Hi,

On Tue, Mar 11, 2025 at 07:39:47AM -0500, Nishanth Menon wrote:
> On 15:04-20250228, Andreas Kemnade wrote:
> > On the GTA04A5 writing a reset command to the gyroscope causes IRQ
> > storms because NACK IRQs are enabled and therefore triggered but not
> > acked.
> >
> > Sending a reset command to the gyroscope by
> > i2cset 1 0x69 0x14 0xb6
> > with an additional debug print in the ISR (not the thread) itself
> > causes
> >
> > [ 363.353515] i2c i2c-1: ioctl, cmd=0x720, arg=0xbe801b00
> > [ 363.359039] omap_i2c 48072000.i2c: addr: 0x0069, len: 2, flags: 0x0, stop: 1
> > [ 363.366180] omap_i2c 48072000.i2c: IRQ LL (ISR = 0x1110)
> > [ 363.371673] omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
> > [ 363.376892] omap_i2c 48072000.i2c: IRQ LL (ISR = 0x0102)
> > [ 363.382263] omap_i2c 48072000.i2c: IRQ LL (ISR = 0x0102)
> > [ 363.387664] omap_i2c 48072000.i2c: IRQ LL (ISR = 0x0102)
> > repeating till infinity
> > [...]
> > (0x2 = NACK, 0x100 = Bus free, which is not enabled)
> > Apparently no other IRQ bit gets set, so this stalls.
> >
> > Do not ignore enabled interrupts and make sure they are acked.
> > If the NACK IRQ is not needed, it should simply not enabled, but
> > according to the above log, caring about it is necessary unless
> > the Bus free IRQ is enabled and handled. The assumption that is
> > will always come with a ARDY IRQ, which was the idea behind
> > ignoring it, proves wrong.
> > It is true for simple reads from an unused address.
> >
> > To still avoid the i2cdetect trouble which is the reason for
> > commit c770657bd261 ("i2c: omap: Fix standard mode false ACK readings"),
> > avoid doing much about NACK in omap_i2c_xfer_data() which is used
> > by both IRQ mode and polling mode, so also the false detection fix
> > is extended to polling usage and IRQ storms are avoided.
> >
> > By changing this, the hardirq handler is not needed anymore to filter
> > stuff.
> >
> > The mentioned gyro reset now just causes a -ETIMEDOUT instead of
> > hanging the system.
> >
> > Fixes: c770657bd261 ("i2c: omap: Fix standard mode false ACK readings").
> > CC: <stable@kernel.org>
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> > This needs at least to be tested on systems where false acks were
> > detected.
>
> At least on BeaglePlay, I have not been able to reproduce the original
> bug which was the trigger for commit c770657bd261
>
> I also ran basic boot tests on other K3 platforms and none seem to show
> regressions at the very least.
>
> Tested-by: Nishanth Menon <nm@ti.com>

Thanks for testing it! I asked some OMAP folks to check this
patch, but no one took action. With Nishanth's test, I can now
sleep soundly. :-)

Merged to i2c/i2c-host-fixes.

Thanks,
Andi