On 9/15/20 3:18 AM, Bin Meng wrote: > Hi Sean, > > On Thu, Sep 10, 2020 at 4:09 AM Sean Anderson <sean...@gmail.com> wrote: >> >> This converts the PLMT driver from the riscv-specific timer interface to be >> a DM-based UCLASS_TIMER driver. >> >> The clock-frequency/clocks properties are preferred over timebase-frequency >> 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. >> >> Signed-off-by: Sean Anderson <sean...@gmail.com> >> Reviewed-by: Rick Chen <r...@andestech.com> >> --- >> This patch builds but has NOT been tested. >> >> Changes in v4: >> - Use timer_timebase_fallback >> >> 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 | 44 +++++++++++++--------------- >> 5 files changed, 23 insertions(+), 30 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>; > > Is this change required to make the PLMT timer work?
With the addition of timer_timebase_fallback it is not. >> }; >> }; >> >> 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..a28c14c1eb 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 <r...@andestech.com> >> + * Copyright (C) 2020, Sean Anderson <sean...@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(); >> - >> - *time = readq((void __iomem *)MTIME_REG(gd->arch.plmt)); >> + *count = readq((void __iomem *)MTIME_REG(dev->priv)); >> >> 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 timer_timebase_fallback(dev); >> +} >> + >> 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 > >> + { .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 >