| Message ID | 20200909200930.232174-4-seanga2@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: Clean up timer drivers | expand |
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
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 >
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..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 }, + { .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, };