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

[Hans de Goede <hdegoede@redhat.com>]

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.

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.


> FWIW I notice there is some OMAP specific code right before the "bl
> cpu_init_cp15", although that looks like an even more special case, so
> perhaps not really precedent.
>
>> This does not seem to make any noticable difference, but:
>
> "noticeable"
>
>> 1) According to the manual it is the right thing to do
>> 2) We need to do other magic really early on for sun9i (A80) support, so we
>>     need to introduce a lowlevel_init.S / save_boot_params function anyways
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   arch/arm/cpu/armv7/sunxi/Makefile        |  1 +
>>   arch/arm/cpu/armv7/sunxi/board.c         |  8 --------
>>   arch/arm/cpu/armv7/sunxi/lowlevel_init.S | 23 +++++++++++++++++++++++
>>   3 files changed, 24 insertions(+), 8 deletions(-)
>>   create mode 100644 arch/arm/cpu/armv7/sunxi/lowlevel_init.S
>>
>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
>> index 1720f7d..3a6aa6d 100644
>> --- a/arch/arm/cpu/armv7/sunxi/Makefile
>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
>> @@ -7,6 +7,7 @@
>>   #
>>   # SPDX-License-Identifier:	GPL-2.0+
>>   #
>> +obj-y	+= lowlevel_init.o
>>   obj-y	+= timer.o
>>   obj-y	+= board.o
>>   obj-y	+= clock.o
>> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
>> index bc98c56..4449942 100644
>> --- a/arch/arm/cpu/armv7/sunxi/board.c
>> +++ b/arch/arm/cpu/armv7/sunxi/board.c
>> @@ -120,14 +120,6 @@ void s_init(void)
>>   	 * access gets messed up (seems cache related) */
>>   	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
>>   #endif
>> -#if !defined CONFIG_SPL_BUILD && (defined CONFIG_MACH_SUN7I || \
>> -		defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I)
>> -	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
>> -	asm volatile(
>> -		"mrc p15, 0, r0, c1, c0, 1\n"
>> -		"orr r0, r0, #1 << 6\n"
>> -		"mcr p15, 0, r0, c1, c0, 1\n");
>> -#endif
>>
>>   	clock_init();
>>   	timer_init();
>> diff --git a/arch/arm/cpu/armv7/sunxi/lowlevel_init.S b/arch/arm/cpu/armv7/sunxi/lowlevel_init.S
>> new file mode 100644
>> index 0000000..b80b3eb
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv7/sunxi/lowlevel_init.S
>> @@ -0,0 +1,23 @@
>> +/*
>> + * (C) Copyright 2015 Hans de Goede <hdegoede@redhat.com>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <config.h>
>> +#include <version.h>
>> +#include <linux/linkage.h>
>> +
>> +/*
>> + * On sunxi we need to do some setup *before* cpu_init_cp15 from start.S
>> + * runs, we (ab)use save_boot_params for this.
>> + */
>> +ENTRY(save_boot_params)
>> +#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN7I || \
>> +    defined CONFIG_MACH_SUN8I
>> +	mrc	p15, 0, r0, c1, c0, 1
>> +	orr	r0, r0, #(1<<6)
>> +	mcr	p15, 0, r0, c1, c0, 1
>> +#endif
>> +	bx	lr
>> +ENDPROC(save_boot_params)
>
>

Regards,

Hans