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

Bin Meng
2020-09-15T07:18:11Z

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?

> };
> };
>
> 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