[U-Boot] [PATCH RFC 2/3] arm920t: do not use r8 for relocation

Albert ARIBAUD
2010-11-30T08:22:16Z

Le 30/11/2010 08:06, Andreas Bie?mann a ?crit :
> r8 is used for gd and should therefore be left alone

I'm surprised that this did not break things so far... Whatever value r8
ended with was used as the address of GD.

After a quick look I haven't found out where r8 is *set* to GD,
though... Will have to look this up tonight.

> Signed-off-by: Andreas Bie?mann<andreas.devel@googlemail.com>
> ---
>
> I don't know if this is really needed, but we use --fixed-r8 compiler
> flag for all arm boards. Albert, can you shed some light on that?

ffixed-r8 is for the C compiler. The assembler cannot honor ffixed-r8
since register usage is decided by the programmer in each instruction.

> arch/arm/cpu/arm920t/start.S | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/cpu/arm920t/start.S b/arch/arm/cpu/arm920t/start.S
> index 71de373..629be3f 100644
> --- a/arch/arm/cpu/arm920t/start.S
> +++ b/arch/arm/cpu/arm920t/start.S
> @@ -201,7 +201,6 @@ 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:
> @@ -226,7 +225,7 @@ copy_loop:
> * fix .rel.dyn relocations
> */
> ldr r0, _TEXT_BASE /* r0<- Text base */
> - sub r9, r7, r0 /* r9<- relocation offset */
> + sub r9, r6, r0 /* r9<- relocation offset */
> ldr r10, _dynsym_start_ofs /* r10<- sym table ofs */
> add r10, r10, r0 /* r10<- sym table in FLASH */
> ldr r2, _rel_dyn_start_ofs /* r2<- rel dyn start ofs */
> @@ -237,10 +236,10 @@ fixloop:
> ldr r0, [r2] /* r0<- location to fix up, IN FLASH! */
> add r0, r0, r9 /* r0<- location to fix up in RAM */
> ldr r1, [r2, #4]
> - and r8, r1, #0xff
> - cmp r8, #23 /* relative fixup? */
> + and r7, r1, #0xff
> + cmp r7, #23 /* relative fixup? */
> beq fixrel
> - cmp r8, #2 /* absolute fixup? */
> + cmp r7, #2 /* absolute fixup? */
> beq fixabs
> /* ignore unknown type of fixup */
> b fixnext
> @@ -253,10 +252,10 @@ fixabs:
> b fixnext
> fixrel:
> /* relative fix: increase location by offset */
> - ldr r1, [r0]
> - add r1, r1, r9
> + ldr r1, [r0] /* r1<- address of symbol */
> + add r1, r1, r9 /* r1<- relocated address of symbol */

I'd like to see a less ambiguous comment here, but I'm not sure what's
best. Any suggestions?

> fixnext:
> - str r1, [r0]
> + str r1, [r0] /* store back content of r1 */

Nak. This comment paraphrases the instruction.

> add r2, r2, #8 /* each rel.dyn entry is 8 bytes */
> cmp r2, r3
> blo fixloop
> @@ -267,7 +266,7 @@ clear_bss:
> ldr r0, _bss_start_ofs
> ldr r1, _bss_end_ofs
> ldr r3, _TEXT_BASE /* Text base */
> - mov r4, r7 /* reloc addr */
> + mov r4, r6 /* reloc addr */
> add r0, r0, r4
> add r1, r1, r4
> mov r2, #0x00000000 /* clear */
> @@ -295,10 +294,10 @@ _nand_boot_ofs:
> ldr r0, _board_init_r_ofs
> adr r1, _start
> add lr, r0, r1
> - add lr, lr, r9
> + add lr, lr, r9 /* position from board_init_r in RAM */
> /* setup parameters for board_init_r */
> mov r0, r5 /* gd_t */
> - mov r1, r7 /* dest_addr */
> + mov r1, r6 /* dest_addr */
> /* jump to it ... */
> mov pc, lr
>

Amicalement,
--
Albert.