diff mbox series

[v2,2/7] riscv: Rework Andes PLMT as a UCLASS_TIMER driver

Message ID 20200729095636.1077054-3-seanga2@gmail.com
State Superseded
Delegated to: Andes
Headers show
Series riscv: Clean up timer drivers | expand

Commit Message

Sean Anderson July 29, 2020, 9:56 a.m. UTC
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@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(-)

Comments

Bin Meng Aug. 20, 2020, 7:40 a.m. UTC | #1
On Wed, Jul 29, 2020 at 5:56 PM 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.
>
> Signed-off-by: Sean Anderson <seanga2@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?

>                 };
>         };
>
> 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@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();
> +       *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.

> +       { .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
Sean Anderson Aug. 20, 2020, 10:15 a.m. UTC | #2
On 8/20/20 3:40 AM, Bin Meng wrote:
> On Wed, Jul 29, 2020 at 5:56 PM 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.
>>
>> Signed-off-by: Sean Anderson <seanga2@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@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();
>> +       *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
>
diff mbox series

Patch

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>;
 		};
 	};
 
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@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();
+	*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 },
+	{ .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,
 };