Hi Albert,
On Fri, 1 Mar 2013 22:56:50 +0100, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi Benoît,
On Fri, 1 Mar 2013 16:50:44 +0100 (CET), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Hi Albert,
On Friday, March 1, 2013 4:46:07 PM, Albert ARIBAUD wrote:
Hi Benoît,
On Fri, 1 Mar 2013 13:10:40 +0100, Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Factorize start.S code as much as possible.
Functions that may need to be customized for some start.S are defined weak for that purpose.
relocate_code_prepare() and relocate_code_finish() are introduced as hooks to be executed at the beginning and at the end of relocate_code() if needed by some start.S, e.g. for special cache or MMU operations.
NAK.
- I don't like this idea of planting hooks inside relocate-code().
This function is about relocating code, not about MMU stuff. If there are any MMU steps to be performed between calls to board_init_f(), relocate_code() or board_init_r(), I want them laid out as calls of their own right in crt0.S.
I also don't like it. The finish hook was used by SMDK6400 before it was removed, and the prepare hook is still used by pxa.
So is it OK for you if I just drop relocate_code_finish() and move and rename the call to relocate_code_prepare() to crt0.S?
Fine, except for the name: "prepare for relocation" is what every instruction does from board_init_f() return to relocate_code() entry. This 'hook' does only a small part, if at all, of preparing for relocation, and this part must get a less improper name. If we are enabling the I-cache here, then let's name the function accordingly. Better yet, let us find out if we do need to enable the instruction cache here at all.
- If we're going to factorize out relocate_code() from the various
start.S files, I want it moved not in crt0.S but in its own file.
It is not in crt0.S, but in arch/arm/include/asm/start_marco.S, which is almost its own file apart from another macro.
I do not want it as a macro. It is and should stay a function. Regarding your added comment:
Sorry for the mixup here. Drop the "Regarding..." just above...
Actually, I'd stopped dead at the relocate_code() changes, but the other macros I don't like much either; I don't see the point of it.
To be faire, I don't see the point of the whole patch wrt the objective.
... it should go here.
And in case you ask, with relocate_code() as a function in its own file instead of a macro called from start.S, it does not work because of the _start-relative word values that require relocate_code() to be in _start's section.
How does it "not work" exactly?
This way, i) people can easily create binaries which use crt0.S but do not relocate, ii) people who want to make relocate_code() a C function will have it easier, and iii) crt0.S keeps being the ugly ASM glue needed for flash inits, relocation and RAM inits to have a C proper run-time environment.
Which is already the case with this implementation?
Not with relocate_code() as a macro, though.
This whole thing/way of "factorizing" start.S using macros feels wrong to me; this departs from what I have started with crt0.S, where code is actually really factorized in a single file which is actually a compilation unit, not a helper file.
Do you need patch 31/31 in your series?
Incidentally, CC:ing Simon:
Signed-off-by: Benoît Thébaudeau benoit.thebaudeau@advansee.com
Changes in v8:
- New patch.
Changes in v7: None Changes in v6: None Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None
Is this produced by patman?
Yes [...]
Ok, then, don't bother to fix patman's behavior manually in your own patches -- I'll try and see if I can submit a patch to fix patman itself.
Best regards, Benoît
Amicalement,
Amicalement,