aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndriy Gapon <avg@FreeBSD.org>2021-11-26 08:07:27 +0000
committerAndriy Gapon <avg@FreeBSD.org>2021-11-26 14:17:24 +0000
commit04622a7f21157827fe75276a4520a52d3a571a86 (patch)
tree9516e74b24267ac0ff09e7247c1f841a97ed52af
parenta4fe8922085dfc1e5e6a6bac73ccc738e373165f (diff)
downloadsrc-04622a7f21157827fe75276a4520a52d3a571a86.tar.gz
src-04622a7f21157827fe75276a4520a52d3a571a86.zip
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
-rw-r--r--sys/dev/iicbus/twsi/twsi.c40
1 files 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: