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.
--
:wq Claudio
