[PATCH] i2c: xiic: NACK the last byte of an SMBus block read

[Abdurrahman Hussain]

Commit e6fe3ea04f01 ("i2c: xiic: defer RX_FULL until all trailing bytes
are in FIFO") raised the Rx FIFO threshold for an SMBus block read whose
trailing bytes fit in the FIFO, so that the drain takes xiic_read_rx()'s
bytes_rem == 0 path. That path skips the bytes_rem == 1 branch, the only
place that sets NO_ACK, so every trailing byte is now ACKed. Having had
its last byte ACKed, the target goes on to drive the next one, and the
STOP that follows lands in the middle of it.

Some targets ride this out; others are left wedged until the next
transactions clock them free. A TDK FS1412 fails the two transfers that
follow any block read, so it never probes: pmbus reads MFR_ID as a block
and then VOUT_MODE as a byte. A Renesas RAA228246 and one RAA228244 on
the same controller show the same pattern.

Program the threshold so that RX_FULL fires with one trailing byte still
to come, letting xiic_read_rx() set NO_ACK for it on the bytes_rem == 1
path, and set NO_ACK straight away when only one trailing byte remains.

Twenty rounds per device of a block read and a PEC block read of MFR_ID,
each followed by a byte read:

               before                      after
            block  PEC   byte after     block  PEC   byte after
  FS1412    20/20  0/20  20/40 fail     20/20 20/20   0/40 fail
  RAA228246  0/20 20/20  20/40 fail     20/20 20/20   0/40 fail
  RAA228244  0/20 20/20  20/40 fail     20/20 20/20   0/40 fail

A second RAA228244 and an Infineon XDPE1A2G5B passed every round before
and after. PEC block reads, which that commit fixed, still pass.

Fixes: e6fe3ea04f01 ("i2c: xiic: defer RX_FULL until all trailing bytes are in FIFO")
Signed-off-by: Abdurrahman Hussain <abdurrahman@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # v6.3+
---
A follow-up to my xiic block-read series, now in 7.3-rc as
b7e6df2f52ed, e6fe3ea04f01 and 840d8acc8792. The second of those
stopped NACKing the final byte of a block read that fits in the Rx
FIFO, which leaves some PMBus targets wedged for the next couple of
transfers.

Tested on a 6.12 kernel carrying that series, against an AXI IIC
controller with an FS1412, RAA228244/RAA228246 and XDPE1A2G5B behind
it, using plain and PEC block reads and the byte reads that follow.

The same happens on a second board, where two of four ISL68225s behind
the same AXI IIC core fail plain block reads and the two transfers
after them. A capture of SCL/SDA taken inside the FPGA shows the
mechanism: after the last byte of an MFR_ID block read the master
drives ACK, the target starts on the next byte with SDA low, and the
STOP the master then attempts never appears, leaving SDA held low. With
this change the last byte is NACKed and the STOP is clean, and all four
pass.

With this change a block read ends with a NACK, and so with TX_ERROR,
like any other read. That puts it in the window Siddarth's patch closes,
where xiic_process() sees TX_ERROR before RX_FULL and resets the core
even though the data is already in the Rx FIFO:

  https://lore.kernel.org/linux-i2c/20261005-i2c-xiic-rx-fifo-v1-1-0177ac5f72d7@xxxxxxxxxx/

The two touch different functions and apply cleanly to v7.3-rc6 in
either order.
---
 drivers/i2c/busses/i2c-xiic.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 5e397a7e63f6..2714fd2231b4 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -568,12 +568,24 @@ static void xiic_smbus_block_read_setup(struct xiic_i2c *i2c)
 			i2c->rx_msg->len = SMBUS_BLOCK_READ_MIN_LEN;
 			i2c->smbus_actual_len = 1 + rxmsg_len + pec_len;
 		} else {
+			unsigned int tail = rxmsg_len + pec_len;
+
 			/*
-			 * All trailing bytes fit in the Rx FIFO. Defer RX_FULL
-			 * until every one of them is buffered, so the drain
-			 * takes xiic_read_rx()'s bytes_rem == 0 path.
+			 * All trailing bytes fit in the Rx FIFO. The last one
+			 * must be NACKed or the slave keeps driving SDA into
+			 * the STOP. With one byte left, NACK it now; otherwise
+			 * fire RX_FULL with one byte still to come so
+			 * xiic_read_rx() sets NACK in its bytes_rem == 1 path.
 			 */
-			rfd_set = rxmsg_len + pec_len - 1;
+			if (tail == 1) {
+				u8 cr = xiic_getreg8(i2c, XIIC_CR_REG_OFFSET);
+
+				xiic_setreg8(i2c, XIIC_CR_REG_OFFSET,
+					     cr | XIIC_CR_NO_ACK_MASK);
+				rfd_set = 0;
+			} else {
+				rfd_set = tail - 2;
+			}
 			i2c->rx_msg->len = rxmsg_len + 1 + pec_len;
 		}
 		xiic_setreg8(i2c, XIIC_RFD_REG_OFFSET, rfd_set);

---
base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
change-id: 20261007-i2c-xiic-block-read-nack-d2b735baddd3

Best regards,
--  
Abdurrahman Hussain <abdurrahman@xxxxxxxxxx>