Hi,
Am 01.09.2022 um 11:27 schrieb Stefan Roese:
Hi Tony,
On 01.09.22 09:39, Tony Dinh wrote:
<snip>
Some ideas.
The get_timer() function looks wrong assigning an uint64_t to ulong.
lib/time.c
static uint64_t notrace tick_to_time(uint64_t tick)
uint64_t notrace get_ticks(void)
uint64_t __weak notrace get_ticks(void)
ulong __weak get_timer(ulong base)
{
return tick_to_time(get_ticks()) - base;
}
Most of the timer infrastructure is using uint64_t. I'm seeing this
__weak function get_timer was invoked in Kirkwood boards. Both in
sleep and timer commands.
The get_ticks() thing can run at 1MHz but the timer is 1KHz, so that
is why we don't need a u64 for the timer.
Thanks for your explanation! However, would you agree that code is
problematic and needed some improvement ? IOW, depending what the
compiler does, it might return the 1st 32 bit of the 64-bit integer
result?
It will return the lower 32 bits if the system is 32bit, yes.
To check if we have a problem here, please add this (totally untested)
code and extend it if it makes sense:
diff --git a/lib/time.c b/lib/time.c
index bbf191f67323..ef5252419f3b 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -146,7 +146,15 @@ int __weak timer_init(void)
/* Returns time in milliseconds */
ulong __weak get_timer(ulong base)
{
- return tick_to_time(get_ticks()) - base;
+ u64 ticks = get_ticks();
+ u64 time_ms = tick_to_time(ticks);
+
+ if (time_ms & 0xffffffff00000000ULL)
+ printf("ticks=%lld time_ms=%lld\n", ticks, time_ms);
+ if ((time_ms - base) & 0xffffffff80000000ULL)
+ printf("ticks=%lld time_ms=%lld base=%ld ret=%lld\n",
ticks, time_ms, base, time_ms - base);
+
+ return time_ms - base;
}
At least here, you seem to have a wrap around with the 32bits AFAICT:
GoFlexHome> sleep 20.5
do_sleep got a timer start = 15031
do_sleep delay = 20000
do_sleep delay = 20500
do_sleep sleeping...
do_sleep start 15031 current 100
<snip>
do_sleep start 15031 current 6400
do_sleep end of sleep ... current = 4294952265
*** Something strange happened here. current should be 6500, but it
seems to have garbage. So the loop exits prematurely.
4294952265 = 0xFFFFC549!
Does the driver use a 32 bit counter without the timer_conv_64 function
inside the get_count function?
Regards
Stefan
