[PATCH v4 3/8] riscv: Rework Andes PLMT as a UCLASS_TIMER driver

Sean Anderson
2020-09-15T10:04:21Z

On 9/15/20 3:18 AM, Bin Meng wrote:
> Hi Sean,
>
> On Thu, Sep 10, 2020 at 4:09 AM 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.
>>
>> 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 <seanga2@gmail.com>
>> Reviewed-by: Rick Chen <rick@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 <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();
>> -
>> - *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
>