[RESEND,PATCH v2 1/1] riscv: Add timer_get_us() for tracing
Rick Chen
rickchen36 at gmail.com
Tue Nov 10 08:46:43 CET 2020
Hi Pragnesh
> Hi Rick,
>
> >-----Original Message-----
> >From: Rick Chen <rickchen36 at gmail.com>
> >Sent: 09 November 2020 13:44
> >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>; 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>; Simon Glass <sjg at chromium.org>; Sean
> >Anderson <seanga2 at gmail.com>; palmerdabbelt at google.com; rick
> ><rick at andestech.com>; Alan Kao <alankao at 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.patel at sifive.com]
> >> Sent: Thursday, November 05, 2020 7:31 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; 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.patel at 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.patel@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
> >>
More information about the U-Boot
mailing list