> Date: Fri, 25 Dec 2020 11:52:03 -0600
> From: Scott Cheloha <scottchel...@gmail.com>
>
> On Wed, Dec 16, 2020 at 10:04:48PM +0100, Mark Kettenis wrote:
> > > Date: Wed, 16 Dec 2020 12:50:46 -0600
> > > From: Scott Cheloha <scottchel...@gmail.com>
> > >
> > > On Tue, Dec 15, 2020 at 01:47:24PM +0100, Mark Kettenis wrote:
> > > > > Date: Tue, 15 Dec 2020 13:32:22 +0100
> > > > > From: Claudio Jeker <cje...@diehard.n-r-g.com>
> > > > >
> > > > > On Fri, Dec 11, 2020 at 07:07:56PM -0600, Scott Cheloha wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I'd like to remove lbolt from the kernel. I think having it in the
> > > > > > kernel complicates otherwise simple code.
> > > > > >
> > > > > > We can start with sdmmc(4).
> > > > > >
> > > > > > The goal in sdmmc_io_function_enable() is calling
> > > > > > sdmmc_io_function_ready()
> > > > > > up to six times and sleep 1 second between each attempt. Here's
> > > > > > rewritten
> > > > > > code that does with without lbolt.
> > > > > >
> > > > > > ok?
> > > > > >
> > > > > > Index: sdmmc_io.c
> > > > > > ===================================================================
> > > > > > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> > > > > > retrieving revision 1.41
> > > > > > diff -u -p -r1.41 sdmmc_io.c
> > > > > > --- sdmmc_io.c 31 Dec 2019 10:05:33 -0000 1.41
> > > > > > +++ sdmmc_io.c 12 Dec 2020 01:04:59 -0000
> > > > > > @@ -231,8 +231,8 @@ sdmmc_io_function_enable(struct sdmmc_fu
> > > > > > {
> > > > > > struct sdmmc_softc *sc = sf->sc;
> > > > > > struct sdmmc_function *sf0 = sc->sc_fn0;
> > > > > > + int chan, retry = 5;
> > > > > > u_int8_t rv;
> > > > > > - int retry = 5;
> > > > > >
> > > > > > rw_assert_wrlock(&sc->sc_lock);
> > > > > >
> > > > > > @@ -244,7 +244,7 @@ sdmmc_io_function_enable(struct sdmmc_fu
> > > > > > sdmmc_io_write_1(sf0, SD_IO_CCCR_FN_ENABLE, rv);
> > > > > >
> > > > > > while (!sdmmc_io_function_ready(sf) && retry-- > 0)
> > > > > > - tsleep_nsec(&lbolt, PPAUSE, "pause", INFSLP);
> > > > > > + tsleep_nsec(&chan, PPAUSE, "pause", SEC_TO_NSEC(1));
> > > > > > return (retry >= 0) ? 0 : ETIMEDOUT;
> > > > > > }
> > > > > >
> > > > >
> > > > > Why not use &retry as wait channel instead of adding a new variable
> > > > > chan? Result is the same. Would it make sense to allow NULL as wait
> > > > > channel to make the tsleep not wakeable. At least that could be used
> > > > > in a
> > > > > few places where timeouts are implemented with tsleep and would make
> > > > > the
> > > > > intent more obvious.
> > > >
> > > > Or have an appropriately named global variable? Something like "int
> > > > nowake"?
> > >
> > > Something like the attached patch?
> > >
> > > I think the idea of a "dead channel" communicates the intent. Nobody
> > > broadcasts wakeups on the dead channel. If you don't want to receive
> > > wakeup broadcasts you sleep on the dead channel. Hence, "deadchan".
> >
> > Yeah, that's a reasonable name. Not sure if we need the indirection
> > though. Using &deadchan isn't an enormous burden I'd say.
>
> Now that we've settled on using &nowake, are we ok with the attached?
sure, ok kettenis@
> Index: sdmmc_io.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_io.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 sdmmc_io.c
> --- sdmmc_io.c 31 Dec 2019 10:05:33 -0000 1.41
> +++ sdmmc_io.c 25 Dec 2020 17:03:44 -0000
> @@ -244,7 +244,7 @@ sdmmc_io_function_enable(struct sdmmc_fu
> sdmmc_io_write_1(sf0, SD_IO_CCCR_FN_ENABLE, rv);
>
> while (!sdmmc_io_function_ready(sf) && retry-- > 0)
> - tsleep_nsec(&lbolt, PPAUSE, "pause", INFSLP);
> + tsleep_nsec(&nowake, PPAUSE, "pause", SEC_TO_NSEC(1));
> return (retry >= 0) ? 0 : ETIMEDOUT;
> }
>
>
