This series adds SMP support for two Broadcom mobile SoC families.
It uses CPU_METHOD_OF_DECLARE() (or rather a new variant of that)
so that SMP operations are assigned using device tree rather than
adding it to a machine definition in a board file.
The first patch adds the ability to extract information from device
tree nodes having a matching "enable-method" value. This allows
additional parameters to be associated with the enable method. This
is used for the Broadcom SoCs to efficiently determine the location
of a special register used in coordinating the start of secondary
CPUs.
I have not included a device tree document update here; I would like
suggestions on where the new enable method and its parameter should
be documented. (I presume in .../arm/cpus.txt, but that could get
out of hand as more boards define new enable methods.)
-Alex
Note:
This series is based on the current arm-soc/for-next branch:
e98cd72 arm-soc: document samsung merges
Plus one more recently-posted patch:
http://permalink.gmane.org/gmane.linux.kernel/1677714
This series is available here:
http://git.linaro.org/landing-teams/working/broadcom/kernel.git
Branch review/bcm-smp
Alex Elder (5):
ARM: introduce CPU_METHOD_OF_DECLARE_SETUP()
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
arch/arm/boot/dts/bcm11351.dtsi | 19 ++++
arch/arm/boot/dts/bcm21664.dtsi | 19 ++++
arch/arm/configs/bcm_defconfig | 2 +
arch/arm/include/asm/smp.h | 10 +-
arch/arm/kernel/devtree.c | 31 +++++--
arch/arm/mach-bcm/Kconfig | 18 +++-
arch/arm/mach-bcm/Makefile | 5 +-
arch/arm/mach-bcm/platsmp.c | 194 +++++++++++++++++++++++++++++++++++++++
8 files changed, 286 insertions(+), 12 deletions(-)
create mode 100644 arch/arm/mach-bcm/platsmp.c
--
1.7.9.5
Define nodes representing the two Cortex A9 CPUs in a bcm28155 SoC.
Signed-off-by: Ray Jui <[email protected]>
Signed-off-by: Alex Elder <[email protected]>
---
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.7.9.5
This patch adds SMP support for BCM281XX and BCM21664 family SoCs.
This feature is controlled with a distinct config option such that a
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 <[email protected]>
Signed-off-by: Alex Elder <[email protected]>
---
arch/arm/mach-bcm/Kconfig | 18 +++-
arch/arm/mach-bcm/Makefile | 5 +-
arch/arm/mach-bcm/platsmp.c | 194 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 213 insertions(+), 4 deletions(-)
create mode 100644 arch/arm/mach-bcm/platsmp.c
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 183fdef..863c623 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 b2279e3..929579f 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -15,7 +15,10 @@ obj-$(CONFIG_ARCH_BCM_281XX) += board_bcm281xx.o
plus_sec := $(call as-instr,.arch_extension sec,+sec)
# BCM21664
-obj-$(CONFIG_ARCH_BCM_21664) += board_bcm21664.o
+obj-$(CONFIG_ARCH_BCM_21664) := board_bcm21664.o
+
+# BCM281XX and BCM21664 SMP support
+obj-$(CONFIG_ARCH_BCM_MOBILE_SMP) += platsmp.o
# BCM281XX and BCM21664 L2 cache control
obj-$(CONFIG_ARCH_BCM_MOBILE) += bcm_kona_smc.o bcm_kona_smc_asm.o kona.o
diff --git a/arch/arm/mach-bcm/platsmp.c b/arch/arm/mach-bcm/platsmp.c
new file mode 100644
index 0000000..46a64f2
--- /dev/null
+++ b/arch/arm/mach-bcm/platsmp.c
@@ -0,0 +1,194 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * 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 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; disabling SMP\n");
+ return -ENOENT;
+ }
+
+ scu_base = ioremap((phys_addr_t)config_base, SCU_SIZE);
+ if (!scu_base) {
+ pr_err("failed to remap config base (%lu/%u) for SCU\n",
+ config_base, 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 };
+ int ret;
+
+ /* Enable SCU on Cortex A9 based SoCs */
+ ret = scu_a9_enable();
+ if (!ret)
+ return;
+
+ /*
+ * If we have a uniprocessor, make sure the CPU present map
+ * reflects that. Bail on any other failure to enable the SCU.
+ */
+ if (ret == -ENOENT)
+ init_cpu_present(&only_cpu_0);
+ else
+ BUG();
+}
+
+/*
+ * Secondary startup method setup routine to extract the location of
+ * the secondary boot register from a "cpu" or "cpus" device tree
+ * node. Only the first seen secondary boot register value is used;
+ * any others are ignored. The secondary boot register value must be
+ * non-zero.
+ *
+ * Returns 0 if successful or an error code otherwise.
+ */
+static int __init of_enable_method_setup(struct device_node *node)
+{
+ int ret;
+
+ /* Ignore all but the first one specified */
+ if (secondary_boot)
+ return 0;
+
+ ret = of_property_read_u32(node, OF_SECONDARY_BOOT, &secondary_boot);
+ if (ret)
+ pr_err("%s: missing/invalid " OF_SECONDARY_BOOT " property\n",
+ node->name);
+
+ return ret;
+}
+
+/*
+ * 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;
+ }
+
+ boot_func = virt_to_phys(secondary_startup);
+ BUG_ON(boot_func & BOOT_ADDR_CPUID_MASK);
+ BUG_ON(boot_func > (phys_addr_t)U32_MAX);
+
+ boot_val = (u32)boot_func | cpu_id;
+ writel_relaxed(boot_val, boot_reg);
+
+ sev();
+
+ 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_SETUP(bcm_smp_bcm281xx, "brcm,bcm11351-cpu-method",
+ &of_enable_method_setup, &bcm_smp_ops);
--
1.7.9.5
Also explicitly set CONFIG_NR_CPUS to 2, limiting it to the most we
currently need.
Signed-off-by: Ray Jui <[email protected]>
Signed-off-by: Alex Elder <[email protected]>
---
arch/arm/configs/bcm_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/bcm_defconfig b/arch/arm/configs/bcm_defconfig
index 0100464..ea374a7 100644
--- a/arch/arm/configs/bcm_defconfig
+++ b/arch/arm/configs/bcm_defconfig
@@ -24,6 +24,8 @@ CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
# CONFIG_BLK_DEV_BSG is not set
CONFIG_PARTITION_ADVANCED=y
+CONFIG_SMP=y
+CONFIG_NR_CPUS=2
CONFIG_ARCH_BCM=y
CONFIG_ARCH_BCM_MOBILE=y
CONFIG_ARM_THUMBEE=y
--
1.7.9.5
Define nodes representing the two Cortex A9 CPUs in a bcm21644 SoC.
Signed-off-by: Alex Elder <[email protected]>
---
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.7.9.5
The CPU_METHOD_OF_DECLARE() macro allows methods for assigning
SMP/hotplug operations to CPUS to be defined using device tree,
without the need for machine-dependent code.
And although it allows the *method* to be specified, it does *not*
allow any parameterization of that method. For example, there is no
efficient way to define a machine-specific address or other
property one might want to define for secondary CPUs.
Define a new of_cpu_method->setup() function, which (if defined) is
called for nodes found having a matching "enable-method" property.
The matching node is supplied as the function's argument, allowing
additional required information to be extracted from that node.
A new macro CPU_METHOD_OF_DECLARE_SETUP() allows a setup method
to be supplied when a method is declared.
Extend the interface for set_smp_ops_by_method() so that it can
return a negative error code to allow DT parsing errors to be
reported by the setup function.
(Note that only the first "cpu" (or "cpus") node having a matching
method is used by set_smp_ops_by_method(); this logic is not
changed.)
Signed-off-by: Alex Elder <[email protected]>
---
arch/arm/include/asm/smp.h | 10 ++++++++--
arch/arm/kernel/devtree.c | 31 +++++++++++++++++++++++++------
2 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 2ec765c..ab4a5a9 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -115,15 +115,21 @@ struct smp_operations {
#endif
};
+struct device_node;
struct of_cpu_method {
const char *method;
+ int (*setup)(struct device_node *node);
struct smp_operations *ops;
};
-#define CPU_METHOD_OF_DECLARE(name, _method, _ops) \
+#define CPU_METHOD_OF_DECLARE_SETUP(name, _method, _setup, _ops) \
static const struct of_cpu_method __cpu_method_of_table_##name \
__used __section(__cpu_method_of_table) \
- = { .method = _method, .ops = _ops }
+ = { .method = _method, .setup = _setup, .ops = _ops }
+
+#define CPU_METHOD_OF_DECLARE(name, _method, _ops) \
+ CPU_METHOD_OF_DECLARE_SETUP(name, _method, NULL, _ops)
+
/*
* set platform specific SMP operations
*/
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index c7419a5..1a0cca3 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -76,11 +76,18 @@ static int __init set_smp_ops_by_method(struct device_node *node)
if (of_property_read_string(node, "enable-method", &method))
return 0;
- for (; m < __cpu_method_of_table_end; m++)
+ for (; m < __cpu_method_of_table_end; m++) {
if (!strcmp(m->method, method)) {
- smp_set_ops(m->ops);
- return 1;
+ int ret = 0;
+
+ if (m->setup)
+ ret = m->setup(node);
+ if (!ret)
+ smp_set_ops(m->ops);
+
+ return ret ? ret : 1;
}
+ }
return 0;
}
@@ -181,16 +188,28 @@ void __init arm_dt_init_cpu_maps(void)
tmp_map[i] = hwid;
- if (!found_method)
+ if (!found_method) {
found_method = set_smp_ops_by_method(cpu);
+ if (WARN(found_method < 0,
+ "error %d getting enable-method for "
+ "DT /cpu %u\n", found_method, cpuidx)) {
+ return;
+ }
+ }
}
/*
* Fallback to an enable-method in the cpus node if nothing found in
* a cpu node.
*/
- if (!found_method)
- set_smp_ops_by_method(cpus);
+ if (!found_method) {
+ found_method = set_smp_ops_by_method(cpus);
+ if (WARN(found_method < 0,
+ "error %d getting enable-method for "
+ "DT /cpus node\n", found_method)) {
+ return;
+ }
+ }
if (!bootcpu_valid) {
pr_warn("DT missing boot CPU MPIDR[23:0], fall back to default cpu_logical_map\n");
--
1.7.9.5
On 04/03/2014 09:18 PM, Alex Elder wrote:
> This patch adds SMP support for BCM281XX and BCM21664 family SoCs.
>
> This feature is controlled with a distinct config option such that a
> 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 <[email protected]>
>
> Signed-off-by: Alex Elder <[email protected]>
> ---
. . .
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index b2279e3..929579f 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -15,7 +15,10 @@ obj-$(CONFIG_ARCH_BCM_281XX) += board_bcm281xx.o
> plus_sec := $(call as-instr,.arch_extension sec,+sec)
>
> # BCM21664
> -obj-$(CONFIG_ARCH_BCM_21664) += board_bcm21664.o
> +obj-$(CONFIG_ARCH_BCM_21664) := board_bcm21664.o
The above was a mistake, it should still be +=.
(I'll fix it.)
. . .
On 04/03/2014 09:18 PM, Alex Elder wrote:
> Also explicitly set CONFIG_NR_CPUS to 2, limiting it to the most we
> currently need.
>
> Signed-off-by: Ray Jui <[email protected]>
> Signed-off-by: Alex Elder <[email protected]>
> ---
> arch/arm/configs/bcm_defconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/configs/bcm_defconfig b/arch/arm/configs/bcm_defconfig
> index 0100464..ea374a7 100644
> --- a/arch/arm/configs/bcm_defconfig
> +++ b/arch/arm/configs/bcm_defconfig
> @@ -24,6 +24,8 @@ CONFIG_MODULES=y
> CONFIG_MODULE_UNLOAD=y
> # CONFIG_BLK_DEV_BSG is not set
> CONFIG_PARTITION_ADVANCED=y
> +CONFIG_SMP=y
> +CONFIG_NR_CPUS=2
Since this will be part of a MULTIPLATFORM build, I don't
want to set CONFIG_NR_CPUS. I will delete this line.
-Alex
> CONFIG_ARCH_BCM=y
> CONFIG_ARCH_BCM_MOBILE=y
> CONFIG_ARM_THUMBEE=y
>
On Thu, Apr 3, 2014 at 7:18 PM, Alex Elder <[email protected]> wrote:
> diff --git a/arch/arm/mach-bcm/platsmp.c b/arch/arm/mach-bcm/platsmp.c
> new file mode 100644
> index 0000000..46a64f2
> --- /dev/null
> +++ b/arch/arm/mach-bcm/platsmp.c
> +/* Size of mapped Cortex A9 SCU address space */
> +#define SCU_SIZE 0x58
> +/*
> + * 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; disabling SMP\n");
> + return -ENOENT;
> + }
> +
> + scu_base = ioremap((phys_addr_t)config_base, SCU_SIZE);
> + if (!scu_base) {
> + pr_err("failed to remap config base (%lu/%u) for SCU\n",
> + config_base, SCU_SIZE);
> + return -ENOMEM;
> + }
> +
> + scu_enable(scu_base);
> +
> + iounmap(scu_base); /* That's the last we'll need of this */
> +
> + return 0;
> +}
This function seems useful for Cortex A9 MPCore in general.
While you gave it a generic name, you put it in a Broadcom file.
Is there a better location for this code?
On 04/03/14 19:18, Alex Elder wrote:
> +
> +/*
> + * Secondary startup method setup routine to extract the location of
> + * the secondary boot register from a "cpu" or "cpus" device tree
> + * node. Only the first seen secondary boot register value is used;
> + * any others are ignored. The secondary boot register value must be
> + * non-zero.
> + *
> + * Returns 0 if successful or an error code otherwise.
> + */
> +static int __init of_enable_method_setup(struct device_node *node)
> +{
> + int ret;
> +
> + /* Ignore all but the first one specified */
> + if (secondary_boot)
> + return 0;
> +
> + ret = of_property_read_u32(node, OF_SECONDARY_BOOT, &secondary_boot);
> + if (ret)
> + pr_err("%s: missing/invalid " OF_SECONDARY_BOOT " property\n",
> + node->name);
> +
> + return ret;
> +}
I don't understand why we need this. Why can't we get the secondary boot
address from the /cpus node in the smp_prepare_cpus op. It isn't that
hard to get access to the cpus node there via of_find_node_by_path().
Then we don't need patch 1 at all. If it turns out to be common stuff,
we can always have the common function live in arm common code or maybe
even be a devicetree API.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On 04/04/2014 10:30 AM, Tim Kryger wrote:
> On Thu, Apr 3, 2014 at 7:18 PM, Alex Elder <[email protected]> wrote:
>
>> diff --git a/arch/arm/mach-bcm/platsmp.c b/arch/arm/mach-bcm/platsmp.c
>> new file mode 100644
>> index 0000000..46a64f2
>> --- /dev/null
>> +++ b/arch/arm/mach-bcm/platsmp.c
>
>> +/* Size of mapped Cortex A9 SCU address space */
>> +#define SCU_SIZE 0x58
>
>> +/*
>> + * 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; disabling SMP\n");
>> + return -ENOENT;
>> + }
>> +
>> + scu_base = ioremap((phys_addr_t)config_base, SCU_SIZE);
>> + if (!scu_base) {
>> + pr_err("failed to remap config base (%lu/%u) for SCU\n",
>> + config_base, SCU_SIZE);
>> + return -ENOMEM;
>> + }
>> +
>> + scu_enable(scu_base);
>> +
>> + iounmap(scu_base); /* That's the last we'll need of this */
>> +
>> + return 0;
>> +}
>
> This function seems useful for Cortex A9 MPCore in general.
>
> While you gave it a generic name, you put it in a Broadcom file.
>
> Is there a better location for this code?
I think it belongs in arch/arm/kernel/smp_scu.c. I was thinking
it might be generally useful when I wrote it (hence the more
complete header comment, for example). And I'll gladly move
it there, I just didn't want anybody to get hung up on that.
-Alex
On 04/04/2014 12:55 PM, Stephen Boyd wrote:
> On 04/03/14 19:18, Alex Elder wrote:
>> +
>> +/*
>> + * Secondary startup method setup routine to extract the location of
>> + * the secondary boot register from a "cpu" or "cpus" device tree
>> + * node. Only the first seen secondary boot register value is used;
>> + * any others are ignored. The secondary boot register value must be
>> + * non-zero.
>> + *
>> + * Returns 0 if successful or an error code otherwise.
>> + */
>> +static int __init of_enable_method_setup(struct device_node *node)
>> +{
>> + int ret;
>> +
>> + /* Ignore all but the first one specified */
>> + if (secondary_boot)
>> + return 0;
>> +
>> + ret = of_property_read_u32(node, OF_SECONDARY_BOOT, &secondary_boot);
>> + if (ret)
>> + pr_err("%s: missing/invalid " OF_SECONDARY_BOOT " property\n",
>> + node->name);
>> +
>> + return ret;
>> +}
>
> I don't understand why we need this. Why can't we get the secondary boot
> address from the /cpus node in the smp_prepare_cpus op. It isn't that
> hard to get access to the cpus node there via of_find_node_by_path().
> Then we don't need patch 1 at all. If it turns out to be common stuff,
> we can always have the common function live in arm common code or maybe
> even be a devicetree API.
You're right, that is one of several ways this could have
also been done.
Part of my thinking about this was affected by seeing how
arm_dt_init_cpu_maps() works. (Well, at least how it's
structured.) It looks for an enable method in each "cpu"
node, then falls back to looking for one in the "cpus" node
if none was seen. It doesn't assume the extra properties
are in the "cpus" node (or a "cpu" node for that matter).
It directly supplies a node that's known to have a matching
"enable-method" property--at the time that match is found--to
make it possible to extract other relevant information from
that matching node.
(As an aside, the documentation doesn't mention that "cpus"
nodes will contain "enable-method" properties, even though
it seems that's more a function of "cpus" than each individual
"cpu".)
I think it offers useful flexibility to do it this way,
and I like that it is unambiguous which node should be
searched for the additional information.
-Alex
On 04/04/2014 01:56 PM, Alex Elder wrote:
> On 04/04/2014 10:30 AM, Tim Kryger wrote:
>> On Thu, Apr 3, 2014 at 7:18 PM, Alex Elder <[email protected]> wrote:
>>
>>> diff --git a/arch/arm/mach-bcm/platsmp.c b/arch/arm/mach-bcm/platsmp.c
>>> new file mode 100644
>>> index 0000000..46a64f2
>>> --- /dev/null
>>> +++ b/arch/arm/mach-bcm/platsmp.c
>>
>>> +/* Size of mapped Cortex A9 SCU address space */
>>> +#define SCU_SIZE 0x58
>>
>>> +/*
>>> + * 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; disabling SMP\n");
>>> + return -ENOENT;
>>> + }
>>> +
>>> + scu_base = ioremap((phys_addr_t)config_base, SCU_SIZE);
>>> + if (!scu_base) {
>>> + pr_err("failed to remap config base (%lu/%u) for SCU\n",
>>> + config_base, SCU_SIZE);
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + scu_enable(scu_base);
>>> +
>>> + iounmap(scu_base); /* That's the last we'll need of this */
>>> +
>>> + return 0;
>>> +}
>>
>> This function seems useful for Cortex A9 MPCore in general.
>>
>> While you gave it a generic name, you put it in a Broadcom file.
>>
>> Is there a better location for this code?
>
> I think it belongs in arch/arm/kernel/smp_scu.c. I was thinking
> it might be generally useful when I wrote it (hence the more
> complete header comment, for example). And I'll gladly move
> it there, I just didn't want anybody to get hung up on that.
I'm going to re-submit this series this morning. I looked at
where this function might be used, and it looks like only one
other platform could use it (in hi3xxx_smp_prepare_cpus()).
Man of the others define get their scu base address some
other way (using a fixed constant or using device tree).
For now I'm going to keep it where it is. If you or someone else
reiterate the suggestion I'll move it to arch/arm/kernel/smp_scu.c.
-Alex