[PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Pragnesh Patel
pragnesh.patel at openfive.com
Sun Nov 15 12:28:00 CET 2020
Hi Rick,
>-----Original Message-----
>From: Rick Chen <rickchen36 at gmail.com>
>Sent: 13 November 2020 13:37
>To: Pragnesh Patel <pragnesh.patel at openfive.com>
>Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra
><atish.patra at wdc.com>; palmerdabbelt at google.com; Bin Meng
><bmeng.cn at gmail.com>; Paul Walmsley ( Sifive) <paul.walmsley at sifive.com>;
>Anup Patel <anup.patel at wdc.com>; Sagar Kadam
><sagar.kadam at openfive.com>; Sean Anderson <seanga2 at gmail.com>; rick
><rick at andestech.com>; Alan Kao <alankao at andestech.com>; Leo Liang
><ycliang at andestech.com>
>Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>Hi Pragnesh
>
>> From: Pragnesh Patel [mailto:pragnesh.patel at sifive.com]
>> Sent: Wednesday, November 11, 2020 6:15 PM
>> To: u-boot at lists.denx.de
>> Cc: atish.patra at wdc.com; palmerdabbelt at google.com;
>bmeng.cn at gmail.com;
>> paul.walmsley at sifive.com; anup.patel at wdc.com; sagar.kadam at sifive.com;
>> Rick Jian-Zhi Chen(陳建志); Pragnesh Patel; Sean Anderson; Heinrich
>> Schuchardt; Simon Glass
>> Subject: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>>
>> Add timer_get_us() which is useful for tracing.
>> For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
>> and For M-mode U-Boot, mtime register will provide the same.
>>
>> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
>> ---
>>
>> Changes in v3:
>> - Added gd->arch.plmt in global data
>> - For timer_get_us(), use readq() instead of andes_plmt_get_count()
>> and sifive_clint_get_count()
>>
>> Changes in v2:
>> - Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
>> and andes_plmt_timer.c.
>>
>>
>> arch/riscv/include/asm/global_data.h | 3 +++
>> drivers/timer/andes_plmt_timer.c | 19 ++++++++++++++++++-
>> drivers/timer/riscv_timer.c | 14 +++++++++++++-
>> drivers/timer/sifive_clint_timer.c | 19 ++++++++++++++++++-
>> 4 files changed, 52 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/global_data.h
>> b/arch/riscv/include/asm/global_data.h
>> index d3a0b1d221..4e22ceb83f 100644
>> --- a/arch/riscv/include/asm/global_data.h
>> +++ b/arch/riscv/include/asm/global_data.h
>> @@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLIC
>> void __iomem *plic; /* plic base address */
>> #endif
>> +#ifdef CONFIG_ANDES_PLMT
>
>It shall be CONFIG_ANDES_PLMT, or it will compile fail as below:
>
>drivers/timer/andes_plmt_timer.c: In function 'timer_get_us':
>drivers/timer/andes_plmt_timer.c:36:15: error: 'struct arch_global_data' has no
>member named 'plmt'; did you mean 'plic'?
> if (gd->arch.plmt) {
> ^~~~
> plic
>drivers/timer/andes_plmt_timer.c:37:52: error: 'struct arch_global_data' has no
>member named 'plmt'; did you mean 'plic'?
> ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
No I did not get this. Why are you getting this error because plmt is already defined ?
>
>And it is not proper to have dependency of data structure between
>/arch/riscv/include/asm/* and /drivers/timer/* Maybe enable TIMER_EARLY and
>use timer_get_us() of /lib/time.c will be better.
>I also found that without dm_timer_init() in initf_dm(),
>andes_plmt_get_count() will be executed ahead of andes_plmt_probe().
Thanks Rick but me and Heinrich are working on different approach to use existing dm_timer_init() from lib/time.c
https://patchwork.ozlabs.org/project/uboot/patch/20201112071411.4202-1-xypron.glpk@gmx.de/
>
>Thanks,
>Rick
>
>
>> + void __iomem *plmt; /* plmt base address */
>> +#endif
>> #if CONFIG_IS_ENABLED(SMP)
>> struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git
>> a/drivers/timer/andes_plmt_timer.c b/drivers/timer/andes_plmt_timer.c
>> index cec86718c7..7c50c46d9e 100644
>> --- a/drivers/timer/andes_plmt_timer.c
>> +++ b/drivers/timer/andes_plmt_timer.c
>> @@ -13,11 +13,12 @@
>> #include <timer.h>
>> #include <asm/io.h>
>> #include <linux/err.h>
>> +#include <div64.h>
>>
>> /* mtime register */
>> #define MTIME_REG(base) ((ulong)(base))
>>
>> -static u64 andes_plmt_get_count(struct udevice *dev)
>> +static u64 notrace andes_plmt_get_count(struct udevice *dev)
>> {
>> return readq((void __iomem *)MTIME_REG(dev->priv)); } @@
>> -26,12 +27,28 @@ static const struct timer_ops andes_plmt_ops = {
>> .get_count = andes_plmt_get_count, };
>>
>> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
>> +unsigned long notrace timer_get_us(void) {
>> + u64 ticks;
>> +
>> + /* FIXME: gd->arch.plmt should contain valid base address */
>> + if (gd->arch.plmt) {
>> + ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
>> + do_div(ticks, CONFIG_SYS_HZ);
>> + }
>> +
>> + return ticks;
>> +}
>> +#endif
>> +
>> static int andes_plmt_probe(struct udevice *dev) {
>> dev->priv = dev_read_addr_ptr(dev);
>> if (!dev->priv)
>> return -EINVAL;
>>
>> + gd->arch.plmt = dev->priv;
>> return timer_timebase_fallback(dev); }
>>
>> diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
>> index 21ae184057..7fa8773da3 100644
>> --- a/drivers/timer/riscv_timer.c
>> +++ b/drivers/timer/riscv_timer.c
>> @@ -15,8 +15,9 @@
>> #include <errno.h>
>> #include <timer.h>
>> #include <asm/csr.h>
>> +#include <div64.h>
>>
>> -static u64 riscv_timer_get_count(struct udevice *dev)
>> +static u64 notrace riscv_timer_get_count(struct udevice *dev)
>> {
>> __maybe_unused u32 hi, lo;
>>
>> @@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice *dev)
>> return ((u64)hi << 32) | lo;
>> }
>>
>> +#if CONFIG_IS_ENABLED(RISCV_SMODE)
>> +unsigned long notrace timer_get_us(void) {
>> + u64 ticks;
>> +
>> + ticks = riscv_timer_get_count(NULL);
>> + do_div(ticks, CONFIG_SYS_HZ);
>> + return ticks;
>> +}
>> +#endif
>> +
>> static int riscv_timer_probe(struct udevice *dev) {
>> struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>> diff --git a/drivers/timer/sifive_clint_timer.c
>> b/drivers/timer/sifive_clint_timer.c
>> index 00ce0f08d6..c341f7789b 100644
>> --- a/drivers/timer/sifive_clint_timer.c
>> +++ b/drivers/timer/sifive_clint_timer.c
>> @@ -10,11 +10,12 @@
>> #include <timer.h>
>> #include <asm/io.h>
>> #include <linux/err.h>
>> +#include <div64.h>
>>
>> /* mtime register */
>> #define MTIME_REG(base) ((ulong)(base) + 0xbff8)
>>
>> -static u64 sifive_clint_get_count(struct udevice *dev)
>> +static u64 notrace sifive_clint_get_count(struct udevice *dev)
>> {
>> return readq((void __iomem *)MTIME_REG(dev->priv)); } @@
>> -23,12 +24,28 @@ static const struct timer_ops sifive_clint_ops = {
>> .get_count = sifive_clint_get_count, };
>>
>> +#if CONFIG_IS_ENABLED(RISCV_MMODE)
>> +unsigned long notrace timer_get_us(void) {
>> + u64 ticks;
>> +
>> + /* FIXME: gd->arch.clint should contain valid base address */
>> + if (gd->arch.clint) {
>> + ticks = readq((void __iomem *)MTIME_REG(gd->arch.clint));
>> + do_div(ticks, CONFIG_SYS_HZ);
>> + }
>> +
>> + return ticks;
>> +}
>> +#endif
>> +
>> static int sifive_clint_probe(struct udevice *dev) {
>> dev->priv = dev_read_addr_ptr(dev);
>> if (!dev->priv)
>> return -EINVAL;
>>
>> + gd->arch.clint = dev->priv;
>> return timer_timebase_fallback(dev); }
>>
>> --
>> 2.17.1
More information about the U-Boot
mailing list