[PATCH] riscv: timer: Fix M-Mode timer

[PATCH] riscv: timer: Fix M-Mode timer

[Nikita Shubin]

RISC-V timer is missing timer_early_get_rate() for M-Mode.

Also timer_early_get_count() is guarded by RISCV_SMODE for no
clear reason.

Make timer_early_get_rate() use RISCV_MMODE_TIMER_FREQ in M-Mode and
move timer_early_get_count() out of define.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/timer/riscv_timer.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 1f4980ceb38..65f174fb879 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -33,14 +33,22 @@ static u64 notrace riscv_timer_get_count(struct udevice *dev)
 	return ((u64)hi << 32) | lo;
 }
 
-#if CONFIG_IS_ENABLED(RISCV_SMODE) && IS_ENABLED(CONFIG_TIMER_EARLY)
 /**
  * timer_early_get_rate() - Get the timer rate before driver model
  */
+#if IS_ENABLED(CONFIG_TIMER_EARLY)
+#if CONFIG_IS_ENABLED(RISCV_SMODE)
 unsigned long notrace timer_early_get_rate(void)
 {
 	return RISCV_SMODE_TIMER_FREQ;
 }
+#elif CONFIG_IS_ENABLED(RISCV_MMODE)
+unsigned long notrace timer_early_get_rate(void)
+{
+	return RISCV_MMODE_TIMER_FREQ;
+}
+#endif
+#endif
 
 /**
  * timer_early_get_count() - Get the timer count before driver model
@@ -50,7 +58,6 @@ u64 notrace timer_early_get_count(void)
 {
 	return riscv_timer_get_count(NULL);
 }
-#endif
 
 #if CONFIG_IS_ENABLED(RISCV_SMODE) && CONFIG_IS_ENABLED(BOOTSTAGE)
 ulong timer_get_boot_us(void)

---
base-commit: cc557af4553382f6f50e3ed62b9577054e7bc54f
change-id: 20260904-riscv_fix_early_timer_mmode-174f3ee4e106

Best regards,
-- 
Nikita Shubin

Re: [PATCH] riscv: timer: Fix M-Mode timer

[Yao Zi]

On Fri, Sep 04, 2026 at 09:12:55AM +0300, Nikita Shubin wrote:
> RISC-V timer is missing timer_early_get_rate() for M-Mode.
> 
> Also timer_early_get_count() is guarded by RISCV_SMODE for no
> clear reason.

This driver is described as "support for a generic RISC-V S-Mode timer
driver", which should be the original reason to mark it as S-Mode only.
It should be intentionally to split M-mode (riscv_aclint_timer.c,
guarded by CONFIG_RISCV_ACLINT) and S-mode timers, since riscv_timer.c
reads ticks from TIME CSR, while at least SiFive cores miss it in
M-mode, and rely on OpenSBI to emulate them.

But this separation is indeed unnecessary, there are as well cores
implementing TIME CSR, so riscv_timer.c works on them even in M-mode,
too, e.g., T-Head ones.

I've summarized the situation here[1], but didn't find enough time and
energy to send out a series to fix issues up...

Anyway, please fix the Kconfig help text, too.

> Make timer_early_get_rate() use RISCV_MMODE_TIMER_FREQ in M-Mode and
> move timer_early_get_count() out of define.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  drivers/timer/riscv_timer.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
> index 1f4980ceb38..65f174fb879 100644
> --- a/drivers/timer/riscv_timer.c
> +++ b/drivers/timer/riscv_timer.c

At start of the file, a comment mentions "This driver provides generic
timer support for S-mode U-Boot". Please update it along the way.

> @@ -33,14 +33,22 @@ static u64 notrace riscv_timer_get_count(struct udevice *dev)
>  	return ((u64)hi << 32) | lo;
>  }
>  
> -#if CONFIG_IS_ENABLED(RISCV_SMODE) && IS_ENABLED(CONFIG_TIMER_EARLY)
>  /**
>   * timer_early_get_rate() - Get the timer rate before driver model
>   */
> +#if IS_ENABLED(CONFIG_TIMER_EARLY)
> +#if CONFIG_IS_ENABLED(RISCV_SMODE)
>  unsigned long notrace timer_early_get_rate(void)
>  {
>  	return RISCV_SMODE_TIMER_FREQ;
>  }
> +#elif CONFIG_IS_ENABLED(RISCV_MMODE)
> +unsigned long notrace timer_early_get_rate(void)
> +{
> +	return RISCV_MMODE_TIMER_FREQ;
> +}
> +#endif
> +#endif

For platforms like starfive_visionfive2_defconfig where both
CONFIG_RISCV_TIMER and CONFIG_RISCV_ACLINT are enabled, riscv_timer.c
and riscv_aclint_timer.c would each provide their own early timer
symbols and causing linking errors when building with defconfig.

These platforms might depend on the IPI functionality provided by
aclint_ipi.c, which is also guarded by CONFIG_RISCV_ACLINT; and
riscv_timer.c might not work on them, either, thus simply disabling
CONFIG_RISCV_ACLINT isn't an option.

Regards,
Yao Zi

[1]: https://lore.kernel.org/all/Z7dMY_GqLHsC-5gl@pie.lan/

Re: [PATCH] riscv: timer: Fix M-Mode timer

[Nikita Shubin]

Hi Yao Zi,

> https://lore.kernel.org/all/Z7dMY_GqLHsC-5gl@pie.lan/

> TIME CSR only when Zicntr extension exists,

Indeed, also TIME CSR obviously is available for M-Mode in that case.

> - Remove the timer-binding code in drivers/cpu/riscv_cpu.c

There is a caveat that, we have "riscv,timer" compatible which is
defined in documentation, but never used anywhere.

So we must manually instantiate it somewhere. May be it's a good idea
to move it into `riscv_cpu_setup()`, we can also check extension there
(in case they specified in dts for those who rely on it).

> - Correct help text and comments for riscv_timer.c, just like what
has
> been done in this patch.

Agree.

> - Rename RISCV_SMODE_TIMER_FREQ to RISCV_EARLY_TIMER_FREQ, clean up
>  preprocessor instructions that limit some functions to S-Mode only.
>  They shouldn't be S-mode only stuff.

Agree. But still RISCV_SMODE_TIMER_FREQ/RISCV_MMODE_TIMER_FREQ in
theory might differ, if, for example we are switching clock source
somewhere in SPL phase.

> - For future RISC-V cores that are capable of reading timestamp from
>  TIME CSR, we could either register the compatible string of its
timer
>  to riscv_timer.c (T-Head case, the underlying CLINT isn't a real
>  SSTC-capable device) or add a "riscv,timer" node.

> +	{ .compatible = "thead,c900-clint" },

I think Zicntr is orthogonal to a particular CLINT implementation.
There might be some cases when MMIO access is compatible with some
existing CLINT implementations, but the hart lacks Zicntr.


On Fri, 2026-09-04 at 19:17 +0000, Yao Zi wrote:
> On Fri, Sep 04, 2026 at 09:12:55AM +0300, Nikita Shubin wrote:
> > RISC-V timer is missing timer_early_get_rate() for M-Mode.
> > 
> > Also timer_early_get_count() is guarded by RISCV_SMODE for no
> > clear reason.
> 
> This driver is described as "support for a generic RISC-V S-Mode
> timer
> driver", which should be the original reason to mark it as S-Mode
> only.
> It should be intentionally to split M-mode (riscv_aclint_timer.c,
> guarded by CONFIG_RISCV_ACLINT) and S-mode timers, since
> riscv_timer.c
> reads ticks from TIME CSR, while at least SiFive cores miss it in
> M-mode, and rely on OpenSBI to emulate them.
> 
> But this separation is indeed unnecessary, there are as well cores
> implementing TIME CSR, so riscv_timer.c works on them even in M-mode,
> too, e.g., T-Head ones.
> 
> I've summarized the situation here[1], but didn't find enough time
> and
> energy to send out a series to fix issues up...
> 
> Anyway, please fix the Kconfig help text, too.

Agree.

> 
> > Make timer_early_get_rate() use RISCV_MMODE_TIMER_FREQ in M-Mode
> > and
> > move timer_early_get_count() out of define.
> > 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> > ---
> >  drivers/timer/riscv_timer.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/timer/riscv_timer.c
> > b/drivers/timer/riscv_timer.c
> > index 1f4980ceb38..65f174fb879 100644
> > --- a/drivers/timer/riscv_timer.c
> > +++ b/drivers/timer/riscv_timer.c
> 
> At start of the file, a comment mentions "This driver provides
> generic
> timer support for S-mode U-Boot". Please update it along the way.
> 
> > @@ -33,14 +33,22 @@ static u64 notrace riscv_timer_get_count(struct
> > udevice *dev)
> >  	return ((u64)hi << 32) | lo;
> >  }
> >  
> > -#if CONFIG_IS_ENABLED(RISCV_SMODE) &&
> > IS_ENABLED(CONFIG_TIMER_EARLY)
> >  /**
> >   * timer_early_get_rate() - Get the timer rate before driver model
> >   */
> > +#if IS_ENABLED(CONFIG_TIMER_EARLY)
> > +#if CONFIG_IS_ENABLED(RISCV_SMODE)
> >  unsigned long notrace timer_early_get_rate(void)
> >  {
> >  	return RISCV_SMODE_TIMER_FREQ;
> >  }
> > +#elif CONFIG_IS_ENABLED(RISCV_MMODE)
> > +unsigned long notrace timer_early_get_rate(void)
> > +{
> > +	return RISCV_MMODE_TIMER_FREQ;
> > +}
> > +#endif
> > +#endif
> 
> For platforms like starfive_visionfive2_defconfig where both
> CONFIG_RISCV_TIMER and CONFIG_RISCV_ACLINT are enabled, riscv_timer.c
> and riscv_aclint_timer.c would each provide their own early timer
> symbols and causing linking errors when building with defconfig.
> 
> These platforms might depend on the IPI functionality provided by
> aclint_ipi.c, which is also guarded by CONFIG_RISCV_ACLINT; and
> riscv_timer.c might not work on them, either, thus simply disabling
> CONFIG_RISCV_ACLINT isn't an option.
> 
> Regards,
> Yao Zi
> 
> [1]: https://lore.kernel.org/all/Z7dMY_GqLHsC-5gl@pie.lan/

Re: [PATCH] riscv: timer: Fix M-Mode timer

[Yao Zi]

On Mon, Sep 07, 2026 at 10:22:54AM +0300, Nikita Shubin wrote:
> Hi Yao Zi,
> 
> > https://lore.kernel.org/all/Z7dMY_GqLHsC-5gl@pie.lan/
> 
> > TIME CSR only when Zicntr extension exists,
> 
> Indeed, also TIME CSR obviously is available for M-Mode in that case.
> 
> > - Remove the timer-binding code in drivers/cpu/riscv_cpu.c
> 
> There is a caveat that, we have "riscv,timer" compatible which is
> defined in documentation, but never used anywhere.

Yes, I later noticed this.

> So we must manually instantiate it somewhere. May be it's a good idea
> to move it into `riscv_cpu_setup()`, we can also check extension there
> (in case they specified in dts for those who rely on it).

We already have similar logic in riscv_cpu.c, but I have concerns for it,

- Instantiate the timer in riscv_cpu.c is quite surprising
- This prevents timer-uclass.c from probing the timer driver ahead of
  time when udelay(), and etc. is called.

  For example, a specific board might want to make use of udelay() in
  board_init_f(); if we only instantiate the timer in riscv_cpu.c, the
  board code must manually bind the CPU, or the timer isn't available.

> > - Correct help text and comments for riscv_timer.c, just like what
> has
> > been done in this patch.
> 
> Agree.
> 
> > - Rename RISCV_SMODE_TIMER_FREQ to RISCV_EARLY_TIMER_FREQ, clean up
> >  preprocessor instructions that limit some functions to S-Mode only.
> >  They shouldn't be S-mode only stuff.
> 
> Agree. But still RISCV_SMODE_TIMER_FREQ/RISCV_MMODE_TIMER_FREQ in
> theory might differ, if, for example we are switching clock source
> somewhere in SPL phase.

Yes, you're right. And in case that S-Mode timer (TIME CSR) is emulated
by M-mode firmware, they might not derive from the clock source at all.

> > - For future RISC-V cores that are capable of reading timestamp from
> >  TIME CSR, we could either register the compatible string of its
> timer
> >  to riscv_timer.c (T-Head case, the underlying CLINT isn't a real
> >  SSTC-capable device) or add a "riscv,timer" node.
> 
> > +	{ .compatible = "thead,c900-clint" },
> 
> I think Zicntr is orthogonal to a particular CLINT implementation.
> There might be some cases when MMIO access is compatible with some
> existing CLINT implementations, but the hart lacks Zicntr.

Yes. But T-Head's CLINT is kind of special, it lacks of a MTIME
register, but only exports the timer through the TIMER CSR in HART...
which is different from SiFive's implementation. I think in this case
it's not valuable to split the HART implementation and CLINT
implementation, and discuss them separately.

Anyway, you're not required to fix all these issues up to get this
patch merged :)

Thanks,
Yao Zi