Re: [PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support

[Stefan Roese <sr@denx.de>]

Adding Simon to Cc...

On 30.08.22 14:00, Michael Walle wrote:
> Am 2022-08-30 13:53, schrieb Stefan Roese:
>> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE
>> enabled, like pogo_v4.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> ---
>>  drivers/timer/orion-timer.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
>> index 02ed138642b8..7e920eaeaa40 100644
>> --- a/drivers/timer/orion-timer.c
>> +++ b/drivers/timer/orion-timer.c
>> @@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void)
>>  }
>>  #endif
>>
>> +#if CONFIG_IS_ENABLED(BOOTSTAGE)
>> +ulong timer_get_boot_us(void)
>> +{
>> +    u64 ticks = 0;
>> +    u32 rate = 1;
>> +    u64 us;
>> +    int ret;
>> +
>> +    ret = dm_timer_init();
>> +    if (!ret) {
>> +        /* The timer is available */
>> +        rate = timer_get_rate(gd->timer);
>> +        timer_get_count(gd->timer, &ticks);
>> +    } else {
>> +        return 0;
>> +    }
>> +
>> +    us = (ticks * 1000) / rate;
>> +    return us;
>> +}
>> +#endif
> 
> This is duplicate code in almost all the timer drivers, shouldn't
> this be a (weak) default implementation in timer-uclass.c?

Yes. I was lazy and just copied this function and did not notice, that
even more timer drivers have this function. Of course it makes sense
to not duplicate code here, but have a common function for this. Frankly
I don't even know why exactly this function is needed, as I did not
look into BOOTSTAGE yet.

Simon, do we really need this function? Can't bootstage just use the
"normal" timer functionality instead?

> Also,
> do you need to guard it with CONFIG_IS_ENABLED(BOOTSTAGE), aren't
> unused functions discarded anyway?

Yes, this should be the case. Also just a result of my lazy copy-and-
past.

Thanks,
Stefan