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

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hello Tom,

On Thu, 22 Jan 2015 11:20:58 -0500, Tom Rini <trini@ti.com> 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@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.

I don't like all of it, specifically the double call, to cpu_init_cp15
then soc_init, for two reasons:

0) soc_init might touch cp15, but it's hidden in the name whereas it is
   shown in cpu_init_cp15. Either both names should explicitly say they
   are touching cp15, or both should not.

1) we cannot tell if cpu_init_cp15 should always happen before
   soc_init. Having a single call to soc_init, with the understanding
   that it will handle the cpu_init_cp15 call itself, makes it
   possible for soc_init yo decide whether it wants to call
   cpu_init_cp15 first, last or at all.

Amicalement,
-- 
Albert.