diff mbox

[U-Boot,2/9] ARM: Factor out reusable psci_cpu_entry

Message ID 1dc8d9e0c2b8fa245aa26580000fb4f30af5da13.1423949325.git.jan.kiszka@web.de
State Superseded
Delegated to: Tom Warren
Headers show

Commit Message

Jan Kiszka Feb. 14, 2015, 9:28 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

_sunxi_cpu_entry can be converted completely into a reusable
psci_cpu_entry. Tegra124 will use it as well.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/arm/cpu/armv7/psci.S       | 19 +++++++++++++++++++
 arch/arm/cpu/armv7/sunxi/psci.S | 21 ++-------------------
 2 files changed, 21 insertions(+), 19 deletions(-)

Comments

Chen-Yu Tsai Feb. 15, 2015, 2:01 a.m. UTC | #1
Hi,

On Sun, Feb 15, 2015 at 5:28 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> _sunxi_cpu_entry can be converted completely into a reusable
> psci_cpu_entry. Tegra124 will use it as well.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/arm/cpu/armv7/psci.S       | 19 +++++++++++++++++++
>  arch/arm/cpu/armv7/sunxi/psci.S | 21 ++-------------------
>  2 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S
> index d688607..e916d71 100644
> --- a/arch/arm/cpu/armv7/psci.S
> +++ b/arch/arm/cpu/armv7/psci.S
> @@ -170,4 +170,23 @@ ENTRY(psci_cpu_off_common)
>         bx      lr
>  ENDPROC(psci_cpu_off_common)
>
> +ENTRY(psci_cpu_entry)
> +       @ Set SMP bit
> +       mrc     p15, 0, r0, c1, c0, 1           @ ACTLR
> +       orr     r0, r0, #(1 << 6)               @ Set SMP bit
> +       mcr     p15, 0, r0, c1, c0, 1           @ ACTLR
> +       isb
> +
> +       bl      _nonsec_init
> +       bl      psci_arch_init
> +
> +       adr     r0, _psci_target_pc
> +       ldr     r0, [r0]
> +       b       _do_nonsec_entry
> +ENDPROC(psci_cpu_entry)
> +
> +.globl _psci_target_pc
> +_psci_target_pc:
> +       .word   0

The sunxi version didn't have a per-core target_pc variable.
It is still the case here. Is this the correct way to implement
it? I see per-core storage of this in some of the kernel's smp
ops.

On sunxi it works because the only platform using it only has
one secondary core.


ChenYu

> +
>         .popsection
> diff --git a/arch/arm/cpu/armv7/sunxi/psci.S b/arch/arm/cpu/armv7/sunxi/psci.S
> index 6785fdd..c3a8dc1 100644
> --- a/arch/arm/cpu/armv7/sunxi/psci.S
> +++ b/arch/arm/cpu/armv7/sunxi/psci.S
> @@ -138,7 +138,7 @@ out:        mcr     p15, 0, r7, c1, c1, 0
>         @ r2 = target PC
>  .globl psci_cpu_on
>  psci_cpu_on:
> -       adr     r0, _target_pc
> +       ldr     r0, =_psci_target_pc
>         str     r2, [r0]
>         dsb
>
> @@ -150,7 +150,7 @@ psci_cpu_on:
>         mov     r4, #1
>         lsl     r4, r4, r1
>
> -       adr     r6, _sunxi_cpu_entry
> +       ldr     r6, =psci_cpu_entry
>         str     r6, [r0, #0x1a4] @ PRIVATE_REG (boot vector)
>
>         @ Assert reset on target CPU
> @@ -196,23 +196,6 @@ psci_cpu_on:
>         mov     r0, #ARM_PSCI_RET_SUCCESS       @ Return PSCI_RET_SUCCESS
>         mov     pc, lr
>
> -_target_pc:
> -       .word   0
> -
> -_sunxi_cpu_entry:
> -       @ Set SMP bit
> -       mrc     p15, 0, r0, c1, c0, 1
> -       orr     r0, r0, #0x40
> -       mcr     p15, 0, r0, c1, c0, 1
> -       isb
> -
> -       bl      _nonsec_init
> -       bl      psci_arch_init
> -
> -       adr     r0, _target_pc
> -       ldr     r0, [r0]
> -       b       _do_nonsec_entry
> -
>  .globl psci_cpu_off
>  psci_cpu_off:
>         bl      psci_cpu_off_common
> --
> 2.1.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Jan Kiszka Feb. 15, 2015, 6:29 a.m. UTC | #2
On 2015-02-15 03:01, Chen-Yu Tsai wrote:
> Hi,
> 
> On Sun, Feb 15, 2015 at 5:28 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> _sunxi_cpu_entry can be converted completely into a reusable
>> psci_cpu_entry. Tegra124 will use it as well.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  arch/arm/cpu/armv7/psci.S       | 19 +++++++++++++++++++
>>  arch/arm/cpu/armv7/sunxi/psci.S | 21 ++-------------------
>>  2 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S
>> index d688607..e916d71 100644
>> --- a/arch/arm/cpu/armv7/psci.S
>> +++ b/arch/arm/cpu/armv7/psci.S
>> @@ -170,4 +170,23 @@ ENTRY(psci_cpu_off_common)
>>         bx      lr
>>  ENDPROC(psci_cpu_off_common)
>>
>> +ENTRY(psci_cpu_entry)
>> +       @ Set SMP bit
>> +       mrc     p15, 0, r0, c1, c0, 1           @ ACTLR
>> +       orr     r0, r0, #(1 << 6)               @ Set SMP bit
>> +       mcr     p15, 0, r0, c1, c0, 1           @ ACTLR
>> +       isb
>> +
>> +       bl      _nonsec_init
>> +       bl      psci_arch_init
>> +
>> +       adr     r0, _psci_target_pc
>> +       ldr     r0, [r0]
>> +       b       _do_nonsec_entry
>> +ENDPROC(psci_cpu_entry)
>> +
>> +.globl _psci_target_pc
>> +_psci_target_pc:
>> +       .word   0
> 
> The sunxi version didn't have a per-core target_pc variable.
> It is still the case here. Is this the correct way to implement
> it? I see per-core storage of this in some of the kernel's smp
> ops.
> 
> On sunxi it works because the only platform using it only has
> one secondary core.
> 

With homogeneous SMP, it probably works as well because reset vectors
may not differ across the cores. But this remains a valid point.

I'm considering to push this variable to the top of the per-CPU stack.
Calculating the stack position is actually another function to factor out.

Thanks,
Jan
Jan Kiszka Feb. 15, 2015, 9:46 a.m. UTC | #3
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2015-02-15 07:29, Jan Kiszka wrote:
> On 2015-02-15 03:01, Chen-Yu Tsai wrote:
>> Hi,
>> 
>> On Sun, Feb 15, 2015 at 5:28 AM, Jan Kiszka <jan.kiszka@web.de>
>> wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>> 
>>> _sunxi_cpu_entry can be converted completely into a reusable 
>>> psci_cpu_entry. Tegra124 will use it as well.
>>> 
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- 
>>> arch/arm/cpu/armv7/psci.S       | 19 +++++++++++++++++++ 
>>> arch/arm/cpu/armv7/sunxi/psci.S | 21 ++------------------- 2
>>> files changed, 21 insertions(+), 19 deletions(-)
>>> 
>>> diff --git a/arch/arm/cpu/armv7/psci.S
>>> b/arch/arm/cpu/armv7/psci.S index d688607..e916d71 100644 ---
>>> a/arch/arm/cpu/armv7/psci.S +++ b/arch/arm/cpu/armv7/psci.S @@
>>> -170,4 +170,23 @@ ENTRY(psci_cpu_off_common) bx      lr 
>>> ENDPROC(psci_cpu_off_common)
>>> 
>>> +ENTRY(psci_cpu_entry) +       @ Set SMP bit +       mrc
>>> p15, 0, r0, c1, c0, 1           @ ACTLR +       orr     r0, r0,
>>> #(1 << 6)               @ Set SMP bit +       mcr     p15, 0,
>>> r0, c1, c0, 1           @ ACTLR +       isb + +       bl
>>> _nonsec_init +       bl      psci_arch_init + +       adr
>>> r0, _psci_target_pc +       ldr     r0, [r0] +       b
>>> _do_nonsec_entry +ENDPROC(psci_cpu_entry) + +.globl
>>> _psci_target_pc +_psci_target_pc: +       .word   0
>> 
>> The sunxi version didn't have a per-core target_pc variable. It
>> is still the case here. Is this the correct way to implement it?
>> I see per-core storage of this in some of the kernel's smp ops.
>> 
>> On sunxi it works because the only platform using it only has one
>> secondary core.
>> 
> 
> With homogeneous SMP, it probably works as well because reset
> vectors may not differ across the cores. But this remains a valid
> point.
> 
> I'm considering to push this variable to the top of the per-CPU
> stack. Calculating the stack position is actually another function
> to factor out.
> 

https://github.com/siemens/u-boot/commits/jetson-tk1-v2

works fine on the TK1, but I'd like to give it a try on a Banana Pi as
well (currently out of reach) before reposting.

Jan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlTgawwACgkQitSsb3rl5xQCqgCg1cQM3fGHRbU4VhvHlfvwMkFa
2MwAnRy3lYcAXeCYiCfk8h5WlRLxIxRx
=tcEk
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S
index d688607..e916d71 100644
--- a/arch/arm/cpu/armv7/psci.S
+++ b/arch/arm/cpu/armv7/psci.S
@@ -170,4 +170,23 @@  ENTRY(psci_cpu_off_common)
 	bx	lr
 ENDPROC(psci_cpu_off_common)
 
+ENTRY(psci_cpu_entry)
+	@ Set SMP bit
+	mrc	p15, 0, r0, c1, c0, 1		@ ACTLR
+	orr	r0, r0, #(1 << 6)		@ Set SMP bit
+	mcr	p15, 0, r0, c1, c0, 1		@ ACTLR
+	isb
+
+	bl	_nonsec_init
+	bl	psci_arch_init
+
+	adr	r0, _psci_target_pc
+	ldr	r0, [r0]
+	b	_do_nonsec_entry
+ENDPROC(psci_cpu_entry)
+
+.globl _psci_target_pc
+_psci_target_pc:
+	.word	0
+
 	.popsection
diff --git a/arch/arm/cpu/armv7/sunxi/psci.S b/arch/arm/cpu/armv7/sunxi/psci.S
index 6785fdd..c3a8dc1 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.S
+++ b/arch/arm/cpu/armv7/sunxi/psci.S
@@ -138,7 +138,7 @@  out:	mcr	p15, 0, r7, c1, c1, 0
 	@ r2 = target PC
 .globl	psci_cpu_on
 psci_cpu_on:
-	adr	r0, _target_pc
+	ldr	r0, =_psci_target_pc
 	str	r2, [r0]
 	dsb
 
@@ -150,7 +150,7 @@  psci_cpu_on:
 	mov	r4, #1
 	lsl	r4, r4, r1
 
-	adr	r6, _sunxi_cpu_entry
+	ldr	r6, =psci_cpu_entry
 	str	r6, [r0, #0x1a4] @ PRIVATE_REG (boot vector)
 
 	@ Assert reset on target CPU
@@ -196,23 +196,6 @@  psci_cpu_on:
 	mov	r0, #ARM_PSCI_RET_SUCCESS	@ Return PSCI_RET_SUCCESS
 	mov	pc, lr
 
-_target_pc:
-	.word	0
-
-_sunxi_cpu_entry:
-	@ Set SMP bit
-	mrc	p15, 0, r0, c1, c0, 1
-	orr	r0, r0, #0x40
-	mcr	p15, 0, r0, c1, c0, 1
-	isb
-
-	bl	_nonsec_init
-	bl	psci_arch_init
-
-	adr	r0, _target_pc
-	ldr	r0, [r0]
-	b	_do_nonsec_entry
-
 .globl	psci_cpu_off
 psci_cpu_off:
 	bl	psci_cpu_off_common