Re: [PATCH v4 2/2] riscv: timer: Enable early timer for M‑mode

[Yao Zi <me@ziyao.cc>]

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 <nikita.shubin@maquefel.me>
> 
> 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 :)

> 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