[U-Boot] OMAP-common/timer: Fix bss usage

This patch fixes the bss usage in ARMv7/omap-common/timer.c

The .bss section cannot be used before the relocation, because this
section is overlayed with .rel.dyn section.

Suggested-by: Andreas Biessmann <andreas.devel@gmail.com>
Signed-off-by: Thomas Weber <weber@corscience.de>
---
 arch/arm/cpu/armv7/omap-common/timer.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

Hello Thomas,

Thomas Weber wrote:
> This patch fixes the bss usage in ARMv7/omap-common/timer.c
> 
> The .bss section cannot be used before the relocation, because this
> section is overlayed with .rel.dyn section.
> 
> Suggested-by: Andreas Biessmann <andreas.devel@gmail.com>
> Signed-off-by: Thomas Weber <weber@corscience.de>
> ---
>  arch/arm/cpu/armv7/omap-common/timer.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/omap-common/timer.c b/arch/arm/cpu/armv7/omap-common/timer.c
> index 6b8cf7b..b649b93 100644
> --- a/arch/arm/cpu/armv7/omap-common/timer.c
> +++ b/arch/arm/cpu/armv7/omap-common/timer.c
> @@ -35,8 +35,8 @@
>  #include <common.h>
>  #include <asm/io.h>
>  
> -static ulong timestamp;
> -static ulong lastinc;
> +static ulong timestamp = 0;
> +static ulong lastinc = 0;
>  static struct gptimer *timer_base = (struct gptimer *)CONFIG_SYS_TIMERBASE;
>  
>  /*
> @@ -54,8 +54,6 @@ int timer_init(void)
>  	writel((CONFIG_SYS_PTV << 2) | TCLR_PRE | TCLR_AR | TCLR_ST,
>  		&timer_base->tclr);
>  
> -	reset_timer_masked();	/* init the timestamp and lastinc value */
> -
>  	return 0;
>  }

Is this Ok? In actual reset_timer_masked() lastinc gets initialized with:

lastinc = readl(&timer_base->tcrr) / (TIMER_CLOCK / CONFIG_SYS_HZ);

and your patch changes that to lastinc = 0 ...

bye,
Heiko

Hello Heiko,

On Mon, Nov 29, 2010 at 6:25 PM, Heiko Schocher <hs@denx.de> wrote:
> Hello Thomas,
>
> Thomas Weber wrote:
>> This patch fixes the bss usage in ARMv7/omap-common/timer.c
>>
>> The .bss section cannot be used before the relocation, because this
>> section is overlayed with .rel.dyn section.
>>
>> Suggested-by: Andreas Biessmann <andreas.devel@gmail.com>
>> Signed-off-by: Thomas Weber <weber@corscience.de>
>> ---
>>  arch/arm/cpu/armv7/omap-common/timer.c |    6 ++----
>>  1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/omap-common/timer.c b/arch/arm/cpu/armv7/omap-common/timer.c
>> index 6b8cf7b..b649b93 100644
>> --- a/arch/arm/cpu/armv7/omap-common/timer.c
>> +++ b/arch/arm/cpu/armv7/omap-common/timer.c
>> @@ -35,8 +35,8 @@
>>  #include <common.h>
>>  #include <asm/io.h>
>>
>> -static ulong timestamp;
>> -static ulong lastinc;
>> +static ulong timestamp = 0;
>> +static ulong lastinc = 0;
>>  static struct gptimer *timer_base = (struct gptimer *)CONFIG_SYS_TIMERBASE;
>>
>>  /*
>> @@ -54,8 +54,6 @@ int timer_init(void)
>>       writel((CONFIG_SYS_PTV << 2) | TCLR_PRE | TCLR_AR | TCLR_ST,
>>               &timer_base->tclr);
>>
>> -     reset_timer_masked();   /* init the timestamp and lastinc value */
>> -
>>       return 0;
>>  }
>
> Is this Ok? In actual reset_timer_masked() lastinc gets initialized with:
>
> lastinc = readl(&timer_base->tcrr) / (TIMER_CLOCK / CONFIG_SYS_HZ);
>
> and your patch changes that to lastinc = 0 ...
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

or should it be moved into global_data struct like in commit 5dca710a for AT91?

Thomas

Dear Thomas Weber,

In message <1291046912-15167-1-git-send-email-weber@corscience.de> you wrote:
> This patch fixes the bss usage in ARMv7/omap-common/timer.c
> 
> The .bss section cannot be used before the relocation, because this
> section is overlayed with .rel.dyn section.

No, this is not correct. It cannot be used because it simply has not
been created at all.  The overlay thing is just an optimization to
shring the image size, but otherwise it has no influence here.  The
thing is that before relocation we simply do not have a full standard
conforming C runtime environment: we havce only a very small stack, we
have a read-only data segment and no bss at all.

> -static ulong timestamp;
> -static ulong lastinc;
> +static ulong timestamp = 0;
> +static ulong lastinc = 0;

This does not change anything. The compiler still generates exactly
the same code.

> -	reset_timer_masked();	/* init the timestamp and lastinc value */
> -

And this is most probably wrong.


Best regards,

Wolfgang Denk