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

[Hans de Goede]

Hi,

On 20-01-15 11:22, Albert ARIBAUD wrote:
> 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?)

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 ?

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.

Regards,

Hans