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

Albert ARIBAUD
2010-11-30T08:58:20Z

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.