> Date: Thu, 26 Nov 2020 10:29:50 +0100
> From: Claudio Jeker <cje...@diehard.n-r-g.com>
> 
> On Thu, Nov 26, 2020 at 08:25:48PM +1100, Jonathan Gray wrote:
> > On Tue, Nov 24, 2020 at 07:20:46PM -0600, Scott Cheloha wrote:
> > > Hi,
> > > 
> > > Both kettenis@ and mpi@ have mentioned in private that my proposed
> > > changes to tsleep_nsec(9) etc. would be nicer if we could just get rid
> > > of tsleep(9) etc. entirely.
> > > 
> > > This is difficult, but I'll try.
> > > 
> > > Worst case, we thin out the remaining callers.  There are not many
> > > left.
> > > 
> > > --
> > > 
> > > So, an(4) is one such caller.
> > > 
> > > In an_wait() we spin for (3 * hz) ticks waiting for CSR_WRITE_2 to
> > > return the AN_EV_CMD flag.  There is no code handling a case where
> > > this fails to happen.
> > > 
> > > What we do in practice is very nearly equivalent to spinning for 3
> > > seconds waiting for CSR_WRITE_2 to return the AN_EV_CMD flag, so I
> > > have converted it to use tsleep_nsec(9).
> > > 
> > > This compiles on amd64 but I can't test it.
> > > 
> > > Thoughts?  ok?
> > 
> > I don't see why the upper bound would have to be so precise.
> > 
> > Why not just
> > 
> > for (i = 0; i < 3000; i += 100) {
> >     if (CSR_READ_2(sc, AN_EVENT_STAT) & AN_EV_CMD)
> >             break;
> >     tsleep_nsec(sc, PWAIT, "anatch", MSEC_TO_NSEC(100));
> > }
> > 
> 
> Agreed, that seems more in the spirit of such wait loops.

+1

> > > Index: an.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/ic/an.c,v
> > > retrieving revision 1.76
> > > diff -u -p -r1.76 an.c
> > > --- an.c  10 Jul 2020 13:26:37 -0000      1.76
> > > +++ an.c  25 Nov 2020 01:19:16 -0000
> > > @@ -678,13 +678,18 @@ an_linkstat_intr(struct an_softc *sc)
> > >  void
> > >  an_wait(struct an_softc *sc)
> > >  {
> > > - int i;
> > > + struct timespec now, end;
> > > +
> > > + nanouptime(&now);
> > > + end = now;
> > > + end.tv_sec += 3;        /* spin for at most three seconds */
> > >  
> > >   CSR_WRITE_2(sc, AN_COMMAND, AN_CMD_NOOP2);
> > > - for (i = 0; i < 3*hz; i++) {
> > > -         if (CSR_READ_2(sc, AN_EVENT_STAT) & AN_EV_CMD)
> > > + while ((CSR_READ_2(sc, AN_EVENT_STAT) & AN_EV_CMD) == 0) {
> > > +         nanouptime(&now);
> > > +         if (timespeccmp(&end, &now, <=))
> > >                   break;
> > > -         (void)tsleep(sc, PWAIT, "anatch", 1);
> > > +         tsleep_nsec(sc, PWAIT, "anatch", MSEC_TO_NSEC(10));
> > >   }
> > >   CSR_WRITE_2(sc, AN_EVENT_ACK, AN_EV_CMD);
> > >  }
> > > 
> > > 
> > 
> 
> -- 
> :wq Claudio
> 
> 

Reply via email to