| Message ID | 20200824144429.12641-2-pragnesh.patel@sifive.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | RISC-V tracing support | expand |
Hi Pragnesh, On Mon, Aug 24, 2020 at 10:45 PM Pragnesh Patel <pragnesh.patel@sifive.com> wrote: > > timer_get_us() will use timer_ops->get_count() function for timer counter. > For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer counter and > For M-mode U-Boot, mtime register will provide the same. > > Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> > --- > arch/riscv/lib/Makefile | 1 + > arch/riscv/lib/timer.c | 50 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > create mode 100644 arch/riscv/lib/timer.c > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > index 10ac5b06d3..fbb68e583b 100644 > --- a/arch/riscv/lib/Makefile > +++ b/arch/riscv/lib/Makefile > @@ -26,6 +26,7 @@ obj-y += setjmp.o > obj-$(CONFIG_$(SPL_)SMP) += smp.o > obj-$(CONFIG_SPL_BUILD) += spl.o > obj-y += fdt_fixup.o > +obj-$(CONFIG_TIMER) += timer.o > > # For building EFI apps > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > diff --git a/arch/riscv/lib/timer.c b/arch/riscv/lib/timer.c > new file mode 100644 > index 0000000000..3e423f2805 > --- /dev/null > +++ b/arch/riscv/lib/timer.c > @@ -0,0 +1,50 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2020 SiFive, Inc > + * > + * Authors: > + * Pragnesh Patel <pragnesh.patel@sifive.com> > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <timer.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +static struct udevice *timer; > + > +ulong notrace timer_get_us(void) Does the weak one in lib/time.c not work on RISC-V? > +{ > + u64 count; > + ulong rate; > + int ret; > + > + /** > + * gd->timer will become NULL in initr_dm(), so assign gd->timer > + * to other static global timer, so that timer_get_us() can use it. > + */ > + if (!timer && gd->timer) > + timer = (struct udevice *)gd->timer; > + > + if (timer) { > + ret = timer_get_count(timer, &count); > + if (ret) > + return ret; > + > + rate = timer_get_rate(timer); > + } > + > + return (ulong)count / rate; > +} > + > +int timer_init(void) Why is this function necessary? > +{ > + int ret; > + > + ret = dm_timer_init(); Does enabling CONFIG_TIMER_EARLY help? > + if (ret) > + return ret; > + > + return 0; > +} Regards, Bin
Hi Pragnesh > From: Bin Meng [mailto:bmeng.cn@gmail.com] > Sent: Friday, September 11, 2020 2:48 PM > To: Pragnesh Patel > Cc: U-Boot Mailing List; Atish Patra; Anup Patel; Sagar Kadam; Rick Jian-Zhi Chen(陳建志); Paul Walmsley; Bin Meng; Lukas Auer; Sean Anderson > Subject: Re: [PATCH 1/3] riscv: Add timer_get_us() for tracing > > Hi Pragnesh, > > On Mon, Aug 24, 2020 at 10:45 PM Pragnesh Patel > <pragnesh.patel@sifive.com> wrote: > > > > timer_get_us() will use timer_ops->get_count() function for timer counter. > > For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer counter and > > For M-mode U-Boot, mtime register will provide the same. > > > > Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> > > --- > > arch/riscv/lib/Makefile | 1 + > > arch/riscv/lib/timer.c | 50 +++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+) > > create mode 100644 arch/riscv/lib/timer.c > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > index 10ac5b06d3..fbb68e583b 100644 > > --- a/arch/riscv/lib/Makefile > > +++ b/arch/riscv/lib/Makefile > > @@ -26,6 +26,7 @@ obj-y += setjmp.o > > obj-$(CONFIG_$(SPL_)SMP) += smp.o > > obj-$(CONFIG_SPL_BUILD) += spl.o > > obj-y += fdt_fixup.o > > +obj-$(CONFIG_TIMER) += timer.o > > > > # For building EFI apps > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > diff --git a/arch/riscv/lib/timer.c b/arch/riscv/lib/timer.c > > new file mode 100644 > > index 0000000000..3e423f2805 > > --- /dev/null > > +++ b/arch/riscv/lib/timer.c > > @@ -0,0 +1,50 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2020 SiFive, Inc > > + * > > + * Authors: > > + * Pragnesh Patel <pragnesh.patel@sifive.com> > > + */ > > + > > +#include <common.h> > > +#include <dm.h> > > +#include <timer.h> > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +static struct udevice *timer; > > + > > +ulong notrace timer_get_us(void) > > Does the weak one in lib/time.c not work on RISC-V? Do you have any comments about Bin's reply ? Thanks, Rick > > > +{ > > + u64 count; > > + ulong rate; > > + int ret; > > + > > + /** > > + * gd->timer will become NULL in initr_dm(), so assign gd->timer > > + * to other static global timer, so that timer_get_us() can use it. > > + */ > > + if (!timer && gd->timer) > > + timer = (struct udevice *)gd->timer; > > + > > + if (timer) { > > + ret = timer_get_count(timer, &count); > > + if (ret) > > + return ret; > > + > > + rate = timer_get_rate(timer); > > + } > > + > > + return (ulong)count / rate; > > +} > > + > > +int timer_init(void) > > Why is this function necessary? > > > +{ > > + int ret; > > + > > + ret = dm_timer_init(); > > Does enabling CONFIG_TIMER_EARLY help? > > > + if (ret) > > + return ret; > > + > > + return 0; > > +} > > Regards, > Bin
Hi Bin, >-----Original Message----- >From: Rick Chen <rickchen36@gmail.com> >Sent: 21 October 2020 08:58 >To: Pragnesh Patel <pragnesh.patel@openfive.com> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra ><atish.patra@wdc.com>; Anup Patel <anup.patel@wdc.com>; Sagar Kadam ><sagar.kadam@openfive.com>; Bin Meng <bmeng.cn@gmail.com>; Lukas Auer ><lukas.auer@aisec.fraunhofer.de>; Sean Anderson <seanga2@gmail.com>; rick ><rick@andestech.com>; Alan Kao <alankao@andestech.com> >Subject: Re: [PATCH 1/3] 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: Bin Meng [mailto:bmeng.cn@gmail.com] >> Sent: Friday, September 11, 2020 2:48 PM >> To: Pragnesh Patel >> Cc: U-Boot Mailing List; Atish Patra; Anup Patel; Sagar Kadam; Rick >> Jian-Zhi Chen(陳建志); Paul Walmsley; Bin Meng; Lukas Auer; Sean Anderson >> Subject: Re: [PATCH 1/3] riscv: Add timer_get_us() for tracing >> >> Hi Pragnesh, >> >> On Mon, Aug 24, 2020 at 10:45 PM Pragnesh Patel >> <pragnesh.patel@sifive.com> wrote: >> > >> > timer_get_us() will use timer_ops->get_count() function for timer counter. >> > For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer >> > counter and For M-mode U-Boot, mtime register will provide the same. >> > >> > Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> >> > --- >> > arch/riscv/lib/Makefile | 1 + >> > arch/riscv/lib/timer.c | 50 >> > +++++++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 51 insertions(+) >> > create mode 100644 arch/riscv/lib/timer.c >> > >> > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index >> > 10ac5b06d3..fbb68e583b 100644 >> > --- a/arch/riscv/lib/Makefile >> > +++ b/arch/riscv/lib/Makefile >> > @@ -26,6 +26,7 @@ obj-y += setjmp.o >> > obj-$(CONFIG_$(SPL_)SMP) += smp.o >> > obj-$(CONFIG_SPL_BUILD) += spl.o >> > obj-y += fdt_fixup.o >> > +obj-$(CONFIG_TIMER) += timer.o >> > >> > # For building EFI apps >> > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git >> > a/arch/riscv/lib/timer.c b/arch/riscv/lib/timer.c new file mode >> > 100644 index 0000000000..3e423f2805 >> > --- /dev/null >> > +++ b/arch/riscv/lib/timer.c >> > @@ -0,0 +1,50 @@ >> > +// SPDX-License-Identifier: GPL-2.0+ >> > +/* >> > + * Copyright (C) 2020 SiFive, Inc >> > + * >> > + * Authors: >> > + * Pragnesh Patel <pragnesh.patel@sifive.com> >> > + */ >> > + >> > +#include <common.h> >> > +#include <dm.h> >> > +#include <timer.h> >> > + >> > +DECLARE_GLOBAL_DATA_PTR; >> > + >> > +static struct udevice *timer; >> > + >> > +ulong notrace timer_get_us(void) >> >> Does the weak one in lib/time.c not work on RISC-V? No because if "gd->timer" is set early then also it will become NULL in initr_dm() static int initr_dm(void) { ... #ifdef CONFIG_TIMER gd->timer = NULL; #endif ... } So timer_get_us() again try to call dm_timer_init() to initialize "gd->timer" and it got stuck in tracing. Not all the functins are marked with notrace in dm_timer_init(). > >Do you have any comments about Bin's reply ? > >Thanks, >Rick > >> >> > +{ >> > + u64 count; >> > + ulong rate; >> > + int ret; >> > + >> > + /** >> > + * gd->timer will become NULL in initr_dm(), so assign gd->timer >> > + * to other static global timer, so that timer_get_us() can use it. >> > + */ >> > + if (!timer && gd->timer) >> > + timer = (struct udevice *)gd->timer; >> > + >> > + if (timer) { >> > + ret = timer_get_count(timer, &count); >> > + if (ret) >> > + return ret; >> > + >> > + rate = timer_get_rate(timer); >> > + } >> > + >> > + return (ulong)count / rate; >> > +} >> > + >> > +int timer_init(void) >> >> Why is this function necessary? >> >> > +{ >> > + int ret; >> > + >> > + ret = dm_timer_init(); >> >> Does enabling CONFIG_TIMER_EARLY help? I need to implement timer_early_get_count() and timer_early_get_rate() for that. Will look into this >> >> > + if (ret) >> > + return ret; >> > + >> > + return 0; >> > +} >> >> Regards, >> Bin
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index 10ac5b06d3..fbb68e583b 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -26,6 +26,7 @@ obj-y += setjmp.o obj-$(CONFIG_$(SPL_)SMP) += smp.o obj-$(CONFIG_SPL_BUILD) += spl.o obj-y += fdt_fixup.o +obj-$(CONFIG_TIMER) += timer.o # For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/timer.c b/arch/riscv/lib/timer.c new file mode 100644 index 0000000000..3e423f2805 --- /dev/null +++ b/arch/riscv/lib/timer.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2020 SiFive, Inc + * + * Authors: + * Pragnesh Patel <pragnesh.patel@sifive.com> + */ + +#include <common.h> +#include <dm.h> +#include <timer.h> + +DECLARE_GLOBAL_DATA_PTR; + +static struct udevice *timer; + +ulong notrace timer_get_us(void) +{ + u64 count; + ulong rate; + int ret; + + /** + * gd->timer will become NULL in initr_dm(), so assign gd->timer + * to other static global timer, so that timer_get_us() can use it. + */ + if (!timer && gd->timer) + timer = (struct udevice *)gd->timer; + + if (timer) { + ret = timer_get_count(timer, &count); + if (ret) + return ret; + + rate = timer_get_rate(timer); + } + + return (ulong)count / rate; +} + +int timer_init(void) +{ + int ret; + + ret = dm_timer_init(); + if (ret) + return ret; + + return 0; +}
timer_get_us() will use timer_ops->get_count() function for timer counter. For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer counter and For M-mode U-Boot, mtime register will provide the same. Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> --- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/timer.c | 50 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 arch/riscv/lib/timer.c