[U-Boot] v2010-rc2: OMAP3 broken

[U-Boot] v2010-rc2: OMAP3 broken

[Wolfgang Denk]

Hi,

it seems that commit 566d49a3f has broken OMAP3 - at least beagle and
devkit8000 hang in or after relocation.

The culprit is this the "$(sort $(LIBS)) in Makefile, i. e. the
following change will unbreak the boards:

@@ -263,7 +263,7 @@ ifeq ($(SOC),s5pc2xx)
 LIBS += $(CPUDIR)/s5p-common/libs5p-common.o
 endif
 
-LIBS := $(addprefix $(obj),$(sort $(LIBS)))
+LIBS := $(addprefix $(obj),$(LIBS))
 .PHONY : $(LIBS) $(TIMESTAMP_FILE) $(VERSION_FILE)
 
 LIBBOARD = board/$(BOARDDIR)/lib$(BOARD).o


Unfortunately we need the sort to avoild duplicate listing of libs /
objects on PowerPC.  And I don;t understand yet how this change can
cause such an affect.

Any help welcome.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek

[U-Boot] v2010-rc2: OMAP3 broken

[Hiremath, Vaibhav]

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de]
> On Behalf Of Wolfgang Denk
> Sent: Monday, November 29, 2010 6:19 PM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] v2010-rc2: OMAP3 broken
> 
> Hi,
> 
> it seems that commit 566d49a3f has broken OMAP3 - at least beagle and
> devkit8000 hang in or after relocation.
> 
[Hiremath, Vaibhav] AM3517 also used to hangs in relocate_code function.



> The culprit is this the "$(sort $(LIBS)) in Makefile, i. e. the
> following change will unbreak the boards:
> 
> @@ -263,7 +263,7 @@ ifeq ($(SOC),s5pc2xx)
>  LIBS += $(CPUDIR)/s5p-common/libs5p-common.o
>  endif
> 
> -LIBS := $(addprefix $(obj),$(sort $(LIBS)))
> +LIBS := $(addprefix $(obj),$(LIBS))
>  .PHONY : $(LIBS) $(TIMESTAMP_FILE) $(VERSION_FILE)
> 
>  LIBBOARD = board/$(BOARDDIR)/lib$(BOARD).o
> 
> 
[Hiremath, Vaibhav] The above patch works for me as well.

> Unfortunately we need the sort to avoild duplicate listing of libs /
> objects on PowerPC.  And I don;t understand yet how this change can
> cause such an affect.
> 
> Any help welcome.
> 
[Hiremath, Vaibhav] I can put my debug observations here,


The system is hanging at 

fixrel:
        /* relative fix: increase location by offset */
        ldr     r1, [r0]

Here the value of r0 is relocation offset and not an address.

fixloop:
        ldr     r0, [r2]        /* r0 <- location to fix up, IN FLASH! */
        add     r0, r0, r9      /* r0 <- location to fix up in RAM */

In the above two instructions, load from [r2] is coming as 0, causing reference to invalid address (relocation offset).

After applying your patch, the value from above load instruction comes as 0x80008020, which further gets added to offset (valid address).


I hope this might give you some pointers, and I am not an expert on makefile, so not sure about relation between library sort and above finding.

Thanks,
Vaibhav
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> It seems intuitively obvious to me, which  means  that  it  might  be
> wrong.                                                 -- Chris Torek
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] v2010-rc2: OMAP3 broken

[Hiremath, Vaibhav]

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de]
> On Behalf Of Hiremath, Vaibhav
> Sent: Monday, November 29, 2010 6:50 PM
> To: Wolfgang Denk; u-boot at lists.denx.de
> Subject: Re: [U-Boot] v2010-rc2: OMAP3 broken
> 
> 
> > -----Original Message-----
> > From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de]
> > On Behalf Of Wolfgang Denk
> > Sent: Monday, November 29, 2010 6:19 PM
> > To: u-boot at lists.denx.de
> > Subject: [U-Boot] v2010-rc2: OMAP3 broken
> >
> > Hi,
> >
> > it seems that commit 566d49a3f has broken OMAP3 - at least beagle and
> > devkit8000 hang in or after relocation.
> >
> [Hiremath, Vaibhav] AM3517 also used to hangs in relocate_code function.
> 
> 
> 
> > The culprit is this the "$(sort $(LIBS)) in Makefile, i. e. the
> > following change will unbreak the boards:
> >
> > @@ -263,7 +263,7 @@ ifeq ($(SOC),s5pc2xx)
> >  LIBS += $(CPUDIR)/s5p-common/libs5p-common.o
> >  endif
> >
> > -LIBS := $(addprefix $(obj),$(sort $(LIBS)))
> > +LIBS := $(addprefix $(obj),$(LIBS))
> >  .PHONY : $(LIBS) $(TIMESTAMP_FILE) $(VERSION_FILE)
> >
> >  LIBBOARD = board/$(BOARDDIR)/lib$(BOARD).o
> >
> >
> [Hiremath, Vaibhav] The above patch works for me as well.
> 
> > Unfortunately we need the sort to avoild duplicate listing of libs /
> > objects on PowerPC.  And I don;t understand yet how this change can
> > cause such an affect.
> >
> > Any help welcome.
> >
> [Hiremath, Vaibhav] I can put my debug observations here,
> 
> 
> The system is hanging at
> 
> fixrel:
>         /* relative fix: increase location by offset */
>         ldr     r1, [r0]
> 
[Hiremath, Vaibhav] pasting error, the actual place is 

fixnext:
        str     r1, [r0]

Thanks,
Vaibhav

> Here the value of r0 is relocation offset and not an address.
> 
> fixloop:
>         ldr     r0, [r2]        /* r0 <- location to fix up, IN FLASH! */
>         add     r0, r0, r9      /* r0 <- location to fix up in RAM */
> 
> In the above two instructions, load from [r2] is coming as 0, causing
> reference to invalid address (relocation offset).
> 
> After applying your patch, the value from above load instruction comes as
> 0x80008020, which further gets added to offset (valid address).
> 
> 
> I hope this might give you some pointers, and I am not an expert on
> makefile, so not sure about relation between library sort and above
> finding.
> 
> Thanks,
> Vaibhav
> > Best regards,
> >
> > Wolfgang Denk
> >
> > --
> > DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> > It seems intuitively obvious to me, which  means  that  it  might  be
> > wrong.                                                 -- Chris Torek
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] v2010-rc2: OMAP3 broken

[Albert ARIBAUD]

Hi all,

I'll take a look this evening at builds with and without the SORT() from 
an ELF relocation tables perspective.

Amicalement,
-- 
Albert.

[U-Boot] v2010-rc2: OMAP3 broken

[Heiko Schocher]

Hello Albert,

Albert ARIBAUD wrote:
> I'll take a look this evening at builds with and without the SORT() from 
> an ELF relocation tables perspective.

I debugged on the beagle board a little bit in this problem, and here
what I found:

Hier it goes wrong:

arch/arm/cpu/armv7/start.S

fixloop:
        ldr     r0, [r2]                /* r0 <- location to fix up, IN FLASH! */
 104:   e5920000        ldr     r0, [r2]
        add     r0, r0, r9              /* r0 <- location to fix up in RAM */
 108:   e0800009        add     r0, r0, r9

and later here

fixrel:
        /* relative fix: increase location by offset */
        ldr     r1, [r0]

Here the version with sort:

OMAP35xx>t;r
    Core number       : 0
    Core state        : debug mode (ARM)
    Debug entry cause : Single Step
    Current PC        : 0x80008104
    Current CPSR      : 0x200001d3 (Supervisor)
GPR00: 80008000 8ff1df84 80046d7c 8004d6ac
GPR04: 8ff1df80 8ff1df84 8ffbcd80 8ff7e000
GPR08: 4020ffa0 0ff76000 8004d6ac 00000000
GPR12: 00000000 8ff1df80 8000aef0 80008104
PC   : 80008104    CPSR: 200001d3
OMAP35xx>t;r
    Core number       : 0
    Core state        : debug mode (ARM)
    Debug entry cause : Single Step
    Current PC        : 0x80008108
    Current CPSR      : 0x200001d3 (Supervisor)
GPR00: 00000000 8ff1df84 80046d7c 8004d6ac
       ^^^^^^^^
       Ups... not good

GPR04: 8ff1df80 8ff1df84 8ffbcd80 8ff7e000
GPR08: 4020ffa0 0ff76000 8004d6ac 00000000
GPR12: 00000000 8ff1df80 8000aef0 80008108
PC   : 80008108    CPSR: 200001d3
OMAP35xx>t;r


Here without sort:

GPR00: 80008000 8ff1df84 80046d74 8004d6a4
GPR04: 8ff1df80 8ff1df84 8ffbcd78 8ff7e000
GPR08: 4020ffa0 0ff76000 8004d6a4 00000000
GPR12: 00000000 8ff1df80 80010730 80008104
PC   : 80008104    CPSR: 200001d3
OMAP35xx>ti;r
    Core number       : 0
    Core state        : debug mode (ARM)
    Debug entry cause : Single Step
    Current PC        : 0x80008108
    Current CPSR      : 0x200001d3 (Supervisor)
GPR00: 80008020 8ff1df84 80046d74 8004d6a4
       ^^^^^^^^
       Yep, thats better
GPR04: 8ff1df80 8ff1df84 8ffbcd78 8ff7e000
GPR08: 4020ffa0 0ff76000 8004d6a4 00000000
GPR12: 00000000 8ff1df80 80010730 80008108
PC   : 80008108    CPSR: 200001d3
OMAP35xx>ti;r
    Core number       : 0
    Core state        : debug mode (ARM)
    Debug entry cause : Single Step
    Current PC        : 0x8000810c
    Current CPSR      : 0x200001d3 (Supervisor)


System Map:

with sort:

80046d7c B __bss_start
80046d7c R __rel_dyn_start
80046d7c b timestamp
80046d80 b lastinc
80046d84 B gpmc_cfg

without sort:

80046d74 R __rel_dyn_start
80046d78 b htab
80046d84 B ___strtok
80046d88 B z_verbose

timestamp comes after the "__rel_dyn_end" entry in this case!

Further debugging pointed my that in:

in arch/arm/cpu/armv7/omap-common/timer.c timer_init() sets
timestamp to 0, before relocation is executed, which leads
that the memory @80046d7c gets overwritten to 0, which
results in crashing in the fixrel case ...

So it seems to me the "sort" version intermix the "rel dyn"
section entries with "normal" vars in bss ... Which raises
the question:

Why is the rel.dyn Section in the bss section?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] v2010-rc2: OMAP3 broken

[Andreas Bießmann]

Dear Heiko Schocher,

Am 29.11.2010 16:04, schrieb Heiko Schocher:
[snip]
> So it seems to me the "sort" version intermix the "rel dyn"
> section entries with "normal" vars in bss ... Which raises
> the question:
> 
> Why is the rel.dyn Section in the bss section?

cause of the
---8<---
.bss __rel_dyn_start (OVERLAY) : {
--->8---
in u-boot.lds

But the error is to use bss variables before setting up the bss section,
isn't it?

regards

Andreas Bie?mann

[U-Boot] v2010-rc2: OMAP3 broken

[Heiko Schocher]

Hello Andreas

Andreas Bie?mann wrote:
> Am 29.11.2010 16:04, schrieb Heiko Schocher:
> [snip]
>> So it seems to me the "sort" version intermix the "rel dyn"
>> section entries with "normal" vars in bss ... Which raises
>> the question:
>>
>> Why is the rel.dyn Section in the bss section?
> 
> cause of the
> ---8<---
> .bss __rel_dyn_start (OVERLAY) : {
> --->8---
> in u-boot.lds
> 
> But the error is to use bss variables before setting up the bss section,
> isn't it?

Yep, thats right.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] v2010-rc2: OMAP3 broken

[Albert ARIBAUD]

Le 29/11/2010 16:04, Heiko Schocher a ?crit :
> Hello Albert,
>
> Albert ARIBAUD wrote:
>> I'll take a look this evening at builds with and without the SORT() from
>> an ELF relocation tables perspective.
>
> I debugged on the beagle board a little bit in this problem, and here
> what I found:
>
> Hier it goes wrong:
>
> arch/arm/cpu/armv7/start.S
>
> fixloop:
>          ldr     r0, [r2]                /* r0<- location to fix up, IN FLASH! */
>   104:   e5920000        ldr     r0, [r2]
>          add     r0, r0, r9              /* r0<- location to fix up in RAM */
>   108:   e0800009        add     r0, r0, r9
>
> and later here
>
> fixrel:
>          /* relative fix: increase location by offset */
>          ldr     r1, [r0]
>
> Here the version with sort:
>
> OMAP35xx>t;r
>      Core number       : 0
>      Core state        : debug mode (ARM)
>      Debug entry cause : Single Step
>      Current PC        : 0x80008104
>      Current CPSR      : 0x200001d3 (Supervisor)
> GPR00: 80008000 8ff1df84 80046d7c 8004d6ac
> GPR04: 8ff1df80 8ff1df84 8ffbcd80 8ff7e000
> GPR08: 4020ffa0 0ff76000 8004d6ac 00000000
> GPR12: 00000000 8ff1df80 8000aef0 80008104
> PC   : 80008104    CPSR: 200001d3
> OMAP35xx>t;r
>      Core number       : 0
>      Core state        : debug mode (ARM)
>      Debug entry cause : Single Step
>      Current PC        : 0x80008108
>      Current CPSR      : 0x200001d3 (Supervisor)
> GPR00: 00000000 8ff1df84 80046d7c 8004d6ac
>         ^^^^^^^^
>         Ups... not good
>
> GPR04: 8ff1df80 8ff1df84 8ffbcd80 8ff7e000
> GPR08: 4020ffa0 0ff76000 8004d6ac 00000000
> GPR12: 00000000 8ff1df80 8000aef0 80008108
> PC   : 80008108    CPSR: 200001d3
> OMAP35xx>t;r
>
>
> Here without sort:
>
> GPR00: 80008000 8ff1df84 80046d74 8004d6a4
> GPR04: 8ff1df80 8ff1df84 8ffbcd78 8ff7e000
> GPR08: 4020ffa0 0ff76000 8004d6a4 00000000
> GPR12: 00000000 8ff1df80 80010730 80008104
> PC   : 80008104    CPSR: 200001d3
> OMAP35xx>ti;r
>      Core number       : 0
>      Core state        : debug mode (ARM)
>      Debug entry cause : Single Step
>      Current PC        : 0x80008108
>      Current CPSR      : 0x200001d3 (Supervisor)
> GPR00: 80008020 8ff1df84 80046d74 8004d6a4
>         ^^^^^^^^
>         Yep, thats better
> GPR04: 8ff1df80 8ff1df84 8ffbcd78 8ff7e000
> GPR08: 4020ffa0 0ff76000 8004d6a4 00000000
> GPR12: 00000000 8ff1df80 80010730 80008108
> PC   : 80008108    CPSR: 200001d3
> OMAP35xx>ti;r
>      Core number       : 0
>      Core state        : debug mode (ARM)
>      Debug entry cause : Single Step
>      Current PC        : 0x8000810c
>      Current CPSR      : 0x200001d3 (Supervisor)
>
>
> System Map:
>
> with sort:
>
> 80046d7c B __bss_start
> 80046d7c R __rel_dyn_start
> 80046d7c b timestamp
> 80046d80 b lastinc
> 80046d84 B gpmc_cfg
>
> without sort:
>
> 80046d74 R __rel_dyn_start
> 80046d78 b htab
> 80046d84 B ___strtok
> 80046d88 B z_verbose
>
> timestamp comes after the "__rel_dyn_end" entry in this case!
>
> Further debugging pointed my that in:
>
> in arch/arm/cpu/armv7/omap-common/timer.c timer_init() sets
> timestamp to 0, before relocation is executed, which leads
> that the memory @80046d7c gets overwritten to 0, which
> results in crashing in the fixrel case ...
>
> So it seems to me the "sort" version intermix the "rel dyn"
> section entries with "normal" vars in bss ... Which raises
> the question:
>
> Why is the rel.dyn Section in the bss section?
>
The .rel.dyn section is not "in" or "out" of the BSS: they are 
overlapped, i.e. they should start at the same address.

As indicated in the documentation, BSS should never be used before 
relocation; it actually *cannot* be written into by boards which start 
from NOR, and is set to zero for all boards right after relocation.

Since BSS does not exist before relocation, and .rel.dyn does not exist 
after relocation, they can use the same location: overlaying them is the 
best approach in order to get the smallest binary in Flash *and* the 
smallest image in RAM.

We can add code to try and catch the most obvious corruption of the 
relocation tables due to BSS being accessed before relocation, but we 
can't be sure to catch them all. The real solution is to fix the BSS usage.

> bye,
> Heiko

Amicalement,
-- 
Albert.

[U-Boot] v2010-rc2: OMAP3 broken

[Andreas Bießmann]

Dear all,

Am 29.11.2010 16:19, schrieb Albert ARIBAUD:

[snip]

> The real solution is to fix the BSS usage.

I'd like to mention old thread facing this issue for timestamp/lastinc
values in arch/arm/cpu/*/timer.c

http://article.gmane.org/gmane.comp.boot-loaders.u-boot/88545
and
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/88383/focus=88551

regards

Andreas Bie?mann

[U-Boot] v2010-rc2: OMAP3 broken

[Heiko Schocher]

Hello Albert,

Albert ARIBAUD wrote:
> Le 29/11/2010 16:04, Heiko Schocher a ?crit :
>> Hello Albert,
>>
>> Albert ARIBAUD wrote:
>>> I'll take a look this evening at builds with and without the SORT() from
>>> an ELF relocation tables perspective.
>>
>> I debugged on the beagle board a little bit in this problem, and here
>> what I found:
[...]
>> Why is the rel.dyn Section in the bss section?
>>
> The .rel.dyn section is not "in" or "out" of the BSS: they are
> overlapped, i.e. they should start at the same address.
> 
> As indicated in the documentation, BSS should never be used before
> relocation; it actually *cannot* be written into by boards which start
> from NOR, and is set to zero for all boards right after relocation.

Yep.

> Since BSS does not exist before relocation, and .rel.dyn does not exist
> after relocation, they can use the same location: overlaying them is the
> best approach in order to get the smallest binary in Flash *and* the
> smallest image in RAM.

Ah, thats what I missed, thanks!

> We can add code to try and catch the most obvious corruption of the
> relocation tables due to BSS being accessed before relocation, but we
> can't be sure to catch them all. The real solution is to fix the BSS usage.

Yep.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] v2010-rc2: OMAP3 broken

[Premi, Sanjeev]

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Heiko Schocher
> Sent: Monday, November 29, 2010 8:35 PM
> To: Albert ARIBAUD
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] v2010-rc2: OMAP3 broken
> 

[snip]

> in arch/arm/cpu/armv7/omap-common/timer.c timer_init() sets
> timestamp to 0, before relocation is executed, which leads
> that the memory @80046d7c gets overwritten to 0, which
> results in crashing in the fixrel case ...
> 
> So it seems to me the "sort" version intermix the "rel dyn"
> section entries with "normal" vars in bss ... Which raises
> the question:
> 
> Why is the rel.dyn Section in the bss section?

I have been facing the linker errors due to overlap of these
sections. Using git-bisect, I was able to narrow down the
failure to commit 6d8962e814c15807dd6ac5757904be2a02d187b8.

I had the same question above. See these:
  http://marc.info/?l=u-boot&m=129043627514796&w=2
  http://marc.info/?l=u-boot&m=129041727622164&w=2

~sanjeev

> 
> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] v2010-rc2: OMAP3 broken

[Andreas Bießmann]

Dear all,

Am 29.11.2010 14:23, schrieb Hiremath, Vaibhav:

>> The system is hanging at
>>
>> fixrel:
>>         /* relative fix: increase location by offset */
>>         ldr     r1, [r0]
>>
> [Hiremath, Vaibhav] pasting error, the actual place is 
> 
> fixnext:
>         str     r1, [r0]

> Here the value of r0 is relocation offset and not an address.

I do have this error on at92rm9200ek too. I got it down to corrupted
.rel.dyn table by setting two bss values in arch/arm/cpu/at91/timer.c.

Can you try to change your linker script to _not_ overlay _rel_dyn_start
with with .bss?

The other way is to break on board_init_f, display the relevant
(defective) locations in .rel.dyn and step through the for loop for
init_sequence if one destroys the relevant part in .rel.dyn

regards

Andreas Bie?mann

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

[Thomas Weber]

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;
 }
 
-- 
1.7.3.2

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

[Heiko Schocher]

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] [PATCH] OMAP-common/timer: Fix bss usage

[Thomas Weber]

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 at 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

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I usually tell my classes "if you are using @ and [] together in this
class, you will almost certainly NOT get what you want. That's  going
down  the wrong tunnel. There's no cheese at the end of that tunnel."
         -- Randal L. Schwartz in <8czptuomey.fsf@gadget.cscaper.com>