[PATCH 0/6] riscv: Clean up timer drivers

[PATCH 0/6] riscv: Clean up timer drivers

[Sean Anderson]

This series cleans up the timer drivers in RISC-V and converts them to DM.

This series depends on [1]. This series needs to be tested! I have only tested
it on QEMU and the K210. Notably, this means that the HiFive and anything Andes
is completely untested. CI for this series is located at [2].

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=190862
[2] https://dev.azure.com/seanga2/u-boot/_build/results?buildId=4


Sean Anderson (6):
  riscv: Rework riscv timer driver to only support S-mode
  riscv: Rework Andes PLMT as a UCLASS_TIMER driver
  riscv: Clean up initialization in Andes PLIC
  riscv: Rework Sifive CLINT as UCLASS_TIMER driver
  riscv: Update Kendryte device tree for new CLINT driver
  riscv: Update SiFive device tree for new CLINT driver

 arch/riscv/Kconfig                      |  8 ---
 arch/riscv/dts/ae350_32.dts             |  1 +
 arch/riscv/dts/ae350_64.dts             |  1 +
 arch/riscv/dts/fu540-c000-u-boot.dtsi   |  7 +-
 arch/riscv/dts/k210.dtsi                | 10 +--
 arch/riscv/include/asm/global_data.h    |  3 -
 arch/riscv/lib/Makefile                 |  1 -
 arch/riscv/lib/andes_plic.c             | 58 +++++++----------
 arch/riscv/lib/andes_plmt.c             | 42 ++++++------
 arch/riscv/lib/rdtime.c                 | 38 -----------
 arch/riscv/lib/sifive_clint.c           | 87 ++++++++++++++++---------
 common/spl/spl_opensbi.c                |  5 ++
 drivers/clk/kendryte/clk.c              |  4 ++
 drivers/ram/sifive/Kconfig              |  2 +
 drivers/timer/Kconfig                   |  6 +-
 drivers/timer/riscv_timer.c             | 39 +++++------
 include/dt-bindings/clock/k210-sysctl.h |  1 +
 17 files changed, 151 insertions(+), 162 deletions(-)
 delete mode 100644 arch/riscv/lib/rdtime.c

-- 
2.27.0

[PATCH 1/6] riscv: Rework riscv timer driver to only support S-mode

[Sean Anderson]

The riscv-timer driver currently serves as a shim for several riscv timer
drivers. This is not too desirable because it bypasses the usual timer
selection via the driver model. There is no easy way to specify an
alternate timing driver, or have the tick rate depend on the cpu's
configured frequency. The timer drivers also do not have device structs,
and so have to rely on storing parameters in gd_t. Lastly, there is no
initialization call, so driver init is done in the same function which
reads the time. This can result in confusing error messages. To a user, it
looks like the driver failed when trying to read the time, whereas it may
have failed while initializing.

This patch removes the shim functionality from the riscv-timer driver, and
has it instead implement the former rdtime.c timer driver. This is because
existing u-boot users who pass in a device tree (e.g. qemu) do not create a
timer device for S-mode u-boot. The existing behavior of creating the
riscv-timer device in the riscv cpu driver must be kept. The actual reading
of the CSRs has been redone in the style of Linux's get_cycles64.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 arch/riscv/lib/Makefile     |  1 -
 arch/riscv/lib/rdtime.c     | 38 ------------------------------------
 drivers/timer/Kconfig       |  6 +++---
 drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------
 4 files changed, 23 insertions(+), 61 deletions(-)
 delete mode 100644 arch/riscv/lib/rdtime.c

diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index 6c503ff2b2..10ac5b06d3 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
 obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
 obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o
 else
-obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
 obj-$(CONFIG_SBI) += sbi.o
 obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
 endif
diff --git a/arch/riscv/lib/rdtime.c b/arch/riscv/lib/rdtime.c
deleted file mode 100644
index e128d7fce6..0000000000
--- a/arch/riscv/lib/rdtime.c
+++ /dev/null
@@ -1,38 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright (C) 2018, Anup Patel <anup@brainfault.org>
- * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
- *
- * The riscv_get_time() API implementation that is using the
- * standard rdtime instruction.
- */
-
-#include <common.h>
-
-/* Implement the API required by RISC-V timer driver */
-int riscv_get_time(u64 *time)
-{
-#ifdef CONFIG_64BIT
-	u64 n;
-
-	__asm__ __volatile__ (
-		"rdtime %0"
-		: "=r" (n));
-
-	*time = n;
-#else
-	u32 lo, hi, tmp;
-
-	__asm__ __volatile__ (
-		"1:\n"
-		"rdtimeh %0\n"
-		"rdtime %1\n"
-		"rdtimeh %2\n"
-		"bne %0, %2, 1b"
-		: "=&r" (hi), "=&r" (lo), "=&r" (tmp));
-
-	*time = ((u64)hi << 32) | lo;
-#endif
-
-	return 0;
-}
diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index 637024445c..b85fa33e47 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -144,10 +144,10 @@ config OMAP_TIMER
 
 config RISCV_TIMER
 	bool "RISC-V timer support"
-	depends on TIMER && RISCV
+	depends on TIMER && RISCV_SMODE
 	help
-	  Select this to enable support for the timer as defined
-	  by the RISC-V privileged architecture spec.
+	  Select this to enable support for a generic RISC-V S-Mode timer
+	  driver.
 
 config ROCKCHIP_TIMER
 	bool "Rockchip timer support"
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 9f9f070e0b..449fcfcfd5 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -1,36 +1,37 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
+ * Copyright (C) 2020, Sean Anderson <seanga2@gmail.com>
  * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
+ * Copyright (C) 2018, Anup Patel <anup@brainfault.org>
+ * Copyright (C) 2012 Regents of the University of California
  *
- * RISC-V privileged architecture defined generic timer driver
+ * RISC-V architecturally-defined generic timer driver
  *
- * This driver relies on RISC-V platform codes to provide the essential API
- * riscv_get_time() which is supposed to return the timer counter as defined
- * by the RISC-V privileged architecture spec.
- *
- * This driver can be used in both M-mode and S-mode U-Boot.
+ * This driver provides generic timer support for S-mode U-Boot.
  */
 
 #include <common.h>
 #include <dm.h>
 #include <errno.h>
 #include <timer.h>
-#include <asm/io.h>
-
-/**
- * riscv_get_time() - get the timer counter
- *
- * Platform codes should provide this API in order to make this driver function.
- *
- * @time:	the 64-bit timer count  as defined by the RISC-V privileged
- *		architecture spec.
- * @return:	0 on success, -ve on error.
- */
-extern int riscv_get_time(u64 *time);
+#include <asm/csr.h>
 
 static int riscv_timer_get_count(struct udevice *dev, u64 *count)
 {
-	return riscv_get_time(count);
+	if (IS_ENABLED(CONFIG_64BIT)) {
+		*count = csr_read(CSR_TIME);
+	} else {
+		u32 hi, lo;
+
+		do {
+			hi = csr_read(CSR_TIMEH);
+			lo = csr_read(CSR_TIME);
+		} while (hi != csr_read(CSR_TIMEH));
+
+		*count = ((u64)hi << 32) | lo;
+	}
+
+	return 0;
 }
 
 static int riscv_timer_probe(struct udevice *dev)
-- 
2.27.0

[PATCH 2/6] riscv: Rework Andes PLMT as a UCLASS_TIMER driver

[Sean Anderson]

This converts the PLMT driver from the riscv-specific timer interface to be
a DM-based UCLASS_TIMER driver.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
This patch builds but has NOT been tested.

 arch/riscv/Kconfig                   |  4 ---
 arch/riscv/dts/ae350_32.dts          |  1 +
 arch/riscv/dts/ae350_64.dts          |  1 +
 arch/riscv/include/asm/global_data.h |  3 --
 arch/riscv/lib/andes_plmt.c          | 42 +++++++++++++---------------
 5 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 009a545fcf..502f27b92d 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -177,10 +177,6 @@ config ANDES_PLIC
 config ANDES_PLMT
 	bool
 	depends on RISCV_MMODE || SPL_RISCV_MMODE
-	select REGMAP
-	select SYSCON
-	select SPL_REGMAP if SPL
-	select SPL_SYSCON if SPL
 	help
 	  The Andes PLMT block holds memory-mapped mtime register
 	  associated with timer tick.
diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts
index 3f8525fe56..afcb9cfbbf 100644
--- a/arch/riscv/dts/ae350_32.dts
+++ b/arch/riscv/dts/ae350_32.dts
@@ -162,6 +162,7 @@
 				&CPU2_intc 7
 				&CPU3_intc 7>;
 			reg = <0xe6000000 0x100000>;
+			clock-frequency = <60000000>;
 		};
 	};
 
diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts
index 482c707503..1c37879049 100644
--- a/arch/riscv/dts/ae350_64.dts
+++ b/arch/riscv/dts/ae350_64.dts
@@ -162,6 +162,7 @@
 				&CPU2_intc 7
 				&CPU3_intc 7>;
 			reg = <0x0 0xe6000000 0x0 0x100000>;
+			clock-frequency = <60000000>;
 		};
 	};
 
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
index 2eb14815bc..0dec5e669e 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -24,9 +24,6 @@ struct arch_global_data {
 #ifdef CONFIG_ANDES_PLIC
 	void __iomem *plic;	/* plic base address */
 #endif
-#ifdef CONFIG_ANDES_PLMT
-	void __iomem *plmt;	/* plmt base address */
-#endif
 #if CONFIG_IS_ENABLED(SMP)
 	struct ipi_data ipi[CONFIG_NR_CPUS];
 #endif
diff --git a/arch/riscv/lib/andes_plmt.c b/arch/riscv/lib/andes_plmt.c
index a7e90ca992..b0245d0b52 100644
--- a/arch/riscv/lib/andes_plmt.c
+++ b/arch/riscv/lib/andes_plmt.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
  * Copyright (C) 2019, Rick Chen <rick@andestech.com>
+ * Copyright (C) 2020, Sean Anderson <seanga2@gmail.com>
  *
  * U-Boot syscon driver for Andes's Platform Level Machine Timer (PLMT).
  * The PLMT block holds memory-mapped mtime register
@@ -9,46 +10,43 @@
 
 #include <common.h>
 #include <dm.h>
-#include <regmap.h>
-#include <syscon.h>
+#include <timer.h>
 #include <asm/io.h>
-#include <asm/syscon.h>
 #include <linux/err.h>
 
 /* mtime register */
 #define MTIME_REG(base)			((ulong)(base))
 
-DECLARE_GLOBAL_DATA_PTR;
-
-#define PLMT_BASE_GET(void)						\
-	do {								\
-		long *ret;						\
-									\
-		if (!gd->arch.plmt) {					\
-			ret = syscon_get_first_range(RISCV_SYSCON_PLMT); \
-			if (IS_ERR(ret))				\
-				return PTR_ERR(ret);			\
-			gd->arch.plmt = ret;				\
-		}							\
-	} while (0)
-
-int riscv_get_time(u64 *time)
+static int andes_plmt_get_count(struct udevice *dev, u64 *count)
 {
-	PLMT_BASE_GET();
+	*count = readq((void __iomem *)MTIME_REG(dev->priv));
 
-	*time = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
+	return 0;
+}
+
+static const struct timer_ops andes_plmt_ops = {
+	.get_count = andes_plmt_get_count,
+};
+
+static int andes_plmt_probe(struct udevice *dev)
+{
+	dev->priv = dev_read_addr_ptr(dev);
+	if (!dev->priv)
+		return -EINVAL;
 
 	return 0;
 }
 
 static const struct udevice_id andes_plmt_ids[] = {
-	{ .compatible = "riscv,plmt0", .data = RISCV_SYSCON_PLMT },
+	{ .compatible = "riscv,plmt0" },
 	{ }
 };
 
 U_BOOT_DRIVER(andes_plmt) = {
 	.name		= "andes_plmt",
-	.id		= UCLASS_SYSCON,
+	.id		= UCLASS_TIMER,
 	.of_match	= andes_plmt_ids,
+	.ops		= &andes_plmt_ops,
+	.probe		= andes_plmt_probe,
 	.flags		= DM_FLAG_PRE_RELOC,
 };
-- 
2.27.0

[PATCH 3/6] riscv: Clean up initialization in Andes PLIC

[Sean Anderson]

This merges the PLIC initialization code from two functions into one.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
This patch builds but has NOT been tested.

 arch/riscv/lib/andes_plic.c | 58 ++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andes_plic.c
index 5cf29df670..267d6a191b 100644
--- a/arch/riscv/lib/andes_plic.c
+++ b/arch/riscv/lib/andes_plic.c
@@ -41,53 +41,45 @@ static int enable_ipi(int hart)
 	return 0;
 }
 
-static int init_plic(void)
+int riscv_init_ipi(void)
 {
-	struct udevice *dev;
-	ofnode node;
 	int ret;
+	long *base = syscon_get_first_range(RISCV_SYSCON_PLIC);
+	ofnode node;
+	struct udevice *dev;
 	u32 reg;
 
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+	gd->arch.plic = base;
+
 	ret = uclass_find_first_device(UCLASS_CPU, &dev);
 	if (ret)
 		return ret;
+	else if (!dev)
+		return -ENODEV;
 
-	if (ret == 0 && dev) {
-		ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
-			const char *device_type;
+	ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
+		const char *device_type;
 
-			device_type = ofnode_read_string(node, "device_type");
-			if (!device_type)
-				continue;
+		device_type = ofnode_read_string(node, "device_type");
+		if (!device_type)
+			continue;
 
-			if (strcmp(device_type, "cpu"))
-				continue;
+		if (strcmp(device_type, "cpu"))
+			continue;
 
-			/* skip if hart is marked as not available */
-			if (!ofnode_is_available(node))
-				continue;
+		/* skip if hart is marked as not available */
+		if (!ofnode_is_available(node))
+			continue;
 
-			/* read hart ID of CPU */
-			ret = ofnode_read_u32(node, "reg", &reg);
-			if (ret == 0)
-				enable_ipi(reg);
-		}
-
-		return 0;
+		/* read hart ID of CPU */
+		ret = ofnode_read_u32(node, "reg", &reg);
+		if (ret == 0)
+			enable_ipi(reg);
 	}
 
-	return -ENODEV;
-}
-
-int riscv_init_ipi(void)
-{
-	long *ret = syscon_get_first_range(RISCV_SYSCON_PLIC);
-
-	if (IS_ERR(ret))
-		return PTR_ERR(ret);
-	gd->arch.plic = ret;
-
-	return init_plic();
+	return 0;
 }
 
 int riscv_send_ipi(int hart)
-- 
2.27.0

[PATCH 4/6] riscv: Rework Sifive CLINT as UCLASS_TIMER driver

[Sean Anderson]

This converts the clint driver from the riscv-specific interface to be a
DM-based UCLASS_TIMER driver. We also need to re-add the initialization for
IPI back into the SPL code. This was previously implicitly done when the
timer was initialized. In addition, the SiFive DDR driver previously
implicitly depended on the CLINT to select REGMAP.

Unlike Andes's PLMT/PLIC (which AFAIK never have anything pass it a dtb),
the SiFive CLINT is part of the device tree passed in by qemu. This device
tree doesn't have a clocks or clock-frequency property on clint, so we need
to fall back on the timebase-frequency property. Perhaps in the future we
can get a clock-frequency property added to the qemu dtb.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
f

---
This patch builds but has only been tested on the K210 and QEMU. It has NOT
been tested on a HiFive.

 arch/riscv/Kconfig            |  4 --
 arch/riscv/lib/sifive_clint.c | 87 +++++++++++++++++++++++------------
 common/spl/spl_opensbi.c      |  5 ++
 drivers/ram/sifive/Kconfig    |  2 +
 4 files changed, 64 insertions(+), 34 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 502f27b92d..f2641ffc53 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -155,10 +155,6 @@ config 64BIT
 config SIFIVE_CLINT
 	bool
 	depends on RISCV_MMODE || SPL_RISCV_MMODE
-	select REGMAP
-	select SYSCON
-	select SPL_REGMAP if SPL
-	select SPL_SYSCON if SPL
 	help
 	  The SiFive CLINT block holds memory-mapped control and status registers
 	  associated with software and timer interrupts.
diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c
index b9a2c649cc..3345a17ad2 100644
--- a/arch/riscv/lib/sifive_clint.c
+++ b/arch/riscv/lib/sifive_clint.c
@@ -8,9 +8,9 @@
  */
 
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
-#include <regmap.h>
-#include <syscon.h>
+#include <timer.h>
 #include <asm/io.h>
 #include <asm/syscon.h>
 #include <linux/err.h>
@@ -24,35 +24,19 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int riscv_get_time(u64 *time)
-{
-	/* ensure timer register base has a sane value */
-	riscv_init_ipi();
-
-	*time = readq((void __iomem *)MTIME_REG(gd->arch.clint));
-
-	return 0;
-}
-
-int riscv_set_timecmp(int hart, u64 cmp)
-{
-	/* ensure timer register base has a sane value */
-	riscv_init_ipi();
-
-	writeq(cmp, (void __iomem *)MTIMECMP_REG(gd->arch.clint, hart));
-
-	return 0;
-}
-
 int riscv_init_ipi(void)
 {
-	if (!gd->arch.clint) {
-		long *ret = syscon_get_first_range(RISCV_SYSCON_CLINT);
+	int ret;
+	struct udevice *dev;
 
-		if (IS_ERR(ret))
-			return PTR_ERR(ret);
-		gd->arch.clint = ret;
-	}
+	ret = uclass_get_device_by_driver(UCLASS_TIMER,
+					  DM_GET_DRIVER(sifive_clint), &dev);
+	if (ret)
+		return ret;
+
+	gd->arch.clint = dev_read_addr_ptr(dev);
+	if (!gd->arch.clint)
+		return -EINVAL;
 
 	return 0;
 }
@@ -78,14 +62,57 @@ int riscv_get_ipi(int hart, int *pending)
 	return 0;
 }
 
+static int sifive_clint_get_count(struct udevice *dev, u64 *count)
+{
+	*count = readq((void __iomem *)MTIME_REG(dev->priv));
+
+	return 0;
+}
+
+static const struct timer_ops sifive_clint_ops = {
+	.get_count = sifive_clint_get_count,
+};
+
+static int sifive_clint_probe(struct udevice *dev)
+{
+	int ret;
+	ofnode cpu;
+	struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+	u32 rate;
+
+	dev->priv = dev_read_addr_ptr(dev);
+	if (!dev->priv)
+		return -EINVAL;
+
+	/* Did we get our clock rate from the device tree? */
+	if (uc_priv->clock_rate)
+		return 0;
+
+	/* Fall back to timebase-frequency */
+	cpu = ofnode_path("/cpus");
+	if (!ofnode_valid(cpu))
+		return -EINVAL;
+
+	ret = ofnode_read_u32(cpu, "timebase-frequency", &rate);
+	if (ret)
+		return ret;
+
+	log_warning("missing clocks or clock-frequency property, falling back on timebase-frequency\n");
+	uc_priv->clock_rate = rate;
+
+	return 0;
+}
+
 static const struct udevice_id sifive_clint_ids[] = {
-	{ .compatible = "riscv,clint0", .data = RISCV_SYSCON_CLINT },
+	{ .compatible = "riscv,clint0" },
 	{ }
 };
 
 U_BOOT_DRIVER(sifive_clint) = {
 	.name		= "sifive_clint",
-	.id		= UCLASS_SYSCON,
+	.id		= UCLASS_TIMER,
 	.of_match	= sifive_clint_ids,
+	.probe		= sifive_clint_probe,
+	.ops		= &sifive_clint_ops,
 	.flags		= DM_FLAG_PRE_RELOC,
 };
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index 14f335f75f..3440bc0294 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -79,6 +79,11 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
 	invalidate_icache_all();
 
 #ifdef CONFIG_SPL_SMP
+	/* Initialize the IPI before we use it */
+	ret = riscv_init_ipi();
+	if (ret)
+		hang();
+
 	/*
 	 * Start OpenSBI on all secondary harts and wait for acknowledgment.
 	 *
diff --git a/drivers/ram/sifive/Kconfig b/drivers/ram/sifive/Kconfig
index 6aca22ab2a..31ad0a7e45 100644
--- a/drivers/ram/sifive/Kconfig
+++ b/drivers/ram/sifive/Kconfig
@@ -9,5 +9,7 @@ config SIFIVE_FU540_DDR
 	bool "SiFive FU540 DDR driver"
 	depends on RAM_SIFIVE
 	default y if TARGET_SIFIVE_FU540
+	select REGMAP
+	select SPL_REGMAP if SPL
 	help
 	  This enables DDR support for the platforms based on SiFive FU540 SoC.
-- 
2.27.0

[PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver

[Sean Anderson]

AFAIK because the K210 clock driver does not come up until after
relocation, the clint will always use the clock-frequency parameter.
Ideally, it should update itself after relocation to take into account the
actual CPU frequency.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 arch/riscv/dts/k210.dtsi                | 10 ++++++----
 drivers/clk/kendryte/clk.c              |  4 ++++
 include/dt-bindings/clock/k210-sysctl.h |  1 +
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 2546c7d4e0..9583694c46 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -17,6 +17,8 @@
 	compatible = "kendryte,k210";
 
 	aliases {
+		cpu0 = &cpu0;
+		cpu1 = &cpu1;
 		dma0 = &dmac0;
 		gpio0 = &gpio0;
 		gpio1 = &gpio1_0;
@@ -40,7 +42,6 @@
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		timebase-frequency = <7800000>;
 		cpu0: cpu at 0 {
 			device_type = "cpu";
 			compatible = "kendryte,k210", "sifive,rocket0", "riscv";
@@ -126,14 +127,15 @@
 			read-only;
 		};
 
-		clint0: interrupt-controller at 2000000 {
+		clint0: clint at 2000000 {
 			#interrupt-cells = <1>;
 			compatible = "kendryte,k210-clint", "riscv,clint0";
 			reg = <0x2000000 0xC000>;
-			interrupt-controller;
 			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>,
 					      <&cpu1_intc 3>, <&cpu1_intc 7>;
-			clocks = <&sysclk K210_CLK_CPU>;
+			clocks = <&sysclk K210_CLK_CLINT>;
+			/* sysclk is only available post-relocation */
+			clock-frequency = <7800000>;
 		};
 
 		plic0: interrupt-controller at C000000 {
diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
index 981b3b7699..bb196961af 100644
--- a/drivers/clk/kendryte/clk.c
+++ b/drivers/clk/kendryte/clk.c
@@ -646,6 +646,10 @@ static int k210_clk_probe(struct udevice *dev)
 	REGISTER_GATE(K210_CLK_RTC,   "rtc",   in0);
 #undef REGISTER_GATE
 
+	/* The MTIME register in CLINT runs at one 50th the CPU clock speed */
+	clk_dm(K210_CLK_CLINT,
+	       clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
+
 	return 0;
 }
 
diff --git a/include/dt-bindings/clock/k210-sysctl.h b/include/dt-bindings/clock/k210-sysctl.h
index 0e3ed3fb9f..fe852bbd92 100644
--- a/include/dt-bindings/clock/k210-sysctl.h
+++ b/include/dt-bindings/clock/k210-sysctl.h
@@ -55,5 +55,6 @@
 #define K210_CLK_OTP    43
 #define K210_CLK_RTC    44
 #define K210_CLK_ACLK   45
+#define K210_CLK_CLINT  46
 
 #endif /* CLOCK_K210_SYSCTL_H */
-- 
2.27.0

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Sean Anderson]

We may need to add a clock-frequency binding like for the K210.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
This patch builds but has NOT been tested.

 arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
index afdb4f4402..e56bfc7595 100644
--- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
+++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
@@ -55,8 +55,13 @@
 		};
 		clint at 2000000 {
 			compatible = "riscv,clint0";
-			interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
+			interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
+					       &cpu1_intc 3 &cpu1_intc 7
+					       &cpu2_intc 3 &cpu2_intc 7
+					       &cpu3_intc 3 &cpu3_intc 7
+					       &cpu4_intc 3 &cpu4_intc 7>;
 			reg = <0x0 0x2000000 0x0 0xc0000>;
+			clocks = <&prci PRCI_CLK_COREPLL>;
 			u-boot,dm-spl;
 		};
 		dmc: dmc at 100b0000 {
-- 
2.27.0

[PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver

[Sagar Kadam]

Hello Sean,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Sean Anderson
> Sent: Wednesday, July 22, 2020 9:21 PM
> To: u-boot at lists.denx.de
> Cc: Bin Meng <bmeng.cn@gmail.com>; Rick Chen <rickchen36@gmail.com>;
> Sean Anderson <seanga2@gmail.com>
> Subject: [PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> AFAIK because the K210 clock driver does not come up until after
> relocation, the clint will always use the clock-frequency parameter.
> Ideally, it should update itself after relocation to take into account the
> actual CPU frequency.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>  arch/riscv/dts/k210.dtsi                | 10 ++++++----
>  drivers/clk/kendryte/clk.c              |  4 ++++
>  include/dt-bindings/clock/k210-sysctl.h |  1 +

Can you please consider splitting the dt-bindings include into separate patch
so as to avoid checkpatch warning.

Thanks & BR,
Sagar

>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
> index 2546c7d4e0..9583694c46 100644
> --- a/arch/riscv/dts/k210.dtsi
> +++ b/arch/riscv/dts/k210.dtsi
> @@ -17,6 +17,8 @@
>         compatible = "kendryte,k210";
> 
>         aliases {
> +               cpu0 = &cpu0;
> +               cpu1 = &cpu1;
>                 dma0 = &dmac0;
>                 gpio0 = &gpio0;
>                 gpio1 = &gpio1_0;
> @@ -40,7 +42,6 @@
>         cpus {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> -               timebase-frequency = <7800000>;
>                 cpu0: cpu at 0 {
>                         device_type = "cpu";
>                         compatible = "kendryte,k210", "sifive,rocket0", "riscv";
> @@ -126,14 +127,15 @@
>                         read-only;
>                 };
> 
> -               clint0: interrupt-controller at 2000000 {
> +               clint0: clint at 2000000 {
>                         #interrupt-cells = <1>;
>                         compatible = "kendryte,k210-clint", "riscv,clint0";
>                         reg = <0x2000000 0xC000>;
> -                       interrupt-controller;
>                         interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>,
>                                               <&cpu1_intc 3>, <&cpu1_intc 7>;
> -                       clocks = <&sysclk K210_CLK_CPU>;
> +                       clocks = <&sysclk K210_CLK_CLINT>;
> +                       /* sysclk is only available post-relocation */
> +                       clock-frequency = <7800000>;
>                 };
> 
>                 plic0: interrupt-controller at C000000 {
> diff --git a/drivers/clk/kendryte/clk.c b/drivers/clk/kendryte/clk.c
> index 981b3b7699..bb196961af 100644
> --- a/drivers/clk/kendryte/clk.c
> +++ b/drivers/clk/kendryte/clk.c
> @@ -646,6 +646,10 @@ static int k210_clk_probe(struct udevice *dev)
>         REGISTER_GATE(K210_CLK_RTC,   "rtc",   in0);
>  #undef REGISTER_GATE
> 
> +       /* The MTIME register in CLINT runs at one 50th the CPU clock speed */
> +       clk_dm(K210_CLK_CLINT,
> +              clk_register_fixed_factor(NULL, "clint", "cpu", 0, 1, 50));
> +
>         return 0;
>  }
> 
> diff --git a/include/dt-bindings/clock/k210-sysctl.h b/include/dt-
> bindings/clock/k210-sysctl.h
> index 0e3ed3fb9f..fe852bbd92 100644
> --- a/include/dt-bindings/clock/k210-sysctl.h
> +++ b/include/dt-bindings/clock/k210-sysctl.h
> @@ -55,5 +55,6 @@
>  #define K210_CLK_OTP    43
>  #define K210_CLK_RTC    44
>  #define K210_CLK_ACLK   45
> +#define K210_CLK_CLINT  46
> 
>  #endif /* CLOCK_K210_SYSCTL_H */
> --
> 2.27.0

[PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver

[Sean Anderson]

On 7/23/20 7:49 AM, Sagar Kadam wrote:
> Hello Sean,
> 
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Sean Anderson
>> Sent: Wednesday, July 22, 2020 9:21 PM
>> To: u-boot at lists.denx.de
>> Cc: Bin Meng <bmeng.cn@gmail.com>; Rick Chen <rickchen36@gmail.com>;
>> Sean Anderson <seanga2@gmail.com>
>> Subject: [PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver
>>
>> [External Email] Do not click links or attachments unless you recognize the
>> sender and know the content is safe
>>
>> AFAIK because the K210 clock driver does not come up until after
>> relocation, the clint will always use the clock-frequency parameter.
>> Ideally, it should update itself after relocation to take into account the
>> actual CPU frequency.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>  arch/riscv/dts/k210.dtsi                | 10 ++++++----
>>  drivers/clk/kendryte/clk.c              |  4 ++++
>>  include/dt-bindings/clock/k210-sysctl.h |  1 +
> 
> Can you please consider splitting the dt-bindings include into separate patch
> so as to avoid checkpatch warning.

If you'd like. AFAIK this is mostly a kernel thing since dt-bindings
often have separate maintainers than the rest of the series. Can anyone
comment on whether this applies to U-Boot as well?

--Sean

[PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver

[Bin Meng]

Hi Sean,

On Thu, Jul 23, 2020 at 7:56 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 7/23/20 7:49 AM, Sagar Kadam wrote:
> > Hello Sean,
> >
> >> -----Original Message-----
> >> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Sean Anderson
> >> Sent: Wednesday, July 22, 2020 9:21 PM
> >> To: u-boot at lists.denx.de
> >> Cc: Bin Meng <bmeng.cn@gmail.com>; Rick Chen <rickchen36@gmail.com>;
> >> Sean Anderson <seanga2@gmail.com>
> >> Subject: [PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver
> >>
> >> [External Email] Do not click links or attachments unless you recognize the
> >> sender and know the content is safe
> >>
> >> AFAIK because the K210 clock driver does not come up until after
> >> relocation, the clint will always use the clock-frequency parameter.
> >> Ideally, it should update itself after relocation to take into account the
> >> actual CPU frequency.
> >>
> >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >> ---
> >>
> >>  arch/riscv/dts/k210.dtsi                | 10 ++++++----
> >>  drivers/clk/kendryte/clk.c              |  4 ++++
> >>  include/dt-bindings/clock/k210-sysctl.h |  1 +
> >
> > Can you please consider splitting the dt-bindings include into separate patch
> > so as to avoid checkpatch warning.
>
> If you'd like. AFAIK this is mostly a kernel thing since dt-bindings
> often have separate maintainers than the rest of the series. Can anyone
> comment on whether this applies to U-Boot as well?

If the changes are from upstream Linux kernel, it's fine to keep the
changes in the k210.dtsi. But if the changes are only needed in
U-Boot, as Sagar mentioned they should be in k210-uboot.dtsi.

Regards,
Bin

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Bin Meng]

Hi Sean,

On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> We may need to add a clock-frequency binding like for the K210.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> This patch builds but has NOT been tested.
>
>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> index afdb4f4402..e56bfc7595 100644
> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> @@ -55,8 +55,13 @@
>                 };
>                 clint at 2000000 {
>                         compatible = "riscv,clint0";
> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> +                                              &cpu1_intc 3 &cpu1_intc 7
> +                                              &cpu2_intc 3 &cpu2_intc 7
> +                                              &cpu3_intc 3 &cpu3_intc 7
> +                                              &cpu4_intc 3 &cpu4_intc 7>;
>                         reg = <0x0 0x2000000 0x0 0xc0000>;
> +                       clocks = <&prci PRCI_CLK_COREPLL>;

This looks wrong to me. The CLINT timer frequency should come from the RTC node.

+Pragnesh Patel

+Sagar Kadam

>                         u-boot,dm-spl;
>                 };
>                 dmc: dmc at 100b0000 {

Regards,
Bin

[PATCH 2/6] riscv: Rework Andes PLMT as a UCLASS_TIMER driver

[Bin Meng]

Hi Sean,

On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> This converts the PLMT driver from the riscv-specific timer interface to be
> a DM-based UCLASS_TIMER driver.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> This patch builds but has NOT been tested.
>
>  arch/riscv/Kconfig                   |  4 ---
>  arch/riscv/dts/ae350_32.dts          |  1 +
>  arch/riscv/dts/ae350_64.dts          |  1 +
>  arch/riscv/include/asm/global_data.h |  3 --
>  arch/riscv/lib/andes_plmt.c          | 42 +++++++++++++---------------
>  5 files changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 009a545fcf..502f27b92d 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -177,10 +177,6 @@ config ANDES_PLIC
>  config ANDES_PLMT
>         bool
>         depends on RISCV_MMODE || SPL_RISCV_MMODE
> -       select REGMAP
> -       select SYSCON
> -       select SPL_REGMAP if SPL
> -       select SPL_SYSCON if SPL
>         help
>           The Andes PLMT block holds memory-mapped mtime register
>           associated with timer tick.
> diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts
> index 3f8525fe56..afcb9cfbbf 100644
> --- a/arch/riscv/dts/ae350_32.dts
> +++ b/arch/riscv/dts/ae350_32.dts
> @@ -162,6 +162,7 @@
>                                 &CPU2_intc 7
>                                 &CPU3_intc 7>;
>                         reg = <0xe6000000 0x100000>;
> +                       clock-frequency = <60000000>;

Where is this value from?

>                 };
>         };
>
> diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts
> index 482c707503..1c37879049 100644
> --- a/arch/riscv/dts/ae350_64.dts
> +++ b/arch/riscv/dts/ae350_64.dts
> @@ -162,6 +162,7 @@
>                                 &CPU2_intc 7
>                                 &CPU3_intc 7>;
>                         reg = <0x0 0xe6000000 0x0 0x100000>;
> +                       clock-frequency = <60000000>;
>                 };
>         };
>
> diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> index 2eb14815bc..0dec5e669e 100644
> --- a/arch/riscv/include/asm/global_data.h
> +++ b/arch/riscv/include/asm/global_data.h
> @@ -24,9 +24,6 @@ struct arch_global_data {
>  #ifdef CONFIG_ANDES_PLIC
>         void __iomem *plic;     /* plic base address */
>  #endif
> -#ifdef CONFIG_ANDES_PLMT
> -       void __iomem *plmt;     /* plmt base address */
> -#endif
>  #if CONFIG_IS_ENABLED(SMP)
>         struct ipi_data ipi[CONFIG_NR_CPUS];
>  #endif
> diff --git a/arch/riscv/lib/andes_plmt.c b/arch/riscv/lib/andes_plmt.c
> index a7e90ca992..b0245d0b52 100644
> --- a/arch/riscv/lib/andes_plmt.c
> +++ b/arch/riscv/lib/andes_plmt.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Copyright (C) 2019, Rick Chen <rick@andestech.com>
> + * Copyright (C) 2020, Sean Anderson <seanga2@gmail.com>
>   *
>   * U-Boot syscon driver for Andes's Platform Level Machine Timer (PLMT).
>   * The PLMT block holds memory-mapped mtime register
> @@ -9,46 +10,43 @@
>
>  #include <common.h>
>  #include <dm.h>
> -#include <regmap.h>
> -#include <syscon.h>
> +#include <timer.h>
>  #include <asm/io.h>
> -#include <asm/syscon.h>
>  #include <linux/err.h>
>
>  /* mtime register */
>  #define MTIME_REG(base)                        ((ulong)(base))
>
> -DECLARE_GLOBAL_DATA_PTR;
> -
> -#define PLMT_BASE_GET(void)                                            \
> -       do {                                                            \
> -               long *ret;                                              \
> -                                                                       \
> -               if (!gd->arch.plmt) {                                   \
> -                       ret = syscon_get_first_range(RISCV_SYSCON_PLMT); \
> -                       if (IS_ERR(ret))                                \
> -                               return PTR_ERR(ret);                    \
> -                       gd->arch.plmt = ret;                            \
> -               }                                                       \
> -       } while (0)
> -
> -int riscv_get_time(u64 *time)
> +static int andes_plmt_get_count(struct udevice *dev, u64 *count)
>  {
> -       PLMT_BASE_GET();
> +       *count = readq((void __iomem *)MTIME_REG(dev->priv));
>
> -       *time = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
> +       return 0;
> +}
> +
> +static const struct timer_ops andes_plmt_ops = {
> +       .get_count = andes_plmt_get_count,
> +};
> +
> +static int andes_plmt_probe(struct udevice *dev)
> +{
> +       dev->priv = dev_read_addr_ptr(dev);
> +       if (!dev->priv)
> +               return -EINVAL;
>
>         return 0;
>  }
>
>  static const struct udevice_id andes_plmt_ids[] = {
> -       { .compatible = "riscv,plmt0", .data = RISCV_SYSCON_PLMT },
> +       { .compatible = "riscv,plmt0" },
>         { }
>  };
>
>  U_BOOT_DRIVER(andes_plmt) = {
>         .name           = "andes_plmt",
> -       .id             = UCLASS_SYSCON,
> +       .id             = UCLASS_TIMER,
>         .of_match       = andes_plmt_ids,
> +       .ops            = &andes_plmt_ops,
> +       .probe          = andes_plmt_probe,
>         .flags          = DM_FLAG_PRE_RELOC,
>  };

Regards,
Bin

[PATCH 2/6] riscv: Rework Andes PLMT as a UCLASS_TIMER driver

[Sean Anderson]

On 7/23/20 9:51 AM, Bin Meng wrote:
> Hi Sean,
> 
> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> This converts the PLMT driver from the riscv-specific timer interface to be
>> a DM-based UCLASS_TIMER driver.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>> This patch builds but has NOT been tested.
>>
>>  arch/riscv/Kconfig                   |  4 ---
>>  arch/riscv/dts/ae350_32.dts          |  1 +
>>  arch/riscv/dts/ae350_64.dts          |  1 +
>>  arch/riscv/include/asm/global_data.h |  3 --
>>  arch/riscv/lib/andes_plmt.c          | 42 +++++++++++++---------------
>>  5 files changed, 22 insertions(+), 29 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 009a545fcf..502f27b92d 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -177,10 +177,6 @@ config ANDES_PLIC
>>  config ANDES_PLMT
>>         bool
>>         depends on RISCV_MMODE || SPL_RISCV_MMODE
>> -       select REGMAP
>> -       select SYSCON
>> -       select SPL_REGMAP if SPL
>> -       select SPL_SYSCON if SPL
>>         help
>>           The Andes PLMT block holds memory-mapped mtime register
>>           associated with timer tick.
>> diff --git a/arch/riscv/dts/ae350_32.dts b/arch/riscv/dts/ae350_32.dts
>> index 3f8525fe56..afcb9cfbbf 100644
>> --- a/arch/riscv/dts/ae350_32.dts
>> +++ b/arch/riscv/dts/ae350_32.dts
>> @@ -162,6 +162,7 @@
>>                                 &CPU2_intc 7
>>                                 &CPU3_intc 7>;
>>                         reg = <0xe6000000 0x100000>;
>> +                       clock-frequency = <60000000>;
> 
> Where is this value from?

/cpus/timebase-frequency

> 
>>                 };
>>         };
>>
>> diff --git a/arch/riscv/dts/ae350_64.dts b/arch/riscv/dts/ae350_64.dts
>> index 482c707503..1c37879049 100644
>> --- a/arch/riscv/dts/ae350_64.dts
>> +++ b/arch/riscv/dts/ae350_64.dts
>> @@ -162,6 +162,7 @@
>>                                 &CPU2_intc 7
>>                                 &CPU3_intc 7>;
>>                         reg = <0x0 0xe6000000 0x0 0x100000>;
>> +                       clock-frequency = <60000000>;
>>                 };
>>         };
>>
>> diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
>> index 2eb14815bc..0dec5e669e 100644
>> --- a/arch/riscv/include/asm/global_data.h
>> +++ b/arch/riscv/include/asm/global_data.h
>> @@ -24,9 +24,6 @@ struct arch_global_data {
>>  #ifdef CONFIG_ANDES_PLIC
>>         void __iomem *plic;     /* plic base address */
>>  #endif
>> -#ifdef CONFIG_ANDES_PLMT
>> -       void __iomem *plmt;     /* plmt base address */
>> -#endif
>>  #if CONFIG_IS_ENABLED(SMP)
>>         struct ipi_data ipi[CONFIG_NR_CPUS];
>>  #endif
>> diff --git a/arch/riscv/lib/andes_plmt.c b/arch/riscv/lib/andes_plmt.c
>> index a7e90ca992..b0245d0b52 100644
>> --- a/arch/riscv/lib/andes_plmt.c
>> +++ b/arch/riscv/lib/andes_plmt.c
>> @@ -1,6 +1,7 @@
>>  // SPDX-License-Identifier: GPL-2.0+
>>  /*
>>   * Copyright (C) 2019, Rick Chen <rick@andestech.com>
>> + * Copyright (C) 2020, Sean Anderson <seanga2@gmail.com>
>>   *
>>   * U-Boot syscon driver for Andes's Platform Level Machine Timer (PLMT).
>>   * The PLMT block holds memory-mapped mtime register
>> @@ -9,46 +10,43 @@
>>
>>  #include <common.h>
>>  #include <dm.h>
>> -#include <regmap.h>
>> -#include <syscon.h>
>> +#include <timer.h>
>>  #include <asm/io.h>
>> -#include <asm/syscon.h>
>>  #include <linux/err.h>
>>
>>  /* mtime register */
>>  #define MTIME_REG(base)                        ((ulong)(base))
>>
>> -DECLARE_GLOBAL_DATA_PTR;
>> -
>> -#define PLMT_BASE_GET(void)                                            \
>> -       do {                                                            \
>> -               long *ret;                                              \
>> -                                                                       \
>> -               if (!gd->arch.plmt) {                                   \
>> -                       ret = syscon_get_first_range(RISCV_SYSCON_PLMT); \
>> -                       if (IS_ERR(ret))                                \
>> -                               return PTR_ERR(ret);                    \
>> -                       gd->arch.plmt = ret;                            \
>> -               }                                                       \
>> -       } while (0)
>> -
>> -int riscv_get_time(u64 *time)
>> +static int andes_plmt_get_count(struct udevice *dev, u64 *count)
>>  {
>> -       PLMT_BASE_GET();
>> +       *count = readq((void __iomem *)MTIME_REG(dev->priv));
>>
>> -       *time = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
>> +       return 0;
>> +}
>> +
>> +static const struct timer_ops andes_plmt_ops = {
>> +       .get_count = andes_plmt_get_count,
>> +};
>> +
>> +static int andes_plmt_probe(struct udevice *dev)
>> +{
>> +       dev->priv = dev_read_addr_ptr(dev);
>> +       if (!dev->priv)
>> +               return -EINVAL;
>>
>>         return 0;
>>  }
>>
>>  static const struct udevice_id andes_plmt_ids[] = {
>> -       { .compatible = "riscv,plmt0", .data = RISCV_SYSCON_PLMT },
>> +       { .compatible = "riscv,plmt0" },
>>         { }
>>  };
>>
>>  U_BOOT_DRIVER(andes_plmt) = {
>>         .name           = "andes_plmt",
>> -       .id             = UCLASS_SYSCON,
>> +       .id             = UCLASS_TIMER,
>>         .of_match       = andes_plmt_ids,
>> +       .ops            = &andes_plmt_ops,
>> +       .probe          = andes_plmt_probe,
>>         .flags          = DM_FLAG_PRE_RELOC,
>>  };
> 
> Regards,
> Bin
> 

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Sean Anderson]

On 7/23/20 9:50 AM, Bin Meng wrote:
> Hi Sean,
> 
> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> We may need to add a clock-frequency binding like for the K210.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>> This patch builds but has NOT been tested.
>>
>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>> index afdb4f4402..e56bfc7595 100644
>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>> @@ -55,8 +55,13 @@
>>                 };
>>                 clint at 2000000 {
>>                         compatible = "riscv,clint0";
>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>> +                                              &cpu1_intc 3 &cpu1_intc 7
>> +                                              &cpu2_intc 3 &cpu2_intc 7
>> +                                              &cpu3_intc 3 &cpu3_intc 7
>> +                                              &cpu4_intc 3 &cpu4_intc 7>;
>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
> 
> This looks wrong to me. The CLINT timer frequency should come from the RTC node.
> 
> +Pragnesh Patel
> 
> +Sagar Kadam

On further review, I think you are right that this should be RTCCLK_FREQ.

Perhaps the clocks part should be moved into
arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to something
like

clocks = <&rtcclk>;

--Sean

[PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver

[Sean Anderson]

On 7/23/20 9:49 AM, Bin Meng wrote:
> Hi Sean,
> 
> On Thu, Jul 23, 2020 at 7:56 PM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> On 7/23/20 7:49 AM, Sagar Kadam wrote:
>>> Hello Sean,
>>>
>>>> -----Original Message-----
>>>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Sean Anderson
>>>> Sent: Wednesday, July 22, 2020 9:21 PM
>>>> To: u-boot at lists.denx.de
>>>> Cc: Bin Meng <bmeng.cn@gmail.com>; Rick Chen <rickchen36@gmail.com>;
>>>> Sean Anderson <seanga2@gmail.com>
>>>> Subject: [PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver
>>>>
>>>> [External Email] Do not click links or attachments unless you recognize the
>>>> sender and know the content is safe
>>>>
>>>> AFAIK because the K210 clock driver does not come up until after
>>>> relocation, the clint will always use the clock-frequency parameter.
>>>> Ideally, it should update itself after relocation to take into account the
>>>> actual CPU frequency.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> ---
>>>>
>>>>  arch/riscv/dts/k210.dtsi                | 10 ++++++----
>>>>  drivers/clk/kendryte/clk.c              |  4 ++++
>>>>  include/dt-bindings/clock/k210-sysctl.h |  1 +
>>>
>>> Can you please consider splitting the dt-bindings include into separate patch
>>> so as to avoid checkpatch warning.
>>
>> If you'd like. AFAIK this is mostly a kernel thing since dt-bindings
>> often have separate maintainers than the rest of the series. Can anyone
>> comment on whether this applies to U-Boot as well?
> 
> If the changes are from upstream Linux kernel, it's fine to keep the
> changes in the k210.dtsi. But if the changes are only needed in
> U-Boot, as Sagar mentioned they should be in k210-uboot.dtsi.

Here the question is whether the patch should be split, not what file
the changes should go in. At the moment, the dts is not synced between
U-Boot and Linux for the K210, so there is no need to have a separate
device tree file.

--Sean

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Pragnesh Patel]

Hi Sean,

>-----Original Message-----
>From: Sean Anderson <seanga2@gmail.com>
>Sent: 23 July 2020 19:27
>To: Bin Meng <bmeng.cn@gmail.com>; Pragnesh Patel
><pragnesh.patel@sifive.com>; Sagar Kadam <sagar.kadam@sifive.com>
>Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
><rickchen36@gmail.com>
>Subject: Re: [PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>On 7/23/20 9:50 AM, Bin Meng wrote:
>> Hi Sean,
>>
>> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com>
>wrote:
>>>
>>> We may need to add a clock-frequency binding like for the K210.
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> ---
>>> This patch builds but has NOT been tested.
>>>
>>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>> b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>> index afdb4f4402..e56bfc7595 100644
>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>> @@ -55,8 +55,13 @@
>>>                 };
>>>                 clint at 2000000 {
>>>                         compatible = "riscv,clint0";
>>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc
>3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7
>&cpu4_intc 3 &cpu4_intc 7>;
>>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>>> +                                              &cpu1_intc 3 &cpu1_intc 7
>>> +                                              &cpu2_intc 3 &cpu2_intc 7
>>> +                                              &cpu3_intc 3 &cpu3_intc 7
>>> +                                              &cpu4_intc 3
>>> + &cpu4_intc 7>;
>>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
>>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
>>
>> This looks wrong to me. The CLINT timer frequency should come from the
>RTC node.
>>
>> +Pragnesh Patel
>>
>> +Sagar Kadam
>
>On further review, I think you are right that this should be RTCCLK_FREQ.
>
>Perhaps the clocks part should be moved into arch/riscv/dts/hifive-
>unleashed-a00-u-boot.dts and changed to something like
>
>clocks = <&rtcclk>;

I am okay with your suggestion, better to add in "hifive-unleashed-a00-u-boot.dtsi".

>
>--Sean

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Bin Meng]

Hi Sean,

On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 7/23/20 9:50 AM, Bin Meng wrote:
> > Hi Sean,
> >
> > On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
> >>
> >> We may need to add a clock-frequency binding like for the K210.
> >>
> >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >> ---
> >> This patch builds but has NOT been tested.
> >>
> >>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >> index afdb4f4402..e56bfc7595 100644
> >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >> @@ -55,8 +55,13 @@
> >>                 };
> >>                 clint at 2000000 {
> >>                         compatible = "riscv,clint0";
> >> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
> >> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> >> +                                              &cpu1_intc 3 &cpu1_intc 7
> >> +                                              &cpu2_intc 3 &cpu2_intc 7
> >> +                                              &cpu3_intc 3 &cpu3_intc 7
> >> +                                              &cpu4_intc 3 &cpu4_intc 7>;
> >>                         reg = <0x0 0x2000000 0x0 0xc0000>;
> >> +                       clocks = <&prci PRCI_CLK_COREPLL>;
> >
> > This looks wrong to me. The CLINT timer frequency should come from the RTC node.
> >
> > +Pragnesh Patel
> >
> > +Sagar Kadam
>
> On further review, I think you are right that this should be RTCCLK_FREQ.
>
> Perhaps the clocks part should be moved into
> arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to something
> like
>
> clocks = <&rtcclk>;

How does the device tree look like in the upstream Linux to work with
the new CLINT timer driver?

Regards,
Bin

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Sean Anderson]

On 7/23/20 10:47 AM, Bin Meng wrote:
> Hi Sean,
> 
> On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> On 7/23/20 9:50 AM, Bin Meng wrote:
>>> Hi Sean,
>>>
>>> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
>>>>
>>>> We may need to add a clock-frequency binding like for the K210.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> ---
>>>> This patch builds but has NOT been tested.
>>>>
>>>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>> index afdb4f4402..e56bfc7595 100644
>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>> @@ -55,8 +55,13 @@
>>>>                 };
>>>>                 clint at 2000000 {
>>>>                         compatible = "riscv,clint0";
>>>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
>>>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>>>> +                                              &cpu1_intc 3 &cpu1_intc 7
>>>> +                                              &cpu2_intc 3 &cpu2_intc 7
>>>> +                                              &cpu3_intc 3 &cpu3_intc 7
>>>> +                                              &cpu4_intc 3 &cpu4_intc 7>;
>>>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
>>>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
>>>
>>> This looks wrong to me. The CLINT timer frequency should come from the RTC node.
>>>
>>> +Pragnesh Patel
>>>
>>> +Sagar Kadam
>>
>> On further review, I think you are right that this should be RTCCLK_FREQ.
>>
>> Perhaps the clocks part should be moved into
>> arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to something
>> like
>>
>> clocks = <&rtcclk>;
> 
> How does the device tree look like in the upstream Linux to work with
> the new CLINT timer driver?
> 
> Regards,
> Bin

Anup's patch doesn't change the device tree at all so I think they are still using timebase-frequency.

--Sean

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Sagar Kadam]

Hello Sean,

> -----Original Message-----
> From: Sean Anderson <seanga2@gmail.com>
> Sent: Thursday, July 23, 2020 8:22 PM
> To: Bin Meng <bmeng.cn@gmail.com>
> Cc: Pragnesh Patel <pragnesh.patel@sifive.com>; Sagar Kadam
> <sagar.kadam@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; Rick
> Chen <rickchen36@gmail.com>
> Subject: Re: [PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> On 7/23/20 10:47 AM, Bin Meng wrote:
> > Hi Sean,
> >
> > On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson <seanga2@gmail.com>
> wrote:
> >>
> >> On 7/23/20 9:50 AM, Bin Meng wrote:
> >>> Hi Sean,
> >>>
> >>> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com>
> wrote:
> >>>>
> >>>> We may need to add a clock-frequency binding like for the K210.
> >>>>
> >>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >>>> ---
> >>>> This patch builds but has NOT been tested.
> >>>>
> >>>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
> >>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> index afdb4f4402..e56bfc7595 100644
> >>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> @@ -55,8 +55,13 @@
> >>>>                 };
> >>>>                 clint at 2000000 {
> >>>>                         compatible = "riscv,clint0";
> >>>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3
> &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
> >>>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> >>>> +                                              &cpu1_intc 3 &cpu1_intc 7
> >>>> +                                              &cpu2_intc 3 &cpu2_intc 7
> >>>> +                                              &cpu3_intc 3 &cpu3_intc 7
> >>>> +                                              &cpu4_intc 3
> >>>> + &cpu4_intc 7>;
> >>>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
> >>>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
> >>>
> >>> This looks wrong to me. The CLINT timer frequency should come from the
> RTC node.
> >>>
> >>> +Pragnesh Patel
> >>>
> >>> +Sagar Kadam
> >>
> >> On further review, I think you are right that this should be RTCCLK_FREQ.
> >>
> >> Perhaps the clocks part should be moved into
> >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to
> >> something like
> >>
> >> clocks = <&rtcclk>;
> >

Yes, CLINT is driven by RTC clock.  

> > How does the device tree look like in the upstream Linux to work with
> > the new CLINT timer driver?
> >
> > Regards,
> > Bin
> 
> Anup's patch doesn't change the device tree at all so I think they are still using
> timebase-frequency.
> 
In that case I was wondering what would be better:
1. IMHO we can skip the property as in your patch [4/6] riscv: Rework Sifive CLINT as UCLASS_TIMER driver
    has a fallback to timebase-frequency, in case clock for clint is not specified and is falling back to timebase-frequency
    which is again set to RTCCLK_FREQ 
2. OR Shift the clock part to board -uboot dts file as you are suggesting in order to keep it synonymous with other TIMER
     changes done as a part of this series.

Thanks & BR,
Sagar
> --Sean

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Sean Anderson]

On 7/23/20 12:51 PM, Sagar Kadam wrote:
> Hello Sean,
> 
>> -----Original Message-----
>> From: Sean Anderson <seanga2@gmail.com>
>> Sent: Thursday, July 23, 2020 8:22 PM
>> To: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Pragnesh Patel <pragnesh.patel@sifive.com>; Sagar Kadam
>> <sagar.kadam@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>; Rick
>> Chen <rickchen36@gmail.com>
>> Subject: Re: [PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver
>>
>> [External Email] Do not click links or attachments unless you recognize the
>> sender and know the content is safe
>>
>> On 7/23/20 10:47 AM, Bin Meng wrote:
>>> Hi Sean,
>>>
>>> On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson <seanga2@gmail.com>
>> wrote:
>>>>
>>>> On 7/23/20 9:50 AM, Bin Meng wrote:
>>>>> Hi Sean,
>>>>>
>>>>> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com>
>> wrote:
>>>>>>
>>>>>> We may need to add a clock-frequency binding like for the K210.
>>>>>>
>>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>>>> ---
>>>>>> This patch builds but has NOT been tested.
>>>>>>
>>>>>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
>>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>>>> b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>>>> index afdb4f4402..e56bfc7595 100644
>>>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
>>>>>> @@ -55,8 +55,13 @@
>>>>>>                 };
>>>>>>                 clint at 2000000 {
>>>>>>                         compatible = "riscv,clint0";
>>>>>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>> &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3
>> &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
>>>>>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
>>>>>> +                                              &cpu1_intc 3 &cpu1_intc 7
>>>>>> +                                              &cpu2_intc 3 &cpu2_intc 7
>>>>>> +                                              &cpu3_intc 3 &cpu3_intc 7
>>>>>> +                                              &cpu4_intc 3
>>>>>> + &cpu4_intc 7>;
>>>>>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
>>>>>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
>>>>>
>>>>> This looks wrong to me. The CLINT timer frequency should come from the
>> RTC node.
>>>>>
>>>>> +Pragnesh Patel
>>>>>
>>>>> +Sagar Kadam
>>>>
>>>> On further review, I think you are right that this should be RTCCLK_FREQ.
>>>>
>>>> Perhaps the clocks part should be moved into
>>>> arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to
>>>> something like
>>>>
>>>> clocks = <&rtcclk>;
>>>
> 
> Yes, CLINT is driven by RTC clock.  
> 
>>> How does the device tree look like in the upstream Linux to work with
>>> the new CLINT timer driver?
>>>
>>> Regards,
>>> Bin
>>
>> Anup's patch doesn't change the device tree at all so I think they are still using
>> timebase-frequency.
>>
> In that case I was wondering what would be better:
> 1. IMHO we can skip the property as in your patch [4/6] riscv: Rework Sifive CLINT as UCLASS_TIMER driver
>     has a fallback to timebase-frequency, in case clock for clint is not specified and is falling back to timebase-frequency
>     which is again set to RTCCLK_FREQ
> 2. OR Shift the clock part to board -uboot dts file as you are suggesting in order to keep it synonymous with other TIMER
>      changes done as a part of this series.

I prefer the second option for two reasons. First, properties which
affect a device should be located near its binding in the device tree.
Using timebase-frequency only really makes sense when the cpu itself is
the timer device. This is the case when we read the time from a CSR, but
not when there is a separate device. Second, it lets the device use the
clock subsystem which adds flexibility. If the device is configured for
a different clock speed, the timer can adjust itself.

--Sean

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Bin Meng]

+Anup Patel

On Thu, Jul 23, 2020 at 10:52 PM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 7/23/20 10:47 AM, Bin Meng wrote:
> > Hi Sean,
> >
> > On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson <seanga2@gmail.com> wrote:
> >>
> >> On 7/23/20 9:50 AM, Bin Meng wrote:
> >>> Hi Sean,
> >>>
> >>> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson <seanga2@gmail.com> wrote:
> >>>>
> >>>> We may need to add a clock-frequency binding like for the K210.
> >>>>
> >>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >>>> ---
> >>>> This patch builds but has NOT been tested.
> >>>>
> >>>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
> >>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> index afdb4f4402..e56bfc7595 100644
> >>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>> @@ -55,8 +55,13 @@
> >>>>                 };
> >>>>                 clint at 2000000 {
> >>>>                         compatible = "riscv,clint0";
> >>>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3 &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
> >>>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> >>>> +                                              &cpu1_intc 3 &cpu1_intc 7
> >>>> +                                              &cpu2_intc 3 &cpu2_intc 7
> >>>> +                                              &cpu3_intc 3 &cpu3_intc 7
> >>>> +                                              &cpu4_intc 3 &cpu4_intc 7>;
> >>>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
> >>>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
> >>>
> >>> This looks wrong to me. The CLINT timer frequency should come from the RTC node.
> >>>
> >>> +Pragnesh Patel
> >>>
> >>> +Sagar Kadam
> >>
> >> On further review, I think you are right that this should be RTCCLK_FREQ.
> >>
> >> Perhaps the clocks part should be moved into
> >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to something
> >> like
> >>
> >> clocks = <&rtcclk>;
> >
> > How does the device tree look like in the upstream Linux to work with
> > the new CLINT timer driver?
> >
> > Regards,
> > Bin
>
> Anup's patch doesn't change the device tree at all so I think they are still using timebase-frequency.

Anup, could you please look at this and suggest whether we should add
this in the kernel upstream DTS?

Regards,
Bin

[PATCH 5/6] riscv: Update Kendryte device tree for new CLINT driver

[Sagar Kadam]

Hi Sean/Bin,

> -----Original Message-----
> From: Sean Anderson <seanga2@gmail.com>
> Sent: Thursday, July 23, 2020 7:30 PM
> To: Bin Meng <bmeng.cn@gmail.com>
> Cc: Sagar Kadam <sagar.kadam@sifive.com>; u-boot at lists.denx.de; Rick
> Chen <rickchen36@gmail.com>
> Subject: Re: [PATCH 5/6] riscv: Update Kendryte device tree for new CLINT
> driver
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> On 7/23/20 9:49 AM, Bin Meng wrote:
> > Hi Sean,
> >
> > On Thu, Jul 23, 2020 at 7:56 PM Sean Anderson <seanga2@gmail.com>
> wrote:
> >>
> >> On 7/23/20 7:49 AM, Sagar Kadam wrote:
> >>> Hello Sean,
> >>>
> >>>> -----Original Message-----
> >>>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Sean
> >>>> Anderson
> >>>> Sent: Wednesday, July 22, 2020 9:21 PM
> >>>> To: u-boot at lists.denx.de
> >>>> Cc: Bin Meng <bmeng.cn@gmail.com>; Rick Chen
> >>>> <rickchen36@gmail.com>; Sean Anderson <seanga2@gmail.com>
> >>>> Subject: [PATCH 5/6] riscv: Update Kendryte device tree for new
> >>>> CLINT driver
> >>>>
> >>>> [External Email] Do not click links or attachments unless you
> >>>> recognize the sender and know the content is safe
> >>>>
> >>>> AFAIK because the K210 clock driver does not come up until after
> >>>> relocation, the clint will always use the clock-frequency parameter.
> >>>> Ideally, it should update itself after relocation to take into
> >>>> account the actual CPU frequency.
> >>>>
> >>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >>>> ---
> >>>>
> >>>>  arch/riscv/dts/k210.dtsi                | 10 ++++++----
> >>>>  drivers/clk/kendryte/clk.c              |  4 ++++
> >>>>  include/dt-bindings/clock/k210-sysctl.h |  1 +
> >>>
> >>> Can you please consider splitting the dt-bindings include into
> >>> separate patch so as to avoid checkpatch warning.
> >>
> >> If you'd like. AFAIK this is mostly a kernel thing since dt-bindings
> >> often have separate maintainers than the rest of the series. Can
> >> anyone comment on whether this applies to U-Boot as well?
> >
> > If the changes are from upstream Linux kernel, it's fine to keep the
> > changes in the k210.dtsi. But if the changes are only needed in
> > U-Boot, as Sagar mentioned they should be in k210-uboot.dtsi.
> 
> Here the question is whether the patch should be split, not what file the
> changes should go in. At the moment, the dts is not synced between U-Boot
> and Linux for the K210, so there is no need to have a separate device tree
> file.
> 
Sorry if my statement turned confusing, (I didn't mean it to be into k210-uboot.dtsi), what I meant
 to say is we might consider splitting the dt-bindings header change into another patch within this series.
cause the checkpatch show's a warning as below:
#/scripts/checkpatch.pl 0005-riscv-Update-Kendryte-device-tree-for-new-CLINT-driv.patch 
WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.txt

total: 0 errors, 1 warnings, 0 checks, 49 lines checked

i.e IIUC,  the changes in include/dt-bindings/clock/k210-sysctl.h is expected to be as a separate patch 
within this series. So the split of this patch can be as

-Patch 5 can be :" riscv: Update Kendryte device tree for new CLINT driver" 
arch/riscv/dts/k210.dtsi                | 10 ++++++----
drivers/clk/kendryte/clk.c              |  4 ++++

-Patch 6 can be something like: "dt-bindings: clk: k210: add clint clock identifier"  
include/dt-bindings/clock/k210-sysctl.h |  1 +

Thanks & BR,
Sagar

> --Sean

[PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver

[Sagar Kadam]

> -----Original Message-----
> From: Sean Anderson <seanga2@gmail.com>
> Sent: Friday, July 24, 2020 1:58 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>; Bin Meng
> <bmeng.cn@gmail.com>
> Cc: Pragnesh Patel <pragnesh.patel@sifive.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Rick Chen <rickchen36@gmail.com>
> Subject: Re: [PATCH 6/6] riscv: Update SiFive device tree for new CLINT driver
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> On 7/23/20 12:51 PM, Sagar Kadam wrote:
> > Hello Sean,
> >
> >> -----Original Message-----
> >> From: Sean Anderson <seanga2@gmail.com>
> >> Sent: Thursday, July 23, 2020 8:22 PM
> >> To: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Pragnesh Patel <pragnesh.patel@sifive.com>; Sagar Kadam
> >> <sagar.kadam@sifive.com>; U-Boot Mailing List <u-boot@lists.denx.de>;
> >> Rick Chen <rickchen36@gmail.com>
> >> Subject: Re: [PATCH 6/6] riscv: Update SiFive device tree for new
> >> CLINT driver
> >>
> >> [External Email] Do not click links or attachments unless you
> >> recognize the sender and know the content is safe
> >>
> >> On 7/23/20 10:47 AM, Bin Meng wrote:
> >>> Hi Sean,
> >>>
> >>> On Thu, Jul 23, 2020 at 9:57 PM Sean Anderson <seanga2@gmail.com>
> >> wrote:
> >>>>
> >>>> On 7/23/20 9:50 AM, Bin Meng wrote:
> >>>>> Hi Sean,
> >>>>>
> >>>>> On Wed, Jul 22, 2020 at 11:51 PM Sean Anderson
> <seanga2@gmail.com>
> >> wrote:
> >>>>>>
> >>>>>> We may need to add a clock-frequency binding like for the K210.
> >>>>>>
> >>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >>>>>> ---
> >>>>>> This patch builds but has NOT been tested.
> >>>>>>
> >>>>>>  arch/riscv/dts/fu540-c000-u-boot.dtsi | 7 ++++++-
> >>>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>>>> b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>>>> index afdb4f4402..e56bfc7595 100644
> >>>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
> >>>>>> @@ -55,8 +55,13 @@
> >>>>>>                 };
> >>>>>>                 clint at 2000000 {
> >>>>>>                         compatible = "riscv,clint0";
> >>>>>> -                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> >> &cpu1_intc 3 &cpu1_intc 7 &cpu2_intc 3 &cpu2_intc 7 &cpu3_intc 3
> >> &cpu3_intc 7 &cpu4_intc 3 &cpu4_intc 7>;
> >>>>>> +                       interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7
> >>>>>> +                                              &cpu1_intc 3 &cpu1_intc 7
> >>>>>> +                                              &cpu2_intc 3 &cpu2_intc 7
> >>>>>> +                                              &cpu3_intc 3 &cpu3_intc 7
> >>>>>> +                                              &cpu4_intc 3
> >>>>>> + &cpu4_intc 7>;
> >>>>>>                         reg = <0x0 0x2000000 0x0 0xc0000>;
> >>>>>> +                       clocks = <&prci PRCI_CLK_COREPLL>;
> >>>>>
> >>>>> This looks wrong to me. The CLINT timer frequency should come from
> >>>>> the
> >> RTC node.
> >>>>>
> >>>>> +Pragnesh Patel
> >>>>>
> >>>>> +Sagar Kadam
> >>>>
> >>>> On further review, I think you are right that this should be
> RTCCLK_FREQ.
> >>>>
> >>>> Perhaps the clocks part should be moved into
> >>>> arch/riscv/dts/hifive-unleashed-a00-u-boot.dts and changed to
> >>>> something like
> >>>>
> >>>> clocks = <&rtcclk>;
> >>>
> >
> > Yes, CLINT is driven by RTC clock.
> >
> >>> How does the device tree look like in the upstream Linux to work
> >>> with the new CLINT timer driver?
> >>>
> >>> Regards,
> >>> Bin
> >>
> >> Anup's patch doesn't change the device tree at all so I think they
> >> are still using timebase-frequency.
> >>
> > In that case I was wondering what would be better:
> > 1. IMHO we can skip the property as in your patch [4/6] riscv: Rework Sifive
> CLINT as UCLASS_TIMER driver
> >     has a fallback to timebase-frequency, in case clock for clint is not
> specified and is falling back to timebase-frequency
> >     which is again set to RTCCLK_FREQ
> > 2. OR Shift the clock part to board -uboot dts file as you are suggesting in
> order to keep it synonymous with other TIMER
> >      changes done as a part of this series.
> 
> I prefer the second option for two reasons. First, properties which affect a
> device should be located near its binding in the device tree.
> Using timebase-frequency only really makes sense when the cpu itself is the
> timer device. This is the case when we read the time from a CSR, but not
> when there is a separate device. Second, it lets the device use the clock
> subsystem which adds flexibility. If the device is configured for a different
> clock speed, the timer can adjust itself.
>
Okay, agreed. 
Thanks for clarification.

BR,
Sagar
> --Sean

[PATCH 1/6] riscv: Rework riscv timer driver to only support S-mode

[Rick Chen]

Hi Sean

> The riscv-timer driver currently serves as a shim for several riscv timer
> drivers. This is not too desirable because it bypasses the usual timer
> selection via the driver model. There is no easy way to specify an
> alternate timing driver, or have the tick rate depend on the cpu's
> configured frequency. The timer drivers also do not have device structs,
> and so have to rely on storing parameters in gd_t. Lastly, there is no
> initialization call, so driver init is done in the same function which
> reads the time. This can result in confusing error messages. To a user, it
> looks like the driver failed when trying to read the time, whereas it may
> have failed while initializing.
>
> This patch removes the shim functionality from the riscv-timer driver, and
> has it instead implement the former rdtime.c timer driver. This is because
> existing u-boot users who pass in a device tree (e.g. qemu) do not create a
> timer device for S-mode u-boot. The existing behavior of creating the
> riscv-timer device in the riscv cpu driver must be kept. The actual reading
> of the CSRs has been redone in the style of Linux's get_cycles64.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  arch/riscv/lib/Makefile     |  1 -
>  arch/riscv/lib/rdtime.c     | 38 ------------------------------------
>  drivers/timer/Kconfig       |  6 +++---
>  drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------
>  4 files changed, 23 insertions(+), 61 deletions(-)
>  delete mode 100644 arch/riscv/lib/rdtime.c
>
> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> index 6c503ff2b2..10ac5b06d3 100644
> --- a/arch/riscv/lib/Makefile
> +++ b/arch/riscv/lib/Makefile
> @@ -15,7 +15,6 @@ obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
>  obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
>  obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o
>  else
> -obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
>  obj-$(CONFIG_SBI) += sbi.o
>  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
>  endif
> diff --git a/arch/riscv/lib/rdtime.c b/arch/riscv/lib/rdtime.c
> deleted file mode 100644
> index e128d7fce6..0000000000
> --- a/arch/riscv/lib/rdtime.c
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0+
> -/*
> - * Copyright (C) 2018, Anup Patel <anup@brainfault.org>
> - * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
> - *
> - * The riscv_get_time() API implementation that is using the
> - * standard rdtime instruction.
> - */
> -
> -#include <common.h>
> -
> -/* Implement the API required by RISC-V timer driver */
> -int riscv_get_time(u64 *time)
> -{
> -#ifdef CONFIG_64BIT
> -       u64 n;
> -
> -       __asm__ __volatile__ (
> -               "rdtime %0"
> -               : "=r" (n));
> -
> -       *time = n;
> -#else
> -       u32 lo, hi, tmp;
> -
> -       __asm__ __volatile__ (
> -               "1:\n"
> -               "rdtimeh %0\n"
> -               "rdtime %1\n"
> -               "rdtimeh %2\n"
> -               "bne %0, %2, 1b"
> -               : "=&r" (hi), "=&r" (lo), "=&r" (tmp));
> -
> -       *time = ((u64)hi << 32) | lo;
> -#endif
> -
> -       return 0;
> -}

config RISCV_RDTIME in arch/riscv/Kconfig shall be removed meanwhile.

Thanks,
Rick

> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> index 637024445c..b85fa33e47 100644
> --- a/drivers/timer/Kconfig
> +++ b/drivers/timer/Kconfig
> @@ -144,10 +144,10 @@ config OMAP_TIMER
>
>  config RISCV_TIMER
>         bool "RISC-V timer support"
> -       depends on TIMER && RISCV
> +       depends on TIMER && RISCV_SMODE
>         help
> -         Select this to enable support for the timer as defined
> -         by the RISC-V privileged architecture spec.
> +         Select this to enable support for a generic RISC-V S-Mode timer
> +         driver.
>
>  config ROCKCHIP_TIMER
>         bool "Rockchip timer support"
> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
> index 9f9f070e0b..449fcfcfd5 100644
> --- a/drivers/timer/riscv_timer.c
> +++ b/drivers/timer/riscv_timer.c
> @@ -1,36 +1,37 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> + * Copyright (C) 2020, Sean Anderson <seanga2@gmail.com>
>   * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
> + * Copyright (C) 2018, Anup Patel <anup@brainfault.org>
> + * Copyright (C) 2012 Regents of the University of California
>   *
> - * RISC-V privileged architecture defined generic timer driver
> + * RISC-V architecturally-defined generic timer driver
>   *
> - * This driver relies on RISC-V platform codes to provide the essential API
> - * riscv_get_time() which is supposed to return the timer counter as defined
> - * by the RISC-V privileged architecture spec.
> - *
> - * This driver can be used in both M-mode and S-mode U-Boot.
> + * This driver provides generic timer support for S-mode U-Boot.
>   */
>
>  #include <common.h>
>  #include <dm.h>
>  #include <errno.h>
>  #include <timer.h>
> -#include <asm/io.h>
> -
> -/**
> - * riscv_get_time() - get the timer counter
> - *
> - * Platform codes should provide this API in order to make this driver function.
> - *
> - * @time:      the 64-bit timer count  as defined by the RISC-V privileged
> - *             architecture spec.
> - * @return:    0 on success, -ve on error.
> - */
> -extern int riscv_get_time(u64 *time);
> +#include <asm/csr.h>
>
>  static int riscv_timer_get_count(struct udevice *dev, u64 *count)
>  {
> -       return riscv_get_time(count);
> +       if (IS_ENABLED(CONFIG_64BIT)) {
> +               *count = csr_read(CSR_TIME);
> +       } else {
> +               u32 hi, lo;
> +
> +               do {
> +                       hi = csr_read(CSR_TIMEH);
> +                       lo = csr_read(CSR_TIME);
> +               } while (hi != csr_read(CSR_TIMEH));
> +
> +               *count = ((u64)hi << 32) | lo;
> +       }
> +
> +       return 0;
>  }
>
>  static int riscv_timer_probe(struct udevice *dev)
> --
> 2.27.0
>

[PATCH 1/6] riscv: Rework riscv timer driver to only support S-mode

[Sean Anderson]

On 7/28/20 5:11 AM, Rick Chen wrote:
> Hi Sean
> 
>> The riscv-timer driver currently serves as a shim for several riscv timer
>> drivers. This is not too desirable because it bypasses the usual timer
>> selection via the driver model. There is no easy way to specify an
>> alternate timing driver, or have the tick rate depend on the cpu's
>> configured frequency. The timer drivers also do not have device structs,
>> and so have to rely on storing parameters in gd_t. Lastly, there is no
>> initialization call, so driver init is done in the same function which
>> reads the time. This can result in confusing error messages. To a user, it
>> looks like the driver failed when trying to read the time, whereas it may
>> have failed while initializing.
>>
>> This patch removes the shim functionality from the riscv-timer driver, and
>> has it instead implement the former rdtime.c timer driver. This is because
>> existing u-boot users who pass in a device tree (e.g. qemu) do not create a
>> timer device for S-mode u-boot. The existing behavior of creating the
>> riscv-timer device in the riscv cpu driver must be kept. The actual reading
>> of the CSRs has been redone in the style of Linux's get_cycles64.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>  arch/riscv/lib/Makefile     |  1 -
>>  arch/riscv/lib/rdtime.c     | 38 ------------------------------------
>>  drivers/timer/Kconfig       |  6 +++---
>>  drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------
>>  4 files changed, 23 insertions(+), 61 deletions(-)
>>  delete mode 100644 arch/riscv/lib/rdtime.c
>>
>> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
>> index 6c503ff2b2..10ac5b06d3 100644
>> --- a/arch/riscv/lib/Makefile
>> +++ b/arch/riscv/lib/Makefile
>> @@ -15,7 +15,6 @@ obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
>>  obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
>>  obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o
>>  else
>> -obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
>>  obj-$(CONFIG_SBI) += sbi.o
>>  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
>>  endif
>> diff --git a/arch/riscv/lib/rdtime.c b/arch/riscv/lib/rdtime.c
>> deleted file mode 100644
>> index e128d7fce6..0000000000
>> --- a/arch/riscv/lib/rdtime.c
>> +++ /dev/null
>> @@ -1,38 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0+
>> -/*
>> - * Copyright (C) 2018, Anup Patel <anup@brainfault.org>
>> - * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
>> - *
>> - * The riscv_get_time() API implementation that is using the
>> - * standard rdtime instruction.
>> - */
>> -
>> -#include <common.h>
>> -
>> -/* Implement the API required by RISC-V timer driver */
>> -int riscv_get_time(u64 *time)
>> -{
>> -#ifdef CONFIG_64BIT
>> -       u64 n;
>> -
>> -       __asm__ __volatile__ (
>> -               "rdtime %0"
>> -               : "=r" (n));
>> -
>> -       *time = n;
>> -#else
>> -       u32 lo, hi, tmp;
>> -
>> -       __asm__ __volatile__ (
>> -               "1:\n"
>> -               "rdtimeh %0\n"
>> -               "rdtime %1\n"
>> -               "rdtimeh %2\n"
>> -               "bne %0, %2, 1b"
>> -               : "=&r" (hi), "=&r" (lo), "=&r" (tmp));
>> -
>> -       *time = ((u64)hi << 32) | lo;
>> -#endif
>> -
>> -       return 0;
>> -}
> 
> config RISCV_RDTIME in arch/riscv/Kconfig shall be removed meanwhile.
> 
> Thanks,
> Rick

Ok, I will remove that in V2.

>> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
>> index 637024445c..b85fa33e47 100644
>> --- a/drivers/timer/Kconfig
>> +++ b/drivers/timer/Kconfig
>> @@ -144,10 +144,10 @@ config OMAP_TIMER
>>
>>  config RISCV_TIMER
>>         bool "RISC-V timer support"
>> -       depends on TIMER && RISCV
>> +       depends on TIMER && RISCV_SMODE
>>         help
>> -         Select this to enable support for the timer as defined
>> -         by the RISC-V privileged architecture spec.
>> +         Select this to enable support for a generic RISC-V S-Mode timer
>> +         driver.
>>
>>  config ROCKCHIP_TIMER
>>         bool "Rockchip timer support"
>> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
>> index 9f9f070e0b..449fcfcfd5 100644
>> --- a/drivers/timer/riscv_timer.c
>> +++ b/drivers/timer/riscv_timer.c
>> @@ -1,36 +1,37 @@
>>  // SPDX-License-Identifier: GPL-2.0+
>>  /*
>> + * Copyright (C) 2020, Sean Anderson <seanga2@gmail.com>
>>   * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
>> + * Copyright (C) 2018, Anup Patel <anup@brainfault.org>
>> + * Copyright (C) 2012 Regents of the University of California
>>   *
>> - * RISC-V privileged architecture defined generic timer driver
>> + * RISC-V architecturally-defined generic timer driver
>>   *
>> - * This driver relies on RISC-V platform codes to provide the essential API
>> - * riscv_get_time() which is supposed to return the timer counter as defined
>> - * by the RISC-V privileged architecture spec.
>> - *
>> - * This driver can be used in both M-mode and S-mode U-Boot.
>> + * This driver provides generic timer support for S-mode U-Boot.
>>   */
>>
>>  #include <common.h>
>>  #include <dm.h>
>>  #include <errno.h>
>>  #include <timer.h>
>> -#include <asm/io.h>
>> -
>> -/**
>> - * riscv_get_time() - get the timer counter
>> - *
>> - * Platform codes should provide this API in order to make this driver function.
>> - *
>> - * @time:      the 64-bit timer count  as defined by the RISC-V privileged
>> - *             architecture spec.
>> - * @return:    0 on success, -ve on error.
>> - */
>> -extern int riscv_get_time(u64 *time);
>> +#include <asm/csr.h>
>>
>>  static int riscv_timer_get_count(struct udevice *dev, u64 *count)
>>  {
>> -       return riscv_get_time(count);
>> +       if (IS_ENABLED(CONFIG_64BIT)) {
>> +               *count = csr_read(CSR_TIME);
>> +       } else {
>> +               u32 hi, lo;
>> +
>> +               do {
>> +                       hi = csr_read(CSR_TIMEH);
>> +                       lo = csr_read(CSR_TIME);
>> +               } while (hi != csr_read(CSR_TIMEH));
>> +
>> +               *count = ((u64)hi << 32) | lo;
>> +       }
>> +
>> +       return 0;
>>  }
>>
>>  static int riscv_timer_probe(struct udevice *dev)
>> --
>> 2.27.0
>>