diff mbox

[RESEND,3/4] ARM: BCM: Add SMP support for Broadcom NSP

Message ID 1446702681-45339-4-git-send-email-kapilh@broadcom.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kapil Hali Nov. 5, 2015, 5:51 a.m. UTC
Add SMP support for Broadcom's Northstar Plus SoC,
cpu enable method and pen_release procedures. This
changes also consolidates iProc family's - BCM NSP
and BCM Kona, SMP handling in a common file.

Northstar Plus SoC is based on ARM Cortex-A9
revision r3p0 which requires configuration for ARM
Errata 764369 for SMP. This change adds the needed
configuration option.

Signed-off-by: Kapil Hali <kapilh@broadcom.com>
---
 arch/arm/mach-bcm/Kconfig                   |   2 +
 arch/arm/mach-bcm/Makefile                  |   8 +-
 arch/arm/mach-bcm/bcm_nsp.h                 |  19 ++++
 arch/arm/mach-bcm/headsmp.S                 |  37 ++++++++
 arch/arm/mach-bcm/{kona_smp.c => platsmp.c} | 142 ++++++++++++++++++++++++++--
 5 files changed, 197 insertions(+), 11 deletions(-)
 create mode 100644 arch/arm/mach-bcm/bcm_nsp.h
 create mode 100644 arch/arm/mach-bcm/headsmp.S
 rename arch/arm/mach-bcm/{kona_smp.c => platsmp.c} (63%)

Comments

Linus Walleij Nov. 5, 2015, 9:44 a.m. UTC | #1
On Thu, Nov 5, 2015 at 6:51 AM, Kapil Hali <kapilh@broadcom.com> wrote:

> Add SMP support for Broadcom's Northstar Plus SoC,
> cpu enable method and pen_release procedures. This
> changes also consolidates iProc family's - BCM NSP
> and BCM Kona, SMP handling in a common file.
>
> Northstar Plus SoC is based on ARM Cortex-A9
> revision r3p0 which requires configuration for ARM
> Errata 764369 for SMP. This change adds the needed
> configuration option.
>
> Signed-off-by: Kapil Hali <kapilh@broadcom.com>
(...)
> +/*
> + * iProc specific entry point for secondary CPUs.  This provides
> + * a "holding pen" into which all secondary cores are held until
> + * we are ready for them to initialise.
> + */

Do you *REALLY* need the complex pen hold/release semaphore
stuff?

Consult for example commit
c00def71efd919e8ae835a25f4f4c80a4b2d36d3
"ARM: ux500: simplify secondary CPU boot"

I realized the pen hold/release stuff was totally surplus on the
ux500 platform, it was probably just copied from the Vexpress
or RealView reference design and kept around because ARM
did things that way. And you are copying the same code from
other platforms again. Do you *really* understand the pen hold/release
code? (I don't.)

In our case it turned out that all that was really needed was:

- Map and enable SCU at .smp_prepare_cpus()

- Write the start address for the secondary CPU(s) in their
  magic registers/addresses i.e what you do in
  nsp_write_lut() and then call
  arch_send_wakeup_ipi_mask(cpumask_of(cpu)); in
  .smp_boot_secondary()
  It seems you may not even need the IPI, I can't see
  that call in your patch so I guess your secondary
  CPUs aren't in WFI at boot.

No pen hold/release magic! Works like a charm for us.
So see of you really need this horror story with copied
assembly code from realview etc.

Yours,
Linus Walleij
Kapil Hali Nov. 6, 2015, 12:15 p.m. UTC | #2
On 11/5/2015 3:14 PM, Linus Walleij wrote:
> On Thu, Nov 5, 2015 at 6:51 AM, Kapil Hali <kapilh@broadcom.com> wrote:
> 
>> Add SMP support for Broadcom's Northstar Plus SoC,
>> cpu enable method and pen_release procedures. This
>> changes also consolidates iProc family's - BCM NSP
>> and BCM Kona, SMP handling in a common file.
>>
>> Northstar Plus SoC is based on ARM Cortex-A9
>> revision r3p0 which requires configuration for ARM
>> Errata 764369 for SMP. This change adds the needed
>> configuration option.
>>
>> Signed-off-by: Kapil Hali <kapilh@broadcom.com>
> (...)
>> +/*
>> + * iProc specific entry point for secondary CPUs.  This provides
>> + * a "holding pen" into which all secondary cores are held until
>> + * we are ready for them to initialise.
>> + */
> 
> Do you *REALLY* need the complex pen hold/release semaphore
> stuff?
> 
> Consult for example commit
> c00def71efd919e8ae835a25f4f4c80a4b2d36d3
> "ARM: ux500: simplify secondary CPU boot"
> 
> I realized the pen hold/release stuff was totally surplus on the
> ux500 platform, it was probably just copied from the Vexpress
> or RealView reference design and kept around because ARM
> did things that way. And you are copying the same code from
> other platforms again. Do you *really* understand the pen hold/release
> code? (I don't.)
> 
> In our case it turned out that all that was really needed was:
> 
> - Map and enable SCU at .smp_prepare_cpus()
> 
> - Write the start address for the secondary CPU(s) in their
>   magic registers/addresses i.e what you do in
>   nsp_write_lut() and then call
>   arch_send_wakeup_ipi_mask(cpumask_of(cpu)); in
>   .smp_boot_secondary()
>   It seems you may not even need the IPI, I can't see
>   that call in your patch so I guess your secondary
>   CPUs aren't in WFI at boot.
> 
There is a dsb_sev() to get secondary core out of standby. However,
I think arch_send_wakeup_ipi_mask() is a better way as it is per
core. I will make the required changes in the next patch set.
> No pen hold/release magic! Works like a charm for us.
> So see of you really need this horror story with copied
> assembly code from realview etc.
> 
> Yours,
> Linus Walleij
> 
Thanks,
Kapil Hali
kernel test robot Nov. 16, 2015, 11:35 p.m. UTC | #3
Hi Kapil,

[auto build test ERROR on: arm-soc/for-next]
[also build test ERROR on: v4.4-rc1 next-20151116]

url:    https://github.com/0day-ci/linux/commits/Kapil-Hali/dt-bindings-add-SMP-enable-method-for-Broadcom-NSP/20151105-135452
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next
config: arm-bcm_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/mach-bcm/built-in.o: In function `__virt_to_phys':
>> arch/arm/include/asm/memory.h:206: undefined reference to `nsp_secondary_startup'
>> arch/arm/include/asm/memory.h:206: undefined reference to `nsp_secondary_startup'

vim +206 arch/arm/include/asm/memory.h

f52bb722 Sricharan R       2013-07-29  200  
ca5a45c0 Santosh Shilimkar 2013-07-31  201  static inline phys_addr_t __virt_to_phys(unsigned long x)
dc21af99 Russell King      2011-01-04  202  {
f52bb722 Sricharan R       2013-07-29  203  	phys_addr_t t;
f52bb722 Sricharan R       2013-07-29  204  
f52bb722 Sricharan R       2013-07-29  205  	if (sizeof(phys_addr_t) == 4) {
cada3c08 Russell King      2011-01-04 @206  		__pv_stub(x, t, "add", __PV_BITS_31_24);
f52bb722 Sricharan R       2013-07-29  207  	} else {
f52bb722 Sricharan R       2013-07-29  208  		__pv_stub_mov_hi(t);
f52bb722 Sricharan R       2013-07-29  209  		__pv_add_carry_stub(x, t);

:::::: The code at line 206 was first introduced by commit
:::::: cada3c0841e1deaec4c0f92654610b028dc683ff ARM: P2V: extend to 16-bit translation offsets

:::::: TO: Russell King <rmk+kernel@arm.linux.org.uk>
:::::: CC: Russell King <rmk+kernel@arm.linux.org.uk>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 1679fa4..2e9dbb5 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -40,6 +40,8 @@  config ARCH_BCM_NSP
 	select ARCH_BCM_IPROC
 	select ARM_ERRATA_754322
 	select ARM_ERRATA_775420
+	select ARM_ERRATA_764369 if SMP
+	select HAVE_SMP
 	help
 	  Support for Broadcom Northstar Plus SoC.
 	  Broadcom Northstar Plus family of SoCs are used for switching control
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 892261f..36a4ca30 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -14,7 +14,11 @@ 
 obj-$(CONFIG_ARCH_BCM_CYGNUS) +=  bcm_cygnus.o
 
 # Northstar Plus
-obj-$(CONFIG_ARCH_BCM_NSP) += bcm_nsp.o
+obj-$(CONFIG_ARCH_BCM_NSP)	+= bcm_nsp.o
+
+ifeq ($(CONFIG_ARCH_BCM_NSP),y)
+obj-$(CONFIG_SMP)		+= headsmp.o platsmp.o
+endif
 
 # BCM281XX
 obj-$(CONFIG_ARCH_BCM_281XX)	+= board_bcm281xx.o
@@ -23,7 +27,7 @@  obj-$(CONFIG_ARCH_BCM_281XX)	+= board_bcm281xx.o
 obj-$(CONFIG_ARCH_BCM_21664)	+= board_bcm21664.o
 
 # BCM281XX and BCM21664 SMP support
-obj-$(CONFIG_ARCH_BCM_MOBILE_SMP) += kona_smp.o
+obj-$(CONFIG_ARCH_BCM_MOBILE_SMP) += platsmp.o
 
 # BCM281XX and BCM21664 L2 cache control
 obj-$(CONFIG_ARCH_BCM_MOBILE_L2_CACHE) += kona_l2_cache.o
diff --git a/arch/arm/mach-bcm/bcm_nsp.h b/arch/arm/mach-bcm/bcm_nsp.h
new file mode 100644
index 0000000..58e1e80
--- /dev/null
+++ b/arch/arm/mach-bcm/bcm_nsp.h
@@ -0,0 +1,19 @@ 
+/*
+ * Copyright (C) 2015 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.
+ */
+
+#ifndef __BCM_NSP_H
+#define __BCM_NSP_H
+
+extern void nsp_secondary_startup(void);
+
+#endif /* __BCM_NSP_H */
diff --git a/arch/arm/mach-bcm/headsmp.S b/arch/arm/mach-bcm/headsmp.S
new file mode 100644
index 0000000..0da13b2
--- /dev/null
+++ b/arch/arm/mach-bcm/headsmp.S
@@ -0,0 +1,37 @@ 
+/*
+ * Copyright (C) 2015 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/linkage.h>
+
+/*
+ * iProc specific entry point for secondary CPUs.  This provides
+ * a "holding pen" into which all secondary cores are held until
+ * we are ready for them to initialise.
+ */
+ENTRY(nsp_secondary_startup)
+	mrc     p15, 0, r0, c0, c0, 5
+	and     r0, r0, #15
+	adr     r4, 1f
+	ldmia   r4, {r5, r6}
+	sub     r4, r4, r5
+	add     r6, r6, r4
+pen:	ldr     r7, [r6]
+	cmp     r7, r0
+	bne     pen
+
+	b    secondary_startup
+
+1:	.long   .
+	.long   pen_release
+
+ENDPROC(nsp_secondary_startup)
diff --git a/arch/arm/mach-bcm/kona_smp.c b/arch/arm/mach-bcm/platsmp.c
similarity index 63%
rename from arch/arm/mach-bcm/kona_smp.c
rename to arch/arm/mach-bcm/platsmp.c
index 66a0465..619030e 100644
--- a/arch/arm/mach-bcm/kona_smp.c
+++ b/arch/arm/mach-bcm/platsmp.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (C) 2014 Broadcom Corporation
+ * Copyright (C) 2014-2015 Broadcom Corporation
  * Copyright 2014 Linaro Limited
  *
  * This program is free software; you can redistribute it and/or
@@ -12,16 +12,23 @@ 
  * GNU General Public License for more details.
  */
 
-#include <linux/init.h>
+#include <linux/cpumask.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/init.h>
 #include <linux/io.h>
+#include <linux/jiffies.h>
 #include <linux/of.h>
 #include <linux/sched.h>
+#include <linux/smp.h>
 
+#include <asm/cacheflush.h>
 #include <asm/smp.h>
 #include <asm/smp_plat.h>
 #include <asm/smp_scu.h>
 
+#include "bcm_nsp.h"
+
 /* Size of mapped Cortex A9 SCU address space */
 #define CORTEX_A9_SCU_SIZE	0x58
 
@@ -34,6 +41,24 @@ 
 /* I/O address of register used to coordinate secondary core startup */
 static u32	secondary_boot;
 
+static DEFINE_SPINLOCK(boot_lock);
+
+/*
+ * Write pen_release in a way that is guaranteed to be visible to all
+ * observers, irrespective of whether they're taking part in coherency
+ * or not.  This is necessary for the hotplug code to work reliably.
+ */
+static void write_pen_release(int val)
+{
+	pen_release = val;
+	/*
+	 * Ensure write to pen_release is visible to the other cores,
+	 * here - primary core
+	 */
+	smp_wmb();
+	sync_cache_w(&pen_release);
+}
+
 /*
  * Enable the Cortex A9 Snoop Control Unit
  *
@@ -75,6 +100,51 @@  static int __init scu_a9_enable(void)
 	return 0;
 }
 
+static int nsp_write_lut(void (*secondary_startup) (void))
+{
+	void __iomem *sku_rom_lut;
+	phys_addr_t secondary_startup_phy;
+
+	if (!secondary_boot) {
+		pr_warn("required secondary boot register not specified\n");
+		return -EINVAL;
+	}
+
+	sku_rom_lut = ioremap_nocache((phys_addr_t)secondary_boot,
+						sizeof(secondary_boot));
+	if (!sku_rom_lut) {
+		pr_warn("unable to ioremap SKU-ROM LUT register\n");
+		return -ENOMEM;
+	}
+
+	secondary_startup_phy = virt_to_phys(secondary_startup);
+	BUG_ON(secondary_startup_phy > (phys_addr_t)U32_MAX);
+
+	writel_relaxed(secondary_startup_phy, sku_rom_lut);
+	/*
+	 * Ensure the write is visible to the secondary core.
+	 */
+	smp_wmb();
+
+	iounmap(sku_rom_lut);
+
+	return 0;
+}
+
+static void nsp_secondary_init(unsigned int cpu)
+{
+	/*
+	 * Let the primary cpu know we are out of holding pen.
+	 */
+	write_pen_release(-1);
+
+	/*
+	 * Synchronise with the boot thread.
+	 */
+	spin_lock(&boot_lock);
+	spin_unlock(&boot_lock);
+}
+
 static void __init bcm_smp_prepare_cpus(unsigned int max_cpus)
 {
 	static cpumask_t only_cpu_0 = { CPU_BITS_CPU0 };
@@ -95,11 +165,11 @@  static void __init bcm_smp_prepare_cpus(unsigned int max_cpus)
 	/*
 	 * 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
+	 * ROM code boot a secondary core.  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",
+		pr_warn("%s: missing/invalid " OF_SECONDARY_BOOT " property\n",
 			node->name);
 		ret = -ENOENT;		/* Arrange to disable SMP */
 		goto out;
@@ -115,7 +185,6 @@  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);
 	}
@@ -139,7 +208,7 @@  out:
  * - 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)
+static int kona_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
 	void __iomem *boot_reg;
 	phys_addr_t boot_func;
@@ -162,7 +231,7 @@  static int bcm_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	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;
+		return -ENOMEM;
 	}
 
 	/*
@@ -191,12 +260,67 @@  static int bcm_boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 	pr_err("timeout waiting for cpu %u to start\n", cpu_id);
 
-	return -ENOSYS;
+	return -ENXIO;
+}
+
+static int nsp_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+	unsigned long timeout;
+	int ret;
+
+	/*
+	 * After wake up, secondary core branches to the startup
+	 * address programmed at SKU ROM LUT location.
+	 */
+	ret = nsp_write_lut(nsp_secondary_startup);
+	if (ret) {
+		pr_err("unable to write startup addr to SKU ROM LUT\n");
+		goto out;
+	}
+
+	/*
+	 * The secondary processor is waiting to be released from
+	 * the holding pen - release it, then wait for it to flag
+	 * that it has been released by resetting pen_release.
+	 */
+	spin_lock(&boot_lock);
+
+	write_pen_release(cpu_logical_map(cpu));
+	/*
+	 * Send an Event to wake up the secondary core which is in
+	 * WFE state. Updated pen_release should also be visible to
+	 * the secondary core.
+	 */
+	dsb_sev();
+
+	timeout = jiffies + (1 * HZ);
+	while (time_before(jiffies, timeout)) {
+		/* Make sure loads on other CPU is visible */
+		smp_rmb();
+		if (pen_release == -1)
+			break;
+
+		udelay(10);
+	}
+
+	spin_unlock(&boot_lock);
+
+	ret = pen_release != -1 ? -ENXIO : 0;
+
+out:
+	return ret;
 }
 
 static struct smp_operations bcm_smp_ops __initdata = {
 	.smp_prepare_cpus	= bcm_smp_prepare_cpus,
-	.smp_boot_secondary	= bcm_boot_secondary,
+	.smp_boot_secondary	= kona_boot_secondary,
 };
 CPU_METHOD_OF_DECLARE(bcm_smp_bcm281xx, "brcm,bcm11351-cpu-method",
 			&bcm_smp_ops);
+
+struct smp_operations nsp_smp_ops __initdata = {
+	.smp_prepare_cpus	= bcm_smp_prepare_cpus,
+	.smp_secondary_init	= nsp_secondary_init,
+	.smp_boot_secondary	= nsp_boot_secondary,
+};
+CPU_METHOD_OF_DECLARE(bcm_smp_nsp, "brcm,bcm-nsp-smp", &nsp_smp_ops);