[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

Le 20/09/2011 20:09, Wolfgang Denk a écrit :
> Dear "GROYER, Anthony",
>
> In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194D534 at EU-FR-EXBE07.eu.corp.airliquide.com>  you wrote:
>>
>> The use of the initial patches for the CONFIG_SYS_SKIP_ARM_RELOCATION featu
>> res has revealed two issues.
>
> Could you please restict your line length to some 70 characters or so?
> Thanks.
>
>> First issue: the calculation of the relocation offset was done only if the
>> relocation is actually done. So we could reach a point where r9 has a wrong
>>   value, since it has never been used before (in my case, this bug happens w
>
> This is a configuration error then, isn't it?  The relocation offset
> should be either the intended value, or eventually zero, if no
> relocation is intended.

Actually, even though "revision 1083" and "revision 1113" are not git 
references (and thus I can't be sure Anthony is referring to up-to-date 
mainline code), there is a point to what Anthony says: in the case where 
relocation is unneeded (r0 equals r6) then r9 is not set, but is still 
used when branching to board_init_r().

for this bug to have any effect, relocation would have to be unneeded, 
which is a rare case, *and* r9 has to be nonzero, which may or may not 
happen depending on the code executed until relocate_code() is called, 
and thus makes the whole condition rarer yet; probably the rarity of 
these two conjunct conditions explains why it was not noticed until now.

However, since start.S has a code path to handle the non-relocating 
case, this path ought to be bug-free. But then, I want it to be 
consistent: if the relocation offset is computed in r9, then testing 
whether relocation is needed would be done on r9 once computed, not 
before, by replacing

	adr	r0, _start
	cmp	r0, r6
	beq	clear_bss		/* skip relocation */

With

	adr	r0, _start
	sub	r9, r6, r0
	cmp	r0, #0
	beq	clear_bss		/* skip relocation */

> BTW:  your patch has a number ofd coduing style errors, and the
> Signed-off-by: line is missing.

Plus it did not have the commit message separator either. I suspect it 
was not produced using git format-patch / git send-email.

Anthony, please submit a proper [PATCH], without [RFC], and with the 
setting of r9 done as shown above, and applied to all relevant start.S 
files in arch/arm/cpu/*/ -- in next merge window we should try and 
factorize those start.S files, BTW.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.