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

[Heiko Schocher <hs@denx.de>]

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