[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Albert ARIBAUD]

Hello Tom,

On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini <trini at ti.com> wrote:
> On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> > Hi,
> > 
> > On 26-01-15 16:18, Tom Rini wrote:
> > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> > >>Hi,
> > >>
> > >>On 22-01-15 22:03, Tom Rini wrote:
> > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> > >>>>Hi,
> > >>>>
> > >>>>On 22-01-15 17:20, Tom Rini wrote:
> > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> > >>>>>
> > >>>>>>On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the
> > >>>>>>icache, etc. Add a soc_init hook with a weak default which just calls
> > >>>>>>cpu_init_cp15.
> > >>>>>>
> > >>>>>>This way different implementations can be provided to do some extra work
> > >>>>>>before or after cpu_init_cp15, or completely replacing cpu_init_cp15.
> > >>>>>>
> > >>>>>>Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> > >>>>>>---
> > >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> > >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> > >>>>>>
> > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > >>>>>>index fdc05b9..9882b20 100644
> > >>>>>>--- a/arch/arm/cpu/armv7/start.S
> > >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> > >>>>>>@@ -64,7 +64,7 @@ reset:
> > >>>>>>
> > >>>>>>  	/* the mask ROM code should have PLL and others stable */
> > >>>>>>  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
> > >>>>>>-	bl	cpu_init_cp15
> > >>>>>>+	bl	soc_init
> > >>>>>>  	bl	cpu_init_crit
> > >>>>>>  #endif
> > >>>>>
> > >>>>>I like the direction here.  And I want to make sure I get the sunxi
> > >>>>>direction right here too (as I agree with the need / desire for boot0 +
> > >>>>>U-Boot to be a valid combination).  I think we can take this a step
> > >>>>>farther.  cpu_init_crit (on armv7) is basically a call to s_init().
> > >>>>>
> > >>>>>For am33xx (and I bet but need to do and test omap3+) we can, with
> > >>>>>Simon's patch to let us move stack to DDR a tiny bit later, in the SPL
> > >>>>>case make s_init empty, which just leaves us with (with your patch)
> > >>>>>soc_init.  Is there some way we can put all of this together in a
> > >>>>>function?
> > >>>>
> > >>>>You mean essentially call s_init here and have s_init call cpu_init_cp15
> > >>>>I guess we could do that, but it would require auditing all existing armv7
> > >>>>users of s_init. This may require me to rethink how / when I do timer &
> > >>>>gpio init etc. for u-boot.bin on sunxi, but that should not be a (big)
> > >>>>problem.
> > >>>
> > >>>Basically.  From my first pass audit of s_init, it's either empty
> > >>>(Kona), sunxi, or omap/etc so I get to deal with it.  And the default
> > >>>soc_init would just be the call to cpu_init_cp15 as you have it and we
> > >>>drop the lowlevel_init hurdles.
> > >>
> > >>Ok, so what you're suggesting is a patch which:
> > >>
> > >>1) Changes:
> > >>
> > >>#ifndef CONFIG_SKIP_LOWLEVEL_INIT
> > >>	bl	cpu_init_cp15
> > >>    	bl	cpu_init_crit
> > >>#endif
> > >>
> > >>Into:
> > >>
> > >>#ifndef CONFIG_SKIP_LOWLEVEL_INIT
> > >>	bl	lowlevel_init
> > >>#endif
> > >>
> > >>Which will setup the stack and then call the s_init C function
> > >>
> > >>2) Adds a weak default s_init which calls cpu_init_cp15
> > >>
> > >>3) Patch all existing s_init functions to call cpu_init_cp15
> > >>before doing anything else.
> > >
> > >Pretty close.  Simon's SPL DM series and related clean-ups got me
> > >thinking that yes, seemingly too much got shoved into "s_init" that
> > >really could have been done using an existing hook done slightly later.
> > >
> > >>And then in follow up patches we can:
> > >>
> > >>4) Drop cpu_init_crit
> > >>
> > >>5) Cleanup some s_init functions (this will be left to the individual
> > >>SoC maintainers)
> > >>
> > >>I think that is a good idea, Albert what do you think about this ?
> > >
> > >So I'd like to see 5 done "soon" afterwards as it's me (omap*) and
> > >sunxi.  I think we can simplfy the call sequence too, to roughly:
> > >#ifndef CONFIG_SKIP_LOWLEVEL_INIT
> > >     ... Set up stack for C, it's just a few instrs
> > >     bl lowlevel_init
> > >#endif
> > >     bl _main
> > >
> > >__weak asm
> > >lowlevel_init:
> > >   bl cpu_init_cp15
> > >   return to caller
> > >
> > >And comment that anything called via lowlevel_init must be C-callable.
> > >I hope that once #5 is done no one actually has a lowlevel_init that's
> > >done in C but we've kept the door open should it be needed down the
> > >road (as I _think_ we can shuffle both the omap* and sunxi stuff to do
> > >their inits as needed in both SPL and full U-Boot from an early hook in
> > >board_init_r, top of my head is board_init calls some_other_func() in
> > >full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls
> > >same func in SPL, and we can consolidate again further down the road as
> > >we get SPL and full U-Boot more in sync on the call chain).
> > 
> > Sounds good to me, and I'm fine with working the sunxi side of things.
> > 
> > Since you seem to have this all in your head can you do a patch for this
> > replacing my patchset ?
> 
> I suppose that's what happens when you have a detailed plan, will do ;)

I'll comment if the patch is out (going through my mail one at a time)
but I just want to chime in on C contexts: I'd rather have this done
only once and from crt0.S. If this means crt0.S must do the call to
lowlevel_init, then so be it IMO.

> -- 
> Tom



Amicalement,
-- 
Albert.