On Fri, 2026-09-11 at 09:03 +0000, Yao Zi wrote:
> On Fri, Sep 11, 2026 at 11:36:35AM +0300, Nikita Shubin wrote:
> > On Fri, 2026-09-11 at 08:27 +0000, Yao Zi wrote:
> > > On Fri, Sep 11, 2026 at 10:14:06AM +0300, Nikita Shubin wrote:
> > > > The generic RISC-V timer driver currently defines
> > > > timer_early_get_count() only when
> > > > CONFIG_IS_ENABLED(RISCV_SMODE),
> > > > even though reading the TIME CSR is not inherently limited
> > > > to S‑mode; it works in M‑mode as well when the CSR is
> > > > implemented
> > > > in hardware (e.g., with the Zicntr extension).
> > > >
> > > > Moreover, timer_early_get_rate() is missing entirely for
> > > > M‑mode,
> > > > causing early timer functions to be unavailable on such
> > > > systems.
> > > >
> > > > Fix this by:
> > > > - Moving timer_early_get_count() out of the RISCV_SMODE guard
> > > > so it is always available when CONFIG_TIMER_EARLY is set.
> > > > - Adding M‑mode support to timer_early_get_rate(), returning
> > > > RISCV_MMODE_TIMER_FREQ when running in M‑mode
> > > > and RISCV_SMODE_TIMER_FREQ in S‑mode.
> > > >
> > > > This is also necessary because several functions
> > > > (e.g., net_random_ethaddr() via get_ticks()) rely on
> > > > timer_early_get_count() even if CONFIG_TIMER_EARLY is not
> > > > enabled.
> > >
> > > Is this description correct? net_random_ethaddr() does invoke
> > > get_ticks(), but the latter only delegates the work to
> > > timer_early_get_count() when CONFIG_TIMER_EARLY is enabled and
> > > gd-
> > > > timer
> > > hasn't been initialized,
> > >
> > > uint64_t notrace get_ticks(void)
> > > {
> > > u64 count;
> > > int ret;
> > >
> > > if (!gd->timer) {
> > > int ret;
> > >
> > > if (IS_ENABLED(CONFIG_TIMER_EARLY))
> > > return timer_early_get_count();
> > > ...
> > >
> > >
> > > > Signed-off-by: Nikita Shubin <[email protected]>
> >
> > Totally correct as this i how i came into fixing the RISCV TIMER,
> > as
> > if (IS_ENABLED(CONFIG_TIMER_EARLY)) doesn't protect us from linking
> > error if timer_early_get_count() is missing (unless i am missing
> > something).
>
> With optimization, this branch should be turned into dead code and
> the reference is thus eliminated. But anyway, please improve the
> description to mention it's only a possible linking time dependency.
> I'm
> simply confused by "rely on" since this should be dead code without
> CONFIG_TIMER_EARLY :)
Yep it's true - should be rephrased into:
"This is also necessary because several functions
(e.g., net_random_ethaddr() via get_ticks()) rely on
timer_early_get_count() and we should provide one if CONFIG_TIMER_EARLY
is enabled."
>
> > It could be simply avoided by turning on any kind of ACLINT/CLINT
> > MTIMER (everybody has one of some sort) - but why i need it if i
> > can
> > rely on RISCV TIMER in SPL stage ?
> >
> > It seems no RISC-V board is currently using BOOTP/TFTP boot in SPL.
> >
> > >
> > > Otherwise this patch looks good to me.
> > >
> > > Best regards,
> > > Yao Zi
>
> Best regards,
> Yao Zi
