> Date: Wed, 6 Jan 2021 16:16:27 -0600
> From: Scott Cheloha <scottchel...@gmail.com>
> 
> On Wed, Jan 06, 2021 at 12:16:13PM -0600, Scott Cheloha wrote:
> > As mentioned in a prior mail, tpm(4) is the last user of tvtohz(9) in
> > the tree.
> > 
> > However, we don't need to use tvtohz(9) in tpm(4) at all.  Converting
> > from milliseconds to ticks is trivial.  Using an intermediary timeval
> > is just pointless indirection.
> > 
> > With this committed I will be able to remove both tvtohz(9) and
> > tstohz(9) from the tree in a subsequent patch.
> 
> Whoops, made a mistake in the prior diff.
> 
> We want to MAX() the result up to 1 to avoid dividing to zero and
> blocking indefinitely in e.g. tsleep(9).  Previous diff MIN'd the
> result down to 1, which is very wrong.

To be honest I'd just zap the function completely and instead simply do:

    to = TPM_ACCESS_TMO / 10;

and

    to = TPM_BURST_TMO / 10;

There is no magic happening here.  The code is just doing a hardware
register poll loop in 10us steps.

Hmm, actually it seems the code is broken and uses steps of 10
microseconds instead of milliseconds.  So instead it should probably
use:

    to = TPM_ACCESS_TMO * 100;

and

    to = TPM_BURST_TMO * 100;

Cheers,

Mark

> Index: tpm.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/tpm.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 tpm.c
> --- tpm.c     22 May 2020 10:16:37 -0000      1.10
> +++ tpm.c     6 Jan 2021 22:09:49 -0000
> @@ -24,6 +24,7 @@
>   */
>  
>  #include <sys/param.h>
> +#include <sys/kernel.h>
>  #include <sys/systm.h>
>  #include <sys/device.h>
>  #include <sys/malloc.h>
> @@ -455,12 +456,7 @@ tpm_status(struct tpm_softc *sc)
>  int
>  tpm_tmotohz(int tmo)
>  {
> -     struct timeval tv;
> -
> -     tv.tv_sec = tmo / 1000;
> -     tv.tv_usec = 1000 * (tmo % 1000);
> -
> -     return tvtohz(&tv);
> +     return MAX(1, tmo * hz / 1000);
>  }
>  
>  int
> 
> 

Reply via email to