[PATCH v4 0/5] ARM: SMP: support Broadcom mobile SoCs

[PATCH v4 0/5] ARM: SMP: support Broadcom mobile SoCs

[Alex Elder]

[Trouble posting patches today.  I'm very sorry about the duplicates.]

This series adds SMP support for two Broadcom mobile SoC families.
It uses CPU_METHOD_OF_DECLARE() so that SMP operations are assigned
using device tree rather than adding it to a machine definition in a
board file.

The enable method starts a secondary core by writing to a register
monitored by CPUs spinning in a ROM-based holding pen loop.  The
address of this register is recorded as a property in the "cpus"
node of the device tree.

					-Alex

Note:
This series is based on v3.15-rc4, plus one more recently-posted patch:
    http://permalink.gmane.org/gmane.linux.kernel/1683693

History
v3: - Renamed "platsmp.c" to be "kona_smp.c".
    - Rebased onto v3.15-rc5
v3: - Dropped definition and use of CPU_METHOD_OF_DECLARE_SETUP()
    - Added documentation for "enable-method"
    - Rebased onto v3.15-rc4
v2: - Fixed a Makefile error (:= should have been +=)
    - No longer set CONFIG_NR_CPUS in bcm_defconfig
    - Rebased onto v3.15-rc1

This series is available here:
    http://git.linaro.org/landing-teams/working/broadcom/kernel.git
    Branch review/bcm-smp-v4

Alex Elder (5):
  devicetree: bindings: document Broadcom CPU enable method
  ARM: add SMP support for Broadcom mobile SoCs
  ARM: configs: enable SMP in bcm_defconfig
  ARM: dts: enable SMP support for bcm28155
  ARM: dts: enable SMP support for bcm21664

 Documentation/devicetree/bindings/arm/cpus.txt |  12 ++
 arch/arm/boot/dts/bcm11351.dtsi                |  19 +++
 arch/arm/boot/dts/bcm21664.dtsi                |  19 +++
 arch/arm/configs/bcm_defconfig                 |   1 +
 arch/arm/mach-bcm/Kconfig                      |  18 ++-
 arch/arm/mach-bcm/Makefile                     |   3 +
 arch/arm/mach-bcm/kona_smp.c                   | 202 +++++++++++++++++++++++++
 7 files changed, 271 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mach-bcm/kona_smp.c

-- 
1.9.1

[PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Alex Elder]

Broadcom mobile SoCs use a ROM-implemented holding pen for
controlled boot of secondary cores.  A special register is
used to communicate to the ROM that a secondary core should
start executing kernel code.  This enable method is currently
used for members of the bcm281xx and bcm21664 SoC families.

The use of an enable method also allows the SMP operation vector to
be assigned as a result of device tree content for these SoCs.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 333f4ae..c6a2411 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"
+			    "brcm,bcm11351-cpu-method"
 
 	- cpu-release-addr
 		Usage: required for systems that have an "enable-method"
@@ -209,6 +210,17 @@ nodes to be present and contain the properties described below.
 		Value type: <phandle>
 		Definition: Specifies the ACC[2] node associated with this CPU.
 
+	- secondary-boot-reg
+		Usage:
+			Required for systems that have an "enable-method"
+			property value of "brcm,bcm11351-cpu-method".
+		Value type: <u32>
+		Definition:
+			Specifies the physical address of the register used to
+			request the ROM holding pen code release a secondary
+			CPU.  The value written to the register is formed by
+			encoding the target CPU id into the low bits of the
+			physical start address it should jump to.
 
 Example 1 (dual-cluster big.LITTLE system 32-bit):
 
-- 
1.9.1

[PATCH v4 2/5] ARM: add SMP support for Broadcom mobile SoCs

[Alex Elder]

This patch adds SMP support for BCM281XX and BCM21664 family SoCs.

This feature is controlled with a distinct config option such that
an SMP-enabled multi-v7 binary can be configured to run these SoCs
in uniprocessor mode.  Since this SMP functionality is used for
multiple Broadcom mobile chip families the config option is called
ARCH_BCM_MOBILE_SMP (for lack of a better name).

On SoCs of this type, the secondary core is not held in reset on
power-on.  Instead it loops in a ROM-based holding pen.  To release
it, one must write into a special register a jump address whose
low-order bits have been replaced with a secondary core's id, then
trigger an event with SEV.  On receipt of an event, the ROM code
will examine the register's contents, and if the low-order bits
match its cpu id, it will clear them and write the value back to the
register just prior to jumping to the address specified.

The location of the special register is defined in the device tree
using a "secondary-boot-reg" property in a node whose "enable-method"
matches.

Derived from code originally provided by Ray Jui <rjui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Signed-off-by: Alex Elder <elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-bcm/Kconfig    |  18 +++-
 arch/arm/mach-bcm/Makefile   |   3 +
 arch/arm/mach-bcm/kona_smp.c | 202 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mach-bcm/kona_smp.c

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 5f5740f..336fc6f 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -14,7 +14,6 @@ config ARCH_BCM_MOBILE
 	depends on MMU
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_ERRATA_754322
-	select ARM_ERRATA_764369 if SMP
 	select ARM_GIC
 	select GPIO_BCM_KONA
 	select TICK_ONESHOT
@@ -31,18 +30,31 @@ menu "Broadcom Mobile SoC Selection"
 config ARCH_BCM_281XX
 	bool "Broadcom BCM281XX SoC family"
 	default y
+	select HAVE_SMP
 	help
-	  Enable support for the the BCM281XX family, which includes
+	  Enable support for the BCM281XX family, which includes
 	  BCM11130, BCM11140, BCM11351, BCM28145 and BCM28155
 	  variants.
 
 config ARCH_BCM_21664
 	bool "Broadcom BCM21664 SoC family"
 	default y
+	select HAVE_SMP
 	help
-	  Enable support for the the BCM21664 family, which includes
+	  Enable support for the BCM21664 family, which includes
 	  BCM21663 and BCM21664 variants.
 
+config ARCH_BCM_MOBILE_SMP
+	bool "Broadcom mobile SoC SMP support"
+	depends on (ARCH_BCM_281XX || ARCH_BCM_21664) && SMP
+	default y
+	select HAVE_ARM_SCU
+	select ARM_ERRATA_764369
+	help
+	  SMP support for the BCM281XX and BCM21664 SoC families.
+	  Provided as an option so SMP support for SoCs of this type
+	  can be disabled for an SMP-enabled kernel.
+
 endmenu
 
 endif
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 7fb9b04..d8c0dcf 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -16,6 +16,9 @@ obj-$(CONFIG_ARCH_BCM_281XX)	+= board_bcm281xx.o
 # BCM21664
 obj-$(CONFIG_ARCH_BCM_21664)	+= board_bcm21664.o
 
+# BCM281XX and BCM21664 SMP support
+obj-$(CONFIG_ARCH_BCM_MOBILE_SMP) += kona_smp.o
+
 # BCM281XX and BCM21664 L2 cache control
 obj-$(CONFIG_ARCH_BCM_MOBILE)	+= bcm_kona_smc.o bcm_kona_smc_asm.o kona.o
 plus_sec := $(call as-instr,.arch_extension sec,+sec)
diff --git a/arch/arm/mach-bcm/kona_smp.c b/arch/arm/mach-bcm/kona_smp.c
new file mode 100644
index 0000000..66a0465
--- /dev/null
+++ b/arch/arm/mach-bcm/kona_smp.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ * Copyright 2014 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/sched.h>
+
+#include <asm/smp.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+/* Size of mapped Cortex A9 SCU address space */
+#define CORTEX_A9_SCU_SIZE	0x58
+
+#define SECONDARY_TIMEOUT_NS	NSEC_PER_MSEC	/* 1 msec (in nanoseconds) */
+#define BOOT_ADDR_CPUID_MASK	0x3
+
+/* Name of device node property defining secondary boot register location */
+#define OF_SECONDARY_BOOT	"secondary-boot-reg"
+
+/* I/O address of register used to coordinate secondary core startup */
+static u32	secondary_boot;
+
+/*
+ * Enable the Cortex A9 Snoop Control Unit
+ *
+ * By the time this is called we already know there are multiple
+ * cores present.  We assume we're running on a Cortex A9 processor,
+ * so any trouble getting the base address register or getting the
+ * SCU base is a problem.
+ *
+ * Return 0 if successful or an error code otherwise.
+ */
+static int __init scu_a9_enable(void)
+{
+	unsigned long config_base;
+	void __iomem *scu_base;
+
+	if (!scu_a9_has_base()) {
+		pr_err("no configuration base address register!\n");
+		return -ENXIO;
+	}
+
+	/* Config base address register value is zero for uniprocessor */
+	config_base = scu_a9_get_base();
+	if (!config_base) {
+		pr_err("hardware reports only one core\n");
+		return -ENOENT;
+	}
+
+	scu_base = ioremap((phys_addr_t)config_base, CORTEX_A9_SCU_SIZE);
+	if (!scu_base) {
+		pr_err("failed to remap config base (%lu/%u) for SCU\n",
+			config_base, CORTEX_A9_SCU_SIZE);
+		return -ENOMEM;
+	}
+
+	scu_enable(scu_base);
+
+	iounmap(scu_base);	/* That's the last we'll need of this */
+
+	return 0;
+}
+
+static void __init bcm_smp_prepare_cpus(unsigned int max_cpus)
+{
+	static cpumask_t only_cpu_0 = { CPU_BITS_CPU0 };
+	struct device_node *node;
+	int ret;
+
+	BUG_ON(secondary_boot);		/* We're called only once */
+
+	/*
+	 * This function is only called via smp_ops->smp_prepare_cpu().
+	 * That only happens if a "/cpus" device tree node exists
+	 * and has an "enable-method" property that selects the SMP
+	 * operations defined herein.
+	 */
+	node = of_find_node_by_path("/cpus");
+	BUG_ON(!node);
+
+	/*
+	 * Our secondary enable method requires a "secondary-boot-reg"
+	 * property to specify a register address used to request the
+	 * ROM code boot a secondary code.  If we have any trouble
+	 * getting this we fall back to uniprocessor mode.
+	 */
+	if (of_property_read_u32(node, OF_SECONDARY_BOOT, &secondary_boot)) {
+		pr_err("%s: missing/invalid " OF_SECONDARY_BOOT " property\n",
+			node->name);
+		ret = -ENOENT;		/* Arrange to disable SMP */
+		goto out;
+	}
+
+	/*
+	 * Enable the SCU on Cortex A9 based SoCs.  If -ENOENT is
+	 * returned, the SoC reported a uniprocessor configuration.
+	 * We bail on any other error.
+	 */
+	ret = scu_a9_enable();
+out:
+	of_node_put(node);
+	if (ret) {
+		/* Update the CPU present map to reflect uniprocessor mode */
+		BUG_ON(ret != -ENOENT);
+		pr_warn("disabling SMP\n");
+		init_cpu_present(&only_cpu_0);
+	}
+}
+
+/*
+ * The ROM code has the secondary cores looping, waiting for an event.
+ * When an event occurs each core examines the bottom two bits of the
+ * secondary boot register.  When a core finds those bits contain its
+ * own core id, it performs initialization, including computing its boot
+ * address by clearing the boot register value's bottom two bits.  The
+ * core signals that it is beginning its execution by writing its boot
+ * address back to the secondary boot register, and finally jumps to
+ * that address.
+ *
+ * So to start a core executing we need to:
+ * - Encode the (hardware) CPU id with the bottom bits of the secondary
+ *   start address.
+ * - Write that value into the secondary boot register.
+ * - Generate an event to wake up the secondary CPU(s).
+ * - Wait for the secondary boot register to be re-written, which
+ *   indicates the secondary core has started.
+ */
+static int bcm_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	void __iomem *boot_reg;
+	phys_addr_t boot_func;
+	u64 start_clock;
+	u32 cpu_id;
+	u32 boot_val;
+	bool timeout = false;
+
+	cpu_id = cpu_logical_map(cpu);
+	if (cpu_id & ~BOOT_ADDR_CPUID_MASK) {
+		pr_err("bad cpu id (%u > %u)\n", cpu_id, BOOT_ADDR_CPUID_MASK);
+		return -EINVAL;
+	}
+
+	if (!secondary_boot) {
+		pr_err("required secondary boot register not specified\n");
+		return -EINVAL;
+	}
+
+	boot_reg = ioremap_nocache((phys_addr_t)secondary_boot, sizeof(u32));
+	if (!boot_reg) {
+		pr_err("unable to map boot register for cpu %u\n", cpu_id);
+		return -ENOSYS;
+	}
+
+	/*
+	 * Secondary cores will start in secondary_startup(),
+	 * defined in "arch/arm/kernel/head.S"
+	 */
+	boot_func = virt_to_phys(secondary_startup);
+	BUG_ON(boot_func & BOOT_ADDR_CPUID_MASK);
+	BUG_ON(boot_func > (phys_addr_t)U32_MAX);
+
+	/* The core to start is encoded in the low bits */
+	boot_val = (u32)boot_func | cpu_id;
+	writel_relaxed(boot_val, boot_reg);
+
+	sev();
+
+	/* The low bits will be cleared once the core has started */
+	start_clock = local_clock();
+	while (!timeout && readl_relaxed(boot_reg) == boot_val)
+		timeout = local_clock() - start_clock > SECONDARY_TIMEOUT_NS;
+
+	iounmap(boot_reg);
+
+	if (!timeout)
+		return 0;
+
+	pr_err("timeout waiting for cpu %u to start\n", cpu_id);
+
+	return -ENOSYS;
+}
+
+static struct smp_operations bcm_smp_ops __initdata = {
+	.smp_prepare_cpus	= bcm_smp_prepare_cpus,
+	.smp_boot_secondary	= bcm_boot_secondary,
+};
+CPU_METHOD_OF_DECLARE(bcm_smp_bcm281xx, "brcm,bcm11351-cpu-method",
+			&bcm_smp_ops);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/5] ARM: configs: enable SMP in bcm_defconfig

[Alex Elder]

Also explicitly set CONFIG_NR_CPUS to 2, limiting it to the most we
currently need.

Signed-off-by: Ray Jui <rjui@broadcom.com>
Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/configs/bcm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/bcm_defconfig b/arch/arm/configs/bcm_defconfig
index 3df3f3a..af1f911 100644
--- a/arch/arm/configs/bcm_defconfig
+++ b/arch/arm/configs/bcm_defconfig
@@ -27,6 +27,7 @@ CONFIG_PARTITION_ADVANCED=y
 CONFIG_ARCH_BCM=y
 CONFIG_ARCH_BCM_MOBILE=y
 CONFIG_ARM_THUMBEE=y
+CONFIG_SMP=y
 CONFIG_PREEMPT=y
 CONFIG_AEABI=y
 # CONFIG_COMPACTION is not set
-- 
1.9.1

[PATCH v4 4/5] ARM: dts: enable SMP support for bcm28155

[Alex Elder]

Define nodes representing the two Cortex A9 CPUs in a bcm28155 SoC.

Signed-off-by: Ray Jui <rjui@broadcom.com>
Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/boot/dts/bcm11351.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 64d069b..61372a6 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -27,6 +27,25 @@
 		bootargs = "console=ttyS0,115200n8";
 	};
 
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "brcm,bcm11351-cpu-method";
+		secondary-boot-reg = <0x3500417c>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <1>;
+		};
+	};
+
 	gic: interrupt-controller@3ff00100 {
 		compatible = "arm,cortex-a9-gic";
 		#interrupt-cells = <3>;
-- 
1.9.1

[PATCH v4 5/5] ARM: dts: enable SMP support for bcm21664

[Alex Elder]

Define nodes representing the two Cortex A9 CPUs in a bcm21644 SoC.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 arch/arm/boot/dts/bcm21664.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/bcm21664.dtsi b/arch/arm/boot/dts/bcm21664.dtsi
index 08a44d4..a37ded1 100644
--- a/arch/arm/boot/dts/bcm21664.dtsi
+++ b/arch/arm/boot/dts/bcm21664.dtsi
@@ -25,6 +25,25 @@
 		bootargs = "console=ttyS0,115200n8";
 	};
 
+       cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "brcm,bcm11351-cpu-method";
+		secondary-boot-reg = <0x35004178>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			reg = <1>;
+		};
+	};
+
 	gic: interrupt-controller@3ff00100 {
 		compatible = "arm,cortex-a9-gic";
 		#interrupt-cells = <3>;
-- 
1.9.1

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Lorenzo Pieralisi]

On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
> Broadcom mobile SoCs use a ROM-implemented holding pen for
> controlled boot of secondary cores.  A special register is
> used to communicate to the ROM that a secondary core should
> start executing kernel code.  This enable method is currently
> used for members of the bcm281xx and bcm21664 SoC families.
> 
> The use of an enable method also allows the SMP operation vector to
> be assigned as a result of device tree content for these SoCs.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>

This is getting out of control, it is absolutely ghastly. I wonder how
I can manage to keep cpus.txt updated if anyone with a boot method
du jour adds into cpus.txt, and honestly in this specific case it is even
hard to understand why.

Can't it be done with bindings for the relative register address space
(regmap ?) and platform code just calls the registers driver to set-up the
jump address ? It is platform specific code anyway there is no way you
can make this generic.

I really do not see the point in cluttering cpus.txt with this stuff, it
is a platform specific hack, and do not belong in generic bindings in my
opinion.

Thanks,
Lorenzo

> ---
>  Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 333f4ae..c6a2411 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"
> +			    "brcm,bcm11351-cpu-method"
>  
>  	- cpu-release-addr
>  		Usage: required for systems that have an "enable-method"
> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below.
>  		Value type: <phandle>
>  		Definition: Specifies the ACC[2] node associated with this CPU.
>  
> +	- secondary-boot-reg
> +		Usage:
> +			Required for systems that have an "enable-method"
> +			property value of "brcm,bcm11351-cpu-method".
> +		Value type: <u32>
> +		Definition:
> +			Specifies the physical address of the register used to
> +			request the ROM holding pen code release a secondary
> +			CPU.  The value written to the register is formed by
> +			encoding the target CPU id into the low bits of the
> +			physical start address it should jump to.
>  
>  Example 1 (dual-cluster big.LITTLE system 32-bit):
>  
> -- 
> 1.9.1
> 
> 

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Alex Elder]

On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote:
> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
>> Broadcom mobile SoCs use a ROM-implemented holding pen for
>> controlled boot of secondary cores.  A special register is
>> used to communicate to the ROM that a secondary core should
>> start executing kernel code.  This enable method is currently
>> used for members of the bcm281xx and bcm21664 SoC families.
>>
>> The use of an enable method also allows the SMP operation vector to
>> be assigned as a result of device tree content for these SoCs.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
> 
> This is getting out of control, it is absolutely ghastly. I wonder how
> I can manage to keep cpus.txt updated if anyone with a boot method
> du jour adds into cpus.txt, and honestly in this specific case it is even
> hard to understand why.

I concur that it gets out of control to document bindings
in "cpus.txt" like this.

I posted a separate, independent documentation patch, to
address this issue specifically:
    devicetree: bindings: separate CPU enable method descriptions
There I don't even include my new addition but I also
do a little work to sort out some stuff--for example only
defining "cpu-release-addr" (or "qcom,saw") in the one place
where it's relevant.

> Can't it be done with bindings for the relative register address space
> (regmap ?) and platform code just calls the registers driver to set-up the
> jump address ? It is platform specific code anyway there is no way you
> can make this generic.

This is feedback specific to how I implement this enable method.
I am going to address this in a follow-on message, to distinguish
it from the broader question of where best to document these
enable methods.  (I'll be sending that message later today.)

> I really do not see the point in cluttering cpus.txt with this stuff, it
> is a platform specific hack, and do not belong in generic bindings in my
> opinion.

Again, I completely agree with this.

In order to assign the SMP operations vector for my machine
via device tree, I need to define an "enable-method" property
in either the "cpus" node or one of the "cpu" nodes.  I would
prefer to use a generic method, but the method used here is
semantically different from the others in existence, and I
need to document how it works.  The place currently used
to do that is "cpus.txt".

Please look at the patch I mentioned above.  I'd be glad to
do it another way; but it is an attempt to address what I
saw as a problem that I think you are talking about.

Thanks.

					-Alex


> Thanks,
> Lorenzo
> 
>> ---
>>  Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>> index 333f4ae..c6a2411 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"
>> +			    "brcm,bcm11351-cpu-method"
>>  
>>  	- cpu-release-addr
>>  		Usage: required for systems that have an "enable-method"
>> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below.
>>  		Value type: <phandle>
>>  		Definition: Specifies the ACC[2] node associated with this CPU.
>>  
>> +	- secondary-boot-reg
>> +		Usage:
>> +			Required for systems that have an "enable-method"
>> +			property value of "brcm,bcm11351-cpu-method".
>> +		Value type: <u32>
>> +		Definition:
>> +			Specifies the physical address of the register used to
>> +			request the ROM holding pen code release a secondary
>> +			CPU.  The value written to the register is formed by
>> +			encoding the target CPU id into the low bits of the
>> +			physical start address it should jump to.
>>  
>>  Example 1 (dual-cluster big.LITTLE system 32-bit):
>>  
>> -- 
>> 1.9.1
>>
>>
> 

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Alex Elder]

On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote:
> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
>> Broadcom mobile SoCs use a ROM-implemented holding pen for
>> controlled boot of secondary cores.  A special register is
>> used to communicate to the ROM that a secondary core should
>> start executing kernel code.  This enable method is currently
>> used for members of the bcm281xx and bcm21664 SoC families.
>>
>> The use of an enable method also allows the SMP operation vector to
>> be assigned as a result of device tree content for these SoCs.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
> 
> This is getting out of control, it is absolutely ghastly. I wonder how
> I can manage to keep cpus.txt updated if anyone with a boot method
> du jour adds into cpus.txt, and honestly in this specific case it is even
> hard to understand why.

OK, in this message I'll focus on the particulars of this
proposed binding.

> Can't it be done with bindings for the relative register address space
> (regmap ?) and platform code just calls the registers driver to set-up the
> jump address ? It is platform specific code anyway there is no way you
> can make this generic.

I want to clarify what you're after here.

My aim is to add SMP support for a class of Broadcom SMP
machines.  To do so, I'm told I need to use the technique
of assigning the SMP operations vector as a result of
identifying an enable method in the DT.

For 32-bit ARM, there are no generic "enable-method" values.
(I did attempt to create one for "spin-table" but that was
rejected by Russell King.)  For the machines I'm trying to
enable, secondary CPUS start out spinning in a ROM-based
holding pen, and there is no need for a kernel-based one.

However, like a spin-table/holding pen enable method, a
memory location is required for coordination between the
boot CPU running kernel code and secondary CPUs running ROM
code.  My proposal specifies it using a special numeric
property value named "secondary-boot-reg" in the "cpus"
node in the DT.

And as I understand it, the issue you have relates to how
this memory location is specified.

You suggest regmap.  I'm using a single 32-bit register,
only at very early boot time, and thereafter access to
it is meaningless.  It seems like overkill if it's only
used for this purpose.  I could hide the register values
in the code, but with the exception of that, the code I'm
using is generic (in the context of this class of Broadcom
machine).  I could specify the register differently somehow,
in a different node, or with a different property.

The bottom line here is I'm not sure whether I understand
what you're suggesting, or perhaps why what you suggest is
preferable.  I'm very open to suggestions, I just need it
laid out a bit more detail in order to respond directly.

Thanks.

					-Alex

> I really do not see the point in cluttering cpus.txt with this stuff, it
> is a platform specific hack, and do not belong in generic bindings in my
> opinion.
> 
> Thanks,
> Lorenzo
> 
>> ---
>>  Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>> index 333f4ae..c6a2411 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"
>> +			    "brcm,bcm11351-cpu-method"
>>  
>>  	- cpu-release-addr
>>  		Usage: required for systems that have an "enable-method"
>> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below.
>>  		Value type: <phandle>
>>  		Definition: Specifies the ACC[2] node associated with this CPU.
>>  
>> +	- secondary-boot-reg
>> +		Usage:
>> +			Required for systems that have an "enable-method"
>> +			property value of "brcm,bcm11351-cpu-method".
>> +		Value type: <u32>
>> +		Definition:
>> +			Specifies the physical address of the register used to
>> +			request the ROM holding pen code release a secondary
>> +			CPU.  The value written to the register is formed by
>> +			encoding the target CPU id into the low bits of the
>> +			physical start address it should jump to.
>>  
>>  Example 1 (dual-cluster big.LITTLE system 32-bit):
>>  
>> -- 
>> 1.9.1
>>
>>
> 

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Lorenzo Pieralisi]

On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote:
> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote:
> > On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
> >> Broadcom mobile SoCs use a ROM-implemented holding pen for
> >> controlled boot of secondary cores.  A special register is
> >> used to communicate to the ROM that a secondary core should
> >> start executing kernel code.  This enable method is currently
> >> used for members of the bcm281xx and bcm21664 SoC families.
> >>
> >> The use of an enable method also allows the SMP operation vector to
> >> be assigned as a result of device tree content for these SoCs.
> >>
> >> Signed-off-by: Alex Elder <elder@linaro.org>
> > 
> > This is getting out of control, it is absolutely ghastly. I wonder how
> > I can manage to keep cpus.txt updated if anyone with a boot method
> > du jour adds into cpus.txt, and honestly in this specific case it is even
> > hard to understand why.
> 
> OK, in this message I'll focus on the particulars of this
> proposed binding.
> 
> > Can't it be done with bindings for the relative register address space
> > (regmap ?) and platform code just calls the registers driver to set-up the
> > jump address ? It is platform specific code anyway there is no way you
> > can make this generic.
> 
> I want to clarify what you're after here.
> 
> My aim is to add SMP support for a class of Broadcom SMP
> machines.  To do so, I'm told I need to use the technique
> of assigning the SMP operations vector as a result of
> identifying an enable method in the DT.
> 
> For 32-bit ARM, there are no generic "enable-method" values.
> (I did attempt to create one for "spin-table" but that was
> rejected by Russell King.)  For the machines I'm trying to
> enable, secondary CPUS start out spinning in a ROM-based
> holding pen, and there is no need for a kernel-based one.
> 
> However, like a spin-table/holding pen enable method, a
> memory location is required for coordination between the
> boot CPU running kernel code and secondary CPUs running ROM
> code.  My proposal specifies it using a special numeric
> property value named "secondary-boot-reg" in the "cpus"
> node in the DT.
> 
> And as I understand it, the issue you have relates to how
> this memory location is specified.

The issue I have relates to cluttering cpus.txt with all
sorts of platform specific SMP boot hacks.

> You suggest regmap.  I'm using a single 32-bit register,
> only at very early boot time, and thereafter access to
> it is meaningless.  It seems like overkill if it's only
> used for this purpose.  I could hide the register values
> in the code, but with the exception of that, the code I'm
> using is generic (in the context of this class of Broadcom
> machine).  I could specify the register differently somehow,
> in a different node, or with a different property.

Is that register part of a larger registers block ? What I wanted
to say is that you can use a driver "API" (we wish) to write that
register, something like eg vexpress does with sysflags:

drivers/mfd/vexpress-sysreg.c

vexpress_flags_set()

instead of grabbing the reg address from a platform specific boot
method DT entry.

I doubt that register exists on its own, even though I have to say this
would force you to write yet another platform specific driver to control
a bunch of registers, I do not see any other solution.

One thing is for certain: I really do not see the point in adding a boot
method per-SoC, and I do not want to end up having a cpus.txt file with a
gazillion entries just because every given platform reinvents the wheel when
it comes to booting an SMP system, cpus.txt would become a document that
describes platform quirks, not a proper binding anymore.

At least all platform specific quirks must be moved out of cpus.txt and
in platform documentation, I understand it is just a cosmetic change but
I want to prevent cpus.txt to become an abomination.

> The bottom line here is I'm not sure whether I understand
> what you're suggesting, or perhaps why what you suggest is
> preferable.  I'm very open to suggestions, I just need it
> laid out a bit more detail in order to respond directly.

See above.

Thanks !
Lorenzo

> 
> Thanks.
> 
> 					-Alex
> 
> > I really do not see the point in cluttering cpus.txt with this stuff, it
> > is a platform specific hack, and do not belong in generic bindings in my
> > opinion.
> > 
> > Thanks,
> > Lorenzo
> > 
> >> ---
> >>  Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> >> index 333f4ae..c6a2411 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"
> >> +			    "brcm,bcm11351-cpu-method"
> >>  
> >>  	- cpu-release-addr
> >>  		Usage: required for systems that have an "enable-method"
> >> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below.
> >>  		Value type: <phandle>
> >>  		Definition: Specifies the ACC[2] node associated with this CPU.
> >>  
> >> +	- secondary-boot-reg
> >> +		Usage:
> >> +			Required for systems that have an "enable-method"
> >> +			property value of "brcm,bcm11351-cpu-method".
> >> +		Value type: <u32>
> >> +		Definition:
> >> +			Specifies the physical address of the register used to
> >> +			request the ROM holding pen code release a secondary
> >> +			CPU.  The value written to the register is formed by
> >> +			encoding the target CPU id into the low bits of the
> >> +			physical start address it should jump to.
> >>  
> >>  Example 1 (dual-cluster big.LITTLE system 32-bit):
> >>  
> >> -- 
> >> 1.9.1
> >>
> >>
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Alex Elder]

On 05/28/2014 05:36 AM, Lorenzo Pieralisi wrote:
> On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote:
>> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote:
>>> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
>>>> Broadcom mobile SoCs use a ROM-implemented holding pen for
>>>> controlled boot of secondary cores.  A special register is
>>>> used to communicate to the ROM that a secondary core should
>>>> start executing kernel code.  This enable method is currently
>>>> used for members of the bcm281xx and bcm21664 SoC families.
>>>>
>>>> The use of an enable method also allows the SMP operation vector to
>>>> be assigned as a result of device tree content for these SoCs.
>>>>
>>>> Signed-off-by: Alex Elder <elder@linaro.org>
>>>
>>> This is getting out of control, it is absolutely ghastly. I wonder how
>>> I can manage to keep cpus.txt updated if anyone with a boot method
>>> du jour adds into cpus.txt, and honestly in this specific case it is even
>>> hard to understand why.
>>
>> OK, in this message I'll focus on the particulars of this
>> proposed binding.
>>
>>> Can't it be done with bindings for the relative register address space
>>> (regmap ?) and platform code just calls the registers driver to set-up the
>>> jump address ? It is platform specific code anyway there is no way you
>>> can make this generic.
>>
>> I want to clarify what you're after here.
>>
>> My aim is to add SMP support for a class of Broadcom SMP
>> machines.  To do so, I'm told I need to use the technique
>> of assigning the SMP operations vector as a result of
>> identifying an enable method in the DT.
>>
>> For 32-bit ARM, there are no generic "enable-method" values.
>> (I did attempt to create one for "spin-table" but that was
>> rejected by Russell King.)  For the machines I'm trying to
>> enable, secondary CPUS start out spinning in a ROM-based
>> holding pen, and there is no need for a kernel-based one.
>>
>> However, like a spin-table/holding pen enable method, a
>> memory location is required for coordination between the
>> boot CPU running kernel code and secondary CPUs running ROM
>> code.  My proposal specifies it using a special numeric
>> property value named "secondary-boot-reg" in the "cpus"
>> node in the DT.
>>
>> And as I understand it, the issue you have relates to how
>> this memory location is specified.
> 
> The issue I have relates to cluttering cpus.txt with all
> sorts of platform specific SMP boot hacks.

OK, as I mentioned in my other message, I totally
agree with you here.  It's a total (and building)
mess.  I discussed this with Mark Rutland at ELC
last month and suggested splitting that stuff out
of "cpus.txt", which I have now proposed with a
patch.
    https://lkml.org/lkml/2014/5/8/545

>> You suggest regmap.  I'm using a single 32-bit register,
>> only at very early boot time, and thereafter access to
>> it is meaningless.  It seems like overkill if it's only
>> used for this purpose.  I could hide the register values
>> in the code, but with the exception of that, the code I'm
>> using is generic (in the context of this class of Broadcom
>> machine).  I could specify the register differently somehow,
>> in a different node, or with a different property.
> 
> Is that register part of a larger registers block ? What I wanted
> to say is that you can use a driver "API" (we wish) to write that
> register, something like eg vexpress does with sysflags:
> 
> drivers/mfd/vexpress-sysreg.c
> 
> vexpress_flags_set()
> 
> instead of grabbing the reg address from a platform specific boot
> method DT entry.

I had this exact thought when I was working on this.  Yes,
this register *is* part of a register block, "ChipRegs", and
I wanted to have a separate driver/library/API to manage access
to registers in that block.  But the block is sort of a dumping
ground for many unrelated things, and lots of them aren't going
to be used for normal operation (lots of debug and diagnostic
functionality, for example).  After some discussion with others
on my team I made the explicit decision to *not* have any sort
of API in this case, and just specify the register.  We decided
that if and when we had a need for multiple users of addresses
in this range we'd figure out how to do best what was required.

> I doubt that register exists on its own, even though I have to say this
> would force you to write yet another platform specific driver to control
> a bunch of registers, I do not see any other solution.

Yes, exactly, and I was prepared to do that.  But doing a whole
driver for that one address seemed to make less sense.

> One thing is for certain: I really do not see the point in adding a boot
> method per-SoC, and I do not want to end up having a cpus.txt file with a

I'd love to see common, "standard" boot methods.  For ARM32
it seems each one was done separately, and in many cases,
slightly differently.  The one I saw the most was "spin table,"
but making that "standard" was deemed unacceptable.

> gazillion entries just because every given platform reinvents the wheel when
> it comes to booting an SMP system, cpus.txt would become a document that
> describes platform quirks, not a proper binding anymore.

Agreed.

> At least all platform specific quirks must be moved out of cpus.txt and
> in platform documentation, I understand it is just a cosmetic change but
> I want to prevent cpus.txt to become an abomination.

Agreed.

>> The bottom line here is I'm not sure whether I understand
>> what you're suggesting, or perhaps why what you suggest is
>> preferable.  I'm very open to suggestions, I just need it
>> laid out a bit more detail in order to respond directly.
> 
> See above.

I really appreciate your responses.  One problem with this
process is you don't get the benefit of the discussion and
design process that went on prior to posting this stuff
publicly.

If you still feel strongly we should have a custom driver
to manage this "ChipRegs" address space, say so, and I can
put one together.  If my explanations above mitigate your
concerns, please say that as well.  Either way I just want
to make progress toward getting this support upstream.

Thanks a lot.

					-Alex

> 
> Thanks !
> Lorenzo
> 
>>
>> Thanks.
>>
>> 					-Alex
>>
>>> I really do not see the point in cluttering cpus.txt with this stuff, it
>>> is a platform specific hack, and do not belong in generic bindings in my
>>> opinion.
>>>
>>> Thanks,
>>> Lorenzo
>>>
>>>> ---
>>>>  Documentation/devicetree/bindings/arm/cpus.txt | 12 ++++++++++++
>>>>  1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>>>> index 333f4ae..c6a2411 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"
>>>> +			    "brcm,bcm11351-cpu-method"
>>>>  
>>>>  	- cpu-release-addr
>>>>  		Usage: required for systems that have an "enable-method"
>>>> @@ -209,6 +210,17 @@ nodes to be present and contain the properties described below.
>>>>  		Value type: <phandle>
>>>>  		Definition: Specifies the ACC[2] node associated with this CPU.
>>>>  
>>>> +	- secondary-boot-reg
>>>> +		Usage:
>>>> +			Required for systems that have an "enable-method"
>>>> +			property value of "brcm,bcm11351-cpu-method".
>>>> +		Value type: <u32>
>>>> +		Definition:
>>>> +			Specifies the physical address of the register used to
>>>> +			request the ROM holding pen code release a secondary
>>>> +			CPU.  The value written to the register is formed by
>>>> +			encoding the target CPU id into the low bits of the
>>>> +			physical start address it should jump to.
>>>>  
>>>>  Example 1 (dual-cluster big.LITTLE system 32-bit):
>>>>  
>>>> -- 
>>>> 1.9.1
>>>>
>>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Lorenzo Pieralisi]

On Wed, May 28, 2014 at 01:22:06PM +0100, Alex Elder wrote:
> On 05/28/2014 05:36 AM, Lorenzo Pieralisi wrote:
> > On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote:
> >> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote:
> >>> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
> >>>> Broadcom mobile SoCs use a ROM-implemented holding pen for
> >>>> controlled boot of secondary cores.  A special register is
> >>>> used to communicate to the ROM that a secondary core should
> >>>> start executing kernel code.  This enable method is currently
> >>>> used for members of the bcm281xx and bcm21664 SoC families.
> >>>>
> >>>> The use of an enable method also allows the SMP operation vector to
> >>>> be assigned as a result of device tree content for these SoCs.
> >>>>
> >>>> Signed-off-by: Alex Elder <elder@linaro.org>
> >>>
> >>> This is getting out of control, it is absolutely ghastly. I wonder how
> >>> I can manage to keep cpus.txt updated if anyone with a boot method
> >>> du jour adds into cpus.txt, and honestly in this specific case it is even
> >>> hard to understand why.
> >>
> >> OK, in this message I'll focus on the particulars of this
> >> proposed binding.
> >>
> >>> Can't it be done with bindings for the relative register address space
> >>> (regmap ?) and platform code just calls the registers driver to set-up the
> >>> jump address ? It is platform specific code anyway there is no way you
> >>> can make this generic.
> >>
> >> I want to clarify what you're after here.
> >>
> >> My aim is to add SMP support for a class of Broadcom SMP
> >> machines.  To do so, I'm told I need to use the technique
> >> of assigning the SMP operations vector as a result of
> >> identifying an enable method in the DT.
> >>
> >> For 32-bit ARM, there are no generic "enable-method" values.
> >> (I did attempt to create one for "spin-table" but that was
> >> rejected by Russell King.)  For the machines I'm trying to
> >> enable, secondary CPUS start out spinning in a ROM-based
> >> holding pen, and there is no need for a kernel-based one.
> >>
> >> However, like a spin-table/holding pen enable method, a
> >> memory location is required for coordination between the
> >> boot CPU running kernel code and secondary CPUs running ROM
> >> code.  My proposal specifies it using a special numeric
> >> property value named "secondary-boot-reg" in the "cpus"
> >> node in the DT.
> >>
> >> And as I understand it, the issue you have relates to how
> >> this memory location is specified.
> > 
> > The issue I have relates to cluttering cpus.txt with all
> > sorts of platform specific SMP boot hacks.
> 
> OK, as I mentioned in my other message, I totally
> agree with you here.  It's a total (and building)
> mess.  I discussed this with Mark Rutland at ELC
> last month and suggested splitting that stuff out
> of "cpus.txt", which I have now proposed with a
> patch.
>     https://lkml.org/lkml/2014/5/8/545

I think this makes sense, I will review that patchset, and with this
approach agreed I am ok with adding a platform specific boot method,
since it is split up "nicely", do not bother adding a specific driver
to poke a register (it will be fun to see the number of files we have
to add to /cpu-enable-method though, big fun).

I still think that it is high time we started pushing back on these
platform hacks and move towards a common interface like PSCI to boot
(and suspend) ARM processors, there is no reason whatsoever why this
can't be done on the platforms you are trying to get merged unless I am
missing something.

Lorenzo

Re: [PATCH v4 1/5] devicetree: bindings: document Broadcom CPU enable method

[Alex Elder]

On 05/28/2014 08:34 AM, Lorenzo Pieralisi wrote:
> On Wed, May 28, 2014 at 01:22:06PM +0100, Alex Elder wrote:
>> On 05/28/2014 05:36 AM, Lorenzo Pieralisi wrote:
>>> On Wed, May 28, 2014 at 04:30:47AM +0100, Alex Elder wrote:
>>>> On 05/27/2014 06:49 AM, Lorenzo Pieralisi wrote:
>>>>> On Tue, May 20, 2014 at 06:43:46PM +0100, Alex Elder wrote:
>>>>>> Broadcom mobile SoCs use a ROM-implemented holding pen for
>>>>>> controlled boot of secondary cores.  A special register is
>>>>>> used to communicate to the ROM that a secondary core should
>>>>>> start executing kernel code.  This enable method is currently
>>>>>> used for members of the bcm281xx and bcm21664 SoC families.
>>>>>>
>>>>>> The use of an enable method also allows the SMP operation vector to
>>>>>> be assigned as a result of device tree content for these SoCs.
>>>>>>
>>>>>> Signed-off-by: Alex Elder <elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>
>>>>> This is getting out of control, it is absolutely ghastly. I wonder how
>>>>> I can manage to keep cpus.txt updated if anyone with a boot method
>>>>> du jour adds into cpus.txt, and honestly in this specific case it is even
>>>>> hard to understand why.
>>>>
>>>> OK, in this message I'll focus on the particulars of this
>>>> proposed binding.
>>>>
>>>>> Can't it be done with bindings for the relative register address space
>>>>> (regmap ?) and platform code just calls the registers driver to set-up the
>>>>> jump address ? It is platform specific code anyway there is no way you
>>>>> can make this generic.
>>>>
>>>> I want to clarify what you're after here.
>>>>
>>>> My aim is to add SMP support for a class of Broadcom SMP
>>>> machines.  To do so, I'm told I need to use the technique
>>>> of assigning the SMP operations vector as a result of
>>>> identifying an enable method in the DT.
>>>>
>>>> For 32-bit ARM, there are no generic "enable-method" values.
>>>> (I did attempt to create one for "spin-table" but that was
>>>> rejected by Russell King.)  For the machines I'm trying to
>>>> enable, secondary CPUS start out spinning in a ROM-based
>>>> holding pen, and there is no need for a kernel-based one.
>>>>
>>>> However, like a spin-table/holding pen enable method, a
>>>> memory location is required for coordination between the
>>>> boot CPU running kernel code and secondary CPUs running ROM
>>>> code.  My proposal specifies it using a special numeric
>>>> property value named "secondary-boot-reg" in the "cpus"
>>>> node in the DT.
>>>>
>>>> And as I understand it, the issue you have relates to how
>>>> this memory location is specified.
>>>
>>> The issue I have relates to cluttering cpus.txt with all
>>> sorts of platform specific SMP boot hacks.
>>
>> OK, as I mentioned in my other message, I totally
>> agree with you here.  It's a total (and building)
>> mess.  I discussed this with Mark Rutland at ELC
>> last month and suggested splitting that stuff out
>> of "cpus.txt", which I have now proposed with a
>> patch.
>>     https://lkml.org/lkml/2014/5/8/545
> 
> I think this makes sense, I will review that patchset, and with this
> approach agreed I am ok with adding a platform specific boot method,
> since it is split up "nicely", do not bother adding a specific driver
> to poke a register (it will be fun to see the number of files we have
> to add to /cpu-enable-method though, big fun).

Great!

I used the existing documentation and the code as a guide in
crafting the text of those descriptions.  Some of them I had
to speculate though--especially for ARM64 (for which there is
documentation but nothing in the tree that uses it).  So it
needs some informed feedback.

> I still think that it is high time we started pushing back on these
> platform hacks and move towards a common interface like PSCI to boot
> (and suspend) ARM processors, there is no reason whatsoever why this
> can't be done on the platforms you are trying to get merged unless I am
> missing something.

We have no need for anything other than SMP startup at this point
on this platform.  If the framework were there for me to use I
would have used it.

Thanks again for working through this with me.

					-Alex

> Lorenzo
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html