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] [thread-next>] [day] [month] [year] [list]

Message-ID: <t43j7wmwsqvs5f6utld72enobqwkendgtpzfu3mth3bdgpxhsh@qeok5d2ujdm2>
Date: Tue, 11 Mar 2025 23:25:48 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Nishanth Menon <nm@...com>
Cc: 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,

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