> Date: Sun, 22 Dec 2019 16:55:59 +0100 (CET)
> From: Mark Kettenis <mark.kette...@xs4all.nl>
>
> The diff below contains a couple of improvements to dwiic(4). They're
> mostly for making ipmi(4) on the Ampere/Lenovo arm64 boxes work
> better. But they need testing on x86 machines with
> keyboards.touchpads/touchscreens connected over i2c.
>
> Most of the diff is there to properly implement SMBus block write/read
> transactions. Defines for these are taken from NetBSD (even though
> NetBSD doesn't actually implement these). Nothing special needs to
> been done for block writes, but for block reads the slave device sends
> us byte count before sending us the data bytes. So the code is
> changed such that we read the byte count first and then adjust the
> length of the transaction accordingly.
>
> The diff then uses this support for block reads in the ipmi(4) SSIF
> interface code and adds support for multi-part read transactions on
> top of that. That makes ipmi(4) fully functional on the arm64 machine
> mentioned above.
>
> Well, almost. I found that dwiic(4) still craps out every now and
> then on this machine. Limiting the number of commands we send to the
> controller to the size of the Tx FIFO minus one seems to help.
>
> Will probably split up the diff before committing.
>
> ok?
ping? tested this myself on x86 hardware with i2c sensors connected
to dwiic(4).
> Index: dev/i2c/i2c_io.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/i2c/i2c_io.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 i2c_io.h
> --- dev/i2c/i2c_io.h 23 May 2004 17:33:43 -0000 1.1
> +++ dev/i2c/i2c_io.h 22 Dec 2019 15:40:28 -0000
> @@ -1,5 +1,5 @@
> /* $OpenBSD: i2c_io.h,v 1.1 2004/05/23 17:33:43 grange Exp $ */
> -/* $NetBSD: i2c_io.h,v 1.1 2003/09/30 00:35:31 thorpej Exp $ */
> +/* $NetBSD: i2c_io.h,v 1.3 2012/04/22 14:10:36 pgoyette Exp $ */
>
> /*
> * Copyright (c) 2003 Wasabi Systems, Inc.
> @@ -45,16 +45,24 @@
> typedef uint16_t i2c_addr_t;
>
> /* High-level I2C operations. */
> +#define I2C_OPMASK_STOP 1
> +#define I2C_OPMASK_WRITE 2
> +#define I2C_OPMASK_BLKMODE 4
> +
> +#define I2C_OP_STOP_P(x) (((int)(x) & I2C_OPMASK_STOP) != 0)
> +#define I2C_OP_WRITE_P(x) (((int)(x) & I2C_OPMASK_WRITE) != 0)
> +#define I2C_OP_READ_P(x) (!I2C_OP_WRITE_P(x))
> +#define I2C_OP_BLKMODE_P(x) (((int)(x) & I2C_OPMASK_BLKMODE) != 0)
> +
> typedef enum {
> - I2C_OP_READ = 0,
> - I2C_OP_READ_WITH_STOP = 1,
> - I2C_OP_WRITE = 2,
> - I2C_OP_WRITE_WITH_STOP = 3,
> + I2C_OP_READ = 0,
> + I2C_OP_READ_WITH_STOP = I2C_OPMASK_STOP,
> + I2C_OP_WRITE = I2C_OPMASK_WRITE,
> + I2C_OP_WRITE_WITH_STOP = I2C_OPMASK_WRITE | I2C_OPMASK_STOP,
> + I2C_OP_READ_BLOCK = I2C_OPMASK_BLKMODE | I2C_OPMASK_STOP,
> + I2C_OP_WRITE_BLOCK = I2C_OPMASK_BLKMODE | I2C_OPMASK_WRITE |
> + I2C_OPMASK_STOP,
> } i2c_op_t;
> -
> -#define I2C_OP_READ_P(x) (((int)(x) & 2) == 0)
> -#define I2C_OP_WRITE_P(x) (! I2C_OP_READ_P(x))
> -#define I2C_OP_STOP_P(x) (((int)(x) & 1) != 0)
>
> /*
> * This structure describes a single I2C control script fragment.
> Index: dev/i2c/ipmi_i2c.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/i2c/ipmi_i2c.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 ipmi_i2c.c
> --- dev/i2c/ipmi_i2c.c 3 Sep 2019 09:17:10 -0000 1.2
> +++ dev/i2c/ipmi_i2c.c 22 Dec 2019 15:40:28 -0000
> @@ -170,7 +170,7 @@ ssif_sendmsg(struct ipmi_cmd *c)
> cmd[0] = 2;
> cmd[1] = c->c_txlen;
> for (retry = 0; retry < 5; retry++) {
> - error = iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP,
> + error = iic_exec(sc->sc_tag, I2C_OP_WRITE_BLOCK,
> sc->sc_addr, cmd, sizeof(cmd), buf, c->c_txlen, 0);
> if (!error)
> break;
> @@ -185,28 +185,72 @@ int
> ssif_recvmsg(struct ipmi_cmd *c)
> {
> struct ipmi_i2c_softc *sc = (struct ipmi_i2c_softc *)c->c_sc;
> - uint8_t buf[IPMI_MAX_RX + 16];
> + uint8_t buf[33];
> uint8_t cmd[1];
> + uint8_t len;
> int error, retry;
> -
> - /* We only support single-part reads. */
> - if (c->c_maxrxlen > 32)
> - return -1;
> + int blkno = 0;
>
> iic_acquire_bus(sc->sc_tag, 0);
>
> cmd[0] = 3;
> for (retry = 0; retry < 250; retry++) {
> - memset(buf, 0, c->c_maxrxlen + 1);
> - error = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP,
> - sc->sc_addr, cmd, sizeof(cmd), buf, c->c_maxrxlen + 1, 0);
> - if (!error) {
> - c->c_rxlen = buf[0];
> - memcpy(sc->sc.sc_buf, &buf[1], c->c_maxrxlen);
> + memset(buf, 0, sizeof(buf));
> + error = iic_exec(sc->sc_tag, I2C_OP_READ_BLOCK,
> + sc->sc_addr, cmd, sizeof(cmd), buf, sizeof(buf), 0);
> + if (error)
> + continue;
> +
> + if (buf[0] < 1 || buf[0] > 32) {
> + error = EIO;
> + goto release;
> + }
> +
> + if (buf[0] == 32 && buf[1] == 0x00 && buf[2] == 0x01) {
> + /* Multi-part read start. */
> + c->c_rxlen = MIN(30, c->c_maxrxlen);
> + memcpy(sc->sc.sc_buf, &buf[3], c->c_rxlen);
> break;
> + } else {
> + /* Single-part read. */
> + c->c_rxlen = MIN(buf[0], c->c_maxrxlen);
> + memcpy(sc->sc.sc_buf, &buf[1], c->c_rxlen);
> + goto release;
> + }
> + }
> + if (retry == 250)
> + goto release;
> +
> + cmd[0] = 9;
> + while (buf[1] != 0xff && c->c_rxlen < c->c_maxrxlen) {
> + memset(buf, 0, sizeof(buf));
> + error = iic_exec(sc->sc_tag, I2C_OP_READ_BLOCK,
> + sc->sc_addr, cmd, sizeof(cmd), buf, sizeof(buf), 0);
> + if (error)
> + goto release;
> +
> + if (buf[0] < 1 || buf[0] > 32) {
> + error = EIO;
> + goto release;
> + }
> +
> + if (buf[0] == 32 && buf[1] == blkno) {
> + /* Multi-part read middle. */
> + len = MIN(31, c->c_maxrxlen - c->c_rxlen);
> + memcpy(&sc->sc.sc_buf[c->c_rxlen], &buf[2], len);
> + } else if (buf[1] == 0xff) {
> + /* Multi-part read end. */
> + len = MIN(buf[0] - 1, c->c_maxrxlen - c->c_rxlen);
> + memcpy(&sc->sc.sc_buf[c->c_rxlen], &buf[2], len);
> + } else {
> + error = EIO;
> + goto release;
> }
> + c->c_rxlen += len;
> + blkno++;
> }
>
> +release:
> iic_release_bus(sc->sc_tag, 0);
>
> return (error ? -1 : 0);
> Index: dev/ic/dwiic.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/dwiic.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 dwiic.c
> --- dev/ic/dwiic.c 18 Aug 2019 15:52:45 -0000 1.8
> +++ dev/ic/dwiic.c 22 Dec 2019 15:40:28 -0000
> @@ -338,6 +338,13 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>
> dwiic_write(sc, DW_IC_DATA_CMD, cmd);
>
> + /*
> + * For a block read, get the byte count before
> + * continuing to read the data bytes.
> + */
> + if (I2C_OP_READ_P(op) && I2C_OP_BLKMODE_P(op) && readpos == 0)
> + tx_limit = 1;
> +
> tx_limit--;
> x++;
>
> @@ -345,7 +352,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
> * As TXFLR fills up, we need to clear it out by reading all
> * available data.
> */
> - while (I2C_OP_READ_P(op) && (tx_limit == 0 || x == len)) {
> + while (I2C_OP_READ_P(op) && (tx_limit <= 1 || x == len)) {
> DPRINTF(("%s: %s: tx_limit %d, sent %d read reqs\n",
> sc->sc_dev.dv_xname, __func__, tx_limit, x));
>
> @@ -374,8 +381,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
>
> if (rx_avail == 0) {
> printf("%s: timed out reading remaining %d\n",
> - sc->sc_dev.dv_xname,
> - (int)(len - 1 - readpos));
> + sc->sc_dev.dv_xname, (int)(len - readpos));
> sc->sc_i2c_xfer.error = 1;
> sc->sc_busy = 0;
>
> @@ -394,6 +400,13 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op
> }
> rx_avail--;
> }
> +
> + /*
> + * Update the transfer length when doing a
> + * block read.
> + */
> + if (I2C_OP_BLKMODE_P(op) && readpos > 0 && len > b[0])
> + len = b[0] + 1;
>
> if (readpos >= len)
> break;
>
>
