[PATCH v3 0/3] Renesas R9A06G032 SMP enabler

[PATCH v3 0/3] Renesas R9A06G032 SMP enabler

[Michel Pollet]

*WARNING -- this requires the base R9A06G032 support patches 
already posted

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

v3:
  + Removed mentions of rz/?n1d?
  + Rebased on base patch v7 
v2:
  + Added suggestions from Florian Fainelli
  + Use __pa_symbol()
  + Simplified logic in prepare_cpu()
  + Reordered the patches
  + Rebased on RZN1 Base patch v5

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

 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         | 85 ++++++++++++++++++++++++++
 4 files changed, 89 insertions(+)
 create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c

-- 
2.7.4

[PATCH v3 0/3] Renesas R9A06G032 SMP enabler

[Michel Pollet]

*WARNING -- this requires the base R9A06G032 support patches 
already posted

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

v3:
  + Removed mentions of rz/?n1d?
  + Rebased on base patch v7 
v2:
  + Added suggestions from Florian Fainelli
  + Use __pa_symbol()
  + Simplified logic in prepare_cpu()
  + Reordered the patches
  + Rebased on RZN1 Base patch v5

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

 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         | 85 ++++++++++++++++++++++++++
 4 files changed, 89 insertions(+)
 create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c

-- 
2.7.4

[PATCH v3 1/3] dt-bindings: cpu: Add Renesas R9A06G032 SMP enable method.

[Michel Pollet]

Add a special enable method for second CA7 of the R9A06G032

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 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

[PATCH v3 1/3] dt-bindings: cpu: Add Renesas R9A06G032 SMP enable method.

[Michel Pollet]

Add a special enable method for second CA7 of the R9A06G032

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 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

[PATCH v3 1/3] dt-bindings: cpu: Add Renesas R9A06G032 SMP enable method.

[Michel Pollet]

Add a special enable method for second CA7 of the R9A06G032

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 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

[PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Michel Pollet]

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

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
 arch/arm/mach-shmobile/Makefile        |  1 +
 arch/arm/mach-shmobile/smp-r9a06g032.c | 85 ++++++++++++++++++++++++++++++++++
 2 files changed, 86 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..a536e89
--- /dev/null
+++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
@@ -0,0 +1,85 @@
+// 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(__pa_symbol(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.
+	 * The bootloader *does* change this property
+	 */
+	ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
+	of_node_put(dn);
+	if (ret) {
+		pr_err("CPU#1: invalid cpu-release-addr property\n");
+		return;
+	}
+	pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
+
+	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

[PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Michel Pollet]

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

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
---
 arch/arm/mach-shmobile/Makefile        |  1 +
 arch/arm/mach-shmobile/smp-r9a06g032.c | 85 ++++++++++++++++++++++++++++++++++
 2 files changed, 86 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..a536e89
--- /dev/null
+++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
@@ -0,0 +1,85 @@
+// 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(__pa_symbol(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.
+	 * The bootloader *does* change this property
+	 */
+	ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
+	of_node_put(dn);
+	if (ret) {
+		pr_err("CPU#1: invalid cpu-release-addr property\n");
+		return;
+	}
+	pr_info("CPU#1: cpu-release-addr %08x\n", (u32)bootaddr);
+
+	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: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Geert Uytterhoeven]

Hi Michel,

On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
<michel.pollet@bp.renesas.com> wrote:
> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time, it
> requires a special enable method to get it started.
>
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

Thanks for your patch!

>  arch/arm/mach-shmobile/smp-r9a06g032.c | 85 ++++++++++++++++++++++++++++++++++

I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
Source files are not covered by the stable DT ABI, and can be reordered
later at will.

I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
(perhaps with a little bit of extra code to handle deviations).

> --- /dev/null
> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> @@ -0,0 +1,85 @@
> +// 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>

Do you need this?

> +#include <asm/cacheflush.h>
> +#include <asm/smp_plat.h>
> +#include <asm/smp_scu.h>

Do you need these?

> +static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +       struct device_node *dn;
> +       int ret;
> +       u32 bootaddr;

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

The cast is not needed.

> +
> +       cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));
> +       if (!cpu_bootaddr)
> +               pr_err("CPU#1: cpu-release-addr map failed\n");

If this fails, that is basically an OOM condition, which will scream anyway.
So I think you should drop the error message.

> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Geert Uytterhoeven]

Hi Michel,

On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
<michel.pollet@bp.renesas.com> wrote:
> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time, it
> requires a special enable method to get it started.
>
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

Thanks for your patch!

>  arch/arm/mach-shmobile/smp-r9a06g032.c | 85 ++++++++++++++++++++++++++++++++++

I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
Source files are not covered by the stable DT ABI, and can be reordered
later at will.

I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
(perhaps with a little bit of extra code to handle deviations).

> --- /dev/null
> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> @@ -0,0 +1,85 @@
> +// 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>

Do you need this?

> +#include <asm/cacheflush.h>
> +#include <asm/smp_plat.h>
> +#include <asm/smp_scu.h>

Do you need these?

> +static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +       struct device_node *dn;
> +       int ret;
> +       u32 bootaddr;

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

The cast is not needed.

> +
> +       cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));
> +       if (!cpu_bootaddr)
> +               pr_err("CPU#1: cpu-release-addr map failed\n");

If this fails, that is basically an OOM condition, which will scream anyway.
So I think you should drop the error message.

> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Geert Uytterhoeven]

Hi Michel,

On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
<michel.pollet@bp.renesas.com> wrote:
> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time, it
> requires a special enable method to get it started.
>
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

Thanks for your patch!

>  arch/arm/mach-shmobile/smp-r9a06g032.c | 85 ++++++++++++++++++++++++++++++++++

I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
Source files are not covered by the stable DT ABI, and can be reordered
later at will.

I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
(perhaps with a little bit of extra code to handle deviations).

> --- /dev/null
> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> @@ -0,0 +1,85 @@
> +// 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>

Do you need this?

> +#include <asm/cacheflush.h>
> +#include <asm/smp_plat.h>
> +#include <asm/smp_scu.h>

Do you need these?

> +static void __init rzn1_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +       struct device_node *dn;
> +       int ret;
> +       u32 bootaddr;

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

The cast is not needed.

> +
> +       cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr));
> +       if (!cpu_bootaddr)
> +               pr_err("CPU#1: cpu-release-addr map failed\n");

If this fails, that is basically an OOM condition, which will scream anyway.
So I think you should drop the error message.

> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Michel Pollet]

On 25 May 2018 10:49, Geert wrote:
> Subject: Re: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler
> driver
>
> Hi Michel,

Hi Geert,

>
> On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
> <michel.pollet@bp.renesas.com> wrote:
> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
> > it requires a special enable method to get it started.
> >
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>
> Thanks for your patch!
>
> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 85
> > ++++++++++++++++++++++++++++++++++
>
> I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
> Source files are not covered by the stable DT ABI, and can be reordered later
> at will.
>
> I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
> (perhaps with a little bit of extra code to handle deviations).

Now I am completely confused -- you had me remove the mention of rzn1
from everywhere it mattered to handle 'family' cases, and now you are
telling me that in *this* case where there is not a single chance of that file
covering another part and there's a clear cut case for it to be part specific
.... I should call it rzn1?!?

I even already renamed the symbols on my tree to match the
rest for v4...

I'd like consistency -- I *thought* I had a consistent naming scheme before,
now I've moved to your part specific one (under duress), I'd rather stick to
something that is consistent and keep everything as r9a06g032 now.

>
> > --- /dev/null
> > +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> > @@ -0,0 +1,85 @@
> > +// 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>
>
> Do you need this?

Fixed all the other remarks, thanks for that!

Michel




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

RE: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Michel Pollet]

On 25 May 2018 10:49, Geert wrote:
> Subject: Re: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler
> driver
>
> Hi Michel,

Hi Geert,

>
> On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
> <michel.pollet@bp.renesas.com> wrote:
> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
> > it requires a special enable method to get it started.
> >
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>
> Thanks for your patch!
>
> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 85
> > ++++++++++++++++++++++++++++++++++
>
> I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
> Source files are not covered by the stable DT ABI, and can be reordered later
> at will.
>
> I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
> (perhaps with a little bit of extra code to handle deviations).

Now I am completely confused -- you had me remove the mention of rzn1
from everywhere it mattered to handle 'family' cases, and now you are
telling me that in *this* case where there is not a single chance of that file
covering another part and there's a clear cut case for it to be part specific
.... I should call it rzn1?!?

I even already renamed the symbols on my tree to match the
rest for v4...

I'd like consistency -- I *thought* I had a consistent naming scheme before,
now I've moved to your part specific one (under duress), I'd rather stick to
something that is consistent and keep everything as r9a06g032 now.

>
> > --- /dev/null
> > +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> > @@ -0,0 +1,85 @@
> > +// 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>
>
> Do you need this?

Fixed all the other remarks, thanks for that!

Michel




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

[PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Michel Pollet]

On 25 May 2018 10:49, Geert wrote:
> Subject: Re: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler
> driver
>
> Hi Michel,

Hi Geert,

>
> On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
> <michel.pollet@bp.renesas.com> wrote:
> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
> > it requires a special enable method to get it started.
> >
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>
> Thanks for your patch!
>
> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 85
> > ++++++++++++++++++++++++++++++++++
>
> I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
> Source files are not covered by the stable DT ABI, and can be reordered later
> at will.
>
> I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
> (perhaps with a little bit of extra code to handle deviations).

Now I am completely confused -- you had me remove the mention of rzn1
from everywhere it mattered to handle 'family' cases, and now you are
telling me that in *this* case where there is not a single chance of that file
covering another part and there's a clear cut case for it to be part specific
.... I should call it rzn1?!?

I even already renamed the symbols on my tree to match the
rest for v4...

I'd like consistency -- I *thought* I had a consistent naming scheme before,
now I've moved to your part specific one (under duress), I'd rather stick to
something that is consistent and keep everything as r9a06g032 now.

>
> > --- /dev/null
> > +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
> > @@ -0,0 +1,85 @@
> > +// 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>
>
> Do you need this?

Fixed all the other remarks, thanks for that!

Michel




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

Re: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Geert Uytterhoeven]

Hi Michel,

On Wed, May 30, 2018 at 10:19 AM, Michel Pollet
<michel.pollet@bp.renesas.com> wrote:
> On 25 May 2018 10:49, Geert wrote:
>> On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
>> <michel.pollet@bp.renesas.com> wrote:
>> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>> > it requires a special enable method to get it started.
>> >
>> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>
>> Thanks for your patch!
>>
>> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 85
>> > ++++++++++++++++++++++++++++++++++
>>
>> I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
>> Source files are not covered by the stable DT ABI, and can be reordered later
>> at will.
>>
>> I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
>> (perhaps with a little bit of extra code to handle deviations).
>
> Now I am completely confused -- you had me remove the mention of rzn1
> from everywhere it mattered to handle 'family' cases, and now you are
> telling me that in *this* case where there is not a single chance of that file
> covering another part and there's a clear cut case for it to be part specific
> .... I should call it rzn1?!?

Sorry for confusing you.
There's a difference between stable DT ABI and (non-existing) kernel stable ABI.
A driver that handles multiple SoCs can be named after the SoC family.

I agree this one is a bit special, as only RZ/N1D has the dual Cortex A7
(RZ/N1S has a single A7, RZ/N1L has no A7), so for now[*] there's no need
to use it on anything but r9a06g032 aka RZ/N1D.

> I'd like consistency -- I *thought* I had a consistent naming scheme before,
> now I've moved to your part specific one (under duress), I'd rather stick to
> something that is consistent and keep everything as r9a06g032 now.

OK for me. Driver names can be changed any time.

[*] Until e.g. a quad A7 arrives :-)
    Perhaps this can also be used to enable the A7s on both RZ/N1D and
    RZ/N1S when running Linux on the Cortex M3?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Geert Uytterhoeven]

Hi Michel,

On Wed, May 30, 2018 at 10:19 AM, Michel Pollet
<michel.pollet@bp.renesas.com> wrote:
> On 25 May 2018 10:49, Geert wrote:
>> On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
>> <michel.pollet@bp.renesas.com> wrote:
>> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>> > it requires a special enable method to get it started.
>> >
>> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>
>> Thanks for your patch!
>>
>> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 85
>> > ++++++++++++++++++++++++++++++++++
>>
>> I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
>> Source files are not covered by the stable DT ABI, and can be reordered later
>> at will.
>>
>> I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
>> (perhaps with a little bit of extra code to handle deviations).
>
> Now I am completely confused -- you had me remove the mention of rzn1
> from everywhere it mattered to handle 'family' cases, and now you are
> telling me that in *this* case where there is not a single chance of that file
> covering another part and there's a clear cut case for it to be part specific
> .... I should call it rzn1?!?

Sorry for confusing you.
There's a difference between stable DT ABI and (non-existing) kernel stable ABI.
A driver that handles multiple SoCs can be named after the SoC family.

I agree this one is a bit special, as only RZ/N1D has the dual Cortex A7
(RZ/N1S has a single A7, RZ/N1L has no A7), so for now[*] there's no need
to use it on anything but r9a06g032 aka RZ/N1D.

> I'd like consistency -- I *thought* I had a consistent naming scheme before,
> now I've moved to your part specific one (under duress), I'd rather stick to
> something that is consistent and keep everything as r9a06g032 now.

OK for me. Driver names can be changed any time.

[*] Until e.g. a quad A7 arrives :-)
    Perhaps this can also be used to enable the A7s on both RZ/N1D and
    RZ/N1S when running Linux on the Cortex M3?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver

[Geert Uytterhoeven]

Hi Michel,

On Wed, May 30, 2018 at 10:19 AM, Michel Pollet
<michel.pollet@bp.renesas.com> wrote:
> On 25 May 2018 10:49, Geert wrote:
>> On Thu, May 24, 2018 at 12:30 PM, Michel Pollet
>> <michel.pollet@bp.renesas.com> wrote:
>> > The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>> > it requires a special enable method to get it started.
>> >
>> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>
>> Thanks for your patch!
>>
>> >  arch/arm/mach-shmobile/smp-r9a06g032.c | 85
>> > ++++++++++++++++++++++++++++++++++
>>
>> I think you can safely call this driver smp-rzn1d.c, or smp-rzn1.c.
>> Source files are not covered by the stable DT ABI, and can be reordered later
>> at will.
>>
>> I expect you will just add more CPU_METHOD_OF_DECLARE() lines later
>> (perhaps with a little bit of extra code to handle deviations).
>
> Now I am completely confused -- you had me remove the mention of rzn1
> from everywhere it mattered to handle 'family' cases, and now you are
> telling me that in *this* case where there is not a single chance of that file
> covering another part and there's a clear cut case for it to be part specific
> .... I should call it rzn1?!?

Sorry for confusing you.
There's a difference between stable DT ABI and (non-existing) kernel stable ABI.
A driver that handles multiple SoCs can be named after the SoC family.

I agree this one is a bit special, as only RZ/N1D has the dual Cortex A7
(RZ/N1S has a single A7, RZ/N1L has no A7), so for now[*] there's no need
to use it on anything but r9a06g032 aka RZ/N1D.

> I'd like consistency -- I *thought* I had a consistent naming scheme before,
> now I've moved to your part specific one (under duress), I'd rather stick to
> something that is consistent and keep everything as r9a06g032 now.

OK for me. Driver names can be changed any time.

[*] Until e.g. a quad A7 arrives :-)
    Perhaps this can also be used to enable the A7s on both RZ/N1D and
    RZ/N1S when running Linux on the Cortex M3?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds