Re: tht(4): more tsleep(9) -> tsleep_nsec(9) conversions
ok
> On 17 Dec 2020, at 04:22, Scott Cheloha wrote:
>
> On Thu, Dec 03, 2020 at 09:59:11PM -0600, Scott Cheloha wrote:
>> Hi,
>>
>> tht(4) is another driver still using tsleep(9).
>>
>> It uses it to spin while it waits for the card to load the firmware.
>> Then it uses it to spin for up to 2 seconds while waiting for
>> THT_REG_INIT_STATUS.
>>
>> In the firmware case we can sleep for 10 milliseconds each iteration.
>>
>> In the THT_REG_INIT_STATUS loop we can sleep for 10 milliseconds each
>> iteration again, but instead of using a timeout to set a flag after 2
>> seconds we can just count how many milliseconds we've slept. This is
>> less precise than using the timeout but it is much simpler. Obviously
>> we then need to remove all the timeout-related stuff from the function
>> and the file.
>>
>> Thoughts? ok?
>
> Two week bump.
>
> Index: if_tht.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_tht.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 if_tht.c
> --- if_tht.c 10 Jul 2020 13:26:38 - 1.142
> +++ if_tht.c 4 Dec 2020 03:57:21 -
> @@ -582,7 +582,6 @@ void tht_lladdr_read(struct
> tht_softc
> void tht_lladdr_write(struct tht_softc *);
> int tht_sw_reset(struct tht_softc *);
> int tht_fw_load(struct tht_softc *);
> -void tht_fw_tick(void *arg);
> void tht_link_state(struct tht_softc *);
>
> /* interface operations */
> @@ -1667,11 +1666,9 @@ tht_sw_reset(struct tht_softc *sc)
> int
> tht_fw_load(struct tht_softc *sc)
> {
> - struct timeout ticker;
> - volatile intok = 1;
> u_int8_t*fw, *buf;
> size_t fwlen, wrlen;
> - int error = 1;
> + int error = 1, msecs, ret;
>
> if (loadfirmware("tht", &fw, &fwlen) != 0)
> return (1);
> @@ -1682,7 +1679,9 @@ tht_fw_load(struct tht_softc *sc)
> buf = fw;
> while (fwlen > 0) {
> while (tht_fifo_writable(sc, &sc->sc_txt) <= THT_FIFO_GAP) {
> - if (tsleep(sc, PCATCH, "thtfw", 1) == EINTR)
> + ret = tsleep_nsec(sc, PCATCH, "thtfw",
> + MSEC_TO_NSEC(10));
> + if (ret == EINTR)
> goto err;
> }
>
> @@ -1695,32 +1694,21 @@ tht_fw_load(struct tht_softc *sc)
> buf += wrlen;
> }
>
> - timeout_set(&ticker, tht_fw_tick, (void *)&ok);
> - timeout_add_sec(&ticker, 2);
> - while (ok) {
> + for (msecs = 0; msecs < 2000; msecs += 10) {
> if (tht_read(sc, THT_REG_INIT_STATUS) != 0) {
> error = 0;
> break;
> }
> -
> - if (tsleep(sc, PCATCH, "thtinit", 1) == EINTR)
> + ret = tsleep_nsec(sc, PCATCH, "thtinit", MSEC_TO_NSEC(10));
> + if (ret == EINTR)
> goto err;
> }
> - timeout_del(&ticker);
>
> tht_write(sc, THT_REG_INIT_SEMAPHORE, 0x1);
>
> err:
> free(fw, M_DEVBUF, fwlen);
> return (error);
> -}
> -
> -void
> -tht_fw_tick(void *arg)
> -{
> - volatile int*ok = arg;
> -
> - *ok = 0;
> }
>
> void
Re: tht(4): more tsleep(9) -> tsleep_nsec(9) conversions
On Thu, Dec 03, 2020 at 09:59:11PM -0600, Scott Cheloha wrote:
> Hi,
>
> tht(4) is another driver still using tsleep(9).
>
> It uses it to spin while it waits for the card to load the firmware.
> Then it uses it to spin for up to 2 seconds while waiting for
> THT_REG_INIT_STATUS.
>
> In the firmware case we can sleep for 10 milliseconds each iteration.
>
> In the THT_REG_INIT_STATUS loop we can sleep for 10 milliseconds each
> iteration again, but instead of using a timeout to set a flag after 2
> seconds we can just count how many milliseconds we've slept. This is
> less precise than using the timeout but it is much simpler. Obviously
> we then need to remove all the timeout-related stuff from the function
> and the file.
>
> Thoughts? ok?
Two week bump.
Index: if_tht.c
===
RCS file: /cvs/src/sys/dev/pci/if_tht.c,v
retrieving revision 1.142
diff -u -p -r1.142 if_tht.c
--- if_tht.c10 Jul 2020 13:26:38 - 1.142
+++ if_tht.c4 Dec 2020 03:57:21 -
@@ -582,7 +582,6 @@ voidtht_lladdr_read(struct
tht_softc
void tht_lladdr_write(struct tht_softc *);
inttht_sw_reset(struct tht_softc *);
inttht_fw_load(struct tht_softc *);
-void tht_fw_tick(void *arg);
void tht_link_state(struct tht_softc *);
/* interface operations */
@@ -1667,11 +1666,9 @@ tht_sw_reset(struct tht_softc *sc)
int
tht_fw_load(struct tht_softc *sc)
{
- struct timeout ticker;
- volatile intok = 1;
u_int8_t*fw, *buf;
size_t fwlen, wrlen;
- int error = 1;
+ int error = 1, msecs, ret;
if (loadfirmware("tht", &fw, &fwlen) != 0)
return (1);
@@ -1682,7 +1679,9 @@ tht_fw_load(struct tht_softc *sc)
buf = fw;
while (fwlen > 0) {
while (tht_fifo_writable(sc, &sc->sc_txt) <= THT_FIFO_GAP) {
- if (tsleep(sc, PCATCH, "thtfw", 1) == EINTR)
+ ret = tsleep_nsec(sc, PCATCH, "thtfw",
+ MSEC_TO_NSEC(10));
+ if (ret == EINTR)
goto err;
}
@@ -1695,32 +1694,21 @@ tht_fw_load(struct tht_softc *sc)
buf += wrlen;
}
- timeout_set(&ticker, tht_fw_tick, (void *)&ok);
- timeout_add_sec(&ticker, 2);
- while (ok) {
+ for (msecs = 0; msecs < 2000; msecs += 10) {
if (tht_read(sc, THT_REG_INIT_STATUS) != 0) {
error = 0;
break;
}
-
- if (tsleep(sc, PCATCH, "thtinit", 1) == EINTR)
+ ret = tsleep_nsec(sc, PCATCH, "thtinit", MSEC_TO_NSEC(10));
+ if (ret == EINTR)
goto err;
}
- timeout_del(&ticker);
tht_write(sc, THT_REG_INIT_SEMAPHORE, 0x1);
err:
free(fw, M_DEVBUF, fwlen);
return (error);
-}
-
-void
-tht_fw_tick(void *arg)
-{
- volatile int*ok = arg;
-
- *ok = 0;
}
void
tht(4): more tsleep(9) -> tsleep_nsec(9) conversions
Hi,
tht(4) is another driver still using tsleep(9).
It uses it to spin while it waits for the card to load the firmware.
Then it uses it to spin for up to 2 seconds while waiting for
THT_REG_INIT_STATUS.
In the firmware case we can sleep for 10 milliseconds each iteration.
In the THT_REG_INIT_STATUS loop we can sleep for 10 milliseconds each
iteration again, but instead of using a timeout to set a flag after 2
seconds we can just count how many milliseconds we've slept. This is
less precise than using the timeout but it is much simpler. Obviously
we then need to remove all the timeout-related stuff from the function
and the file.
Thoughts? ok?
Index: if_tht.c
===
RCS file: /cvs/src/sys/dev/pci/if_tht.c,v
retrieving revision 1.142
diff -u -p -r1.142 if_tht.c
--- if_tht.c10 Jul 2020 13:26:38 - 1.142
+++ if_tht.c4 Dec 2020 03:57:21 -
@@ -582,7 +582,6 @@ voidtht_lladdr_read(struct
tht_softc
void tht_lladdr_write(struct tht_softc *);
inttht_sw_reset(struct tht_softc *);
inttht_fw_load(struct tht_softc *);
-void tht_fw_tick(void *arg);
void tht_link_state(struct tht_softc *);
/* interface operations */
@@ -1667,11 +1666,9 @@ tht_sw_reset(struct tht_softc *sc)
int
tht_fw_load(struct tht_softc *sc)
{
- struct timeout ticker;
- volatile intok = 1;
u_int8_t*fw, *buf;
size_t fwlen, wrlen;
- int error = 1;
+ int error = 1, msecs, ret;
if (loadfirmware("tht", &fw, &fwlen) != 0)
return (1);
@@ -1682,7 +1679,9 @@ tht_fw_load(struct tht_softc *sc)
buf = fw;
while (fwlen > 0) {
while (tht_fifo_writable(sc, &sc->sc_txt) <= THT_FIFO_GAP) {
- if (tsleep(sc, PCATCH, "thtfw", 1) == EINTR)
+ ret = tsleep_nsec(sc, PCATCH, "thtfw",
+ MSEC_TO_NSEC(10));
+ if (ret == EINTR)
goto err;
}
@@ -1695,32 +1694,21 @@ tht_fw_load(struct tht_softc *sc)
buf += wrlen;
}
- timeout_set(&ticker, tht_fw_tick, (void *)&ok);
- timeout_add_sec(&ticker, 2);
- while (ok) {
+ for (msecs = 0; msecs < 2000; msecs += 10) {
if (tht_read(sc, THT_REG_INIT_STATUS) != 0) {
error = 0;
break;
}
-
- if (tsleep(sc, PCATCH, "thtinit", 1) == EINTR)
+ ret = tsleep_nsec(sc, PCATCH, "thtinit", MSEC_TO_NSEC(10));
+ if (ret == EINTR)
goto err;
}
- timeout_del(&ticker);
tht_write(sc, THT_REG_INIT_SEMAPHORE, 0x1);
err:
free(fw, M_DEVBUF, fwlen);
return (error);
-}
-
-void
-tht_fw_tick(void *arg)
-{
- volatile int*ok = arg;
-
- *ok = 0;
}
void
