> Date: Fri, 10 Jan 2020 12:07:51 +0100 > From: Claudio Jeker <cje...@diehard.n-r-g.com> > > On Sun, Dec 22, 2019 at 04:55:59PM +0100, Mark Kettenis wrote: > > 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? > > > > > > 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)); > > > > I'm a bit confused with the previous two hunks. Why change the tx_limit > check? Also you set tx_limit to 1 and then immediatly a tx_limit-- follows > so that the limit is actually 0 which is what you want but then the change > of the check in the while loop seems wrong.
Ok, let's separate things out a bit more. Here is the diff that just implements block mode. There the limit is initially set to 1 to make sure we have a chance to look at the first byte to determine the total length of the transfer. ok? 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 11 Jan 2020 11:37:53 -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++; @@ -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;