Le 20/09/2011 20:09, Wolfgang Denk a écrit :
> Dear "GROYER, Anthony",
>
> In
> message<bc0a2f434d4f39448d24a68ea6effb9f0194d...@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
