My iatp device works the same as before, good on boot, but fails
reading main memory map, etc on resume.
On Wed, May 23, 2018 at 11:18 PM, Mike Larkin <mlar...@azathoth.net> wrote:
> On Tue, May 22, 2018 at 05:43:01PM +0200, Mark Kettenis wrote:
>> > Date: Mon, 21 May 2018 17:25:47 -0700
>> > From: Mike Larkin <mlar...@azathoth.net>
>> >
>> > 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?
>>
>> Testing is always appreciated, and a goood idea if you own a machine
>> that relies on dwiic(4). For example those with machines that have a
>> keyboard or touchpad/screen connected over i2c.
>>
>> Cheers,
>>
>> Mark
>>
>
> No regressions seen here.
>
> -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);
>> > > }
>> > > }
>> > >
>> > >
>> >
>
