Hi Bin,
>-----Original Message-----
>From: Rick Chen <rickche...@gmail.com>
>Sent: 21 October 2020 08:58
>To: Pragnesh Patel <pragnesh.pa...@openfive.com>
>Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra
><atish.pa...@wdc.com>; Anup Patel <anup.pa...@wdc.com>; Sagar Kadam
><sagar.ka...@openfive.com>; Bin Meng <bmeng...@gmail.com>; Lukas Auer
><lukas.a...@aisec.fraunhofer.de>; Sean Anderson <sean...@gmail.com>; rick
><r...@andestech.com>; Alan Kao <alan...@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...@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.pa...@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.pa...@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.pa...@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
