On Mon, Jun 7, 2021 at 1:57 PM <jan.som...@dlr.de> wrote: > > > > -----Original Message----- > > From: Gedare Bloom <ged...@rtems.org> > > Sent: Monday, June 7, 2021 7:00 PM > > To: Sommer, Jan <jan.som...@dlr.de> > > Cc: devel@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.som...@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). > >
No. I'm more worried about boot time. :) > > > 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. > > How does this impact the timeline for boot to first application thread? Is there a period where the system is up but only one core? Any other oddities I might be missing? > > > --- > > > 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@rtems.org > > > http://lists.rtems.org/mailman/listinfo/devel > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel