2014-06-02 09:21:11

by Antoine Tenart

[permalink] [raw]
Subject: [PATCH 0/5] ARM: Berlin: SMP support

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


2014-06-02 09:21:15

by Antoine Tenart

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

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

Signed-off-by: Antoine Ténart <[email protected]>
---
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

2014-06-02 09:21:13

by Antoine Tenart

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

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 <[email protected]>
---
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 <[email protected]>
+ *
+ * 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 <[email protected]>
+ *
+ * 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

2014-06-02 09:21:42

by Antoine Tenart

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

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 <[email protected]>
---
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

2014-06-02 09:22:20

by Antoine Tenart

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

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 <[email protected]>
---
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

2014-06-02 09:22:39

by Antoine Tenart

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

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

Signed-off-by: Antoine Ténart <[email protected]>
---
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

2014-06-02 09:30:55

by Russell King - ARM Linux

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

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.

2014-06-02 09:35:10

by Antoine Tenart

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

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

2014-06-02 09:39:11

by Andrew Lunn

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

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

2014-06-02 09:44:20

by Antoine Tenart

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

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

2014-06-02 09:50:53

by Andrew Lunn

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

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2014-06-02 10:00:54

by Antoine Tenart

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

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

2014-06-02 10:07:10

by Andrew Lunn

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

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

2014-06-02 10:27:56

by Antoine Tenart

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

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

2014-06-02 10:44:43

by Mark Rutland

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

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 <[email protected]>
> ---
> 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.

2014-06-03 06:22:38

by Jisheng Zhang

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

Hi Andrew, Antoine,

On Mon, 2 Jun 2014 03:27:50 -0700
Antoine Ténart <[email protected]> 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

2014-06-03 06:33:41

by Jisheng Zhang

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

Hi Antoine,

On Mon, 2 Jun 2014 02:21:02 -0700
Antoine Ténart <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>
> + *
> + * 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 <[email protected]>
> + *
> + * 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

2014-06-03 07:10:23

by Antoine Tenart

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

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 <[email protected]> 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

2014-06-03 07:21:55

by Jisheng Zhang

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


On Tue, 3 Jun 2014 00:10:17 -0700
Antoine Ténart <[email protected]> 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 <[email protected]> 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

2014-06-03 07:53:51

by Andrew Lunn

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

> 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

2014-06-03 08:05:17

by Jisheng Zhang

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

Hi Andrew,

On Tue, 3 Jun 2014 00:50:11 -0700
Andrew Lunn <[email protected]> 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