The branch main has been updated by avg:
URL:
https://cgit.FreeBSD.org/src/commit/?id=00c07d9559c6197957b00811a0b29876a5c8b573
commit 00c07d9559c6197957b00811a0b29876a5c8b573
Author: Andriy Gapon <a...@freebsd.org>
AuthorDate: 2021-11-26 08:34:42 +0000
Commit: Andriy Gapon <a...@freebsd.org>
CommitDate: 2021-11-26 14:18:51 +0000
twsi: make data receiving code safer
Assert that we are not receiving data beyond the requested length.
Assert that we have not NACK-ed incoming data prematurely.
Abort the current transfer if the incoming data is NACK-ed or not
NACK-ed unexpectedly.
Add debug logging of received data to complement logging of sent data.
MFC after: 3 weeks
---
sys/dev/iicbus/twsi/twsi.c | 62 ++++++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 21 deletions(-)
diff --git a/sys/dev/iicbus/twsi/twsi.c b/sys/dev/iicbus/twsi/twsi.c
index b94396883be5..2f77e2dc69c3 100644
--- a/sys/dev/iicbus/twsi/twsi.c
+++ b/sys/dev/iicbus/twsi/twsi.c
@@ -671,37 +671,57 @@ twsi_intr(void *arg)
break;
case TWSI_STATUS_DATA_RD_ACK:
- debugf(sc, "Ack received after receiving data\n");
- sc->msgs[sc->msg_idx].buf[sc->recv_bytes++] = TWSI_READ(sc,
sc->reg_data);
- debugf(sc, "msg_len=%d recv_bytes=%d\n",
sc->msgs[sc->msg_idx].len, sc->recv_bytes);
+ debugf(sc, "Received and ACK-ed data\n");
+ KASSERT(sc->recv_bytes < sc->msgs[sc->msg_idx].len,
+ ("receiving beyond the end of buffer"));
+
+ sc->msgs[sc->msg_idx].buf[sc->recv_bytes] =
+ TWSI_READ(sc, sc->reg_data);
+ debugf(sc, "Received byte %d (of %d) = 0x%x\n",
+ sc->recv_bytes,
+ sc->msgs[sc->msg_idx].len,
+ sc->msgs[sc->msg_idx].buf[sc->recv_bytes]);
+ sc->recv_bytes++;
/* If we only have one byte left, disable ACK */
- if (sc->msgs[sc->msg_idx].len - sc->recv_bytes == 1)
+ if (sc->msgs[sc->msg_idx].len - sc->recv_bytes == 1) {
sc->control_val &= ~TWSI_CONTROL_ACK;
- if (sc->msgs[sc->msg_idx].len == sc->recv_bytes) {
- debugf(sc, "Done with msg %d\n", sc->msg_idx);
- sc->msg_idx++;
- if (sc->msg_idx == sc->nmsgs - 1) {
- debugf(sc, "No more msgs\n");
- transfer_done = 1;
- sc->error = 0;
- }
+ } else if (sc->msgs[sc->msg_idx].len == sc->recv_bytes) {
+ /*
+ * We should not have ACK-ed the last byte.
+ * The protocol state machine is in invalid state.
+ */
+ debugf(sc, "RX all but asked for more?\n");
+ twsi_error(sc, IIC_ESTATUS);
}
- TWSI_WRITE(sc, sc->reg_control, sc->control_val);
break;
case TWSI_STATUS_DATA_RD_NOACK:
- if (sc->msgs[sc->msg_idx].len - sc->recv_bytes == 1) {
- sc->msgs[sc->msg_idx].buf[sc->recv_bytes++] =
TWSI_READ(sc, sc->reg_data);
- debugf(sc, "Done RX data, send stop (2)\n");
- if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP))
+ debugf(sc, "Received and NACK-ed data\n");
+ KASSERT(sc->recv_bytes == sc->msgs[sc->msg_idx].len - 1,
+ ("sent NACK before receiving all requested data"));
+ sc->msgs[sc->msg_idx].buf[sc->recv_bytes] =
+ TWSI_READ(sc, sc->reg_data);
+ debugf(sc, "Received byte %d (of %d) = 0x%x\n",
+ sc->recv_bytes,
+ sc->msgs[sc->msg_idx].len,
+ sc->msgs[sc->msg_idx].buf[sc->recv_bytes]);
+ sc->recv_bytes++;
+
+ if (sc->msgs[sc->msg_idx].len == sc->recv_bytes) {
+ debugf(sc, "Done RX data\n");
+ if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP)) {
+ debugf(sc, "Send STOP\n");
TWSI_WRITE(sc, sc->reg_control,
sc->control_val | TWSI_CONTROL_STOP);
+ }
} else {
- debugf(sc, "No ack when receiving data, sending stop
anyway\n");
- if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP))
- TWSI_WRITE(sc, sc->reg_control,
- sc->control_val | TWSI_CONTROL_STOP);
+ /*
+ * We should not have NACK-ed yet.
+ * The protocol state machine is in invalid state.
+ */
+ debugf(sc, "NACK-ed before receving all bytes?\n");
+ twsi_error(sc, IIC_ESTATUS);
}
sc->transfer = 0;
transfer_done = 1;
