Hi Pragnesh
> Hi Rick,
>
> >-----Original Message-----
> >From: Rick Chen <rickche...@gmail.com>
> >Sent: 13 November 2020 13:37
> >To: Pragnesh Patel <pragnesh.pa...@openfive.com>
> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra
> ><atish.pa...@wdc.com>; palmerdabb...@google.com; Bin Meng
> ><bmeng...@gmail.com>; Paul Walmsley ( Sifive) <paul.walms...@sifive.com>;
> >Anup Patel <anup.pa...@wdc.com>; Sagar Kadam
> ><sagar.ka...@openfive.com>; Sean Anderson <sean...@gmail.com>; rick
> ><r...@andestech.com>; Alan Kao <alan...@andestech.com>; Leo Liang
> ><ycli...@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.pa...@sifive.com]
> >> Sent: Wednesday, November 11, 2020 6:15 PM
> >> To: u-boot@lists.denx.de
> >> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
> >bmeng...@gmail.com;
> >> paul.walms...@sifive.com; anup.pa...@wdc.com; sagar.ka...@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.pa...@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:
Sorry, it's shall be CONFIG_ANDES_PLMT_TIMER here // because of
obj-$(CONFIG_ANDES_PLMT_TIMER) += andes_plmt_timer.o
Or it will have a compiling error.
> >
> >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.g...@gmx.de/
OK
Thanks,
Rick
>
> >
> >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
