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

[Andreas Bießmann]

Dear Albert ARIBAUD,

Am 30.11.2010 09:22, schrieb 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.

Well r8 is set in arch/arm/include/asm/global_data.h

---8<---
#define DECLARE_GLOBAL_DATA_PTR     register volatile gd_t *gd asm ("r8")
--->8---

The GD is then later on allocated in board_init_f

---8<---
	/* Pointer is writable since we allocated a register for it */
	gd = (gd_t *) (CONFIG_SYS_INIT_SP_ADDR);
	/* compiler optimization barrier needed for GCC >= 3.4 */
	__asm__ __volatile__("": : :"memory");

	memset ((void*)gd, 0, sizeof (gd_t));
--->8---

Therefore r8 is free, when we use it in relocate_code, isn't it?

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

I do know that. Therefore this patch to do _not_ use r8 in assembler.

>>       /* 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?

Not currently, this was just slipped in by fast preperation of that patch.

>>   fixnext:
>> -    str    r1, [r0]
>> +    str    r1, [r0]        /* store back content of r1 */
> 
> Nak. This comment paraphrases the instruction.

dito

regards

Andreas Bießmann