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

[Albert ARIBAUD]

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