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 <[email protected]>
> > 
> > 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 <[email protected]>
> > > > 
> > > > 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 <[email protected]>
> > > > > > 
> > > > > > 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?

Patrick

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)) {

Reply via email to