[U-Boot] Most ARM CPU's have buggy clear_bss?

Heiko Schocher
2010-10-28T06:14:48Z

Hello Darius,

Darius Augulis wrote:
> the code for clearing bss section for most ARM cores looks like this
> or very similar:
>
> clear_bss:
> #ifndef CONFIG_PRELOADER
> ldr r0, _bss_start_ofs
> ldr r1, _bss_end_ofs
> ldr r3, _TEXT_BASE /* Text base */
> mov r4, r7 /* reloc addr */
> add r0, r0, r4
> add r1, r1, r4
> mov r2, #0x00000000 /* clear */
>
> clbss_l:str r2, [r0] /* clear loop... */
> add r0, r0, #4
> cmp r0, r1
> bne clbss_l
> #endif /* #ifndef CONFIG_PRELOADER */
>
>
> IMO, if relocation is skipped, r4 should be loaded with value of
> _TEXT_BASE, not reloc address?

Hmm.. I think the question is, is CONFIG_SKIP_RELOCATE_UBOOT not
obsolete?

> It seems like r3 is prepared for this but, it's somehow missing? It's
> not used at all.

Here you are right. I think r3 is an artifact from the
GOT relocation. This should be reworked (added Albert Aribaud
to cc, because he did the elf relocation work)

> Maybe it could be reason why I'm facing strange problem, when after
> relocating uboot with nand_spl no one command is not working.
> I debugged that command table is empty. I think this ASM code clears
> not bss area but something else.

If you have defined CONFIG_SKIP_RELOCATE_UBOOT, yes.
Please remove this define.

> Also relocation code looks a bit strange:
>
> .globl relocate_code
> relocate_code:
> mov r4, r0 /* save addr_sp */
> mov r5, r1 /* save addr of gd */
> mov r6, r2 /* save addr of destination */
> mov r7, r2 /* save addr of destination */
>
> /* Set up the stack */
> stack_setup:
> mov sp, r4
>
> adr r0, _start
> ldr r2, _TEXT_BASE
> ldr r3, _bss_start_ofs
> add r2, r0, r3 /* r2 <- source end address */
> cmp r0, r6
> beq clear_bss
>
> r0 is compared to r6, which contains reloc address. All instructions
> between loading r0 and comparison are confusing, because they do not
> impact comparison result.
> Also they do not matter in clear_bss so I think would be great to
> change code like this:
>
> mov sp, r4
>
> adr r0, _start
> cmp r0, r6
> beq clear_bss
> ldr r2, _TEXT_BASE
> ldr r3, _bss_start_ofs
> add r2, r0, r3 /* r2 <- source end address */
>
> Any comments are welcome!

Yep, please send a patch for this issue.

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