[PATCH 0/2] riscv: jh7110: visionfive2: fix u-boot crash due to missing timer

Hi all,
Since commit 55171aedda8, U-Boot on the visionfive2 stops at
| initcall sequence 00000000fffd76f8 failed at call 00000000402192c8 (err=-19) | ### ERROR ### Please RESET the board ###
This is init_sequence_r[initr_dm_devices] calling dm_timer_init, which returns ENODEV, because the riscv architectural timer got initialised at the ROM stage, but then discarded at relocation, and never again to be re-registered. A workaround hack was to allow it to register again in drivers/core/root.c line 439:
if (CONFIG_IS_ENABLED(DM_EVENT) /* && !(gd->flags & GD_FLG_RELOC) */ ) {
but that would defeat the purpose of the commit 55171aedda8 cleanup.
However, the timer has a defined device tree binding which, when used, IMHO makes things a lot clearer. AFAIU, the timer roughly corresponds to the x86 TSC, which is defined on many x86 platforms, compare yourself:
grep -r tsc_timer.dtsi arch/x86/dts vs. grep -r '"riscv,timer"' arch/riscv/dts
At first I tried to create a series that converts all riscv platforms to use the DT for the CPU timer and removes the calls from the CPU driver, but this takes too long. Release early, release often -- these two patches fix the visionfive2, for a start. Others may follow and the CPU driver calls can be removed later when they are not needed any longer.
Torsten
Fixes: 55171aedda8 ("dm: Emit the arch_cpu_init_dm() even only before relocation") --- Torsten Duwe (2): riscv: allow riscv timer to be instantiated via device tree riscv: jh7110: enable riscv,timer in the device tree
arch/riscv/dts/jh7110.dtsi | 9 +++++++++ drivers/timer/riscv_timer.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-)

For the architectural timer on riscv, there already is a defined device tree binding[1]. Allow timer instances to be created from device tree matches, but for now retain the old mechanism, which registers the timer biggy-back with the CPU.
[1] linux/Documentation/devicetree/bindings/timer/riscv,timer.yaml
Signed-off-by: Torsten Duwe duwe@suse.de --- drivers/timer/riscv_timer.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c index 3627ed79b8..28a6a6870b 100644 --- a/drivers/timer/riscv_timer.c +++ b/drivers/timer/riscv_timer.c @@ -13,6 +13,7 @@ #include <common.h> #include <dm.h> #include <errno.h> +#include <fdt_support.h> #include <timer.h> #include <asm/csr.h>
@@ -53,9 +54,26 @@ u64 notrace timer_early_get_count(void) static int riscv_timer_probe(struct udevice *dev) { struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + u32 rate;
- /* clock frequency was passed from the cpu driver as driver data */ - uc_priv->clock_rate = dev->driver_data; + /* When this function was called from the CPU driver, clock + * frequency is passed as driver data. + */ + rate = dev->driver_data; + + /* When called from an FDT match, the rate needs to be looked up. */ + if (!rate && gd->fdt_blob) { + rate = fdt_getprop_u32_default(gd->fdt_blob, + "/cpus", "timebase-frequency", 0); + } + + uc_priv->clock_rate = rate; + + /* With rate==0, timer uclass post_probe might later fail with -EINVAL. + * Give a hint at the cause for debugging. + */ + if (!rate) + log_err("riscv_timer_probe with invalid clock rate 0!\n");
return 0; } @@ -64,9 +82,15 @@ static const struct timer_ops riscv_timer_ops = { .get_count = riscv_timer_get_count, };
+static const struct udevice_id riscv_timer_ids[] = { + { .compatible = "riscv,timer", }, + { } +}; + U_BOOT_DRIVER(riscv_timer) = { .name = "riscv_timer", .id = UCLASS_TIMER, + .of_match = of_match_ptr(riscv_timer_ids), .probe = riscv_timer_probe, .ops = &riscv_timer_ops, .flags = DM_FLAG_PRE_RELOC,

On Mon, Aug 14, 2023 at 06:05:28PM +0200, Torsten Duwe wrote:
For the architectural timer on riscv, there already is a defined device tree binding[1]. Allow timer instances to be created from device tree matches, but for now retain the old mechanism, which registers the timer biggy-back with the CPU.
[1] linux/Documentation/devicetree/bindings/timer/riscv,timer.yaml
Signed-off-by: Torsten Duwe duwe@suse.de
drivers/timer/riscv_timer.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com

The JH7110 has the arhitectural CPU timer on all 5 rv64 cores. Note that in the device tree.
Signed-off-by: Torsten Duwe duwe@suse.de --- arch/riscv/dts/jh7110.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi index 081b833331..ec237a46ff 100644 --- a/arch/riscv/dts/jh7110.dtsi +++ b/arch/riscv/dts/jh7110.dtsi @@ -163,6 +163,15 @@ }; };
+ timer { + compatible = "riscv,timer"; + interrupts-extended = <&cpu0_intc 5>, + <&cpu1_intc 5>, + <&cpu2_intc 5>, + <&cpu3_intc 5>, + <&cpu4_intc 5>; + }; + osc: oscillator { compatible = "fixed-clock"; clock-output-names = "osc";

On Mon, Aug 14, 2023 at 06:05:33PM +0200, Torsten Duwe wrote:
The JH7110 has the arhitectural CPU timer on all 5 rv64 cores. Note that in the device tree.
Signed-off-by: Torsten Duwe duwe@suse.de
arch/riscv/dts/jh7110.dtsi | 9 +++++++++ 1 file changed, 9 insertions(+)
Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com
participants (2)
-
duwe@lst.de
-
Leo Liang