Hi Pragnesh > Hi Rick, > > >-----Original Message----- > >From: Rick Chen <rickche...@gmail.com> > >Sent: 09 November 2020 13:44 > >To: Pragnesh Patel <pragnesh.pa...@openfive.com> > >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra > ><atish.pa...@wdc.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>; Simon Glass <s...@chromium.org>; Sean > >Anderson <sean...@gmail.com>; palmerdabb...@google.com; rick > ><r...@andestech.com>; Alan Kao <alan...@andestech.com> > >Subject: Re: [RESEND,PATCH v2 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 > > > >> From: Pragnesh Patel [mailto:pragnesh.pa...@sifive.com] > >> Sent: Thursday, November 05, 2020 7:31 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; Claudiu > >> Beznea; Simon Glass > >> Subject: [RESEND,PATCH v2 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> > >> --- > >> drivers/timer/andes_plmt_timer.c | 16 +++++++++++++++- > >> drivers/timer/riscv_timer.c | 14 +++++++++++++- > >> drivers/timer/sifive_clint_timer.c | 16 +++++++++++++++- > >> 3 files changed, 43 insertions(+), 3 deletions(-) > >> > > > >I verify it fail as below: > > > >U-Boot 2020.10-20532-g0910882 (Nov 09 2020 - 15:51:31 +0800) > > > >DRAM: 1 GiB > >trace: enabled > > > >Do you have any suggestion ? > > On which platform ? > > Do you follow this > https://patchwork.ozlabs.org/project/uboot/cover/20201105113032.26234-1-pragnesh.pa...@sifive.com/ > ? >
I verify on AE350 platform with the following configurations: CONFIG_TRACE=y CONFIG_TRACE_BUFFER_SIZE=0x01000000 CONFIG_TRACE_CALL_DEPTH_LIMIT=15 CONFIG_CMD_TRACE=y make FTRACE=1 ae350_rv64_defconfig make FTRACE=1 After dig in and I found the root cause. You can't use gd->arch.plic to record plmt base. It conflicts with andes plic. Thanks, Rick > > > >Thanks, > >Rick > > > > > >> diff --git a/drivers/timer/andes_plmt_timer.c > >> b/drivers/timer/andes_plmt_timer.c > >> index cec86718c7..9d663e036e 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,25 @@ 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.plic should contain valid base address */ > >> + ticks = andes_plmt_get_count(gd->arch.plic); > > Here andes_plmt_get_count() should be replaced with MTIME_REG() macro. Will > update in v3. > > >> + 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.plic = 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..166655e99d 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,25 @@ 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 */ > >> + ticks = sifive_clint_get_count(gd->arch.clint); > > Here sifive_clint_get_count () should be replaced with MTIME_REG() macro. > Will update in v3. > > >> + 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 > >>