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

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

[Torsten Duwe]

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(-)

-- 
2.35.3

[PATCH 1/2] riscv: allow riscv timer to be instantiated via device tree

[Torsten Duwe]

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,
-- 
2.35.3

Re: [PATCH 1/2] riscv: allow riscv timer to be instantiated via device tree

[Leo Liang]

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>

[PATCH 2/2] riscv: jh7110: enable riscv,timer in the device tree

[Torsten Duwe]

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";
-- 
2.35.3

Re: [PATCH 2/2] riscv: jh7110: enable riscv,timer in the device tree

[Leo Liang]

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>