git: 04622a7f2115 - main - twsi: move handling of TWSI_CONTROL_ACK into the state machine

The branch main has been updated by avg:

URL: https://cgit.FreeBSD.org/src/commit/?id=04622a7f21157827fe75276a4520a52d3a571a86

commit 04622a7f21157827fe75276a4520a52d3a571a86
Author:     Andriy Gapon <avg@FreeBSD.org>
AuthorDate: 2021-11-26 08:07:27 +0000
Commit:     Andriy Gapon <avg@FreeBSD.org>
CommitDate: 2021-11-26 14:17:24 +0000

    twsi: move handling of TWSI_CONTROL_ACK into the state machine
    
    Previously the code set TWSI_CONTROL_ACK in twsi_transfer() based on
    whether the first message had a length of one.  That was done regardless
    of whether the message was a read or write and what kind of messages
    followed it.
    Now the bit is set or cleared while handling TWSI_STATUS_ADDR_R_ACK
    state transition based on the current (read) message.
    
    The old code did not correctly work in a scenario where a single byte
    was read from an EEPROM device with two byte addressing.
    For example:
        i2c -m tr -a 0x50 -d r -w 16 -o 0 -c 1 -v
    The reason is that the first message (a write) has two bytes, so
    TWSI_CONTROL_ACK was set and never cleared.
    Since the controller did not send NACK the EEPROM sent more data resulting
    in a buffer overrun.
    
    While working on TWSI_STATUS_ADDR_R_ACK I also added support for
    the zero-length read access and then I did the same for zero-length write
    access.
    While rare, those types of I2C transactions are completely valid and are
    used by some devices.
    
    PR:             258994
    MFC after:      3 weeks
---
 sys/dev/iicbus/twsi/twsi.c | 40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/sys/dev/iicbus/twsi/twsi.c b/sys/dev/iicbus/twsi/twsi.c
index ecadfade04d9..4a20d7ea35b6 100644
--- a/sys/dev/iicbus/twsi/twsi.c
+++ b/sys/dev/iicbus/twsi/twsi.c
@@ -516,11 +516,9 @@ twsi_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
 	for (int i = 0; i < nmsgs; i++)
 		debugf(sc, "msg %d is %d bytes long\n", i, msgs[i].len);
 #endif
+
 	/* Send start and re-enable interrupts */
-	sc->control_val = TWSI_CONTROL_TWSIEN |
-		TWSI_CONTROL_INTEN | TWSI_CONTROL_ACK;
-	if (sc->msgs[0].len == 1)
-		sc->control_val &= ~TWSI_CONTROL_ACK;
+	sc->control_val = TWSI_CONTROL_TWSIEN | TWSI_CONTROL_INTEN;
 	TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_START);
 	msleep_sbt(sc, &sc->mutex, 0, "twsi", 3000 * SBT_1MS, SBT_1MS, 0);
 	debugf(sc, "pause finish\n");
@@ -597,22 +595,36 @@ twsi_intr(void *arg)
 		break;
 
 	case TWSI_STATUS_ADDR_W_ACK:
-		debugf(sc, "Ack received after transmitting the address (write)\n");
-		/* Directly send the first byte */
-		sc->sent_bytes = 1;
-		debugf(sc, "Sending byte 0 (of %d) = %x\n",
-		    sc->msgs[sc->msg_idx].len,
-		    sc->msgs[sc->msg_idx].buf[0]);
-		TWSI_WRITE(sc, sc->reg_data, sc->msgs[sc->msg_idx].buf[0]);
+		debugf(sc, "Address ACK-ed (write)\n");
 
-		TWSI_WRITE(sc, sc->reg_control, sc->control_val);
+		if (sc->msgs[sc->msg_idx].len > 0) {
+			/* Directly send the first byte */
+			sc->sent_bytes = 1;
+			debugf(sc, "Sending byte 0 (of %d) = %x\n",
+			    sc->msgs[sc->msg_idx].len,
+			    sc->msgs[sc->msg_idx].buf[0]);
+			TWSI_WRITE(sc, sc->reg_data,
+			    sc->msgs[sc->msg_idx].buf[0]);
+		} else {
+			debugf(sc, "Zero-length write, sending STOP\n");
+			TWSI_WRITE(sc, sc->reg_control,
+			    sc->control_val | TWSI_CONTROL_STOP);
+		}
 		break;
 
 	case TWSI_STATUS_ADDR_R_ACK:
-		debugf(sc, "Ack received after transmitting the address (read)\n");
+		debugf(sc, "Address ACK-ed (read)\n");
 		sc->recv_bytes = 0;
 
-		TWSI_WRITE(sc, sc->reg_control, sc->control_val);
+		if (sc->msgs[sc->msg_idx].len == 0) {
+			debugf(sc, "Zero-length read, sending STOP\n");
+			TWSI_WRITE(sc, sc->reg_control,
+			    sc->control_val | TWSI_CONTROL_STOP);
+		} else if (sc->msgs[sc->msg_idx].len == 1) {
+			sc->control_val &= ~TWSI_CONTROL_ACK;
+		} else {
+			sc->control_val |= TWSI_CONTROL_ACK;
+		}
 		break;
 
 	case TWSI_STATUS_ADDR_W_NACK: