[RFC 0/3] Renesas RZ/N1D SMP enabler

[RFC 0/3] Renesas RZ/N1D SMP enabler

[Michel Pollet]

*Warning -- this requires the base RZ/N1 support patches already posted *

This is a tentative patch series for enabling the second CA7 of the RZ/N1D.
It's based on a spin_table method, and it reuses the same binding property
as that driver.

One question is: Do i have to document it separately, or is it sufficiently
clear?

Michel Pollet (3):
  dt-bindings: cpu: Add Renesas RZ/N1D SMP enable method.
  ARM: dts: Renesas RZ/N1D SMP enable method
  arm: shmobile: Add the RZ/N1D SMP enabler driver.

 Documentation/devicetree/bindings/arm/cpus.txt |  1 +
 arch/arm/boot/dts/r9a06g032.dtsi               |  2 +
 arch/arm/mach-shmobile/Makefile                |  1 +
 arch/arm/mach-shmobile/smp-r9a06g032.c         | 87 ++++++++++++++++++++++++++
 4 files changed, 91 insertions(+)
 create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c

-- 
2.7.4

[PATCH 1/1] arm: rzn1: Add support for the second CPU.

[Michel Pollet]

This enables starting the second CA7 core. Also handles the case the
bootloader has had to change the second CPU parking address to allow
booting in NONSEC/HYP.

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
 arch/arm/mach-shmobile/Makefile          |  1 +
 arch/arm/mach-shmobile/r9a06g032.h       |  7 +++
 arch/arm/mach-shmobile/setup-r9a06g032.c |  2 +
 arch/arm/mach-shmobile/smp-r9a06g032.c   | 88 ++++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+)
 create mode 100644 arch/arm/mach-shmobile/r9a06g032.h
 create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index a63e5c2..e0f8c97 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -36,6 +36,7 @@ smp-$(CONFIG_ARCH_R8A7779)	+= smp-r8a7779.o headsmp-scu.o platsmp-scu.o
 smp-$(CONFIG_ARCH_R8A7790)	+= smp-r8a7790.o
 smp-$(CONFIG_ARCH_R8A7791)	+= smp-r8a7791.o
 smp-$(CONFIG_ARCH_EMEV2)	+= smp-emev2.o headsmp-scu.o platsmp-scu.o
+smp-$(CONFIG_ARCH_R9A06G032)	+= smp-r9a06g032.o
 
 # PM objects
 obj-$(CONFIG_SUSPEND)		+= suspend.o
diff --git a/arch/arm/mach-shmobile/r9a06g032.h b/arch/arm/mach-shmobile/r9a06g032.h
new file mode 100644
index 0000000..3992e97
--- /dev/null
+++ b/arch/arm/mach-shmobile/r9a06g032.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __R9A06G032_H__
+#define __R9A06G032_H__
+
+extern const struct smp_operations rzn1_smp_ops;
+
+#endif /* __R9A06G032_H__ */
diff --git a/arch/arm/mach-shmobile/setup-r9a06g032.c b/arch/arm/mach-shmobile/setup-r9a06g032.c
index 65288e1..7bc6216 100644
--- a/arch/arm/mach-shmobile/setup-r9a06g032.c
+++ b/arch/arm/mach-shmobile/setup-r9a06g032.c
@@ -11,6 +11,7 @@
 #include <asm/mach/arch.h>
 #include <dt-bindings/soc/renesas,rzn1-map.h>
 #include <soc/rzn1/sysctrl.h>
+#include "r9a06g032.h"
 
 static void __iomem *sysctrl_base_addr;
 
@@ -50,6 +51,7 @@ static const char *rzn1_boards_compat_dt[] __initconst = {
 };
 
 DT_MACHINE_START(RZN1_DT, "Renesas RZ/N1 (Device Tree)")
+	.smp		= smp_ops(rzn1_smp_ops),
 	.dt_compat      = rzn1_boards_compat_dt,
 	.restart	= rzn1_restart,
 MACHINE_END
diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c b/arch/arm/mach-shmobile/smp-r9a06g032.c
new file mode 100644
index 0000000..e441188
--- /dev/null
+++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SMP support for Renesas RZ/N1D
+ *
+ * Copyright (C) 2018 Renesas Electronics Europe Limited
+ *
+ * Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
+ *
+ * Based on code
+ *  Copyright (C) 2012-2013 Allwinner Ltd.
+ *
+ */
+
+#include <linux/of.h>
+#include <soc/rzn1/sysctrl.h>
+#include "r9a06g032.h"
+
+#define BOOTADDR2_CANARY	0x525a4e31
+
+static void __iomem *pen2_base;
+
+static DEFINE_SPINLOCK(cpu_lock);
+
+/*
+ * The alternate boot address for the second core can be overridden in the DT,
+ * typically this will happen if the bootloader decides to park the second
+ * core somewhere else than the fixed ALT_BOOTADDR address.
+ *
+ * This use case is required for NONSEC, as the SYSCTRL BOOTADDR register isn't
+ * available after switching mode, so the bootloader parks the CPU#2 in a pen
+ * is SRAM to await on an alternate address (followed by a canary) then the
+ * bootloader switches mode, and finaly starts the kernel...
+ * The address of that alternate 'register' is passed in /chosen.
+ */
+static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
+{
+	u32 bootaddr = 0;
+	struct device_node *np = of_find_node_by_path("/chosen");
+
+	if (np)
+		of_property_read_u32(np, "rzn1,bootaddr", &bootaddr);
+
+	if (bootaddr &&
+		bootaddr != RZN1_SYSCTRL_REG_BOOTADDR &&
+		bootaddr != (RZN1_SYSTEM_CTRL_BASE+RZN1_SYSCTRL_REG_BOOTADDR)) {
+
+		pr_info("RZ/N1 CPU#2 boot address %08x\n", bootaddr);
+		pen2_base = ioremap(bootaddr, 8);
+
+		if (!pen2_base)
+			pr_warn("Couldn't map RZ/N1 CPU#2 PEN2\n");
+		return;
+	}
+	pr_info("RZ/N1 CPU#2 boot address not specified - using SYSCTRL reg\n");
+}
+
+static int __init rzn1_smp_boot_secondary(unsigned int cpu,
+				    struct task_struct *idle)
+{
+	u32 t = (u32)virt_to_phys(secondary_startup);
+
+	/* Inform on what is the second CPU boot address */
+	if (pen2_base && (readl(pen2_base + 4) == BOOTADDR2_CANARY))
+		pr_info("RZ/N1 CPU#%d writing %08x to boot address\n", cpu, t);
+	else
+		pr_info("RZ/N1 CPU#%d writing %08x to SYSCTRL reg\n", cpu, t);
+
+	spin_lock(&cpu_lock);
+
+	/* Set CPU boot address */
+	if (pen2_base && (readl(pen2_base + 4) == BOOTADDR2_CANARY))
+		writel(virt_to_phys(secondary_startup), pen2_base);
+	else
+		rzn1_sysctrl_writel(virt_to_phys(secondary_startup),
+			RZN1_SYSCTRL_REG_BOOTADDR);
+
+	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+	spin_unlock(&cpu_lock);
+
+	return 0;
+}
+
+const struct smp_operations rzn1_smp_ops __initconst = {
+	.smp_prepare_cpus	= rzn1_smp_prepare_cpus,
+	.smp_boot_secondary	= rzn1_smp_boot_secondary,
+};
+CPU_METHOD_OF_DECLARE(rzn1_smp, "renesas,r9a06g032", &rzn1_smp_ops);
-- 
2.7.4

[RFC 1/3] dt-bindings: cpu: Add Renesas RZ/N1D SMP enable method.

[Michel Pollet]

Add a special enable method for second CA8 of the Renesas RZ/N1D
(R9A06G032).

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.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 29e1dc5..b395d107 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -219,6 +219,7 @@ described below.
 			    "qcom,kpss-acc-v1"
 			    "qcom,kpss-acc-v2"
 			    "renesas,apmu"
+			    "renesas,r9a06g032-smp"
 			    "rockchip,rk3036-smp"
 			    "rockchip,rk3066-smp"
 			    "ste,dbx500-smp"
-- 
2.7.4

Re: [RFC 1/3] dt-bindings: cpu: Add Renesas RZ/N1D SMP enable method.

[Rob Herring]

On Mon, Apr 16, 2018 at 10:34:57AM +0100, Michel Pollet wrote:
> Add a special enable method for second CA8 of the Renesas RZ/N1D
> (R9A06G032).
> 
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring <robh@kernel.org>

[RFC 2/3] ARM: dts: Renesas RZ/N1D SMP enable method

[Michel Pollet]

Add a special enable method for the second CA7 of the Renesas RZ/N1D
(R9A06G032), as well as the default value for the "cpu-release-addr"
property.

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 7d84b38..50f3043d 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -33,6 +33,8 @@
 			device_type = "cpu";
 			compatible = "arm,cortex-a7";
 			reg = <1>;
+			enable-method = "renesas,r9a06g032-smp";
+			cpu-release-addr = <0x4000c204>;
 		};
 	};
 
-- 
2.7.4

[RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver.

[Michel Pollet]

The Renesas RZ/N1D second CA7 is parked in a ROM pen at boot time, it
requires a special enable method to get it started at boot time.

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
 arch/arm/mach-shmobile/Makefile        |  1 +
 arch/arm/mach-shmobile/smp-r9a06g032.c | 87 ++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)
 create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 1939f52..d7fc98f 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)	+= smp-sh73a0.o headsmp-scu.o platsmp-scu.o
 smp-$(CONFIG_ARCH_R8A7779)	+= smp-r8a7779.o headsmp-scu.o platsmp-scu.o
 smp-$(CONFIG_ARCH_R8A7790)	+= smp-r8a7790.o
 smp-$(CONFIG_ARCH_R8A7791)	+= smp-r8a7791.o
+smp-$(CONFIG_ARCH_R9A06G032)	+= smp-r9a06g032.o
 smp-$(CONFIG_ARCH_EMEV2)	+= smp-emev2.o headsmp-scu.o platsmp-scu.o
 
 # PM objects
diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c b/arch/arm/mach-shmobile/smp-r9a06g032.c
new file mode 100644
index 0000000..59ec287
--- /dev/null
+++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RZ/N1D Second CA7 enabler.
+ *
+ * Copyright (C) 2018 Renesas Electronics Europe Limited
+ *
+ * Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
+ * Derived from action,s500-smp
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+#include <asm/cacheflush.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+/*
+ * The second CPU is parked in ROM at boot time. It requires waking it after
+ * writing an address into the BOOTADDR register of sysctrl.
+ *
+ * So the default value of the "cpu-release-addr" corresponds to BOOTADDR...
+ *
+ * *However* the BOOTADDR register is not available when the kernel
+ * starts in NONSEC mode.
+ *
+ * So for NONSEC mode, the bootloader re-parks the second CPU into a pen
+ * in SRAM, and changes the "cpu-release-addr" of linux's DT to a SRAM address,
+ * which is not restricted.
+ */
+
+static void __iomem *cpu_bootaddr;
+
+static DEFINE_SPINLOCK(cpu_lock);
+
+static int rzn1_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	if (!cpu_bootaddr)
+		return -ENODEV;
+
+	spin_lock(&cpu_lock);
+
+	writel(virt_to_phys(secondary_startup), cpu_bootaddr);
+	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+	spin_unlock(&cpu_lock);
+
+	return 0;
+}
+
+static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *dn;
+	int ret;
+	u32 bootaddr;
+
+	dn = of_get_cpu_node(1, NULL);
+	if (!dn) {
+		pr_err("CPU#1: missing device tree node\n");
+		return;
+	}
+	/*
+	 * Determine the address from which the CPU is polling.
+	 */
+	ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
+	if (ret)
+		pr_err("CPU#1: invalid cpu-release-addr property\n");
+
+	of_node_put(dn);
+	/* The bootloader *does* change this property */
+	pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
+
+	if (!bootaddr)
+		return;
+
+	cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));
+	if (!cpu_bootaddr)
+		pr_err("CPU#1: cpu-release-addr map failed\n");
+}
+
+static const struct smp_operations rzn1_smp_ops __initconst = {
+	.smp_prepare_cpus = rzn1_smp_prepare_cpus,
+	.smp_boot_secondary = rzn1_smp_boot_secondary,
+};
+CPU_METHOD_OF_DECLARE(rzn1_smp, "renesas,r9a06g032-smp", &rzn1_smp_ops);
-- 
2.7.4

Re: [RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver.

[Florian Fainelli]

Hi Michel,

On 04/16/2018 02:34 AM, Michel Pollet wrote:
> The Renesas RZ/N1D second CA7 is parked in a ROM pen at boot time, it
> requires a special enable method to get it started at boot time.
> 
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

Some few comments below. This patch should probably be re-ordered in
your patch series, I would expect you to have this become patch 2 and
have patch 2 be patch 3 (first you add infrastructure for using
something, then you make use of it).

> +static int rzn1_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
> +{
> +	if (!cpu_bootaddr)
> +		return -ENODEV;
> +
> +	spin_lock(&cpu_lock);
> +
> +	writel(virt_to_phys(secondary_startup), cpu_bootaddr);

Consider using __pa_symbol() instead of virt_to_phys() since
secondary_startup is part of the kernel's linear memory map.

> +	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> +
> +	spin_unlock(&cpu_lock);
> +
> +	return 0;
> +}
> +
> +static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *dn;
> +	int ret;
> +	u32 bootaddr;
> +
> +	dn = of_get_cpu_node(1, NULL);
> +	if (!dn) {
> +		pr_err("CPU#1: missing device tree node\n");
> +		return;
> +	}
> +	/*
> +	 * Determine the address from which the CPU is polling.
> +	 */
> +	ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
> +	if (ret)
> +		pr_err("CPU#1: invalid cpu-release-addr property\n");
> +
> +	of_node_put(dn);
> +	/* The bootloader *does* change this property */

This comment should probably be moved above the function that fetches
"cpu-release-addr"

> +	pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
> +
> +	if (!bootaddr)
> +		return;

Would not you want to show a message here to help catch such conditions

> +
> +	cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));

Relying on ioremap() to reject values that might be outside of the
allowed range may be a little fragile, but I can't suggest any better
alternative.

> +	if (!cpu_bootaddr)
> +		pr_err("CPU#1: cpu-release-addr map failed\n");
> +}
> +
> +static const struct smp_operations rzn1_smp_ops __initconst = {
> +	.smp_prepare_cpus = rzn1_smp_prepare_cpus,
> +	.smp_boot_secondary = rzn1_smp_boot_secondary,
> +};
> +CPU_METHOD_OF_DECLARE(rzn1_smp, "renesas,r9a06g032-smp", &rzn1_smp_ops);
> 


-- 
Florian

RE: [RFC 3/3] arm: shmobile: Add the RZ/N1D SMP enabler driver.

[Michel Pollet]

Hi Florian,

On 16 April 2018 22:46, Florian Fainelli:
> Hi Michel,
>
> On 04/16/2018 02:34 AM, Michel Pollet wrote:
> > The Renesas RZ/N1D second CA7 is parked in a ROM pen at boot time, it
> > requires a special enable method to get it started at boot time.
> >
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>
> Some few comments below. This patch should probably be re-ordered in
> your patch series, I would expect you to have this become patch 2 and have
> patch 2 be patch 3 (first you add infrastructure for using something, then you
> make use of it).

Thanks, took that onboard for v2

>
> > +static int rzn1_smp_boot_secondary(unsigned int cpu, struct
> > +task_struct *idle) {
> > +if (!cpu_bootaddr)
> > +return -ENODEV;
> > +
> > +spin_lock(&cpu_lock);
> > +
> > +writel(virt_to_phys(secondary_startup), cpu_bootaddr);
>
> Consider using __pa_symbol() instead of virt_to_phys() since
> secondary_startup is part of the kernel's linear memory map.

Agreed.

>
> > +arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> > +
> > +spin_unlock(&cpu_lock);
> > +
> > +return 0;
> > +}
> > +
> > +static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus) {
> > +struct device_node *dn;
> > +int ret;
> > +u32 bootaddr;
> > +
> > +dn = of_get_cpu_node(1, NULL);
> > +if (!dn) {
> > +pr_err("CPU#1: missing device tree node\n");
> > +return;
> > +}
> > +/*
> > + * Determine the address from which the CPU is polling.
> > + */
> > +ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
> > +if (ret)
> > +pr_err("CPU#1: invalid cpu-release-addr property\n");
> > +
> > +of_node_put(dn);
> > +/* The bootloader *does* change this property */
>
> This comment should probably be moved above the function that fetches
> "cpu-release-addr"

Thanks, I've simplified that bit too..
>
> > +pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
> > +
> > +if (!bootaddr)
> > +return;
>
> Would not you want to show a message here to help catch such conditions
>
> > +
> > +cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));
>
> Relying on ioremap() to reject values that might be outside of the allowed
> range may be a little fragile, but I can't suggest any better alternative.

It's difficult, I'd have to add a memory map header I've been trying not to
use so far for KISS reason...

>
> > +if (!cpu_bootaddr)
> > +pr_err("CPU#1: cpu-release-addr map failed\n"); }
> > +
> > +static const struct smp_operations rzn1_smp_ops __initconst = {
> > +.smp_prepare_cpus = rzn1_smp_prepare_cpus,
> > +.smp_boot_secondary = rzn1_smp_boot_secondary, };
> > +CPU_METHOD_OF_DECLARE(rzn1_smp, "renesas,r9a06g032-smp",
> > +&rzn1_smp_ops);
> >
>

Michel




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.