On Mon, May 21, 2018 at 12:44:47PM +0200, Mark Kettenis wrote: > The diff below fixes I2C_OP_WRITE_WITH_STOP operations. Currently we > run the read completion code for those operations, but since there is > no data to read, this fails. Instead, this waits until a stop is > detected. That doesn't seem to work for I2C_F_POLL operations though, > so in that case we wait until the bus is idle. This also adds > interlocks (using splbio/splx) on the existing tsleep() operations. > > This doesn't fix all the issues with the driver. In particular it > seems that large reads still cause problems. I think a somewhat more > invasive re-write of the dwiic_i2c_exec() function is needed. But I'd > like to get this in before I embark on that. > > ok? >
The diff reads ok to me, do you want us to do any sort of testing here before commit? -ml > > Index: dev/ic/dwiic.c > =================================================================== > RCS file: /cvs/src/sys/dev/ic/dwiic.c,v > retrieving revision 1.3 > diff -u -p -r1.3 dwiic.c > --- dev/ic/dwiic.c 19 Jan 2018 18:20:38 -0000 1.3 > +++ dev/ic/dwiic.c 21 May 2018 10:37:50 -0000 > @@ -192,6 +192,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op > u_int32_t ic_con, st, cmd, resp; > int retries, tx_limit, rx_avail, x, readpos; > uint8_t *b; > + int s; > > if (sc->sc_busy) > return 1; > @@ -245,12 +246,14 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op > if (flags & I2C_F_POLL) > DELAY(200); > else { > + s = splbio(); > dwiic_read(sc, DW_IC_CLR_INTR); > dwiic_write(sc, DW_IC_INTR_MASK, DW_IC_INTR_TX_EMPTY); > > if (tsleep(&sc->sc_writewait, PRIBIO, "dwiic", hz / 2) != 0) > printf("%s: timed out waiting for tx_empty intr\n", > sc->sc_dev.dv_xname); > + splx(s); > } > > /* send our command, one byte at a time */ > @@ -320,7 +323,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 (tx_limit == 0 || x == len) { > + while (I2C_OP_READ_P(op) && (tx_limit == 0 || x == len)) { > DPRINTF(("%s: %s: tx_limit %d, sent %d read reqs\n", > sc->sc_dev.dv_xname, __func__, tx_limit, x)); > > @@ -332,6 +335,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op > DELAY(50); > } > } else { > + s = splbio(); > dwiic_read(sc, DW_IC_CLR_INTR); > dwiic_write(sc, DW_IC_INTR_MASK, > DW_IC_INTR_RX_FULL); > @@ -341,6 +345,7 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op > printf("%s: timed out waiting for " > "rx_full intr\n", > sc->sc_dev.dv_xname); > + splx(s); > > rx_avail = dwiic_read(sc, DW_IC_RXFLR); > } > @@ -378,6 +383,32 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op > } > } > > + if (I2C_OP_STOP_P(op) && I2C_OP_WRITE_P(op)) { > + if (flags & I2C_F_POLL) { > + /* wait for bus to be idle */ > + for (retries = 100; retries > 0; retries--) { > + st = dwiic_read(sc, DW_IC_STATUS); > + if (!(st & DW_IC_STATUS_ACTIVITY)) > + break; > + DELAY(1000); > + } > + if (st & DW_IC_STATUS_ACTIVITY) > + printf("%s: timed out waiting for bus idle\n", > + sc->sc_dev.dv_xname); > + } else { > + s = splbio(); > + while (sc->sc_busy) { > + dwiic_write(sc, DW_IC_INTR_MASK, > + DW_IC_INTR_STOP_DET); > + if (tsleep(&sc->sc_busy, PRIBIO, "dwiic", > + hz / 2) != 0) > + printf("%s: timed out waiting for " > + "stop intr\n", > + sc->sc_dev.dv_xname); > + } > + splx(s); > + } > + } > sc->sc_busy = 0; > > return 0; > @@ -449,6 +480,11 @@ dwiic_intr(void *arg) > DPRINTF(("%s: %s: waking up writer\n", > sc->sc_dev.dv_xname, __func__)); > wakeup(&sc->sc_writewait); > + } > + if (stat & DW_IC_INTR_STOP_DET) { > + dwiic_write(sc, DW_IC_INTR_MASK, 0); > + sc->sc_busy = 0; > + wakeup(&sc->sc_busy); > } > } > >