Le 20/09/2011 20:09, Wolfgang Denk a écrit :
Dear "GROYER, Anthony",
In messageBC0A2F434D4F39448D24A68EA6EFFB9F0194D534@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,