[U-Boot-Users] [Patch 02/17] U-Boot-V2:Common:Clock Handle case of clock rollover for get_time_ns

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, May 21, 2008 at 11:25:15AM -0500, Menon, Nishanth wrote:
> get_time_ns does a simplistic delta = cycle_now - cycle_last. It is possible that the h/w counter reached max and reset back to 0.
> This patch addresses this issue by checking for rollover condition.
> 
> NOTE 1: This does not guarantee that you cannot confuse get_time_ns. You could possibly wait for two reset cycles and then get a messed up value.
> To fix that we may need interrupt mode timer tick - something on the lines of jiffies on linux.
> NOTE 2: the question of cs->mask is not clear. if the mask is for the tick, then it is better done with (cycle_now & cs->mask) - (cs->cycle_last & cs->mask). Do we need min and max for register read?
> 
> Signed-off-by: Nishanth Menon<x0nishan@ti.com>
> 
> ---
>  common/clock.c |   14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> Index: u-boot-v2.git/common/clock.c
> ===================================================================
> --- u-boot-v2.git.orig/common/clock.c	2008-05-20 17:19:44.000000000 -0500
> +++ u-boot-v2.git/common/clock.c	2008-05-20 17:26:31.000000000 -0500
> @@ -37,14 +37,24 @@
>  uint64_t get_time_ns(void)
>  {
>  	struct clocksource *cs = current_clock;
> -        uint64_t cycle_now, cycle_delta;
> +	uint64_t cycle_now, cycle_delta = 0;
>          uint64_t ns_offset;
>  
>          /* read clocksource: */
>          cycle_now = cs->read();
>  
>          /* calculate the delta since the last call: */
> -        cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;

Look closer, the rollover case is handled implicitly by the unsigned
arithmetics.

You are right, we do not have a possibility to detect a double rollover
without interrupts. Normally this is not an issue as this code is used
in timeout polling loops where the current time is polled often enough.
Anyway, maybe some comment should mention that this function is not
useful to measure long periods of time where 'long' is highly
architecture specific.

regards,
  Sascha

> +
> +	/* Handle rollover case */
> +	if (cycle_now < cs->cycle_last) {
> +		/* FIXME: I am not convinced about the cs->mask operation..
> +		 * I am assuming cs->mask is max! Probably need to change
> +		 * clocksource structure to have min and max
> +		 */
> +		cycle_delta = cs->mask - (cs->cycle_last & cs->mask);
> +		cs->cycle_last = 0;
> +	}
> +	cycle_delta += (cycle_now & cs->mask) - (cs->cycle_last & cs->mask);
>  
>          /* convert to nanoseconds: */
>          ns_offset = cyc2ns(cs, cycle_delta);
> 

-- 
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-