Hi Albert, On Fri, 1 Mar 2013 22:56:50 +0100, Albert ARIBAUD <[email protected]> wrote:
> Hi Benoît, > > On Fri, 1 Mar 2013 16:50:44 +0100 (CET), Benoît Thébaudeau > <[email protected]> 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 > > > <[email protected]> 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. > > > > > > 1. 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. > > > > 2. 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 <[email protected]> > > > > --- > > > > 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, -- Albert. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
