git: c7bf34cfe52c - stable/13 - twsi: move handling of TWSI_CONTROL_ACK into the state machine
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 17 Dec 2021 07:31:11 UTC
The branch stable/13 has been updated by avg:
URL: https://cgit.FreeBSD.org/src/commit/?id=c7bf34cfe52c09abf813c784a60d7d9677afb603
commit c7bf34cfe52c09abf813c784a60d7d9677afb603
Author: Andriy Gapon <avg@FreeBSD.org>
AuthorDate: 2021-11-26 08:07:27 +0000
Commit: Andriy Gapon <avg@FreeBSD.org>
CommitDate: 2021-12-17 07:30:41 +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
(cherry picked from commit 04622a7f21157827fe75276a4520a52d3a571a86)
---
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: