port-arm/60912: ti_iic: transfers to absent i2c addresses report success

["trythepostoffice%gmail.com@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60912
>Category:       port-arm
>Synopsis:       ti_iic: transfers to absent i2c addresses report success
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-arm-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 20 22:25:00 +0000 2026
>Originator:     Elias Howell
>Release:        NetBSD 11.0, NetBSD-current 11.99.9 (20261019)
>Organization:
>Environment:
NetBSD armv7 11.0 NetBSD 11.0 (GENERIC) #0: Thu Jul 30 15:23:12 UTC 2026 mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/evbarm/compile/GENERIC evbarm
NetBSD armv7 11.99.9 NetBSD 11.99.9 (GENERIC) #0: Mon Oct 19 09:38:27 UTC 2026  mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/evbarm/compile/GENERIC evbarm
>Description:
On AM335x ti_iic(4) returns success for a lot of transfers to
addresses where nothing answers. Writes return 0 and reads return 0
with a buffer that never got filled.

The SG-1000 only has the EEPROM at 0x50 on I2C0. U-Boot's "i2c probe"
finds only 0x50 and "i2c md" at 0x24, 0x28 and 0x70 fails with "error
waiting for addr ACK (status=0x116)". On 11.0 GENERIC, 20 tries of
each kind of transfer at those three addresses through I2C_IOCTL_EXEC

  write 1 byte + stop          60 of 60 return 0
  read 1 byte (no command)     56 of 60 return 0
  write reg, repeated start,
    read 1 byte                60 of 60 return 0

The counts change from run to run since it's a race. Reads from the
EEPROM at 0x50 are fine (50 of 50). "i2cscan -r /dev/iic0" finds 102
devices.

On -current with tps@24 enabled in the dtb, tps65217pmic(4) at 0x24,
a PMIC this board doesn't have, sometimes "reads" its status register
and gets its uninitialised stack variable back, so it says AC and USB
are online then offline on the next read.

The race is in ti_iic_handle_intr(). With printfs in the interrupt
path a write to an absent address goes

  intr  op WRITE  stat 0x5002 (XDR|BB|NACK)  -> sc_op = TI_I2CERROR
  intr  op ERROR  stat 0x0004 (ARDY)         -> sc_op = TI_I2CDONE
  ti_iic_op() returns 0

NACK and ARDY come in as separate interrupts. The first one sets
TI_I2CERROR and wakes the waiter in ti_iic_op(). ti_iic_handle_intr()
acts on ARDY no matter what state the transfer is in, so if the
second interrupt gets handled before the waiter gets sc_mtx back the
error turns into TI_I2CDONE. Reads look the same (0x1002 then
0x0004). Some NACKs still get reported because it depends on who
wins.

Linux hit the same NACK then ARDY ordering in i2c-omap.c, see
c770657bd261 and 285df995f90e. The patch here still ends the
transfer on NACK so it doesn't depend on ARDY showing up after one,
and ti_iic_intr() still writes back every status bit it read even
when ti_iic_handle_intr() returns early so nothing is left pending.

Not changed by patch. The "clear any pending interrupt" step in
ti_iic_op() reads I2C_IRQSTATUS, the masked status, with all
interrupts disabled so it always reads 0 and clears nothing (45 of 45
in the trace). The only thing I saw left over was BF (0x100) which
isn't in the mask so no harm here, but I2C_IRQSTATUS_RAW is probably
what the comment means.
>How-To-Repeat:
On an AM335x board do a register read at an address with nothing on
it for example a small I2C_IOCTL_EXEC program
(I2C_OP_READ_WITH_STOP, 1 command byte, 1 data byte) at 0x28 on
iic0, 20 times. They should all fail with EIO. Most return 0.
>Fix:
Ignore status once the transfer is over. Tested on the SG-1000
-current b91976112 plus this and my other patches. None of the others
touch ti_iic. The test kernel had this check behind a sysctl plus the
printfs above. The patch below is just the check. Same boot same 180
transfers to 0x24, 0x28 and 0x70

  check off:  114 of 180 return 0
  check on:     0 of 180 return 0 (all EIO)

The EEPROM at 0x50 read right either way
(header aa 55 33 ee 41 33 33 35, and 50 of 50 reads of register 0).

--- a/sys/arch/arm/ti/ti_iic.c
+++ b/sys/arch/arm/ti/ti_iic.c
@@ -563,6 +563,15 @@ ti_iic_handle_intr(struct ti_iic_softc *sc, uint32_t stat)
        KASSERT(stat != 0);
        DPRINTF(("ti_iic_handle_intr stat %#x\n", stat));

+       /*
+        * The transfer is already over.  On a NACK the controller
+        * interrupts with NACK first and with ARDY separately after
+        * that.  Don't let the late ARDY turn TI_I2CERROR into
+        * TI_I2CDONE.
+        */
+       if (sc->sc_op != TI_I2CREAD && sc->sc_op != TI_I2CWRITE)
+               return;
+
        if (stat &
            (I2C_IRQSTATUS_NACK|I2C_IRQSTATUS_AL)) {
                sc->sc_op = TI_I2CERROR;