[PATCH v1] bsps/riscv: Give enough time for clock driver initialization

[Jan.Sommer at dlr.de]

> -----Original Message-----
> From: Gedare Bloom <gedare at rtems.org>
> Sent: Monday, June 7, 2021 7:00 PM
> To: Sommer, Jan <Jan.Sommer at dlr.de>
> Cc: devel at rtems.org
> Subject: Re: [PATCH v1] bsps/riscv: Give enough time for clock driver
> initialization
> 
> On Mon, Jun 7, 2021 at 9:47 AM Jan Sommer <jan.sommer at dlr.de> wrote:
> >
> > - Clock driver initialization for secondary cores had to take less
> > than one tick
> > - If tick time is small (i.e. <= 1ms) setting up all cores could take
> > too long and a fatal error is thrown.
> > - Give at least 10 ms time for clock initialization to avoid this
> > error
> 
> Is there a reason to pick 10?
>

In qemu I (coarsely) measured 1.5ms for 8 cores.
So I thought this should add enough buffer to prevent a fatal error.
I probably could also reduce it to 5 ms.
 
> I assume this blocks/idles the system until the interval elapses, so it would be
> good to minimize waste (subject to Joel's noted rant about premature
> optimization).
> 

If you take a look at the clock initialization of the secondary cores (https://git.rtems.org/rtems/tree/bsps/riscv/riscv/clock/clockdrv.c#n178):

_SMP_Othercast_action(riscv_clock_secondary_action, &cmpval);

  if (cmpval - riscv_clock_read_mtime(&clint->mtime) >= interval) {
    bsp_fatal(RISCV_FATAL_CLOCK_SMP_INIT);
  }

It will put the first clock tick 10ms into the future (instead of just one tick), but it won't block the system initialization.
It only prevents entering the if condition by having a large enough value for interval, but the runtime of the clock initialization is the same.

> > ---
> >  bsps/riscv/riscv/clock/clockdrv.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/bsps/riscv/riscv/clock/clockdrv.c
> > b/bsps/riscv/riscv/clock/clockdrv.c
> > index 3afe86576f..102137aeab 100644
> > --- a/bsps/riscv/riscv/clock/clockdrv.c
> > +++ b/bsps/riscv/riscv/clock/clockdrv.c
> > @@ -211,7 +211,13 @@ static void riscv_clock_initialize(void)
> >    tc->interval = interval;
> >
> >    cmpval = riscv_clock_read_mtime(&clint->mtime);
> > -  cmpval += interval;
> > +  /*
> > +   * For very short intervals the time of 1 tick is not enough to
> > +   * set up the timer on all cores in SMP systems.
> > +   * Give the CPU at least 10 ms.
> > +   */
> > +  interval = (10000 / us_per_tick) * interval;  cmpval += interval;
> >
> >    riscv_clock_clint_init(clint, cmpval, 0);
> >    riscv_clock_secondary_initialization(clint, cmpval, interval);
> > --
> > 2.17.1
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel