Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Pragnesh,
On Mon, Nov 23, 2020 at 06:19:06AM +, Pragnesh Patel wrote:
> Hi Leo,
>
> >-Original Message-
> >From: Leo Liang
> >Sent: 23 November 2020 11:28
> >To: Pragnesh Patel
> >Cc: Rick Chen ; U-Boot Mailing List >b...@lists.denx.de>; Atish Patra ;
> >palmerdabb...@google.com; Bin Meng ; Paul Walmsley
> >( Sifive) ; Anup Patel ; Sagar
> >Kadam ; Sean Anderson ;
> >rick ; Alan Kao
> >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
> >
> >On Tue, Nov 17, 2020 at 08:30:21AM +, Pragnesh Patel wrote:
> >
> >Hi Pragnesh,
> >
> >> Hi Rick,
> >>
> >> >-Original Message-
> >> >From: Rick Chen
> >> >Sent: 13 November 2020 13:37
> >> >To: Pragnesh Patel
> >> >Cc: U-Boot Mailing List ; Atish Patra
> >> >; palmerdabb...@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.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
> >> >> ---
> >> >>
> >> >> 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));
> >> >
> >
> >This patch will cause compile error with ae350 defconfig.
> >
> >${uboot}/drivers/ti
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Leo,
>-Original Message-
>From: Leo Liang
>Sent: 23 November 2020 11:28
>To: Pragnesh Patel
>Cc: Rick Chen ; U-Boot Mailing List b...@lists.denx.de>; Atish Patra ;
>palmerdabb...@google.com; Bin Meng ; Paul Walmsley
>( Sifive) ; Anup Patel ; Sagar
>Kadam ; Sean Anderson ;
>rick ; Alan Kao
>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
>
>On Tue, Nov 17, 2020 at 08:30:21AM +, Pragnesh Patel wrote:
>
>Hi Pragnesh,
>
>> Hi Rick,
>>
>> >-Original Message-
>> >From: Rick Chen
>> >Sent: 13 November 2020 13:37
>> >To: Pragnesh Patel
>> >Cc: U-Boot Mailing List ; Atish Patra
>> >; palmerdabb...@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.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
>> >> ---
>> >>
>> >> 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));
>> >
>
>This patch will cause compile error with ae350 defconfig.
>
>${uboot}/drivers/timer/Makefile is articulated in this way,
>"obj-$(CONFIG_ANDES_PLMT_TIMER) += andes_plmt_timer.o", so the #ifdef
>indicator in ${uboot}/arch/riscv/include/asm/global_data.h should use
>CONFIG_ANDES_PLMT_TIMER.
Thanks Leo but Rick has already informed about this error.
I have discarded this series and added a new patch for Tracing which uses
TIMER_EARLY
https://patchwork.ozlabs.org/project/uboot/patch/20201117110508.25819-1-pragnesh.pa...@sifive.com/
>
>Best regards,
>Leo
>
>> >And it is not proper to have dependency of data s
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On Tue, Nov 17, 2020 at 08:30:21AM +, Pragnesh Patel wrote:
Hi Pragnesh,
> Hi Rick,
>
> >-Original Message-
> >From: Rick Chen
> >Sent: 13 November 2020 13:37
> >To: Pragnesh Patel
> >Cc: U-Boot Mailing List ; Atish Patra
> >; palmerdabb...@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.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
> >> ---
> >>
> >> 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));
> >
This patch will cause compile error with ae350 defconfig.
${uboot}/drivers/timer/Makefile is articulated in this way,
"obj-$(CONFIG_ANDES_PLMT_TIMER) += andes_plmt_timer.o",
so the #ifdef indicator in ${uboot}/arch/riscv/include/asm/global_data.h should
use CONFIG_ANDES_PLMT_TIMER.
Best regards,
Leo
> >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 am planning to use TIMER_EARLY for tracing.
>
> With TIMER_EARLY, I need to implement timer_early_get_rate() and
> timer_early_get_count()
>
> For timer_early_get_count(), I need PLMT or CLINT base address to read mtime
> register.
> So I think it's better to save base address in gd->arch.clint or
> gd->arch.plmt.
>
> Let me know if you have any other idea to save PLM or CLINT base address.
>
> For timer_early_get_rate(), I will add new variable in global data (gd)
> Like, gd->arch.clock_rate; /* Clock rate of timer in Hz */
> This will return Timer frequency in Hz.
>
> Tracing is only useful in U-Boot not in U-Boot SPL so I am doing this for M
> mode U-boot and
> S mode U-Boot.
>
> >I also found that without dm_timer_init() in initf_dm(),
> >andes_plmt_get_count() will be executed ahead of andes_plmt_probe().
>
> If andes_plmt_get_count() exec
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Rick,
>-Original Message-
>From: Rick Chen
>Sent: 13 November 2020 13:37
>To: Pragnesh Patel
>Cc: U-Boot Mailing List ; Atish Patra
>; palmerdabb...@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.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
>> ---
>>
>> 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));
>
>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 am planning to use TIMER_EARLY for tracing.
With TIMER_EARLY, I need to implement timer_early_get_rate() and
timer_early_get_count()
For timer_early_get_count(), I need PLMT or CLINT base address to read mtime
register.
So I think it's better to save base address in gd->arch.clint or gd->arch.plmt.
Let me know if you have any other idea to save PLM or CLINT base address.
For timer_early_get_rate(), I will add new variable in global data (gd)
Like, gd->arch.clock_rate; /* Clock rate of timer in Hz */
This will return Timer frequency in Hz.
Tracing is only useful in U-Boot not in U-Boot SPL so I am doing this for M
mode U-boot and
S mode U-Boot.
>I also found that without dm_timer_init() in initf_dm(),
>andes_plmt_get_count() will be executed ahead of andes_plmt_probe().
If andes_plmt_get_count() executed before andes_plmt_probe() then how did it
will get
PLMT base address ?
>
>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)
>> {
>> ret
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Pragnesh
> Hi Rick,
>
> >-Original Message-
> >From: Rick Chen
> >Sent: 13 November 2020 13:37
> >To: Pragnesh Patel
> >Cc: U-Boot Mailing List ; Atish Patra
> >; palmerdabb...@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.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
> >> ---
> >>
> >> 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
> >> #include
> >> #include
> >> +#include
> >>
> >> /* mtime register */
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Rick,
>-Original Message-
>From: Rick Chen
>Sent: 13 November 2020 13:37
>To: Pragnesh Patel
>Cc: U-Boot Mailing List ; Atish Patra
>; palmerdabb...@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.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
>> ---
>>
>> 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.g...@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 _
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
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
> ---
>
> 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));
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
> + 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)
>
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
>-Original Message- >From: U-Boot On Behalf Of Pragnesh Patel >Sent: 12 November 2020 13:06 >To: Heinrich Schuchardt ; Simon Glass > >Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de; >bmeng...@gmail.com; Paul Walmsley ( Sifive) ; >anup.pa...@wdc.com; Sagar Kadam ; >r...@andestech.com; Sean Anderson >Subject: RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing > >Hi Heinrich, > >>-Original Message- >>From: Heinrich Schuchardt >>Sent: 12 November 2020 12:49 >>To: Pragnesh Patel ; Simon Glass >> >>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; >>u-boot@lists.denx.de; bmeng...@gmail.com; Paul Walmsley ( Sifive) >>; anup.pa...@wdc.com; Sagar Kadam >>; r...@andestech.com; Sean Anderson >> >>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 >> >>On 11/12/20 8:09 AM, Heinrich Schuchardt wrote: >>> On 11/12/20 7:34 AM, Pragnesh Patel wrote: >>>> Hi Heinrich, >>>> >>>>> -Original Message- >>>>> From: Heinrich Schuchardt >>>>> Sent: 11 November 2020 19:18 >>>>> To: Pragnesh Patel >>>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; >>>>> u-boot@lists.denx.de; bmeng...@gmail.com; Paul Walmsley ( Sifive) >>>>> ; anup.pa...@wdc.com; Sagar Kadam >>>>> ; r...@andestech.com; Sean Anderson >>>>> ; Simon Glass >>>>> 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 >>>>> >>>>> On 11/11/20 12:56 PM, Pragnesh Patel wrote: >>>>>> Hi Heinrich, >>>>>> >>>>>>> -Original Message- >>>>>>> From: Heinrich Schuchardt >>>>>>> Sent: 11 November 2020 16:51 >>>>>>> To: Pragnesh Patel ; >>>>>>> u-boot@lists.denx.de >>>>>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; >>>>>>> bmeng...@gmail.com; Paul Walmsley ( Sifive) >>>>>>> ; anup.pa...@wdc.com; Sagar Kadam >>>>>>> ; r...@andestech.com; Sean Anderson >>>>>>> ; Simon Glass >>>>>>> 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 >>>>>>> >>>>>>> On 11.11.20 11:14, Pragnesh Patel wrote: >>>>>>>> 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 >>>>>>> >>>>>>> The default implementation of get_timer_us() in lib/time.c calls >>>>>>> get_ticks() which calls timer_get_count(). The get_count() >>>>>>> operation is implemented in drivers/timer/andes_plmt_timer.c, >>>>>>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c. >>>>>>> >>>>>>> Why do you need special timer_get_us() implementations? >>>>>>> Isn't it enough to supply the get_count() operation in the drivers? >>>>>> >>>>>> get_ticks() is depend on gd->timer and there are 2 cases >>>>>> >>>>>> 1) if gd->timer== NULL then dm_timer_init() gets called and it >>>>>> will call functions which are not marked with "notrace" so tracing got >stuck. >>>>> >>>>> Thanks for the background information. >>>>> >>>>> Please, identify the existing functions that lack "notrace" and fix >>>>> them. This will give us a solution for all existing boards and not >>>>> for a small >>selection. >>>>> Furthermore it will avoid code duplication. >>>> >>>> I tried but There are so many functions which need "notrace". >>> >>> Let's start with
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Heinrich, >-Original Message- >From: Heinrich Schuchardt >Sent: 12 November 2020 12:49 >To: Pragnesh Patel ; Simon Glass > >Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de; >bmeng...@gmail.com; Paul Walmsley ( Sifive) ; >anup.pa...@wdc.com; Sagar Kadam ; >r...@andestech.com; Sean Anderson >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 > >On 11/12/20 8:09 AM, Heinrich Schuchardt wrote: >> On 11/12/20 7:34 AM, Pragnesh Patel wrote: >>> Hi Heinrich, >>> >>>> -Original Message- >>>> From: Heinrich Schuchardt >>>> Sent: 11 November 2020 19:18 >>>> To: Pragnesh Patel >>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; >>>> u-boot@lists.denx.de; bmeng...@gmail.com; Paul Walmsley ( Sifive) >>>> ; anup.pa...@wdc.com; Sagar Kadam >>>> ; r...@andestech.com; Sean Anderson >>>> ; Simon Glass >>>> 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 >>>> >>>> On 11/11/20 12:56 PM, Pragnesh Patel wrote: >>>>> Hi Heinrich, >>>>> >>>>>> -Original Message- >>>>>> From: Heinrich Schuchardt >>>>>> Sent: 11 November 2020 16:51 >>>>>> To: Pragnesh Patel ; >>>>>> u-boot@lists.denx.de >>>>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; >>>>>> bmeng...@gmail.com; Paul Walmsley ( Sifive) >>>>>> ; anup.pa...@wdc.com; Sagar Kadam >>>>>> ; r...@andestech.com; Sean Anderson >>>>>> ; Simon Glass >>>>>> 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 >>>>>> >>>>>> On 11.11.20 11:14, Pragnesh Patel wrote: >>>>>>> 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 >>>>>> >>>>>> The default implementation of get_timer_us() in lib/time.c calls >>>>>> get_ticks() which calls timer_get_count(). The get_count() >>>>>> operation is implemented in drivers/timer/andes_plmt_timer.c, >>>>>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c. >>>>>> >>>>>> Why do you need special timer_get_us() implementations? >>>>>> Isn't it enough to supply the get_count() operation in the drivers? >>>>> >>>>> get_ticks() is depend on gd->timer and there are 2 cases >>>>> >>>>> 1) if gd->timer== NULL then dm_timer_init() gets called and it will >>>>> call functions which are not marked with "notrace" so tracing got stuck. >>>> >>>> Thanks for the background information. >>>> >>>> Please, identify the existing functions that lack "notrace" and fix >>>> them. This will give us a solution for all existing boards and not for a >>>> small >selection. >>>> Furthermore it will avoid code duplication. >>> >>> I tried but There are so many functions which need "notrace". >> >> Let's start with the problem statement: >> >> When tracing functions is enabled this adds calls to >> __cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced >> functions. >> >> __cyg_profile_func_exit() and __cyg_profile_func_exit() invoke >> timer_get_us() to record the entry and exit time. >> >> If timer_get_us() or any function used to implement does not carry >> __attribute__((no_instrument_function)) this will lead to an >> indefinite recursion. >> >> We have to change __cyg_profile_func_enter() and >> __cyg_profile_func_exit() such that during their execution no function >> is traced. We can do so by temporarily setting trace_enabled to false. >> >> Does this match
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11/12/20 8:09 AM, Heinrich Schuchardt wrote:
> On 11/12/20 7:34 AM, Pragnesh Patel wrote:
>> Hi Heinrich,
>>
>>> -Original Message-
>>> From: Heinrich Schuchardt
>>> Sent: 11 November 2020 19:18
>>> To: Pragnesh Patel
>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>>> bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>>> anup.pa...@wdc.com; Sagar Kadam ;
>>> r...@andestech.com; Sean Anderson ; Simon Glass
>>>
>>> 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
>>>
>>> On 11/11/20 12:56 PM, Pragnesh Patel wrote:
>>>> Hi Heinrich,
>>>>
>>>>> -Original Message-
>>>>> From: Heinrich Schuchardt
>>>>> Sent: 11 November 2020 16:51
>>>>> To: Pragnesh Patel ;
>>>>> u-boot@lists.denx.de
>>>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>>>>> bmeng...@gmail.com; Paul Walmsley ( Sifive)
>>>>> ; anup.pa...@wdc.com; Sagar Kadam
>>>>> ; r...@andestech.com; Sean Anderson
>>>>> ; Simon Glass
>>>>> 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
>>>>>
>>>>> On 11.11.20 11:14, Pragnesh Patel wrote:
>>>>>> 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
>>>>>
>>>>> The default implementation of get_timer_us() in lib/time.c calls
>>>>> get_ticks() which calls timer_get_count(). The get_count() operation
>>>>> is implemented in drivers/timer/andes_plmt_timer.c,
>>>>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>>>>>
>>>>> Why do you need special timer_get_us() implementations?
>>>>> Isn't it enough to supply the get_count() operation in the drivers?
>>>>
>>>> get_ticks() is depend on gd->timer and there are 2 cases
>>>>
>>>> 1) if gd->timer== NULL then dm_timer_init() gets called and it will
>>>> call functions which are not marked with "notrace" so tracing got stuck.
>>>
>>> Thanks for the background information.
>>>
>>> Please, identify the existing functions that lack "notrace" and fix them.
>>> This will
>>> give us a solution for all existing boards and not for a small selection.
>>> Furthermore it will avoid code duplication.
>>
>> I tried but There are so many functions which need "notrace".
>
> Let's start with the problem statement:
>
> When tracing functions is enabled this adds calls to
> __cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
> functions.
>
> __cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
> timer_get_us() to record the entry and exit time.
>
> If timer_get_us() or any function used to implement does not carry
> __attribute__((no_instrument_function)) this will lead to an indefinite
> recursion.
>
> We have to change __cyg_profile_func_enter() and
> __cyg_profile_func_exit() such that during their execution no function
> is traced. We can do so by temporarily setting trace_enabled to false.
>
> Does this match your observation?
I just tried to put this into a patch
[PATCH 1/1] trace: avoid infinite recursion
https://lists.denx.de/pipermail/u-boot/2020-November/432589.html
https://patchwork.ozlabs.org/project/uboot/patch/20201112071411.4202-1-xypron.g...@gmx.de/
Does this solve you problem?
Best regards
Heinrich
>
>>
>> Why don’t we consider removing gd->timer=NULL in initr_dm()
>> initr_dm()
>> {
>> #ifdef CONFIG_TIMER
>> gd->timer = NULL;
>> #endif
>> }
>>
>> Or I can use TIMER_EARLY and return ticks and rate by adding
>> timer_early_get_count()
>> and timer_early_get_rate() repectively.
>>
>> Suggestions are welcome
>>
>>>
>>> In lib/trace.c consider replacing
>>> "__attribute__((no_instrument_function))" by "notrace&quo
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11/12/20 7:34 AM, Pragnesh Patel wrote:
> Hi Heinrich,
>
>> -Original Message-
>> From: Heinrich Schuchardt
>> Sent: 11 November 2020 19:18
>> To: Pragnesh Patel
>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>> bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>> anup.pa...@wdc.com; Sagar Kadam ;
>> r...@andestech.com; Sean Anderson ; Simon Glass
>>
>> 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
>>
>> On 11/11/20 12:56 PM, Pragnesh Patel wrote:
>>> Hi Heinrich,
>>>
>>>> -Original Message-
>>>> From: Heinrich Schuchardt
>>>> Sent: 11 November 2020 16:51
>>>> To: Pragnesh Patel ;
>>>> u-boot@lists.denx.de
>>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>>>> bmeng...@gmail.com; Paul Walmsley ( Sifive)
>>>> ; anup.pa...@wdc.com; Sagar Kadam
>>>> ; r...@andestech.com; Sean Anderson
>>>> ; Simon Glass
>>>> 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
>>>>
>>>> On 11.11.20 11:14, Pragnesh Patel wrote:
>>>>> 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
>>>>
>>>> The default implementation of get_timer_us() in lib/time.c calls
>>>> get_ticks() which calls timer_get_count(). The get_count() operation
>>>> is implemented in drivers/timer/andes_plmt_timer.c,
>>>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>>>>
>>>> Why do you need special timer_get_us() implementations?
>>>> Isn't it enough to supply the get_count() operation in the drivers?
>>>
>>> get_ticks() is depend on gd->timer and there are 2 cases
>>>
>>> 1) if gd->timer== NULL then dm_timer_init() gets called and it will
>>> call functions which are not marked with "notrace" so tracing got stuck.
>>
>> Thanks for the background information.
>>
>> Please, identify the existing functions that lack "notrace" and fix them.
>> This will
>> give us a solution for all existing boards and not for a small selection.
>> Furthermore it will avoid code duplication.
>
> I tried but There are so many functions which need "notrace".
Let's start with the problem statement:
When tracing functions is enabled this adds calls to
__cyg_profile_func_enter() and __cyg_profile_func_exit() to the traced
functions.
__cyg_profile_func_exit() and __cyg_profile_func_exit() invoke
timer_get_us() to record the entry and exit time.
If timer_get_us() or any function used to implement does not carry
__attribute__((no_instrument_function)) this will lead to an indefinite
recursion.
We have to change __cyg_profile_func_enter() and
__cyg_profile_func_exit() such that during their execution no function
is traced. We can do so by temporarily setting trace_enabled to false.
Does this match your observation?
Best regards
Heinrich
>
> Why don’t we consider removing gd->timer=NULL in initr_dm()
> initr_dm()
> {
> #ifdef CONFIG_TIMER
> gd->timer = NULL;
> #endif
> }
>
> Or I can use TIMER_EARLY and return ticks and rate by adding
> timer_early_get_count()
> and timer_early_get_rate() repectively.
>
> Suggestions are welcome
>
>>
>> In lib/trace.c consider replacing
>> "__attribute__((no_instrument_function))" by "notrace".
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> 2) if gd->timer is already initialized then still initr_dm() will make
>>> gd->timer = NULL;
>>>
>>> initr_dm()
>>> {
>>> #ifdef CONFIG_TIMER
>>> gd->timer = NULL;
>>> #endif
>>> }
>>>
>>> So again dm_timer_init() gets called and tracing got stuck.
>>>
>>> So I thought better to implement timer_get_us().
>>>
>>>>
>>>> Best regards
>>>>
>>>> Heinrich
>>>>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>&g
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Heinrich,
>-Original Message-
>From: Heinrich Schuchardt
>Sent: 11 November 2020 19:18
>To: Pragnesh Patel
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; u-boot@lists.denx.de;
>bmeng...@gmail.com; Paul Walmsley ( Sifive) ;
>anup.pa...@wdc.com; Sagar Kadam ;
>r...@andestech.com; Sean Anderson ; Simon Glass
>
>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
>
>On 11/11/20 12:56 PM, Pragnesh Patel wrote:
>> Hi Heinrich,
>>
>>> -Original Message-
>>> From: Heinrich Schuchardt
>>> Sent: 11 November 2020 16:51
>>> To: Pragnesh Patel ;
>>> u-boot@lists.denx.de
>>> Cc: atish.pa...@wdc.com; palmerdabb...@google.com;
>>> bmeng...@gmail.com; Paul Walmsley ( Sifive)
>>> ; anup.pa...@wdc.com; Sagar Kadam
>>> ; r...@andestech.com; Sean Anderson
>>> ; Simon Glass
>>> 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
>>>
>>> On 11.11.20 11:14, Pragnesh Patel wrote:
>>>> 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
>>>
>>> The default implementation of get_timer_us() in lib/time.c calls
>>> get_ticks() which calls timer_get_count(). The get_count() operation
>>> is implemented in drivers/timer/andes_plmt_timer.c,
>>> drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>>>
>>> Why do you need special timer_get_us() implementations?
>>> Isn't it enough to supply the get_count() operation in the drivers?
>>
>> get_ticks() is depend on gd->timer and there are 2 cases
>>
>> 1) if gd->timer== NULL then dm_timer_init() gets called and it will
>> call functions which are not marked with "notrace" so tracing got stuck.
>
>Thanks for the background information.
>
>Please, identify the existing functions that lack "notrace" and fix them. This
>will
>give us a solution for all existing boards and not for a small selection.
>Furthermore it will avoid code duplication.
I tried but There are so many functions which need "notrace".
Why don’t we consider removing gd->timer=NULL in initr_dm()
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
Or I can use TIMER_EARLY and return ticks and rate by adding
timer_early_get_count()
and timer_early_get_rate() repectively.
Suggestions are welcome
>
>In lib/trace.c consider replacing
>"__attribute__((no_instrument_function))" by "notrace".
>
>Best regards
>
>Heinrich
>
>>
>> 2) if gd->timer is already initialized then still initr_dm() will make
>> gd->timer = NULL;
>>
>> initr_dm()
>> {
>> #ifdef CONFIG_TIMER
>> gd->timer = NULL;
>> #endif
>> }
>>
>> So again dm_timer_init() gets called and tracing got stuck.
>>
>> So I thought better to implement timer_get_us().
>>
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>> ---
>>>>
>>>> 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
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11/11/20 12:56 PM, Pragnesh Patel wrote:
Hi Heinrich,
-Original Message-
From: Heinrich Schuchardt
Sent: 11 November 2020 16:51
To: Pragnesh Patel ; u-boot@lists.denx.de
Cc: atish.pa...@wdc.com; palmerdabb...@google.com; bmeng...@gmail.com;
Paul Walmsley ( Sifive) ; anup.pa...@wdc.com;
Sagar Kadam ; r...@andestech.com; Sean
Anderson ; Simon Glass
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
On 11.11.20 11:14, Pragnesh Patel wrote:
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
The default implementation of get_timer_us() in lib/time.c calls
get_ticks() which calls timer_get_count(). The get_count() operation is
implemented in drivers/timer/andes_plmt_timer.c,
drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
Why do you need special timer_get_us() implementations?
Isn't it enough to supply the get_count() operation in the drivers?
get_ticks() is depend on gd->timer and there are 2 cases
1) if gd->timer== NULL then dm_timer_init() gets called and it will call
functions
which are not marked with "notrace" so tracing got stuck.
Thanks for the background information.
Please, identify the existing functions that lack "notrace" and fix
them. This will give us a solution for all existing boards and not for a
small selection. Furthermore it will avoid code duplication.
In lib/trace.c consider replacing
"__attribute__((no_instrument_function))" by "notrace".
Best regards
Heinrich
2) if gd->timer is already initialized then still initr_dm() will make
gd->timer = NULL;
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
So again dm_timer_init() gets called and tracing got stuck.
So I thought better to implement timer_get_us().
Best regards
Heinrich
---
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
+ 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
RE: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
Hi Heinrich,
>-Original Message-
>From: Heinrich Schuchardt
>Sent: 11 November 2020 16:51
>To: Pragnesh Patel ; u-boot@lists.denx.de
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; bmeng...@gmail.com;
>Paul Walmsley ( Sifive) ; anup.pa...@wdc.com;
>Sagar Kadam ; r...@andestech.com; Sean
>Anderson ; Simon Glass
>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
>
>On 11.11.20 11:14, Pragnesh Patel wrote:
>> 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
>
>The default implementation of get_timer_us() in lib/time.c calls
>get_ticks() which calls timer_get_count(). The get_count() operation is
>implemented in drivers/timer/andes_plmt_timer.c,
>drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
>
>Why do you need special timer_get_us() implementations?
>Isn't it enough to supply the get_count() operation in the drivers?
get_ticks() is depend on gd->timer and there are 2 cases
1) if gd->timer== NULL then dm_timer_init() gets called and it will call
functions
which are not marked with "notrace" so tracing got stuck.
2) if gd->timer is already initialized then still initr_dm() will make
gd->timer = NULL;
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
So again dm_timer_init() gets called and tracing got stuck.
So I thought better to implement timer_get_us().
>
>Best regards
>
>Heinrich
>
>> ---
>>
>> 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
>> + 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 @@
>>
Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
On 11.11.20 11:14, Pragnesh Patel wrote:
> 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
The default implementation of get_timer_us() in lib/time.c calls
get_ticks() which calls timer_get_count(). The get_count() operation is
implemented in drivers/timer/andes_plmt_timer.c,
drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
Why do you need special timer_get_us() implementations?
Isn't it enough to supply the get_count() operation in the drivers?
Best regards
Heinrich
> ---
>
> 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
> + 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));
> +
[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
+ 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
