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

[Yao Zi <me@ziyao.cc>]

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