| Message ID | 20140713111427.GA20845@amd.pavel.ucw.cz |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On Sunday, July 13, 2014 at 01:14:27 PM, Pavel Machek wrote: [...] > @@ -51,9 +52,10 @@ unsigned long long __weak notrace get_ticks(void) > return ((unsigned long long)gd->timebase_h << 32) | gd->timebase_l; > } > > -static unsigned long long notrace tick_to_time(uint64_t tick) > +/* Returns time in milliseconds */ Please see http://www.denx.de/wiki/U-Boot/CodingStyle at the bottom in case you want to add documentation annotations. [...] Best regards, Marek Vasut
On Sun 2014-07-13 15:26:18, Marek Vasut wrote: > On Sunday, July 13, 2014 at 01:14:27 PM, Pavel Machek wrote: > [...] > > @@ -51,9 +52,10 @@ unsigned long long __weak notrace get_ticks(void) > > return ((unsigned long long)gd->timebase_h << 32) | gd->timebase_l; > > } > > > > -static unsigned long long notrace tick_to_time(uint64_t tick) > > +/* Returns time in milliseconds */ > > Please see http://www.denx.de/wiki/U-Boot/CodingStyle at the bottom in case you > want to add documentation annotations. > [...] Well, I'd like to document return values, but don't think full kerneldoc is not neccessary. That should not be against CodingStyle AFAICT. Thanks, Pavel
On Sun, Jul 13, 2014 at 01:14:27PM +0200, Pavel Machek wrote: > As I initially suspected overflow in time handling, I took a detailed > look at lib/time.c. This adds comments about units being used, reduces > amount of type casting being done, and makes __udelay() always wait at > least one tick. (Current code could do no delaying at all for short > delays). > > Signed-off-by: Pavel Machek <pavel@denx.de> Applied to u-boot/master, thanks!
diff --git a/lib/time.c b/lib/time.c index 73c3b6a..c7b0264 100644 --- a/lib/time.c +++ b/lib/time.c @@ -15,12 +15,13 @@ #endif #ifndef CONFIG_WD_PERIOD -# define CONFIG_WD_PERIOD (10 * 1000 * 1000) /* 10 seconds default*/ +# define CONFIG_WD_PERIOD (10 * 1000 * 1000) /* 10 seconds default */ #endif DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_SYS_TIMER_RATE +/* Returns tick rate in ticks per second */ ulong notrace get_tbclk(void) { return CONFIG_SYS_TIMER_RATE; @@ -51,9 +52,10 @@ unsigned long long __weak notrace get_ticks(void) return ((unsigned long long)gd->timebase_h << 32) | gd->timebase_l; } -static unsigned long long notrace tick_to_time(uint64_t tick) +/* Returns time in milliseconds */ +static unsigned long long notrace tick_to_time(unsigned long long tick) { - unsigned int div = get_tbclk(); + ulong div = get_tbclk(); tick *= CONFIG_SYS_HZ; do_div(tick, div); @@ -65,6 +67,7 @@ int __weak timer_init(void) return 0; } +/* Returns time in milliseconds */ ulong __weak get_timer(ulong base) { return tick_to_time(get_ticks()) - base; @@ -74,9 +77,10 @@ unsigned long __weak notrace timer_get_us(void) { return tick_to_time(get_ticks() * 1000); } + static unsigned long long usec_to_tick(unsigned long usec) { - uint64_t tick = usec; + unsigned long long tick = usec; tick *= get_tbclk(); do_div(tick, 1000000); return tick; @@ -85,12 +89,10 @@ static unsigned long long usec_to_tick(unsigned long usec) void __weak __udelay(unsigned long usec) { unsigned long long tmp; - ulong tmo; - tmo = usec_to_tick(usec); - tmp = get_ticks() + tmo; /* get current timestamp */ + tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ - while (get_ticks() < tmp) /* loop till event */ + while (get_ticks() < tmp+1) /* loop till event */ /*NOP*/; }
As I initially suspected overflow in time handling, I took a detailed look at lib/time.c. This adds comments about units being used, reduces amount of type casting being done, and makes __udelay() always wait at least one tick. (Current code could do no delaying at all for short delays). Signed-off-by: Pavel Machek <pavel@denx.de>