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

[Yao Zi <me@ziyao.cc>]

Hi Nikita,

Sorry for sending out this review late. My system crashed earlier this
day and I forgot this mail in the draft folder.

On Wed, Sep 09, 2026 at 09:30:59AM +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.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  drivers/timer/Kconfig       | 13 +++++++++++--
>  drivers/timer/riscv_timer.c | 12 +++++++++---
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> index 500a25638a9..12c194ff74a 100644
> --- a/drivers/timer/Kconfig
> +++ b/drivers/timer/Kconfig
> @@ -227,8 +227,17 @@ config RISCV_TIMER
>  	bool "RISC-V timer support"
>  	depends on TIMER && RISCV
>  	help
> -	  Select this to enable support for a generic RISC-V S-Mode timer
> -	  driver.
> +	  Enable support for the generic RISC-V timer driver using the TIME CSR.
> +
> +	  This driver works in S-mode and also in M-mode if the TIME CSR is
> +	  implemented in hardware (e.g., when the Zicntr extension is present).
> +	  In M-mode, the timer frequency must be provided via the macro
> +	  RISCV_MMODE_TIMER_FREQ; in S-mode, use RISCV_SMODE_TIMER_FREQ.

This doesn't seem correct. These two constants are only used for the
early timer implementation, and the non-early one looks up the rate from
FDT blob (/cpus/timebase-frequency).

> +	  On platforms that also enable CLINT/ACLINT MTIMER, both drivers
> +	  may provide early timer functions and cause linking conflicts. Ensure
> +	  that only one of them is selected, or adjust the configuration to avoid
> +	  duplicate symbols.

Won't this issue fixed by PATCH 2? And I think this paragraph is too
detailed to be included in the Kconfig help.

Regards,
Yao Zi