Hi Aniket,
On Wed, Mar 12, 2025 at 02:55:38PM +0530, Aniket Limaye wrote:
> On 12/03/25 03:55, Andi Shyti wrote:
> > 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@xxxxxxxxxx>
> > > > Signed-off-by: Andreas Kemnade <andreas@xxxxxxxxxxxx>
> > > > ---
> > > > 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@xxxxxx>
> >
> > 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
> >
>
> I see that the patch got merged so don't know if this is useful at all at
> this point, but yeah looks good to me. Apologies for the slow response.
> Nishanth, Thanks for testing it too!
>
> Reviewed-by: Aniket Limaye <a-limaye@xxxxxx>
thanks for your review, I added it.
Andi
