diff mbox

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

Message ID 1421333554-29822-6-git-send-email-hdegoede@redhat.com
State Superseded
Delegated to: Hans de Goede
Headers show

Commit Message

Hans de Goede Jan. 15, 2015, 2:52 p.m. UTC
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."

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.

This does not seem to make any noticable difference, but:
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

Comments

Ian Campbell Jan. 17, 2015, 10:51 p.m. UTC | #1
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?

> 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?

I'm cc-ing Albert for input these questions.

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)
Hans de Goede Jan. 19, 2015, 7:04 p.m. UTC | #2
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
Albert ARIBAUD Jan. 20, 2015, 7:10 a.m. UTC | #3
Hello Hans,

On Mon, 19 Jan 2015 20:04:58 +0100, Hans de Goede <hdegoede@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.

Will look into this today.

Amicalement,
Ian Campbell Jan. 20, 2015, 8:44 a.m. UTC | #4
On Mon, 2015-01-19 at 20:04 +0100, Hans de Goede 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.
> 
> 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.

Ideally it would be possible to do it in the same hook as sets up the
ACTLR.SMP bit.

In general I'm not a big fan of reusing unrelated hooks just because
they happen to be in a convenient location -- it leads to surprises when
you are reading through/modifying the calling code.

Ian.
Albert ARIBAUD Jan. 20, 2015, 10:22 a.m. UTC | #5
Hello Hans,

On Mon, 19 Jan 2015 20:04:58 +0100, Hans de Goede <hdegoede@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,
Hans de Goede Jan. 20, 2015, 2:32 p.m. UTC | #6
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@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
Albert ARIBAUD Jan. 21, 2015, 6:59 a.m. UTC | #7
Hello Hans,

On Tue, 20 Jan 2015 15:32:34 +0100, Hans de Goede <hdegoede@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,
diff mbox

Patch

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)