[U-Boot] [PATCH] lib/time.c cleanups

[U-Boot] [PATCH] lib/time.c cleanups

[Pavel Machek]

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>


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*/;
 }
 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCH] lib/time.c cleanups

[Marek Vasut]

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

[U-Boot] [PATCH] lib/time.c cleanups

[Pavel Machek]

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] lib/time.c cleanups

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/fc8a0515/attachment.pgp>