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

[Bin Meng]

Hi Sean,

On Thu, Sep 10, 2020 at 4:09 AM 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.
>
> 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 at gmail.com>
> Reviewed-by: Rick Chen <rick at 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 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();
> -
> -       *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