Hi Yao Zi. On Thu, 2026-09-10 at 12:23 +0000, Yao Zi wrote: > Hi Nikita, > > Sorry for sending out this review late. My system crashed earlier > this > day and I forgot this mail in the draft folder.
No problem, all comments have been taken into account and will be corrected. > > 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 <[email protected]> > > --- > > 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
