On Fri, Dec 04, 2020 at 12:08:39PM -0600, Scott Cheloha wrote: > On Fri, Dec 04, 2020 at 10:07:07AM +0100, Claudio Jeker wrote: > > On Thu, Dec 03, 2020 at 10:42:50PM -0600, Scott Cheloha wrote: > > > Hi, > > > > > > mbg(4) is among the few remaining drivers using tsleep(9). > > > > > > In a few spots, when the kernel is not cold, the driver will spin for > > > up to 1/10 seconds waiting for the MBG_BUSY flag to go low. > > > > > > We can approximate this behavior by spinning 10 times and sleeping 10 > > > milliseconds each iteration. 10 x 10ms = 100ms = 1/10 seconds. > > > > > > I can't test this but I was able to compile it on amd64. It isn't > > > normally built for amd64, though. Just i386. > > > > > > I have my doubts that anyone has this card and is able to actually > > > test this diff. > > > > > > Anybody ok? > > > > This code needs to wait for around 70us for the card to process the > > command (according to the comment). The cold code sleeps a max of > > 50 * 20us (1ms). I don't see why the regular code should sleep so much > > longer. I would suggest to use a 1ms timeout and loop 10 times. This is a > > magnitude more than enough and most probably only one cycle will be > > needed. > > > > IIRC someone got a mbg(4) device some time ago apart from mbalmer@ > > Makes sense to me. Updated diff attached.
OK claudio@ > How are we going to find this person? Best way is to commit the diff and wait :) > Index: mbg.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/mbg.c,v > retrieving revision 1.31 > diff -u -p -r1.31 mbg.c > --- mbg.c 29 Nov 2020 03:17:27 -0000 1.31 > +++ mbg.c 4 Dec 2020 18:07:43 -0000 > @@ -417,12 +417,12 @@ mbg_read_amcc_s5920(struct mbg_softc *sc > > /* wait for the BUSY flag to go low (approx 70 us on i386) */ > timer = 0; > - tmax = cold ? 50 : hz / 10; > + tmax = cold ? 50 : 10; > do { > if (cold) > delay(20); > else > - tsleep(tstamp, 0, "mbg", 1); > + tsleep_nsec(tstamp, 0, "mbg", MSEC_TO_NSEC(1)); > status = bus_space_read_1(sc->sc_iot, sc->sc_ioh, > AMCC_IMB4 + 3); > } while ((status & MBG_BUSY) && timer++ < tmax); > @@ -473,12 +473,12 @@ mbg_read_amcc_s5933(struct mbg_softc *sc > > /* wait for the BUSY flag to go low (approx 70 us on i386) */ > timer = 0; > - tmax = cold ? 50 : hz / 10; > + tmax = cold ? 50 : 10; > do { > if (cold) > delay(20); > else > - tsleep(tstamp, 0, "mbg", 1); > + tsleep_nsec(tstamp, 0, "mbg", MSEC_TO_NSEC(1)); > status = bus_space_read_1(sc->sc_iot, sc->sc_ioh, > AMCC_IMB4 + 3); > } while ((status & MBG_BUSY) && timer++ < tmax); > @@ -525,12 +525,12 @@ mbg_read_asic(struct mbg_softc *sc, int > > /* wait for the BUSY flag to go low */ > timer = 0; > - tmax = cold ? 50 : hz / 10; > + tmax = cold ? 50 : 10; > do { > if (cold) > delay(20); > else > - tsleep(tstamp, 0, "mbg", 1); > + tsleep_nsec(tstamp, 0, "mbg", MSEC_TO_NSEC(1)); > status = bus_space_read_1(sc->sc_iot, sc->sc_ioh, ASIC_STATUS); > } while ((status & MBG_BUSY) && timer++ < tmax); > -- :wq Claudio