[PATCH 0/5] ARM: Berlin: SMP support

[PATCH 0/5] ARM: Berlin: SMP support

[Antoine Ténart]

This series adds the SMP support for Marvell Berlin BG2 and BG2Q.

This implementation takes advantage of the reset exception register and
the software reset address register to make the CPUs execute the Berlin
secondary startup when being being reseted. This has the advantage of
not using the pen lock mechanism.

Antoine Ténart (5):
  ARM: berlin: add SMP support
  Documentation: bindings: document the Marvell Berlin enable-method
  Documentation: bindings: add the Berlin CPU control doc
  ARM: dts: berlin: add SMP related nodes and properties for BG2
  ARM: dts: berlin: add SMP related nodes and properties for BG2Q

 Documentation/devicetree/bindings/arm/cpus.txt     |  1 +
 .../devicetree/bindings/arm/marvell,berlin.txt     | 16 ++++
 arch/arm/boot/dts/berlin2.dtsi                     | 11 +++
 arch/arm/boot/dts/berlin2q.dtsi                    | 11 +++
 arch/arm/mach-berlin/Kconfig                       |  3 +
 arch/arm/mach-berlin/Makefile                      |  3 +-
 arch/arm/mach-berlin/headsmp.S                     | 30 +++++++
 arch/arm/mach-berlin/platsmp.c                     | 99 ++++++++++++++++++++++
 8 files changed, 173 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-berlin/headsmp.S
 create mode 100644 arch/arm/mach-berlin/platsmp.c

-- 
1.9.1

[PATCH 1/5] ARM: berlin: add SMP support

[Antoine Ténart]

Adds SMP support for Berlin SoCs. Secondary CPUs are reseted, then
execute the instruction we put in the reset exception register, setting
the pc at the address contained in the software reset address register,
which is the physical address of the Berlin secondary startup.

This implementation avoid using the pen lock mechanism.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/mach-berlin/Kconfig   |  3 ++
 arch/arm/mach-berlin/Makefile  |  3 +-
 arch/arm/mach-berlin/headsmp.S | 30 +++++++++++++
 arch/arm/mach-berlin/platsmp.c | 99 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-berlin/headsmp.S
 create mode 100644 arch/arm/mach-berlin/platsmp.c

diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
index d3c5f14dc142..e3733692f67a 100644
--- a/arch/arm/mach-berlin/Kconfig
+++ b/arch/arm/mach-berlin/Kconfig
@@ -4,6 +4,7 @@ config ARCH_BERLIN
 	select GENERIC_IRQ_CHIP
 	select DW_APB_ICTL
 	select DW_APB_TIMER_OF
+	select SMP
 
 if ARCH_BERLIN
 
@@ -13,6 +14,7 @@ config MACH_BERLIN_BG2
 	bool "Marvell Armada 1500 (BG2)"
 	select CACHE_L2X0
 	select CPU_PJ4B
+	select HAVE_ARM_SCU if SMP
 	select HAVE_ARM_TWD if SMP
 
 config MACH_BERLIN_BG2CD
@@ -24,6 +26,7 @@ config MACH_BERLIN_BG2Q
 	bool "Marvell Armada 1500 Pro (BG2-Q)"
 	select CACHE_L2X0
 	select CPU_V7
+	select HAVE_ARM_SCU if SMP
 	select HAVE_ARM_TWD if SMP
 	select HAVE_SMP
 
diff --git a/arch/arm/mach-berlin/Makefile b/arch/arm/mach-berlin/Makefile
index ab69fe956f49..c0719ecd1890 100644
--- a/arch/arm/mach-berlin/Makefile
+++ b/arch/arm/mach-berlin/Makefile
@@ -1 +1,2 @@
-obj-y += berlin.o
+obj-y			+= berlin.o
+obj-$(CONFIG_SMP)	+= headsmp.o platsmp.o
diff --git a/arch/arm/mach-berlin/headsmp.S b/arch/arm/mach-berlin/headsmp.S
new file mode 100644
index 000000000000..d295b5185598
--- /dev/null
+++ b/arch/arm/mach-berlin/headsmp.S
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+#include <asm/assembler.h>
+
+ENTRY(berlin_secondary_startup)
+ ARM_BE8(setend be)
+	bl	v7_invalidate_l1
+	b       secondary_startup
+ENDPROC(berlin_secondary_startup)
+
+/*
+ * If the following instruction is set in the reset exception register, CPUs
+ * will fetch the value of the software reset address register when being
+ * reseted.
+ */
+.global boot_inst
+boot_inst:
+	ldr	pc, [pc, #140]
+
+	.align
diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
new file mode 100644
index 000000000000..c04c90b81ae3
--- /dev/null
+++ b/arch/arm/mach-berlin/platsmp.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include <asm/cacheflush.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+#define CPU_RESET		0x00
+
+#define RESET_VECT		0x00
+#define SW_RESET_ADDR		0x94
+
+extern void berlin_secondary_startup(void);
+extern u32 boot_inst;
+
+static void __iomem *cpu_ctrl;
+
+static inline void berlin_reset_cpu(unsigned int cpu)
+{
+	u32 val;
+
+	val = readl(cpu_ctrl + CPU_RESET);
+	val |= BIT(cpu_logical_map(cpu));
+	writel(val, cpu_ctrl + CPU_RESET);
+}
+
+static int berlin_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	if (!cpu_ctrl)
+		return -EFAULT;
+
+	/*
+	 * Reset the CPU, making it to execute the instruction in the reset
+	 * exception register.
+	 */
+	berlin_reset_cpu(cpu);
+
+	return 0;
+}
+
+static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *np;
+	void __iomem *scu_base;
+	void __iomem *vectors_base;
+
+	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+	scu_base = of_iomap(np, 0);
+	of_node_put(np);
+	if (!scu_base)
+		return;
+
+	np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
+	cpu_ctrl = of_iomap(np, 0);
+	of_node_put(np);
+	if (!cpu_ctrl)
+		goto unmap_scu;
+
+	vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
+	if (!vectors_base)
+		goto unmap_scu;
+
+	scu_enable(scu_base);
+	flush_cache_all();
+
+	/*
+	 * Write the first instruction the CPU will execute after being reseted
+	 * in the reset exception register.
+	 */
+	writel(boot_inst, vectors_base + RESET_VECT);
+
+	/*
+	 * Write the secondary startup address into the SW reset address
+	 * register. This is used by boot_inst.
+	 */
+	writel(virt_to_phys(berlin_secondary_startup), vectors_base + SW_RESET_ADDR);
+
+	iounmap(vectors_base);
+unmap_scu:
+	iounmap(scu_base);
+}
+
+static struct smp_operations berlin_smp_ops __initdata = {
+	.smp_prepare_cpus	= berlin_smp_prepare_cpus,
+	.smp_boot_secondary	= berlin_boot_secondary,
+};
+CPU_METHOD_OF_DECLARE(berlin_smp, "marvell,berlin-smp", &berlin_smp_ops);
-- 
1.9.1

[PATCH 2/5] Documentation: bindings: document the Marvell Berlin enable-method

[Antoine Ténart]

The SMP support for Marvell Berlin SoCs introduce a new enable-method.
Document it.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 Documentation/devicetree/bindings/arm/cpus.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 333f4aea3029..50ec9ae6de28 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -185,6 +185,7 @@ nodes to be present and contain the properties described below.
 			    "qcom,gcc-msm8660"
 			    "qcom,kpss-acc-v1"
 			    "qcom,kpss-acc-v2"
+			    "marvell,berlin-smp"
 
 	- cpu-release-addr
 		Usage: required for systems that have an "enable-method"
-- 
1.9.1

[PATCH 3/5] Documentation: bindings: add the Berlin CPU control doc

[Antoine Ténart]

Document the CPU control compatible, needed for the SMP support on
Marvell Berlin SoCs.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 Documentation/devicetree/bindings/arm/marvell,berlin.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/marvell,berlin.txt b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
index 0677003e1476..7f9b3ccdf25b 100644
--- a/Documentation/devicetree/bindings/arm/marvell,berlin.txt
+++ b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
@@ -23,3 +23,19 @@ SoC and board used. Currently known SoC compatibles are:
 
 	...
 }
+
+* Marvell Berlin CPU control bindings
+
+CPU control register allows various operations on CPUs, like resetting them
+independently.
+
+Required properties:
+- compatible: should be "marvell,berlin-cpu-ctrl"
+- reg: address and length of the register set
+
+Example:
+
+cpu-ctrl@f7dd0000 {
+	compatible = "marvell,berlin-cpu-ctrl";
+	reg = <0xf7dd0000 0x10000>;
+};
-- 
1.9.1

[PATCH 4/5] ARM: dts: berlin: add SMP related nodes and properties for BG2

[Antoine Ténart]

Add required nodes and properties into the Berlin BG2 device tree to
take advantage of the newly introduced SMP support. Add the scu and
cpu-ctrl nodes along with the CPUs enable-method property.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 56a1af2f1052..f38ddda8a75e 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -21,6 +21,7 @@
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
+		enable-method = "marvell,berlin-smp";
 
 		cpu@0 {
 			compatible = "marvell,pj4b";
@@ -72,6 +73,11 @@
 			cache-level = <2>;
 		};
 
+		scu: snoop-control-unit@ad0000 {
+			compatible = "arm,cortex-a9-scu";
+			reg = <0xad0000 0x58>;
+		};
+
 		gic: interrupt-controller@ad1000 {
 			compatible = "arm,cortex-a9-gic";
 			reg = <0xad1000 0x1000>, <0xad0100 0x0100>;
@@ -86,6 +92,11 @@
 			clocks = <&sysclk>;
 		};
 
+		cpu-ctrl@dd0000 {
+			compatible = "marvell,berlin-cpu-ctrl";
+			reg = <0xdd0000 0x10000>;
+		};
+
 		apb@e80000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
1.9.1

[PATCH 5/5] ARM: dts: berlin: add SMP related nodes and properties for BG2Q

[Antoine Ténart]

Add required nodes and properties into the Berlin BG2Q device tree to
take advantage of the newly introduced SMP support. Add the scu and
cpu-ctrl nodes along with the CPUs enable-method property.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 07452a7483fa..570f12f7e9c4 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -17,6 +17,7 @@
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
+		enable-method = "marvell,berlin-smp";
 
 		cpu@0 {
 			compatible = "arm,cortex-a9";
@@ -94,6 +95,11 @@
 			interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		scu: snoop-control-unit@ad0000 {
+			compatible = "arm,cortex-a9-scu";
+			reg = <0xad0000 0x58>;
+		};
+
 		gic: interrupt-controller@ad1000 {
 			compatible = "arm,cortex-a9-gic";
 			reg = <0xad1000 0x1000>, <0xad0100 0x100>;
@@ -101,6 +107,11 @@
 			#interrupt-cells = <3>;
 		};
 
+		cpu-ctrl@dd0000 {
+			compatible = "marvell,berlin-cpu-ctrl";
+			reg = <0xdd0000 0x10000>;
+		};
+
 		apb@e80000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
1.9.1

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Russell King - ARM Linux]

On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> index d3c5f14dc142..e3733692f67a 100644
> --- a/arch/arm/mach-berlin/Kconfig
> +++ b/arch/arm/mach-berlin/Kconfig
> @@ -4,6 +4,7 @@ config ARCH_BERLIN
>  	select GENERIC_IRQ_CHIP
>  	select DW_APB_ICTL
>  	select DW_APB_TIMER_OF
> +	select SMP

Please don't add selects of symbols which aren't absolutely necessary.

Since this has been merged without SMP support and presumably works without
SMP support, it would appear that SMP support is not mandatory for the
platform to work.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Antoine Ténart]

On Mon, Jun 02, 2014 at 10:29:13AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> > diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> > index d3c5f14dc142..e3733692f67a 100644
> > --- a/arch/arm/mach-berlin/Kconfig
> > +++ b/arch/arm/mach-berlin/Kconfig
> > @@ -4,6 +4,7 @@ config ARCH_BERLIN
> >  	select GENERIC_IRQ_CHIP
> >  	select DW_APB_ICTL
> >  	select DW_APB_TIMER_OF
> > +	select SMP
> 
> Please don't add selects of symbols which aren't absolutely necessary.
> 
> Since this has been merged without SMP support and presumably works without
> SMP support, it would appear that SMP support is not mandatory for the
> platform to work.

It can work without SMP, I'll remove this.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 0/5] ARM: Berlin: SMP support

[Andrew Lunn]

On Mon, Jun 02, 2014 at 11:21:01AM +0200, Antoine Ténart wrote:
> This series adds the SMP support for Marvell Berlin BG2 and BG2Q.
> 
> This implementation takes advantage of the reset exception register and
> the software reset address register to make the CPUs execute the Berlin
> secondary startup when being being reseted. This has the advantage of

Hi Antoine.

One of these funny rules of English is that there is no word
'reseted'. It is just 'reset'. Also, one 'being' is enough!

	   Andrew

Re: [PATCH 0/5] ARM: Berlin: SMP support

[Antoine Ténart]

Hi Andrew,

On Mon, Jun 02, 2014 at 11:35:32AM +0200, Andrew Lunn wrote:
> On Mon, Jun 02, 2014 at 11:21:01AM +0200, Antoine Ténart wrote:
> > This series adds the SMP support for Marvell Berlin BG2 and BG2Q.
> > 
> > This implementation takes advantage of the reset exception register and
> > the software reset address register to make the CPUs execute the Berlin
> > secondary startup when being being reseted. This has the advantage of
> 
> One of these funny rules of English is that there is no word
> 'reseted'. It is just 'reset'. Also, one 'being' is enough!

Good to know! I'll update s/reseted/reset/ in all patches.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Andrew Lunn]

On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> +
> +static inline void berlin_reset_cpu(unsigned int cpu)
> +{
> +	u32 val;
> +
> +	val = readl(cpu_ctrl + CPU_RESET);
> +	val |= BIT(cpu_logical_map(cpu));
> +	writel(val, cpu_ctrl + CPU_RESET);
> +}

Hi Antoine

Is this performing a reset on the CPU, or is it taking it out of reset?

If you are going to implement CPU hotplug at some point, you are going
to want to be able to put the CPU into reset, i.e. power it off, and
take it out of reset, i.e. power it on and getting it running. So it
might help if we get these function names clear now.

      Andrew


> +
> +static int berlin_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	if (!cpu_ctrl)
> +		return -EFAULT;
> +
> +	/*
> +	 * Reset the CPU, making it to execute the instruction in the reset
> +	 * exception register.
> +	 */
> +	berlin_reset_cpu(cpu);
> +
> +	return 0;
> +}
> +
> +static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *np;
> +	void __iomem *scu_base;
> +	void __iomem *vectors_base;
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> +	scu_base = of_iomap(np, 0);
> +	of_node_put(np);
> +	if (!scu_base)
> +		return;
> +
> +	np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
> +	cpu_ctrl = of_iomap(np, 0);
> +	of_node_put(np);
> +	if (!cpu_ctrl)
> +		goto unmap_scu;
> +
> +	vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
> +	if (!vectors_base)
> +		goto unmap_scu;
> +
> +	scu_enable(scu_base);
> +	flush_cache_all();
> +
> +	/*
> +	 * Write the first instruction the CPU will execute after being reseted
> +	 * in the reset exception register.
> +	 */
> +	writel(boot_inst, vectors_base + RESET_VECT);
> +
> +	/*
> +	 * Write the secondary startup address into the SW reset address
> +	 * register. This is used by boot_inst.

Maybe it would be better to call this the reset address vector instead
of address register. It is in the vector space after all.

   Andrew

> +	 */
> +	writel(virt_to_phys(berlin_secondary_startup), vectors_base + SW_RESET_ADDR);
> +
> +	iounmap(vectors_base);
> +unmap_scu:
> +	iounmap(scu_base);
> +}
> +
> +static struct smp_operations berlin_smp_ops __initdata = {
> +	.smp_prepare_cpus	= berlin_smp_prepare_cpus,
> +	.smp_boot_secondary	= berlin_boot_secondary,
> +};
> +CPU_METHOD_OF_DECLARE(berlin_smp, "marvell,berlin-smp", &berlin_smp_ops);
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Antoine Ténart]

Hi Andrew,

On Mon, Jun 02, 2014 at 11:47:15AM +0200, Andrew Lunn wrote:
> On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> > +
> > +static inline void berlin_reset_cpu(unsigned int cpu)
> > +{
> > +	u32 val;
> > +
> > +	val = readl(cpu_ctrl + CPU_RESET);
> > +	val |= BIT(cpu_logical_map(cpu));
> > +	writel(val, cpu_ctrl + CPU_RESET);
> > +}
> 
> Is this performing a reset on the CPU, or is it taking it out of reset?
> 
> If you are going to implement CPU hotplug at some point, you are going
> to want to be able to put the CPU into reset, i.e. power it off, and
> take it out of reset, i.e. power it on and getting it running. So it
> might help if we get these function names clear now.

It is performing a reset on the CPU. berlin_perform_reset_cpu() then?

> > +
> > +static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > +	struct device_node *np;
> > +	void __iomem *scu_base;
> > +	void __iomem *vectors_base;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> > +	scu_base = of_iomap(np, 0);
> > +	of_node_put(np);
> > +	if (!scu_base)
> > +		return;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
> > +	cpu_ctrl = of_iomap(np, 0);
> > +	of_node_put(np);
> > +	if (!cpu_ctrl)
> > +		goto unmap_scu;
> > +
> > +	vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
> > +	if (!vectors_base)
> > +		goto unmap_scu;
> > +
> > +	scu_enable(scu_base);
> > +	flush_cache_all();
> > +
> > +	/*
> > +	 * Write the first instruction the CPU will execute after being reseted
> > +	 * in the reset exception register.
> > +	 */
> > +	writel(boot_inst, vectors_base + RESET_VECT);
> > +
> > +	/*
> > +	 * Write the secondary startup address into the SW reset address
> > +	 * register. This is used by boot_inst.
> 
> Maybe it would be better to call this the reset address vector instead
> of address register. It is in the vector space after all.

I don't have a strong opinion on this. I can update the 'reset exception
register' as well then.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Andrew Lunn]

On Mon, Jun 02, 2014 at 12:00:48PM +0200, Antoine Ténart wrote:
> Hi Andrew,
> 
> On Mon, Jun 02, 2014 at 11:47:15AM +0200, Andrew Lunn wrote:
> > On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> > > +
> > > +static inline void berlin_reset_cpu(unsigned int cpu)
> > > +{
> > > +	u32 val;
> > > +
> > > +	val = readl(cpu_ctrl + CPU_RESET);
> > > +	val |= BIT(cpu_logical_map(cpu));
> > > +	writel(val, cpu_ctrl + CPU_RESET);
> > > +}
> > 
> > Is this performing a reset on the CPU, or is it taking it out of reset?
> > 
> > If you are going to implement CPU hotplug at some point, you are going
> > to want to be able to put the CPU into reset, i.e. power it off, and
> > take it out of reset, i.e. power it on and getting it running. So it
> > might help if we get these function names clear now.
> 
> It is performing a reset on the CPU. berlin_perform_reset_cpu() then?

Yes, that would be better.

What happens if the CPU is powered off? Will a reset power it on?  Or
are you assuming the boot loader has powered it on?

    Andrew

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Antoine Ténart]

On Mon, Jun 02, 2014 at 12:03:32PM +0200, Andrew Lunn wrote:
> On Mon, Jun 02, 2014 at 12:00:48PM +0200, Antoine Ténart wrote:
> > Hi Andrew,
> > 
> > On Mon, Jun 02, 2014 at 11:47:15AM +0200, Andrew Lunn wrote:
> > > On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> > > > +
> > > > +static inline void berlin_reset_cpu(unsigned int cpu)
> > > > +{
> > > > +	u32 val;
> > > > +
> > > > +	val = readl(cpu_ctrl + CPU_RESET);
> > > > +	val |= BIT(cpu_logical_map(cpu));
> > > > +	writel(val, cpu_ctrl + CPU_RESET);
> > > > +}
> > > 
> > > Is this performing a reset on the CPU, or is it taking it out of reset?
> > > 
> > > If you are going to implement CPU hotplug at some point, you are going
> > > to want to be able to put the CPU into reset, i.e. power it off, and
> > > take it out of reset, i.e. power it on and getting it running. So it
> > > might help if we get these function names clear now.
> > 
> > It is performing a reset on the CPU. berlin_perform_reset_cpu() then?
> 
> What happens if the CPU is powered off? Will a reset power it on?  Or
> are you assuming the boot loader has powered it on?

I actually don't have information about this. For now let's assume the
boot loader has powered on the CPUs.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/5] Documentation: bindings: document the Marvell Berlin enable-method

[Mark Rutland]

On Mon, Jun 02, 2014 at 10:21:03AM +0100, Antoine Ténart wrote:
> The SMP support for Marvell Berlin SoCs introduce a new enable-method.
> Document it.
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 333f4aea3029..50ec9ae6de28 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -185,6 +185,7 @@ nodes to be present and contain the properties described below.
>  			    "qcom,gcc-msm8660"
>  			    "qcom,kpss-acc-v1"
>  			    "qcom,kpss-acc-v2"
> +			    "marvell,berlin-smp"

And what does this mean?

The existing bindings are ambiguous enough. It would be nice to have a
proper document describing the method, as Alex Elder is trying to sort
out for the existing bindings.

Cheers,
Mark.

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Jisheng Zhang]

Hi Andrew, Antoine, 

On Mon, 2 Jun 2014 03:27:50 -0700
Antoine Ténart <antoine.tenart@free-electrons.com> wrote:

> On Mon, Jun 02, 2014 at 12:03:32PM +0200, Andrew Lunn wrote:
> > On Mon, Jun 02, 2014 at 12:00:48PM +0200, Antoine Ténart wrote:
> > > Hi Andrew,
> > > 
> > > On Mon, Jun 02, 2014 at 11:47:15AM +0200, Andrew Lunn wrote:
> > > > On Mon, Jun 02, 2014 at 11:21:02AM +0200, Antoine Ténart wrote:
> > > > > +
> > > > > +static inline void berlin_reset_cpu(unsigned int cpu)
> > > > > +{
> > > > > +	u32 val;
> > > > > +
> > > > > +	val = readl(cpu_ctrl + CPU_RESET);
> > > > > +	val |= BIT(cpu_logical_map(cpu));
> > > > > +	writel(val, cpu_ctrl + CPU_RESET);
> > > > > +}
> > > > 
> > > > Is this performing a reset on the CPU, or is it taking it out of
> > > > reset?
> > > > 
> > > > If you are going to implement CPU hotplug at some point, you are going
> > > > to want to be able to put the CPU into reset, i.e. power it off, and
> > > > take it out of reset, i.e. power it on and getting it running. So it
> > > > might help if we get these function names clear now.
> > > 
> > > It is performing a reset on the CPU. berlin_perform_reset_cpu() then?
> > 
> > What happens if the CPU is powered off? Will a reset power it on?  Or
> > are you assuming the boot loader has powered it on?
> 
> I actually don't have information about this. For now let's assume the
> boot loader has powered on the CPUs.
> 

We don't support independently power off cpu on BG2 and BG2Q in linux kernel.
So we can assume that bootloader has powered on the CPUs

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Jisheng Zhang]

Hi Antoine,

On Mon, 2 Jun 2014 02:21:02 -0700
Antoine Ténart <antoine.tenart@free-electrons.com> wrote:

> Adds SMP support for Berlin SoCs. Secondary CPUs are reseted, then
> execute the instruction we put in the reset exception register, setting
> the pc at the address contained in the software reset address register,
> which is the physical address of the Berlin secondary startup.
> 
> This implementation avoid using the pen lock mechanism.
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> ---
>  arch/arm/mach-berlin/Kconfig   |  3 ++
>  arch/arm/mach-berlin/Makefile  |  3 +-
>  arch/arm/mach-berlin/headsmp.S | 30 +++++++++++++
>  arch/arm/mach-berlin/platsmp.c | 99
> ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 134
> insertions(+), 1 deletion(-) create mode 100644
> arch/arm/mach-berlin/headsmp.S create mode 100644
> arch/arm/mach-berlin/platsmp.c
> 
> diff --git a/arch/arm/mach-berlin/Kconfig b/arch/arm/mach-berlin/Kconfig
> index d3c5f14dc142..e3733692f67a 100644
> --- a/arch/arm/mach-berlin/Kconfig
> +++ b/arch/arm/mach-berlin/Kconfig
> @@ -4,6 +4,7 @@ config ARCH_BERLIN
>  	select GENERIC_IRQ_CHIP
>  	select DW_APB_ICTL
>  	select DW_APB_TIMER_OF
> +	select SMP
>  
>  if ARCH_BERLIN
>  
> @@ -13,6 +14,7 @@ config MACH_BERLIN_BG2
>  	bool "Marvell Armada 1500 (BG2)"
>  	select CACHE_L2X0
>  	select CPU_PJ4B
> +	select HAVE_ARM_SCU if SMP
>  	select HAVE_ARM_TWD if SMP
>  
>  config MACH_BERLIN_BG2CD
> @@ -24,6 +26,7 @@ config MACH_BERLIN_BG2Q
>  	bool "Marvell Armada 1500 Pro (BG2-Q)"
>  	select CACHE_L2X0
>  	select CPU_V7
> +	select HAVE_ARM_SCU if SMP
>  	select HAVE_ARM_TWD if SMP
>  	select HAVE_SMP
>  
> diff --git a/arch/arm/mach-berlin/Makefile b/arch/arm/mach-berlin/Makefile
> index ab69fe956f49..c0719ecd1890 100644
> --- a/arch/arm/mach-berlin/Makefile
> +++ b/arch/arm/mach-berlin/Makefile
> @@ -1 +1,2 @@
> -obj-y += berlin.o
> +obj-y			+= berlin.o
> +obj-$(CONFIG_SMP)	+= headsmp.o platsmp.o
> diff --git a/arch/arm/mach-berlin/headsmp.S b/arch/arm/mach-berlin/headsmp.S
> new file mode 100644
> index 000000000000..d295b5185598
> --- /dev/null
> +++ b/arch/arm/mach-berlin/headsmp.S
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/linkage.h>
> +#include <linux/init.h>
> +#include <asm/assembler.h>
> +
> +ENTRY(berlin_secondary_startup)
> + ARM_BE8(setend be)
> +	bl	v7_invalidate_l1
> +	b       secondary_startup
> +ENDPROC(berlin_secondary_startup)
> +
> +/*
> + * If the following instruction is set in the reset exception register,
> CPUs
> + * will fetch the value of the software reset address register when being
> + * reseted.
> + */
> +.global boot_inst
> +boot_inst:
> +	ldr	pc, [pc, #140]
> +
> +	.align
> diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
> new file mode 100644
> index 000000000000..c04c90b81ae3
> --- /dev/null
> +++ b/arch/arm/mach-berlin/platsmp.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/smp_plat.h>
> +#include <asm/smp_scu.h>
> +
> +#define CPU_RESET		0x00
> +
> +#define RESET_VECT		0x00
> +#define SW_RESET_ADDR		0x94
> +
> +extern void berlin_secondary_startup(void);
> +extern u32 boot_inst;
> +
> +static void __iomem *cpu_ctrl;
> +
> +static inline void berlin_reset_cpu(unsigned int cpu)
> +{
> +	u32 val;
> +
> +	val = readl(cpu_ctrl + CPU_RESET);
> +	val |= BIT(cpu_logical_map(cpu));
> +	writel(val, cpu_ctrl + CPU_RESET);

"writel(BIT(cpu_logical_map(cpu)), cpu_ctrl + CPU_RESET)" is enough.
we don't need to read and modify, because we writing 0 has no any effect.

> +}
> +
> +static int berlin_boot_secondary(unsigned int cpu, struct task_struct
> *idle) +{
> +	if (!cpu_ctrl)
> +		return -EFAULT;
> +
> +	/*
> +	 * Reset the CPU, making it to execute the instruction in the reset
> +	 * exception register.
> +	 */
> +	berlin_reset_cpu(cpu);
> +
> +	return 0;
> +}
> +
> +static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *np;
> +	void __iomem *scu_base;
> +	void __iomem *vectors_base;
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> +	scu_base = of_iomap(np, 0);
> +	of_node_put(np);
> +	if (!scu_base)
> +		return;
> +
> +	np = of_find_compatible_node(NULL, NULL,
> "marvell,berlin-cpu-ctrl");
> +	cpu_ctrl = of_iomap(np, 0);
> +	of_node_put(np);
> +	if (!cpu_ctrl)
> +		goto unmap_scu;
> +
> +	vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
> +	if (!vectors_base)
> +		goto unmap_scu;
> +
> +	scu_enable(scu_base);
> +	flush_cache_all();
> +
> +	/*
> +	 * Write the first instruction the CPU will execute after being
> reseted
> +	 * in the reset exception register.
> +	 */
> +	writel(boot_inst, vectors_base + RESET_VECT);

Is it better to let bootloader/firmware handle this writing. Then, we can
also remove the tricky boot_inst in headsmp.S.

Many thanks for this patch,
Jisheng

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Antoine Ténart]

Hi Jisheng,

On Tue, Jun 03, 2014 at 02:31:13PM +0800, Jisheng Zhang wrote:
> On Mon, 2 Jun 2014 02:21:02 -0700
> Antoine Ténart <antoine.tenart@free-electrons.com> wrote:
> > +
> > +extern void berlin_secondary_startup(void);
> > +extern u32 boot_inst;
> > +
> > +static void __iomem *cpu_ctrl;
> > +
> > +static inline void berlin_reset_cpu(unsigned int cpu)
> > +{
> > +	u32 val;
> > +
> > +	val = readl(cpu_ctrl + CPU_RESET);
> > +	val |= BIT(cpu_logical_map(cpu));
> > +	writel(val, cpu_ctrl + CPU_RESET);
> 
> "writel(BIT(cpu_logical_map(cpu)), cpu_ctrl + CPU_RESET)" is enough.
> we don't need to read and modify, because we writing 0 has no any effect.

The reset bit is automatically cleared but I dumped the register value
and it wasn't 0x0, that's why I preferred to read first and only set the
reset bit.

> > +
> > +	/*
> > +	 * Write the first instruction the CPU will execute after being
> > reseted
> > +	 * in the reset exception register.
> > +	 */
> > +	writel(boot_inst, vectors_base + RESET_VECT);
> 
> Is it better to let bootloader/firmware handle this writing. Then, we can
> also remove the tricky boot_inst in headsmp.S.

We thought about it, and since it can be difficult to update the
bootloader for some boards, like BG2 based ones, we preferred to include
this here.

Thanks!

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Jisheng Zhang]

On Tue, 3 Jun 2014 00:10:17 -0700
Antoine Ténart <antoine.tenart@free-electrons.com> wrote:

> Hi Jisheng,
> 
> On Tue, Jun 03, 2014 at 02:31:13PM +0800, Jisheng Zhang wrote:
> > On Mon, 2 Jun 2014 02:21:02 -0700
> > Antoine Ténart <antoine.tenart@free-electrons.com> wrote:
> > > +
> > > +extern void berlin_secondary_startup(void);
> > > +extern u32 boot_inst;
> > > +
> > > +static void __iomem *cpu_ctrl;
> > > +
> > > +static inline void berlin_reset_cpu(unsigned int cpu)
> > > +{
> > > +	u32 val;
> > > +
> > > +	val = readl(cpu_ctrl + CPU_RESET);
> > > +	val |= BIT(cpu_logical_map(cpu));
> > > +	writel(val, cpu_ctrl + CPU_RESET);
> > 
> > "writel(BIT(cpu_logical_map(cpu)), cpu_ctrl + CPU_RESET)" is enough.
> > we don't need to read and modify, because we writing 0 has no any effect.
> 
> The reset bit is automatically cleared but I dumped the register value
> and it wasn't 0x0, that's why I preferred to read first and only set the
> reset bit.
> 
> > > +
> > > +	/*
> > > +	 * Write the first instruction the CPU will execute after being
> > > reseted
> > > +	 * in the reset exception register.
> > > +	 */
> > > +	writel(boot_inst, vectors_base + RESET_VECT);
> > 
> > Is it better to let bootloader/firmware handle this writing. Then, we can
> > also remove the tricky boot_inst in headsmp.S.
> 
> We thought about it, and since it can be difficult to update the
> bootloader for some boards, like BG2 based ones, we preferred to include
> this here.
> 
> Thanks!
> 
> Antoine
> 


Got your points. Thanks very much

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Andrew Lunn]

> We don't support independently power off cpu on BG2 and BG2Q in linux kernel.
> So we can assume that bootloader has powered on the CPUs

Can i assume from your answer that the hardware does actually support
independent power off/on of CPUs? It could be added sometime in the
future.

	Andrew

Re: [PATCH 1/5] ARM: berlin: add SMP support

[Jisheng Zhang]

Hi Andrew,

On Tue, 3 Jun 2014 00:50:11 -0700
Andrew Lunn <andrew@lunn.ch> wrote:

> > We don't support independently power off cpu on BG2 and BG2Q in linux
> > kernel. So we can assume that bootloader has powered on the CPUs
> 
> Can i assume from your answer that the hardware does actually support
> independent power off/on of CPUs? It could be added sometime in the
> future.
> 

The BG2 and BG2Q HW doesn't support independent power off/on CPUs.

Sorry for confusion,
Jisheng