[U-Boot] [PATCH 4/4] arm920t/at91/timer: replace bss variables by gd

[Reinhard Meyer]

Dear Andreas Bießmann,
> Reuse the gd->tbl/tbu values for timestamp/lastinc bss values in
> arm920t/at91/timer driver.
> The usage of bss values in driver before initialisation of bss is
> forbidden. In that special case some data in .rel.dyn gets corrupted by
> the arm920t/at91/timer driver.
> 
> Signed-off-by: Andreas Bießmann <andreas.devel at googlemail.com>
> ---
>  arch/arm/cpu/arm920t/at91/timer.c |   29 ++++++++++++++---------------
>  {
>  	/* reset time */
>  	at91_tc_t *tc = (at91_tc_t *) AT91_TC_BASE;
> -	lastinc = readl(&tc->tc[0].cv) & 0x0000ffff;
> -	timestamp = 0;
> +	gd->tbl = readl(&tc->tc[0].cv) & 0x0000ffff;
> +	gd->tbu = 0;
>  }
>  
>  ulong get_timer_raw(void)
> @@ -109,16 +108,16 @@ ulong get_timer_raw(void)
>  
>  	now = readl(&tc->tc[0].cv) & 0x0000ffff;
>  
> -	if (now >= lastinc) {
> +	if (now >= gd->tbl) {
>  		/* normal mode */
> -		timestamp += now - lastinc;
> +		gd->tbu += now - gd->tbl;
>  	} else {
>  		/* we have an overflow ... */
> -		timestamp += now + TIMER_LOAD_VAL - lastinc;
> +		gd->tbu += now + TIMER_LOAD_VAL - gd->tbl;
>  	}
> -	lastinc = now;
> +	gd->tbl = now;
>  
> -	return timestamp;
> +	return gd->tbu;
>  }

I see your dilemma here.

tbu/tbl were introduced by me to form a true 64 bit monotonous incrementing value
(like on most powerPC).
You use tbl as the last (16 bit) value of the 16 bit hardware timer and
tbu as the actual, only 32 bits worth time value.
If the rest of the timer functions handle this correctly (I doubt that, but I cannot look at
that right now), that "abuse" might be OK.
But I rather have a field, say "u32 last_hw_val" (or a better name) added to the GD inside the
AT91FAMILY define and have tbu/tbl really be a functional 64 bit value.

That would also ease a later unification/factoring of all AT91 timer sources:
A small SoC dependant part that just makes sure tbu/tbl increment with a high frequency,
and a common parts that derives udelay() and get_timer() from that. We should not need any
other functions like *_masked or reset_timer() in the future.

I know there is an endless but not leading anywhere discussion about timers ongoing, but no clean
solution exists right now because of the messed up ways the many different timer functions are used
throughout u-boot.

I would like to reduce it to as few functions as possible...

I am working on a proposal for that, but since that untimately requires to fix *ALL* existing timer
uses I see little change that we will ever get to clean up the mess.

Best Regards,
Reinhard