Skip to content

Commit

Permalink
twsi: further refinements
Browse files Browse the repository at this point in the history 
Most importantly, send the STOP condition after any protocol error.
If we don't finish the current transaction then either the controller
or devices on the bus can get confused about the current protocol state.
E.g., NACK from a slave does not imply by itself that the current
transaction is aborted.
To aid this change, a new function is introduced to handle all common
actions for an error condition.

Also, not only clear TWSI_CONTROL_ACK before a 1-byte read, but also
set that control bit before a multi-byte read.

Some KASSERTs are added to ensure that we never read or write beyond
the end of a caller supplied data buffer.

Support for diagnostic messages is always compiled in.
The run-time behavior is controlled with a hew sysctl, hw.i2c.twsi_debug.

If an interrupt arrives when no transfer is active then the interrupt is
just acknowledged (and reported), but no other action is taken.

Finally, some wording changes of the diagnostic messages.
  • Loading branch information
@avg-I
avg-I committed Sep 23, 2021
1 parent 35f3195 commit 5f2d7e7
Showing 1 changed file with 85 additions and 40 deletions.
125 changes: 85 additions & 40 deletions sys/dev/iicbus/twsi/twsi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,19 @@ __FBSDID("$FreeBSD$");
#include <sys/systm.h>
#include <sys/bus.h>
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/module.h>
#include <sys/resource.h>
#include <sys/rman.h>
#include <sys/sysctl.h>

#include <machine/_inttypes.h>
#include <machine/bus.h>
#include <machine/resource.h>

#include <sys/rman.h>

#include <sys/lock.h>
#include <sys/mutex.h>

#include <dev/iicbus/iiconf.h>
#include <dev/iicbus/iicbus.h>
#include <dev/ofw/ofw_bus.h>
#include <dev/ofw/ofw_bus_subr.h>

#include <dev/iicbus/twsi/twsi.h>

Expand All @@ -72,12 +69,14 @@ __FBSDID("$FreeBSD$");
#define TWSI_CONTROL_TWSIEN (1 << 6)
#define TWSI_CONTROL_INTEN (1 << 7)

#define TWSI_STATUS_BUS_ERROR 0x00
#define TWSI_STATUS_START 0x08
#define TWSI_STATUS_RPTD_START 0x10
#define TWSI_STATUS_ADDR_W_ACK 0x18
#define TWSI_STATUS_ADDR_W_NACK 0x20
#define TWSI_STATUS_DATA_WR_ACK 0x28
#define TWSI_STATUS_DATA_WR_NACK 0x30
#define TWSI_STATUS_ARBITRATION_LOST 0x38
#define TWSI_STATUS_ADDR_R_ACK 0x40
#define TWSI_STATUS_ADDR_R_NACK 0x48
#define TWSI_STATUS_DATA_RD_ACK 0x50
Expand All @@ -86,12 +85,18 @@ __FBSDID("$FreeBSD$");
#define TWSI_DEBUG
#undef TWSI_DEBUG

#define debugf(dev, fmt, args...) if (twsi_debug) device_printf(dev, "%s: " fmt, __func__, ##args)

#ifdef TWSI_DEBUG
#define debugf(dev, fmt, args...) device_printf(dev, "%s: " fmt, __func__, ##args)
static int twsi_debug = 1;
#else
#define debugf(dev, fmt, args...)
static int twsi_debug = 0;
#endif

SYSCTL_DECL(_hw_i2c);
SYSCTL_INT(_hw_i2c, OID_AUTO, twsi_debug, CTLFLAG_RWTUN,
&twsi_debug, 0, "Enable twsi driver debug");

static struct resource_spec res_spec[] = {
{ SYS_RES_MEMORY, 0, RF_ACTIVE },
{ SYS_RES_IRQ, 0, RF_ACTIVE | RF_SHAREABLE},
Expand Down Expand Up @@ -492,18 +497,18 @@ twsi_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
if (!sc->have_intr)
return (iicbus_transfer_gen(dev, msgs, nmsgs));

sc->nmsgs = nmsgs;
sc->msgs = msgs;
sc->msg_idx = 0;
sc->transfer = 1;
sc->error = 0;

sc->control_val = TWSI_CONTROL_TWSIEN |
TWSI_CONTROL_INTEN | TWSI_CONTROL_ACK;
TWSI_WRITE(sc, sc->reg_control, sc->control_val);
debugf(dev, "transmitting %d messages\n", nmsgs);
debugf(sc->dev, "status=%x\n", TWSI_READ(sc, sc->reg_status));

sc->nmsgs = nmsgs;
sc->msgs = msgs;
sc->msg_idx = 0;
sc->transfer = 1;
sc->error = 0;

#ifdef TWSI_DEBUG
for (int i = 0; i < nmsgs; i++)
debugf(sc->dev, "msg %d is %d bytes long\n", i, msgs[i].len);
Expand Down Expand Up @@ -535,6 +540,21 @@ twsi_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs)
return (sc->error);
}

static void
twsi_error(struct twsi_softc *sc, int err)
{
/*
* Must send stop condition to abort the current transfer and
* signal the caller.
*/
debugf(sc->dev, "Sending STOP condition for error %d\n", err);
sc->transfer = 0;
sc->error = err;
sc->control_val = 0;
TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_STOP);
wakeup(sc);
}

static void
twsi_intr(void *arg)
{
Expand All @@ -545,19 +565,22 @@ twsi_intr(void *arg)

sc = arg;

debugf(sc->dev, "Got interrupt Current msg=%x\n", sc->msg_idx);
debugf(sc->dev, "Got interrupt Current msg=%u\n", sc->msg_idx);

status = TWSI_READ(sc, sc->reg_status);
debugf(sc->dev, "reg control=%x\n", TWSI_READ(sc, sc->reg_control));

if (sc->transfer == 0)
device_printf(sc->dev, "interrupt without active transfer\n");
if (sc->transfer == 0) {
device_printf(sc->dev, "interrupt without active transfer, "
"status = 0x%x\n", status);
goto end;
}

switch (status) {
case TWSI_STATUS_START:
case TWSI_STATUS_RPTD_START:
/* Transmit the address */
debugf(sc->dev, "Send the address (%x)", sc->msgs[sc->msg_idx].slave);
debugf(sc->dev, "Send the address %x\n", sc->msgs[sc->msg_idx].slave);

if (sc->msgs[sc->msg_idx].flags & IIC_M_RD)
TWSI_WRITE(sc, sc->reg_data,
Expand All @@ -569,7 +592,7 @@ twsi_intr(void *arg)
break;

case TWSI_STATUS_ADDR_W_ACK:
debugf(sc->dev, "Ack received after transmitting the address (write)\n");
debugf(sc->dev, "ACK received after transmitting the address (write)\n");
/* Directly send the first byte */
sc->sent_bytes = 1;
debugf(sc->dev, "Sending byte 0 (of %d) = %x\n",
Expand All @@ -581,25 +604,30 @@ twsi_intr(void *arg)
break;

case TWSI_STATUS_ADDR_R_ACK:
debugf(sc->dev, "Ack received after transmitting the address (read)\n");
debugf(sc->dev, "ACK received after transmitting the address (read)\n");
sc->recv_bytes = 0;

if (sc->msgs[sc->msg_idx].len == 1)
sc->control_val &= ~TWSI_CONTROL_ACK;
else
sc->control_val |= TWSI_CONTROL_ACK;
TWSI_WRITE(sc, sc->reg_control, sc->control_val);
break;

case TWSI_STATUS_ADDR_W_NACK:
case TWSI_STATUS_ADDR_R_NACK:
debugf(sc->dev, "No ack received after transmitting the address\n");
sc->transfer = 0;
sc->error = IIC_ENOACK;
sc->control_val = 0;
debugf(sc->dev, "NACK received after transmitting the address\n");
twsi_error(sc, IIC_ENOACK);
break;
case TWSI_STATUS_DATA_WR_NACK:
debugf(sc->dev, "NACK received after transmitting data byte\n");
twsi_error(sc, IIC_ENOACK);
wakeup(sc);
break;

case TWSI_STATUS_DATA_WR_ACK:
debugf(sc->dev, "Ack received after transmitting data\n");
KASSERT(sc->sent_bytes <= sc->msgs[sc->msg_idx].len,
("impossible sent_bytes value"));
debugf(sc->dev, "ACK received after transmitting data\n");
if (sc->sent_bytes == sc->msgs[sc->msg_idx].len) {
debugf(sc->dev, "Done sending all the bytes for msg %d\n", sc->msg_idx);
send_byte = 0;
Expand All @@ -614,11 +642,11 @@ twsi_intr(void *arg)
}
sc->msg_idx++;
if (sc->msg_idx == sc->nmsgs) {
debugf(sc->dev, "transfer_done=1\n");
debugf(sc->dev, "All messages transmitted\n");
transfer_done = 1;
sc->error = 0;
} else if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTART)) {
debugf(sc->dev, "Send repeated start\n");
debugf(sc->dev, "Send (repeated) start\n");
TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_START);
} else {
/* Just keep sending data. */
Expand Down Expand Up @@ -646,15 +674,19 @@ twsi_intr(void *arg)
break;

case TWSI_STATUS_DATA_RD_ACK:
debugf(sc->dev, "Ack received after receiving data\n");
debugf(sc->dev, "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->dev, "msg_len=%d recv_bytes=%d\n", sc->msgs[sc->msg_idx].len, sc->recv_bytes);

/* If we only have one byte left, disable ACK */
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->dev, "Done with msg %d\n", sc->msg_idx);
debugf(sc->dev, "Done with msg %d but ACKed for more data\n",
sc->msg_idx);
sc->msg_idx++;
if (sc->msg_idx == sc->nmsgs - 1) {
debugf(sc->dev, "No more msgs\n");
Expand All @@ -666,15 +698,23 @@ twsi_intr(void *arg)
break;

case TWSI_STATUS_DATA_RD_NOACK:
debugf(sc->dev, "Received and NACK-ed data\n");
KASSERT(sc->recv_bytes < sc->msgs[sc->msg_idx].len,
("receiving beyond the end of buffer"));
KASSERT(sc->recv_bytes == sc->msgs[sc->msg_idx].len - 1,
("sent NACK before receiving all requested data"));
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->dev, "Done RX data, send stop (2)\n");
if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP))
debugf(sc->dev, "Done RX data\n");
if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP)) {
debugf(sc->dev, "send stop\n");
TWSI_WRITE(sc, sc->reg_control,
sc->control_val | TWSI_CONTROL_STOP);
}
} else {
debugf(sc->dev, "No ack when receiving data, sending stop anyway\n");
debugf(sc->dev, "NACK-ed too early\n");
if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP))
debugf(sc->dev, "sending stop anyway\n");
TWSI_WRITE(sc, sc->reg_control,
sc->control_val | TWSI_CONTROL_STOP);
}
Expand All @@ -683,12 +723,17 @@ twsi_intr(void *arg)
sc->error = 0;
break;

case TWSI_STATUS_BUS_ERROR:
debugf(sc->dev, "Bus error\n");
twsi_error(sc, IIC_EBUSERR);
break;
case TWSI_STATUS_ARBITRATION_LOST:
debugf(sc->dev, "Arbitration lost\n");
twsi_error(sc, IIC_EBUSBSY);
break;
default:
debugf(sc->dev, "status=%x hot handled\n", status);
sc->transfer = 0;
sc->error = IIC_EBUSERR;
sc->control_val = 0;
wakeup(sc);
debugf(sc->dev, "unexpected status 0x%x\n", status);
twsi_error(sc, IIC_ESTATUS);
break;
}
debugf(sc->dev, "Refresh reg_control\n");
Expand All @@ -699,7 +744,7 @@ twsi_intr(void *arg)
TWSI_WRITE(sc, sc->reg_control, sc->control_val |
(sc->iflag_w1c ? TWSI_CONTROL_IFLG : 0));

debugf(sc->dev, "Done with interrupts\n\n");
debugf(sc->dev, "Done with interrupts\n");
if (transfer_done == 1) {
sc->transfer = 0;
wakeup(sc);
Expand Down

0 comments on commit 5f2d7e7

Please sign in to comment.