> Date: Tue, 13 Jul 2021 21:29:35 +0200 > From: Patrick Wildt <patr...@blueri.se> > > Am Mon, Jul 05, 2021 at 07:52:28PM +0200 schrieb Mark Kettenis: > > > Date: Mon, 5 Jul 2021 19:30:28 +0200 > > > From: Patrick Wildt <patr...@blueri.se> > > > > > > Am Mon, Jul 05, 2021 at 07:07:24PM +0200 schrieb Mark Kettenis: > > > > > Date: Mon, 5 Jul 2021 19:02:32 +0200 > > > > > From: Patrick Wildt <patr...@blueri.se> > > > > > > > > > > Am Mon, Jul 05, 2021 at 06:34:31PM +0200 schrieb Mark Kettenis: > > > > > > > Date: Mon, 5 Jul 2021 00:04:24 +0200 > > > > > > > From: Patrick Wildt <patr...@blueri.se> > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > I had trouble interfacing with a machine's IPMI through dwiic(4). > > > > > > > What > > > > > > > I saw was that when sending 'bigger' commands, it would never > > > > > > > receive > > > > > > > the STOP bit interrupt. > > > > > > > > > > > > > > The trouble is, as can be seen in the log, that we want to send > > > > > > > (it > > > > > > > says read, but it's a write OP, so it's send) 20 bytes, but the tx > > > > > > > limit says 14. > > > > > > > > > > > > > > What we should do is send 14 bytes, then wait for it to send us > > > > > > > the > > > > > > > tx empty interrupt (like we do when we first enable the > > > > > > > controller), > > > > > > > and then re-read the tx limit. The last line in the log is some > > > > > > > debug print I added for myself, but is not part of the diff. > > > > > > > > > > > > > > With this, I was finally able to change the IPMI password and > > > > > > > regain > > > > > > > access to the web interface after updating the BMC's firmware... > > > > > > > > > > > > > > dwiic0: dwiic_i2c_exec: op 7, addr 0x10, cmdlen 2, len 3, flags > > > > > > > 0x00 > > > > > > > dwiic0: dwiic_i2c_exec: need to read 3 bytes, can send 14 read > > > > > > > reqs > > > > > > > dwiic0: dwiic_i2c_exec: op 5, addr 0x10, cmdlen 1, len 33, flags > > > > > > > 0x00 > > > > > > > dwiic0: dwiic_i2c_exec: need to read 33 bytes, can send 15 read > > > > > > > reqs > > > > > > > dwiic0: dwiic_i2c_exec: op 7, addr 0x10, cmdlen 2, len 20, flags > > > > > > > 0x00 > > > > > > > dwiic0: dwiic_i2c_exec: need to read 20 bytes, can send 14 read > > > > > > > reqs > > > > > > > dwiic0: new tx limit 8 > > > > > > > > > > > > > > Opinions? ok? > > > > > > > > > > > > I think you're on to something. But this needs to handle > > > > > > I2C_F_POLL. > > > > > > > > > > True that. The previous code, which waits for the controller to > > > > > accept > > > > > commands, just does DELAY(200), but I'm not sure that's good enough > > > > > for > > > > > inbetween transfers. One can apparently though poll through the raw > > > > > interrupt status register, where the interrupt mask isn't applied. So > > > > > maybe like that? Guess I should try setting ipmi to polling mode... > > > > > > > > Polling the interrupt status register should work I suppose. But for > > > > read operations we actually poll the DW_IC_RXFLR register. > > > > > > Yeah, that would work for TX as well. Maybe something like this, but > > > then the diff still needs to address what happens when we timeout and > > > there's still no tx_limit > 0. Maybe timeout like the read stuff: > > > > > > if (rx_avail == 0) { > > > printf("%s: timed out reading remaining %d\n", > > > sc->sc_dev.dv_xname, (int)(len - readpos)); > > > sc->sc_i2c_xfer.error = 1; > > > sc->sc_busy = 0; > > > > > > return (1); > > > } > > > > Yes. > > This works for me. ok?
ok kettenis@ > diff --git a/sys/dev/ic/dwiic.c b/sys/dev/ic/dwiic.c > index 84d97b8645b..8588b0905ea 100644 > --- a/sys/dev/ic/dwiic.c > +++ b/sys/dev/ic/dwiic.c > @@ -416,6 +416,42 @@ dwiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t > addr, const void *cmdbuf, > tx_limit = sc->tx_fifo_depth - > dwiic_read(sc, DW_IC_TXFLR); > } > + > + if (I2C_OP_WRITE_P(op) && tx_limit == 0 && x < len) { > + if (flags & I2C_F_POLL) { > + for (retries = 1000; retries > 0; retries--) { > + tx_limit = sc->tx_fifo_depth - > + dwiic_read(sc, DW_IC_TXFLR); > + if (tx_limit > 0) > + break; > + DELAY(50); > + } > + } else { > + s = splbio(); > + dwiic_read(sc, DW_IC_CLR_INTR); > + dwiic_write(sc, DW_IC_INTR_MASK, > + DW_IC_INTR_TX_EMPTY); > + > + if (tsleep_nsec(&sc->sc_writewait, PRIBIO, > + "dwiic", MSEC_TO_NSEC(500)) != 0) > + printf("%s: timed out waiting for " > + "tx_empty intr\n", > + sc->sc_dev.dv_xname); > + splx(s); > + > + tx_limit = sc->tx_fifo_depth - > + dwiic_read(sc, DW_IC_TXFLR); > + } > + > + if (tx_limit == 0) { > + printf("%s: timed out writing remaining %d\n", > + sc->sc_dev.dv_xname, (int)(len - x)); > + sc->sc_i2c_xfer.error = 1; > + sc->sc_busy = 0; > + > + return (1); > + } > + } > } > > if (I2C_OP_STOP_P(op) && I2C_OP_WRITE_P(op)) { >