[PATCH v2 0/7] riscv: Clean up timer drivers
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
Changes in v2: - Remove RISCV_RDTIME KConfig option - Split Kendryte binding changes into their own commit - Fix SiFive CLINT not getting tick-rate from rtcclk
Sean Anderson (7): 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: clk: Add CLINT clock to kendryte clock driver riscv: Update Kendryte device tree for new CLINT driver riscv: Update SiFive device tree for new CLINT driver
arch/riscv/Kconfig | 16 ---- arch/riscv/dts/ae350_32.dts | 1 + arch/riscv/dts/ae350_64.dts | 1 + arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 +- .../dts/hifive-unleashed-a00-u-boot.dtsi | 4 + arch/riscv/dts/k210.dtsi | 9 +- 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 + 18 files changed, 155 insertions(+), 170 deletions(-) delete mode 100644 arch/riscv/lib/rdtime.c
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 ---
Changes in v2: - Remove RISCV_RDTIME KConfig option
arch/riscv/Kconfig | 8 -------- arch/riscv/lib/Makefile | 1 - arch/riscv/lib/rdtime.c | 38 ------------------------------------ drivers/timer/Kconfig | 6 +++--- drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------ 5 files changed, 23 insertions(+), 69 deletions(-) delete mode 100644 arch/riscv/lib/rdtime.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 009a545fcf..21e6690f4d 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -185,14 +185,6 @@ config ANDES_PLMT The Andes PLMT block holds memory-mapped mtime register associated with timer tick.
-config RISCV_RDTIME - bool - default y if RISCV_SMODE || SPL_RISCV_SMODE - help - The provides the riscv_get_time() API that is implemented using the - standard rdtime instruction. This is the case for S-mode U-Boot, and - is useful for processors that support rdtime in M-mode too. - config SYS_MALLOC_F_LEN default 0x1000
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)
On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson seanga2@gmail.com wrote:
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
Changes in v2:
- Remove RISCV_RDTIME KConfig option
arch/riscv/Kconfig | 8 -------- arch/riscv/lib/Makefile | 1 - arch/riscv/lib/rdtime.c | 38 ------------------------------------ drivers/timer/Kconfig | 6 +++--- drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------ 5 files changed, 23 insertions(+), 69 deletions(-) delete mode 100644 arch/riscv/lib/rdtime.c
Reviewed-by: Bin Meng bin.meng@windriver.com
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.
(no changes since v1)
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 21e6690f4d..d9155b9bab 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, };
On Wed, Jul 29, 2020 at 5:56 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.
(no changes since v1)
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 21e6690f4d..d9155b9bab 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 REGMAPselect SYSCONselect SPL_REGMAP if SPLselect 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>;
Why not use /cpus/timebase-frequency directly?
}; };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 },
RISCV_SYSCON_PLMT should be removed from arch/riscv/include/asm/syscon.h to make this patch complete.
{ .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
On 8/20/20 3:40 AM, Bin Meng wrote:
On Wed, Jul 29, 2020 at 5:56 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.
(no changes since v1)
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 21e6690f4d..d9155b9bab 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 REGMAPselect SYSCONselect SPL_REGMAP if SPLselect 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>;Why not use /cpus/timebase-frequency directly?
I touched on this in the discussion for the previous version of the sifive clint patch.
I prefer [using clock-frequency/clocks] 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.
Only the first reason applies to the andes plmt, but I think it is good for consistency to move the property here as well.
}; };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 },RISCV_SYSCON_PLMT should be removed from arch/riscv/include/asm/syscon.h to make this patch complete.
Ok.
{ .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
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.
(no changes since v1)
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", ®); - if (ret == 0) - enable_ipi(reg); - } - - return 0; + /* read hart ID of CPU */ + ret = ofnode_read_u32(node, "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)
On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson seanga2@gmail.com wrote:
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.
(no changes since v1)
arch/riscv/lib/andes_plic.c | 58 ++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 33 deletions(-)
Reviewed-by: Bin Meng bin.meng@windriver.com
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 --- This patch builds but has only been tested on the K210 and QEMU. It has NOT been tested on a HiFive.
(no changes since v1)
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 d9155b9bab..aaa3b833a5 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.
On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson seanga2@gmail.com wrote:
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
This patch builds but has only been tested on the K210 and QEMU. It has NOT been tested on a HiFive.
(no changes since v1)
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 d9155b9bab..aaa3b833a5 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 REGMAPselect SYSCONselect SPL_REGMAP if SPLselect 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 },
RISCV_SYSCON_CLINT should be removed from syscon.h
{ .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();
Why above is needed? This is already called in arch_cpu_init_dm() in arch/riscv/cpu/cpu.c
/* * 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 REGMAPselect SPL_REGMAP if SPL
I don't understand why this driver depends on REGMAP?
help This enables DDR support for the platforms based on SiFive FU540 SoC.--
Regards, Bin
On 8/20/20 3:43 AM, Bin Meng wrote:
On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson seanga2@gmail.com wrote:
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
This patch builds but has only been tested on the K210 and QEMU. It has NOT been tested on a HiFive.
(no changes since v1)
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 d9155b9bab..aaa3b833a5 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 REGMAPselect SYSCONselect SPL_REGMAP if SPLselect 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 },RISCV_SYSCON_CLINT should be removed from syscon.h
This can't be removed because the IPI driver still depends on it.
{ .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();Why above is needed? This is already called in arch_cpu_init_dm() in arch/riscv/cpu/cpu.c
I believe I ran into problems with it not getting called in SPL when testing. I can look into whether this is still needed.
/* * 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 REGMAPselect SPL_REGMAP if SPLI don't understand why this driver depends on REGMAP?
I don't either, but it fails to build without these two lines. Previously, regmap was always enabled by the sifive clint. So now that it no longer enables regmap, this dependency became apparent.
help This enables DDR support for the platforms based on SiFive FU540 SoC.--
Regards, Bin
Another "virtual" clock (in the sense that it isn't configurable). This could possibly be done as a clock in the device tree, but I think this is a bit cleaner.
Signed-off-by: Sean Anderson seanga2@gmail.com --- checkpatch still complains about this one, but I don't see any reason to break it up even further. It doesn't make sense to me to split the header file change from everything else.
Changes in v2: - Split Kendryte binding changes into their own commit
drivers/clk/kendryte/clk.c | 4 ++++ include/dt-bindings/clock/k210-sysctl.h | 1 + 2 files changed, 5 insertions(+)
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 */
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 ---
Changes in v2: - New
arch/riscv/dts/k210.dtsi | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi index 2546c7d4e0..2698a33a5c 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; @@ -126,14 +128,15 @@ read-only; };
- clint0: interrupt-controller@2000000 { + clint0: clint@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@C000000 {
On Wed, Jul 29, 2020 at 5:57 PM Sean Anderson seanga2@gmail.com wrote:
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
Changes in v2:
- New
arch/riscv/dts/k210.dtsi | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi index 2546c7d4e0..2698a33a5c 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;@@ -126,14 +128,15 @@ read-only; };
clint0: interrupt-controller@2000000 {
clint0: clint@2000000 { #interrupt-cells = <1>; compatible = "kendryte,k210-clint", "riscv,clint0"; reg = <0x2000000 0xC000>;
interrupt-controller;
Why is this property dropped?
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@C000000 {--
Regards, Bin
On 8/20/20 3:45 AM, Bin Meng wrote:
On Wed, Jul 29, 2020 at 5:57 PM Sean Anderson seanga2@gmail.com wrote:
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
Changes in v2:
- New
arch/riscv/dts/k210.dtsi | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi index 2546c7d4e0..2698a33a5c 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;@@ -126,14 +128,15 @@ read-only; };
clint0: interrupt-controller@2000000 {
clint0: clint@2000000 { #interrupt-cells = <1>; compatible = "kendryte,k210-clint", "riscv,clint0"; reg = <0x2000000 0xC000>;
interrupt-controller;Why is this property dropped?
Because the clint is not an interrupt-controller. That is, no other devices have an interrupt which is controlled by the clint.
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@C000000 {--
Regards, Bin
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.
Changes in v2: - Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; }; - clint@2000000 { + clint: clint@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>; u-boot,dm-spl; }; diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint { + clocks = <&rtcclk>; +}; + &qspi2 { mmc@0 { u-boot,dm-spl;
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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.
Changes in v2:
- Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; };
clint@2000000 {
clint: clint@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>; u-boot,dm-spl; };diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint {
clocks = <&rtcclk>;+};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
&qspi2 { mmc@0 { u-boot,dm-spl;
Regards, Bin
On 8/18/20 5:22 AM, Bin Meng wrote:
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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.
Changes in v2:
- Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; };
clint@2000000 {
clint: clint@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>; u-boot,dm-spl; };diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint {
clocks = <&rtcclk>;+};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
Currently it gets it from /cpus/timebase-frequency.
--Sean
Hi Anup,
On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson seanga2@gmail.com wrote:
On 8/18/20 5:22 AM, Bin Meng wrote:
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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.
Changes in v2:
- Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; };
clint@2000000 {
clint: clint@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>; u-boot,dm-spl; };diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint {
clocks = <&rtcclk>;+};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
Currently it gets it from /cpus/timebase-frequency.
What's your comment on this? I think this should go upstream to the standard bindings.
Regards, Bin
On Thu, Sep 3, 2020 at 7:32 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson seanga2@gmail.com wrote:
On 8/18/20 5:22 AM, Bin Meng wrote:
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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.
Changes in v2:
- Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; };
clint@2000000 {
clint: clint@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>; u-boot,dm-spl; };diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint {
clocks = <&rtcclk>;+};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
Currently it gets it from /cpus/timebase-frequency.
What's your comment on this? I think this should go upstream to the standard bindings.
My apologies for delay. Too many mailing list to track.
I think "clocks" should be an optional DT property for CLINT driver (both Linux and U-Boot). If "clocks" is not available then we should fallback to "/cpus/timebase-frequency"
Please note in any case the "/cpus/timebase-frequency" will remain mandatory for generic RISC-V timer (both Linux and U-Boot).
Regards, Anup
Hi Anup,
On Thu, Sep 3, 2020 at 10:46 AM Anup Patel anup@brainfault.org wrote:
On Thu, Sep 3, 2020 at 7:32 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson seanga2@gmail.com wrote:
On 8/18/20 5:22 AM, Bin Meng wrote:
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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.
Changes in v2:
- Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; };
clint@2000000 {
clint: clint@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>; u-boot,dm-spl; };diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint {
clocks = <&rtcclk>;+};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
Currently it gets it from /cpus/timebase-frequency.
What's your comment on this? I think this should go upstream to the standard bindings.
My apologies for delay. Too many mailing list to track.
I think "clocks" should be an optional DT property for CLINT driver (both Linux and U-Boot). If "clocks" is not available then we should fallback to "/cpus/timebase-frequency"
Agreed. The question is whether this should be mentioned in the DT bindings doc in the kernel tree?
Please note in any case the "/cpus/timebase-frequency" will remain mandatory for generic RISC-V timer (both Linux and U-Boot).
Regards, Bin
On Thu, Sep 3, 2020 at 8:19 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Thu, Sep 3, 2020 at 10:46 AM Anup Patel anup@brainfault.org wrote:
On Thu, Sep 3, 2020 at 7:32 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson seanga2@gmail.com wrote:
On 8/18/20 5:22 AM, Bin Meng wrote:
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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.
Changes in v2:
- Fix SiFive CLINT not getting tick-rate from rtcclk
arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi index afdb4f4402..f126d3e0b3 100644 --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi @@ -53,9 +53,13 @@ reg = <0x0 0x10070000 0x0 0x1000>; fuse-count = <0x1000>; };
clint@2000000 {
clint: clint@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>; u-boot,dm-spl; };diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index e037150520..3275bb1f12 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -26,6 +26,10 @@
};
+&clint {
clocks = <&rtcclk>;+};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
Currently it gets it from /cpus/timebase-frequency.
What's your comment on this? I think this should go upstream to the standard bindings.
My apologies for delay. Too many mailing list to track.
I think "clocks" should be an optional DT property for CLINT driver (both Linux and U-Boot). If "clocks" is not available then we should fallback to "/cpus/timebase-frequency"
Agreed. The question is whether this should be mentioned in the DT bindings doc in the kernel tree?
Yes, the "clocks" DT property should be updated in kernel DT bindings doc and kernel CLINT driver should also prefer "clocks" DT property when available.
Regards, Anup
Please note in any case the "/cpus/timebase-frequency" will remain mandatory for generic RISC-V timer (both Linux and U-Boot).
Regards, Bin
On 9/3/20 1:01 AM, Anup Patel wrote:
On Thu, Sep 3, 2020 at 8:19 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Thu, Sep 3, 2020 at 10:46 AM Anup Patel anup@brainfault.org wrote:
On Thu, Sep 3, 2020 at 7:32 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson seanga2@gmail.com wrote:
On 8/18/20 5:22 AM, Bin Meng wrote:
+Anup Patel
On Wed, Jul 29, 2020 at 5:57 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. > > Changes in v2: > - Fix SiFive CLINT not getting tick-rate from rtcclk > > arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- > arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > index afdb4f4402..f126d3e0b3 100644 > --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > @@ -53,9 +53,13 @@ > reg = <0x0 0x10070000 0x0 0x1000>; > fuse-count = <0x1000>; > }; > - clint@2000000 { > + clint: clint@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>; > u-boot,dm-spl; > }; > diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > index e037150520..3275bb1f12 100644 > --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi > @@ -26,6 +26,10 @@ > > }; > > +&clint { > + clocks = <&rtcclk>; > +};
Can we consider making this property a standard property by the kernel upstream? How does the kernel CLINT timer driver determine its running frequency?
Currently it gets it from /cpus/timebase-frequency.
What's your comment on this? I think this should go upstream to the standard bindings.
My apologies for delay. Too many mailing list to track.
I think "clocks" should be an optional DT property for CLINT driver (both Linux and U-Boot). If "clocks" is not available then we should fallback to "/cpus/timebase-frequency"
Agreed. The question is whether this should be mentioned in the DT bindings doc in the kernel tree?
Yes, the "clocks" DT property should be updated in kernel DT bindings doc and kernel CLINT driver should also prefer "clocks" DT property when available.
Regards, Anup
Should clock-frequency be used as well, or should that be left for timebase-frequency?
--Sean
On Thu, Sep 3, 2020 at 4:23 PM Sean Anderson seanga2@gmail.com wrote:
On 9/3/20 1:01 AM, Anup Patel wrote:
On Thu, Sep 3, 2020 at 8:19 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Thu, Sep 3, 2020 at 10:46 AM Anup Patel anup@brainfault.org wrote:
On Thu, Sep 3, 2020 at 7:32 AM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Tue, Aug 18, 2020 at 6:03 PM Sean Anderson seanga2@gmail.com wrote:
On 8/18/20 5:22 AM, Bin Meng wrote: > +Anup Patel > > On Wed, Jul 29, 2020 at 5:57 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. >> >> Changes in v2: >> - Fix SiFive CLINT not getting tick-rate from rtcclk >> >> arch/riscv/dts/fu540-c000-u-boot.dtsi | 8 ++++++-- >> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi >> index afdb4f4402..f126d3e0b3 100644 >> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi >> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi >> @@ -53,9 +53,13 @@ >> reg = <0x0 0x10070000 0x0 0x1000>; >> fuse-count = <0x1000>; >> }; >> - clint@2000000 { >> + clint: clint@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>; >> u-boot,dm-spl; >> }; >> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >> index e037150520..3275bb1f12 100644 >> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi >> @@ -26,6 +26,10 @@ >> >> }; >> >> +&clint { >> + clocks = <&rtcclk>; >> +}; > > Can we consider making this property a standard property by the kernel > upstream? How does the kernel CLINT timer driver determine its running > frequency?
Currently it gets it from /cpus/timebase-frequency.
What's your comment on this? I think this should go upstream to the standard bindings.
My apologies for delay. Too many mailing list to track.
I think "clocks" should be an optional DT property for CLINT driver (both Linux and U-Boot). If "clocks" is not available then we should fallback to "/cpus/timebase-frequency"
Agreed. The question is whether this should be mentioned in the DT bindings doc in the kernel tree?
Yes, the "clocks" DT property should be updated in kernel DT bindings doc and kernel CLINT driver should also prefer "clocks" DT property when available.
Regards, Anup
Should clock-frequency be used as well, or should that be left for timebase-frequency?
Not sure about clock-frequency DT property. I think very few linux clocksource drivers use "clock-frequency" DT property.
Probably it's fine to first check for "clocks" and "clock-frequency" before falling back to "timebase-frequency".
Regards, Anup
--Sean
Hi Sean,
On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson seanga2@gmail.com wrote:
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
Changes in v2:
- Remove RISCV_RDTIME KConfig option
- Split Kendryte binding changes into their own commit
- Fix SiFive CLINT not getting tick-rate from rtcclk
This series does not apply to u-boot/master. Please rebase.
Regards, Bin
participants (3)
-
Anup Patel -
Bin Meng -
Sean Anderson