[U-Boot] [PATCH 05/12] sunxi: Move setting of CPU system control register SMP bit to save_boot_params

[Albert ARIBAUD]

Hello Hans,

On Mon, 19 Jan 2015 20:04:58 +0100, Hans de Goede <hdegoede at redhat.com>
wrote:
> Hi,
> 
> On 17-01-15 23:51, Ian Campbell wrote:
> > On Thu, 2015-01-15 at 15:52 +0100, Hans de Goede wrote:
> >> According to the "Cortex-A7 MPCore Technical Reference Manual":
> >>
> >> "You must ensure this bit is set to 1 before the caches and MMU are enabled,
> >> or any cache and TLB maintenance operations are performed."
> >
> > Given that this is a feature of the Cortex-A7 (actually, I believe it
> > applies to at least Cortex-A15 too) and not really specific to sunxi,
> > perhaps we can make this more generic?
> 
> Strange enough the bit is different between the A7 and A15, for the A7 the docs
> say it must be set before doing anything with caches, on the A15 it only needs
> to be set for the core to accept cache management operations from other cpu
> cores (or so the docs say), which is likely why it is not in the standard
> init sequence yet, as for u-boot it seems to only be necessary to do this on
> a Cortex A7. I agree that it would be good to move this to the generic start.S
> though, Albert ?

[...]

> >> Since arch/arm/cpu/armv7/start.S: cpu_init_cp15 does several cache operations,
> >> we should thus enable the SMP bit earlier, and the only chance to do that is
> >> to do it at save_boot_params time.
> >
> > Would it be so terrible to add an ifdef CORTEX_A7 here, or to call out
> > to (or call as a macro) a soc_init_cp15?
> 
>  From my pov no that would not be too terrible, but ...
> >
> > I'm cc-ing Albert for input these questions.
> 
> That indeed is Albert's call.

I don't like the idea of #ifdef'ing that much.

OTOH, if we do introduce soc_init_cp15, then we end up with two CP15
init functions, soc_init_cp15 and cpu_init_cp15, with all sorts of
questions on which should be called first and whether one could break
the other's work, and what happens in-between, etc.

Either way, setting CP15 registers is something that all ARM CPUs, and
SoCs require, not just armv7. It's just that other start.S files touch
cp15 directly.

I'm leaning toward grouping all CP15 inits (including cache(s)
and TLB disabling and maybe VBAR setting) in a single CP15 call to
a single soc_init_cp15 function.

Now, SoCs with the same CPU will have a common CP15 init part, and
that part could go into a <cpu>_init_cp15 function which soc_init_cp15
would call. Of course, since we're doing this way before we have any
stack, we will have to handle nested calls by saving and restoring LR
in intermediate function contexts.

> Note that solving this still leaves the A80 magic sram controller poke which
> also needs to happen really really early or otherwise the entire SoC just
> resets as if the watchdog has triggered, I'm fine with using save_boot_params
> for that, it is not its intended purpose, but it works fine for it, so
> I see no reason to complicate things with yet another callback.

Maybe we could turn soc_init_cp15 into a more general soc_init function
which would do whatever is needed, on cp15 or otherwise.

(I see there is one soc_init defined, for spear600, but it is actually
empty and could/should be removed. Patch anyone?)

> Regards,
> 
> Hans

Amicalement,
-- 
Albert.