[PATCH v2 0/3] Add code to release secondary cores from holding pen.

[PATCH v2 0/3] Add code to release secondary cores from holding pen.

[Peter Griffin]

Hi Maxime, Patrice, Srini, Kevin, Olof & Arnd,

This patchset adds in the necessary code to manage the holding pen for STi
platforms.

Due to all the STi upstream devs using JTAG to boot the STi boards, there
is currently a SMP bug when booting upstream kernels via u-boot where
only the primary core is brought up.

This hasn't been noticed until now because when booting via JTAG the
stlinux_arm_boot script sets the PC of the secondary cores directly.

With these patches applied booting via u-boot now works correctly.

[    0.045456] CPU: Testing write buffer coherency: ok
[    0.045597] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    0.045734] Setting up static identity map for 0x40209000 - 0x40209098
[    0.065047] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[    0.065081] Brought up 2 CPUs
[    0.065089] SMP: Total of 2 processors activated (5983.43 BogoMIPS).
[    0.065092] CPU: All CPU(s) started in SVC mode.

Kevin / Olof / Arnd - Can you put this fix into the next ARM SoC fixes pull
request (if there is one)?

regards,

Peter.

Changes since v1:
 - Add STiH418 cpu-release-addr property (Max)
 - Remove trailing '\n' (Max)
 - Add Maximes Acks (Max)

Peter Griffin (3):
  ARM: STi: Add code to release secondary cores from holding pen.
  ARM: STi: DT: STiH407: Add cpu-release-addr dt property.
  ARM: STi: DT: STiH418: Add cpu-release-addr dt property.

 arch/arm/boot/dts/stih407-family.dtsi |  4 +++
 arch/arm/boot/dts/stih418.dtsi        |  4 +++
 arch/arm/mach-sti/headsmp.S           |  1 +
 arch/arm/mach-sti/platsmp.c           | 55 +++++++++++++++++++++++++++++++++--
 arch/arm/mach-sti/smp.h               |  2 ++
 5 files changed, 63 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] ARM: STi: Add code to release secondary cores from holding pen.

[Peter Griffin]

Most upstream devs boot STi platform via JTAG which abuses the
boot process by setting the PC of secondary cores directly. As
a consquence, booting STi platforms via u-boot results in only
the primary core being brought up as the code to manage the
holding pen is not upstream.

This patch adds the necessary code to bring the secondary cores
out of the holding pen. It uses the cpu-release-addr DT property
to get the address of the holding pen from the bootloader.

With this patch booting upstream kernels via u-boot works
correctly:

[    0.045456] CPU: Testing write buffer coherency: ok
[    0.045597] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    0.045734] Setting up static identity map for 0x40209000 - 0x40209098
[    0.065047] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[    0.065081] Brought up 2 CPUs
[    0.065089] SMP: Total of 2 processors activated (5983.43 BogoMIPS).
[    0.065092] CPU: All CPU(s) started in SVC mode.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Maxime Coquelin <maxime.coquelin@st.com>
---
 arch/arm/mach-sti/headsmp.S |  1 +
 arch/arm/mach-sti/platsmp.c | 55 ++++++++++++++++++++++++++++++++++++++++++---
 arch/arm/mach-sti/smp.h     |  2 ++
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-sti/headsmp.S b/arch/arm/mach-sti/headsmp.S
index 4c09bae..e0ad4517 100644
--- a/arch/arm/mach-sti/headsmp.S
+++ b/arch/arm/mach-sti/headsmp.S
@@ -37,6 +37,7 @@ pen:	ldr	r7, [r6]
 	 * should now contain the SVC stack for this core
 	 */
 	b	secondary_startup
+ENDPROC(sti_secondary_startup)
 
 1:	.long	.
 	.long	pen_release
diff --git a/arch/arm/mach-sti/platsmp.c b/arch/arm/mach-sti/platsmp.c
index d4b624f..86bb48d8 100644
--- a/arch/arm/mach-sti/platsmp.c
+++ b/arch/arm/mach-sti/platsmp.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/memblock.h>
 
 #include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
@@ -99,14 +100,62 @@ static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
 {
-	void __iomem *scu_base = NULL;
-	struct device_node *np = of_find_compatible_node(
-					NULL, NULL, "arm,cortex-a9-scu");
+	struct device_node *np;
+	void __iomem *scu_base;
+	u32 __iomem *cpu_strt_ptr;
+	u32 release_phys;
+	int cpu;
+	unsigned long entry_pa = virt_to_phys(sti_secondary_startup);
+
+	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+
 	if (np) {
 		scu_base = of_iomap(np, 0);
 		scu_enable(scu_base);
 		of_node_put(np);
 	}
+
+	if (max_cpus <= 1)
+		return;
+
+	for_each_possible_cpu(cpu) {
+
+		np = of_get_cpu_node(cpu, NULL);
+
+		if (!np)
+			continue;
+
+		if (of_property_read_u32(np, "cpu-release-addr",
+						&release_phys)) {
+			pr_err("CPU %d: missing or invalid cpu-release-addr "
+				"property\n", cpu);
+			continue;
+		}
+
+		/*
+		 * holding pen is usually configured in SBC DMEM but can also be
+		 * in RAM.
+		 */
+
+		if (!memblock_is_memory(release_phys))
+			cpu_strt_ptr =
+				ioremap(release_phys, sizeof(release_phys));
+		else
+			cpu_strt_ptr =
+				(u32 __iomem *)phys_to_virt(release_phys);
+
+		__raw_writel(entry_pa, cpu_strt_ptr);
+
+		/*
+		 * wmb so that data is actually written
+		 * before cache flush is done
+		 */
+		smp_wmb();
+		sync_cache_w(cpu_strt_ptr);
+
+		if (!memblock_is_memory(release_phys))
+			iounmap(cpu_strt_ptr);
+	}
 }
 
 struct smp_operations __initdata sti_smp_ops = {
diff --git a/arch/arm/mach-sti/smp.h b/arch/arm/mach-sti/smp.h
index 1871b72..ae22707 100644
--- a/arch/arm/mach-sti/smp.h
+++ b/arch/arm/mach-sti/smp.h
@@ -14,4 +14,6 @@
 
 extern struct smp_operations	sti_smp_ops;
 
+void sti_secondary_startup(void);
+
 #endif
-- 
1.9.1

[PATCH v2 2/3] ARM: STi: DT: STiH407: Add cpu-release-addr dt property.

[Peter Griffin]

To enable SMP when booting via u-boot we need to specify the
newly implemented cpu-release-addr DT property.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Maxime Coquelin <maxime.coquelin@st.com>
---
 arch/arm/boot/dts/stih407-family.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index c06a546..3c90227 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -19,11 +19,15 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <0>;
+			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
+			cpu-release-addr = <0x94100A4>;
 		};
 		cpu@1 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <1>;
+			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
+			cpu-release-addr = <0x94100A4>;
 		};
 	};
 
-- 
1.9.1

[PATCH v2 3/3] ARM: STi: DT: STiH418: Add cpu-release-addr dt property.

[Peter Griffin]

To enable SMP when booting via u-boot we need to specify the
newly implemented cpu-release-addr DT property for cores 2 & 3.
Cores 0 & 1 are inherited from stih407-family.dtsi.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stih418.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/stih418.dtsi b/arch/arm/boot/dts/stih418.dtsi
index 354d90f..8160a75 100644
--- a/arch/arm/boot/dts/stih418.dtsi
+++ b/arch/arm/boot/dts/stih418.dtsi
@@ -17,11 +17,15 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <2>;
+			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
+			cpu-release-addr = <0x94100A4>;
 		};
 		cpu@3 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a9";
 			reg = <3>;
+			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
+			cpu-release-addr = <0x94100A4>;
 		};
 	};
 
-- 
1.9.1

Re: [PATCH v2 1/3] ARM: STi: Add code to release secondary cores from holding pen.

[Russell King - ARM Linux]

On Tue, Jun 09, 2015 at 02:33:42PM +0100, Peter Griffin wrote:
> Most upstream devs boot STi platform via JTAG which abuses the
> boot process by setting the PC of secondary cores directly. As
> a consquence, booting STi platforms via u-boot results in only
> the primary core being brought up as the code to manage the
> holding pen is not upstream.

Looking at the current mainline code:

static void sti_secondary_init(unsigned int cpu)
{
        trace_hardirqs_off();

Why is this necessary?  If things aren't correctly setup in generic code,
please report a bug against the generic code rather than working around
the problem in platform specific code.  This is the Linux kernel, not
some closed source project.

What bug are you seeing which required the addition of that?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2 0/3] Add code to release secondary cores from holding pen.

[Kevin Hilman]

Peter Griffin <peter.griffin@linaro.org> writes:

> Hi Maxime, Patrice, Srini, Kevin, Olof & Arnd,
>
> This patchset adds in the necessary code to manage the holding pen for STi
> platforms.
>
> Due to all the STi upstream devs using JTAG to boot the STi boards, there
> is currently a SMP bug when booting upstream kernels via u-boot where
> only the primary core is brought up.
>
> This hasn't been noticed until now because when booting via JTAG the
> stlinux_arm_boot script sets the PC of the secondary cores directly.
>
> With these patches applied booting via u-boot now works correctly.
>
> [    0.045456] CPU: Testing write buffer coherency: ok
> [    0.045597] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
> [    0.045734] Setting up static identity map for 0x40209000 - 0x40209098
> [    0.065047] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
> [    0.065081] Brought up 2 CPUs
> [    0.065089] SMP: Total of 2 processors activated (5983.43 BogoMIPS).
> [    0.065092] CPU: All CPU(s) started in SVC mode.
>
> Kevin / Olof / Arnd - Can you put this fix into the next ARM SoC fixes pull
> request (if there is one)?

At this stage of the cycle (we're already at -rc7) we limit things to
strictly regression fixes, and seems this has been broken for some time,
so I think this will have to wait for v4.2.

After addresing the comments, Maxime should queue this up for arm-soc so
we can get it in early in the v4.2-rc cycle.

Kevin

Re: [PATCH v2 1/3] ARM: STi: Add code to release secondary cores from holding pen.

[Peter Griffin]

Hi Russell,

On Tue, 09 Jun 2015, Russell King - ARM Linux wrote:
> On Tue, Jun 09, 2015 at 02:33:42PM +0100, Peter Griffin wrote:
> > Most upstream devs boot STi platform via JTAG which abuses the
> > boot process by setting the PC of secondary cores directly. As
> > a consquence, booting STi platforms via u-boot results in only
> > the primary core being brought up as the code to manage the
> > holding pen is not upstream.
> 
> Looking at the current mainline code:
> 
> static void sti_secondary_init(unsigned int cpu)
> {
>         trace_hardirqs_off();
> 
> Why is this necessary?

After having a dig around, the comment at the top of the file says
this implementation was originally cloned from arch/arm/mach-vexpress/platsmp.c.

After some googling I can see that you submitted this patch
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-December/033678.html
which consolidated this call back in December 2010 from the platform specific
secondary startup code into the common SMP code.

At this time, the STi implementation was being maintained 'out of tree', so
although I can't say for certain, I strongly suspect that this could be why
the call is there, and that when this implementation was submitted in 2013
it was missed during the review process.

> If things aren't correctly setup in generic code,
> please report a bug against the generic code rather than working around
> the problem in platform specific code.  This is the Linux kernel, not
> some closed source project.

100% agree. This is a good example of why upstreaming code is a good idea.

> What bug are you seeing which required the addition of that?

None that I'm aware of, however I'm not the original author or submitter of
the code. I just booted several times with this call removed and everything
appears to work OK.

As I suspect it is due to the implementation being 'out of tree' at the time
of your patch, I will submit another patch which removes this call.

regards,

Peter.

Re: [PATCH v2 0/3] Add code to release secondary cores from holding pen.

[Peter Griffin]

Hi Kevin,

On Tue, 09 Jun 2015, Kevin Hilman wrote:
<snip>
> > Kevin / Olof / Arnd - Can you put this fix into the next ARM SoC fixes pull
> > request (if there is one)?
> 
> At this stage of the cycle (we're already at -rc7) we limit things to
> strictly regression fixes, and seems this has been broken for some time,

Yes your right it has been broken for some time. 

> so I think this will have to wait for v4.2.

Ok
> 
> After addresing the comments, Maxime should queue this up for arm-soc so
> we can get it in early in the v4.2-rc cycle.

Yes Ok, sounds good.

regards,

Peter.

Re: [PATCH v2 0/3] Add code to release secondary cores from holding pen.

[Maxime Coquelin]

Hi Kevin,

On 06/09/2015 05:43 PM, Kevin Hilman wrote:
> Peter Griffin <peter.griffin@linaro.org> writes:
>
>> Hi Maxime, Patrice, Srini, Kevin, Olof & Arnd,
>>
>> This patchset adds in the necessary code to manage the holding pen for STi
>> platforms.
>>
>> Due to all the STi upstream devs using JTAG to boot the STi boards, there
>> is currently a SMP bug when booting upstream kernels via u-boot where
>> only the primary core is brought up.
>>
>> This hasn't been noticed until now because when booting via JTAG the
>> stlinux_arm_boot script sets the PC of the secondary cores directly.
>>
>> With these patches applied booting via u-boot now works correctly.
>>
>> [    0.045456] CPU: Testing write buffer coherency: ok
>> [    0.045597] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
>> [    0.045734] Setting up static identity map for 0x40209000 - 0x40209098
>> [    0.065047] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
>> [    0.065081] Brought up 2 CPUs
>> [    0.065089] SMP: Total of 2 processors activated (5983.43 BogoMIPS).
>> [    0.065092] CPU: All CPU(s) started in SVC mode.
>>
>> Kevin / Olof / Arnd - Can you put this fix into the next ARM SoC fixes pull
>> request (if there is one)?
> At this stage of the cycle (we're already at -rc7) we limit things to
> strictly regression fixes, and seems this has been broken for some time,
> so I think this will have to wait for v4.2.
>
> After addresing the comments, Maxime should queue this up for arm-soc so
> we can get it in early in the v4.2-rc cycle.

Ok, I will take it for v4.2-rc cycle.

Thanks,
Maxime

Re: [PATCH v2 2/3] ARM: STi: DT: STiH407: Add cpu-release-addr dt property.

[Maxime Coquelin]

HI Peter,

On 06/09/2015 03:33 PM, Peter Griffin wrote:
> To enable SMP when booting via u-boot we need to specify the
> newly implemented cpu-release-addr DT property.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Maxime Coquelin <maxime.coquelin@st.com>
> ---
>   arch/arm/boot/dts/stih407-family.dtsi | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
> index c06a546..3c90227 100644
> --- a/arch/arm/boot/dts/stih407-family.dtsi
> +++ b/arch/arm/boot/dts/stih407-family.dtsi
> @@ -19,11 +19,15 @@
>   			device_type = "cpu";
>   			compatible = "arm,cortex-a9";
>   			reg = <0>;
> +			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
> +			cpu-release-addr = <0x94100A4>;
>   		};
>   		cpu@1 {
>   			device_type = "cpu";
>   			compatible = "arm,cortex-a9";
>   			reg = <1>;
> +			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
> +			cpu-release-addr = <0x94100A4>;
>   		};
>   	};
>   

Patch applied for v4.3.

Thanks,
Maxime

Re: [PATCH v2 3/3] ARM: STi: DT: STiH418: Add cpu-release-addr dt property.

[Maxime Coquelin]

HI Peter,

On 06/09/2015 03:33 PM, Peter Griffin wrote:
> To enable SMP when booting via u-boot we need to specify the
> newly implemented cpu-release-addr DT property for cores 2 & 3.
> Cores 0 & 1 are inherited from stih407-family.dtsi.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>   arch/arm/boot/dts/stih418.dtsi | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih418.dtsi b/arch/arm/boot/dts/stih418.dtsi
> index 354d90f..8160a75 100644
> --- a/arch/arm/boot/dts/stih418.dtsi
> +++ b/arch/arm/boot/dts/stih418.dtsi
> @@ -17,11 +17,15 @@
>   			device_type = "cpu";
>   			compatible = "arm,cortex-a9";
>   			reg = <2>;
> +			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
> +			cpu-release-addr = <0x94100A4>;
>   		};
>   		cpu@3 {
>   			device_type = "cpu";
>   			compatible = "arm,cortex-a9";
>   			reg = <3>;
> +			/* u-boot puts hpen in SBC dmem at 0xa4 offset */
> +			cpu-release-addr = <0x94100A4>;
>   		};
>   	};
>   

Patch applied for v4.3.

Thanks,
Maxime

Re: [PATCH v2 1/3] ARM: STi: Add code to release secondary cores from holding pen.

[Maxime Coquelin]

Hi Peter,

On 06/09/2015 03:33 PM, Peter Griffin wrote:
> Most upstream devs boot STi platform via JTAG which abuses the
> boot process by setting the PC of secondary cores directly. As
> a consquence, booting STi platforms via u-boot results in only
> the primary core being brought up as the code to manage the
> holding pen is not upstream.
>
> This patch adds the necessary code to bring the secondary cores
> out of the holding pen. It uses the cpu-release-addr DT property
> to get the address of the holding pen from the bootloader.
>
> With this patch booting upstream kernels via u-boot works
> correctly:
>
> [    0.045456] CPU: Testing write buffer coherency: ok
> [    0.045597] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
> [    0.045734] Setting up static identity map for 0x40209000 - 0x40209098
> [    0.065047] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
> [    0.065081] Brought up 2 CPUs
> [    0.065089] SMP: Total of 2 processors activated (5983.43 BogoMIPS).
> [    0.065092] CPU: All CPU(s) started in SVC mode.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Maxime Coquelin <maxime.coquelin@st.com>
> ---
>   arch/arm/mach-sti/headsmp.S |  1 +
>   arch/arm/mach-sti/platsmp.c | 55 ++++++++++++++++++++++++++++++++++++++++++---
>   arch/arm/mach-sti/smp.h     |  2 ++
>   3 files changed, 55 insertions(+), 3 deletions(-)
>
>

Applied to sti-soc-for-v4.3.

Thanks!
Maxime