[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 07/27] xen/riscv: implement make_timer_node()





On 4/1/26 4:24 PM, Jan Beulich wrote:
On 10.03.2026 18:08, Oleksii Kurochko wrote:
The make_timer_node() function is implemented to return 0,
as RISC-V does not require the creation of a timer node.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Yet: Why is this needed then in the first place? If this node may not
be required, why would the function need providing? Shouldn't the call
site be conditional then?

Generally, in DT for RISC-V there is a document which describes timer node (riscv,timer.yaml or sifive,clint.yaml), but the driver (drivers/clocksource/timer-riscv.c:244) is declared with TIMER_OF_DECLARE(riscv_timer, "riscv", ...). It matches the CPU node (compatible "riscv"), not the timer node itself. It then does of_find_compatible_node(NULL, NULL, "riscv,timer") only to read the optional riscv,timer-cannot-wake-cpu flag. But I don't see too much sense at the moment to enable timer-cannot-wake-cpu option.

Regarding, sifive,clint.yaml basically it has almost the same as riscv,timer node but also additionally provide some MMIO which are acessible only in M-mode (what isn't the case for virtualization as guest isn't ran in M-mode, otherwise emulation of M-mode register will be needed).

So potentially some will want to add riscv,timer node to enable timer-cannot-wake-cpu option, so then some changes additionally should be done in this function.

Considering that it isn't needed now and timebase-frequency property is always part of cpu node not a timer one, I think we can go with an empty implementation of make_timer_node().
I can add some extra information to commit message.

~ Oleksii








 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.