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

[Albert ARIBAUD]

Hello Hans,

On Tue, 20 Jan 2015 15:32:34 +0100, Hans de Goede <hdegoede at redhat.com>
wrote:
> Hi,
> 
> On 20-01-15 11:22, Albert ARIBAUD wrote:
> > Hello Hans,
> >
> > 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?)
> 
> Hmm, so if I'm reading the above correctly, then I think you want to do
> the following:
> 
> 1) Rename cpu_init_cp15 to cpu_init_cp15_common
> 2) Add a new soc_init function, with a weak default which just calls
>     cpu_init_cp15_common
> 3) Add a a7_init_cp15 which sets the smp bit
> 4) Have Cortex A7 SoCs override soc_init with one which first calls
>     a7_init_cp15 and then calls cpu_init_cp15_common
> 5) And on SoC's which need to do something special before or after
>     cp15 init, they can do so by overriding soc_init and do what
>     ever they need to do there before *or* after calling
>     cpu_init_cp15_common
> 
> Have I got that right ?

Almost entirely. My only comments are on 1) :

- cpu_init_cp15_common does not need the "common" suffix IMO; actually,
  it might be more general than just touching cp15, so we could just
  call it "cpu_init" (1).

- if two CPUs need different versions, then we will want to make
  cpu_init a weak function, with a default based on the 'common
  denominator'.

(1) Note that there is already a cpu_init() function in U-Boot, used by
SH and AVR32; if we want 'cpu_init' to be consistent across architectures,
we might have to change "{soc,cpu}_init" to somehting else (for instance
"{soc,cpu}_setup" or "{soc,cpu}_boot_init") but I don't like that much, or
investigate what the existing cpu_init() does and see if /that/ could be
renamed or merged into a common mechanism (I doubt that the second is
practically feasible).

> If so I can try to write a patch-set for this, my arm asm is a bit
> weak, but I should be able to cobble this together using existing code
> as an example.

Thanks!

> Regards,
> 
> Hans

Amicalement,
-- 
Albert.