From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pragnesh Patel Date: Sun, 15 Nov 2020 11:28:00 +0000 Subject: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing In-Reply-To: References: <20201111101435.31455-1-pragnesh.patel@sifive.com> <20201111101435.31455-2-pragnesh.patel@sifive.com> <752D002CFF5D0F4FA35C0100F1D73F3FB2300FF9@ATCPCS16.andestech.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Rick, >-----Original Message----- >From: Rick Chen >Sent: 13 November 2020 13:37 >To: Pragnesh Patel >Cc: U-Boot Mailing List ; Atish Patra >; palmerdabbelt at google.com; Bin Meng >; Paul Walmsley ( Sifive) ; >Anup Patel ; Sagar Kadam >; Sean Anderson ; rick >; Alan Kao ; Leo Liang > >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 >> --- >> >> 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 at 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 >> #include >> #include >> +#include >> >> /* 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 >> #include >> #include >> +#include >> >> -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 >> #include >> #include >> +#include >> >> /* 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