linux-kernel - Re: [PATCH v2] i2c: omap: fix IRQ storms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <k6rzimh2l72xpwpjecrmuqbmfv7pgzpp5uxysqdwvhwhakq4hu@pewez7dbb3mn>
Date: Wed, 12 Mar 2025 12:26:30 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Aniket Limaye <a-limaye@...com>
Cc: Nishanth Menon <nm@...com>, Andreas Kemnade <andreas@...nade.info>, 
	vigneshr@...com, aaro.koskinen@....fi, khilman@...libre.com, rogerq@...nel.org, 
	tony@...mide.com, jmkrzyszt@...il.com, reidt@...com, wsa@...nel.org, 
	linux-omap@...r.kernel.org, linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org, 
	stable@...nel.org
Subject: Re: [PATCH v2] i2c: omap: fix IRQ storms

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@...nel.org>
> > > > Signed-off-by: Andreas Kemnade <andreas@...nade.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@...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
> > 
> 
> 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@...com>

thanks for your review, I added it.

Andi