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

Albert ARIBAUD albert at aribaud.net
Tue Nov 30 09:58:20 CET 2010


Le 30/11/2010 09:35, Andreas Bießmann a écrit :
> 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---

... which is why I could not find "r8" allocation :) ... Thanks. So yes, 
we need to keep r8, and yes, we were lucky that the start.S code did run 
at all with a corrupted r8 -- fixing that may remove some weird 
behaviors we could see,

Can you do a pass on other ARM archs which must be fixed too?

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

I'd rather have no comment than a paraphrase of the code, but I'd rather 
have an imperfect yet at least partly helpful comment than no comment at 
all for assembly language code.

> regards
>
> Andreas Bießmann

Amicalement,
-- 
Albert.


More information about the U-Boot mailing list