[PATCH v2 2/7] riscv: Rework Andes PLMT as a UCLASS_TIMER driver
Sean Anderson
seanga2 at gmail.com
Thu Aug 20 12:15:34 CEST 2020
On 8/20/20 3:40 AM, Bin Meng wrote:
> On Wed, Jul 29, 2020 at 5:56 PM Sean Anderson <seanga2 at 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 at 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>;
>
> 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 at andestech.com>
>> + * Copyright (C) 2020, Sean Anderson <seanga2 at 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
>
More information about the U-Boot
mailing list