[PATCH 0/9] Clean up ARM SMP/CPU hotplug implementations

[PATCH 0/9] Clean up ARM SMP/CPU hotplug implementations

[Russell King - ARM Linux]

There is a lot of apparent copied code in arch/arm for handling SMP/
CPU hotplug, much of which is inappropriate or plain buggy.  This
seems to be a topic that occasionally comes up.

The "pen_release" thing was created for ARM Ltd development platforms
where there was no way to individually control secondary CPUs leaving
the boot loader - they all jumped to whatever physical code address
was supplied at the same time.  This made it necessary for _these_
platforms to have a "holding pen" for the CPUs while the kernel
initialised.

The "boot_lock" thing was also created for ARM Ltd development
platforms which had restricted bus bandwidth, and which used the
loops_per_jiffy delay mechanism, which was calibrated for each
secondary CPU.  With the restricted bus bandwidth, activity from the
boot CPU would affect the delay calibration adversely.

Lastly, the Versatile CPU hotplug implementation is an entirely
ficticious one - these platforms do _not_ support CPU hotplug as
there is no way to actually disable any of the secondary CPUs, or
reset them.  Such an implementation is not acceptable when supporting
features such as suspend or kexec.  As the Versatile platforms are
ARM development platforms which do not have suspend support, this is
acceptable there, but not for production hardware.

None of these three facilities/implementations should be used on
modern production hardware, yet we have a number of copies of this
code.

This series addresses that by removing the inappropriate copies of
some Realview/Versatile Express specific workarounds, and makes it
(hopefully) more clear that introducing this code is really not
acceptable.

To discourage copying the Versatile code, further comments are added
and the functions renamed for CPU hotplug to be "immitation" to make
it clear that it's not a real implementation.

We tried reducing the duplication in the past with ideas around
consolidating the pen_release/boot_lock/immitation hotplug stuff,
but I nacked that because it's not an acceptable implementation for
production hardware.  However, we did decide to consolidate the
"pen_release" definition.  In hind sight, that was a mistake,
because that gave more credence to that way of doing things, and
also gave rise to buggy implementations which only read from that
variable - meaning it served no useful purpose.

There are some rather complex cases that remain, and those need the
SoC folk to fix.

I have left the Actions Semi patch in place since following patches
depend on it, but there is a five-patch series from Linus Walleij
that address this platform which should replace this patch - with
the patch concerned marked as "RFT" - request for testing.

 arch/arm/include/asm/smp.h                         |   1 -
 arch/arm/kernel/smp.c                              |   6 --
 arch/arm/mach-actions/platsmp.c                    |  15 ---
 arch/arm/mach-exynos/headsmp.S                     |   2 +-
 arch/arm/mach-exynos/platsmp.c                     |  31 +++---
 arch/arm/mach-omap2/omap-smp.c                     |  20 ----
 arch/arm/mach-oxnas/Makefile                       |   1 -
 arch/arm/mach-oxnas/hotplug.c                      | 109 --------------------
 arch/arm/mach-oxnas/platsmp.c                      |   4 -
 arch/arm/mach-prima2/common.h                      |   2 +
 arch/arm/mach-prima2/headsmp.S                     |   2 +-
 arch/arm/mach-prima2/hotplug.c                     |   3 +-
 arch/arm/mach-prima2/platsmp.c                     |  17 ++--
 arch/arm/mach-qcom/platsmp.c                       |  26 -----
 arch/arm/mach-realview/Makefile                    |   1 -
 arch/arm/mach-realview/hotplug.c                   | 111 ---------------------
 arch/arm/mach-realview/hotplug.h                   |   1 -
 arch/arm/mach-realview/platsmp-dt.c                |   8 +-
 arch/arm/mach-spear/generic.h                      |   2 +
 arch/arm/mach-spear/headsmp.S                      |   2 +-
 arch/arm/mach-spear/hotplug.c                      |   4 +-
 arch/arm/mach-spear/platsmp.c                      |  27 +++--
 arch/arm/mach-sti/Makefile                         |   2 +-
 arch/arm/mach-sti/headsmp.S                        |  43 --------
 arch/arm/mach-sti/platsmp.c                        |  62 +-----------
 arch/arm/mach-vexpress/Makefile                    |   1 -
 arch/arm/mach-vexpress/core.h                      |   2 -
 arch/arm/mach-vexpress/platsmp.c                   |   7 ++
 arch/arm/plat-versatile/Makefile                   |   1 +
 arch/arm/plat-versatile/headsmp.S                  |   2 +-
 .../{mach-vexpress => plat-versatile}/hotplug.c    |  47 ++++-----
 arch/arm/plat-versatile/include/plat/platsmp.h     |   2 +
 arch/arm/plat-versatile/platsmp.c                  |  47 ++++++---
 33 files changed, 132 insertions(+), 479 deletions(-)
 delete mode 100644 arch/arm/mach-oxnas/hotplug.c
 delete mode 100644 arch/arm/mach-realview/hotplug.c
 delete mode 100644 arch/arm/mach-realview/hotplug.h
 delete mode 100644 arch/arm/mach-sti/headsmp.S
 rename arch/arm/{mach-vexpress => plat-versatile}/hotplug.c (56%)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/9] ARM: omap2: remove unnecessary boot_lock

[Russell King]

The boot_lock is something that was required for ARM development
platforms to ensure that the delay calibration worked properly.  This
is not necessary for modern platforms that have better bus bandwidth
and do not need to calibrate the delay loop for secondary cores.
Remove the boot_lock entirely.

Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-omap2/omap-smp.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
index 1c73694c871a..10e070368f64 100644
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -69,8 +69,6 @@ static const struct omap_smp_config omap5_cfg __initconst = {
 	.startup_addr = omap5_secondary_startup,
 };
 
-static DEFINE_SPINLOCK(boot_lock);
-
 void __iomem *omap4_get_scu_base(void)
 {
 	return cfg.scu_base;
@@ -173,12 +171,6 @@ static void omap4_secondary_init(unsigned int cpu)
 		/* Enable ACR to allow for ICUALLU workaround */
 		omap5_secondary_harden_predictor();
 	}
-
-	/*
-	 * Synchronise with the boot thread.
-	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
 }
 
 static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
@@ -188,12 +180,6 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	static struct powerdomain *cpu1_pwrdm;
 
 	/*
-	 * Set synchronisation state between this boot processor
-	 * and the secondary one
-	 */
-	spin_lock(&boot_lock);
-
-	/*
 	 * Update the AuxCoreBoot0 with boot state for secondary core.
 	 * omap4_secondary_startup() routine will hold the secondary core till
 	 * the AuxCoreBoot1 register is updated with cpu state
@@ -266,12 +252,6 @@ static int omap4_boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	/*
-	 * Now the secondary core is starting up let it run its
-	 * calibrations, then wait for it to finish
-	 */
-	spin_unlock(&boot_lock);
-
 	return 0;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Russell King]

The boot_lock is something that was required for ARM development
platforms to ensure that the delay calibration worked properly.  This
is not necessary for modern platforms that have better bus bandwidth
and do not need to calibrate the delay loop for secondary cores.
Remove the boot_lock entirely.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-qcom/platsmp.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
index 5494c9e0c909..99a6a5e809e0 100644
--- a/arch/arm/mach-qcom/platsmp.c
+++ b/arch/arm/mach-qcom/platsmp.c
@@ -46,8 +46,6 @@
 
 extern void secondary_startup_arm(void);
 
-static DEFINE_SPINLOCK(boot_lock);
-
 #ifdef CONFIG_HOTPLUG_CPU
 static void qcom_cpu_die(unsigned int cpu)
 {
@@ -55,15 +53,6 @@ static void qcom_cpu_die(unsigned int cpu)
 }
 #endif
 
-static void qcom_secondary_init(unsigned int cpu)
-{
-	/*
-	 * Synchronise with the boot thread.
-	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
-}
-
 static int scss_release_secondary(unsigned int cpu)
 {
 	struct device_node *node;
@@ -281,24 +270,12 @@ static int qcom_boot_secondary(unsigned int cpu, int (*func)(unsigned int))
 	}
 
 	/*
-	 * set synchronisation state between this boot processor
-	 * and the secondary one
-	 */
-	spin_lock(&boot_lock);
-
-	/*
 	 * Send the secondary CPU a soft interrupt, thereby causing
 	 * the boot monitor to read the system wide flags register,
 	 * and branch to the address found there.
 	 */
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	/*
-	 * now the secondary core is starting up let it run its
-	 * calibrations, then wait for it to finish
-	 */
-	spin_unlock(&boot_lock);
-
 	return ret;
 }
 
@@ -334,7 +311,6 @@ static void __init qcom_smp_prepare_cpus(unsigned int max_cpus)
 
 static const struct smp_operations smp_msm8660_ops __initconst = {
 	.smp_prepare_cpus	= qcom_smp_prepare_cpus,
-	.smp_secondary_init	= qcom_secondary_init,
 	.smp_boot_secondary	= msm8660_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= qcom_cpu_die,
@@ -344,7 +320,6 @@ CPU_METHOD_OF_DECLARE(qcom_smp, "qcom,gcc-msm8660", &smp_msm8660_ops);
 
 static const struct smp_operations qcom_smp_kpssv1_ops __initconst = {
 	.smp_prepare_cpus	= qcom_smp_prepare_cpus,
-	.smp_secondary_init	= qcom_secondary_init,
 	.smp_boot_secondary	= kpssv1_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= qcom_cpu_die,
@@ -354,7 +329,6 @@ CPU_METHOD_OF_DECLARE(qcom_smp_kpssv1, "qcom,kpss-acc-v1", &qcom_smp_kpssv1_ops)
 
 static const struct smp_operations qcom_smp_kpssv2_ops __initconst = {
 	.smp_prepare_cpus	= qcom_smp_prepare_cpus,
-	.smp_secondary_init	= qcom_secondary_init,
 	.smp_boot_secondary	= kpssv2_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= qcom_cpu_die,
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/9] ARM: oxnas: remove CPU hotplug implementation

[Russell King]

The CPU hotplug implementation on this platform is cargo-culted from
the plat-versatile implementation, and is buggy.  Once a CPU hits the
"low power" loop, it will wait for pen_release to be set to the CPU
number to wake up again - but nothing in this implementation does that.

So, once a CPU has entered cpu_die() it will never, ever leave.

Remove this useless cargo-culted implementation.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-oxnas/Makefile  |   1 -
 arch/arm/mach-oxnas/hotplug.c | 109 ------------------------------------------
 arch/arm/mach-oxnas/platsmp.c |   4 --
 3 files changed, 114 deletions(-)
 delete mode 100644 arch/arm/mach-oxnas/hotplug.c

diff --git a/arch/arm/mach-oxnas/Makefile b/arch/arm/mach-oxnas/Makefile
index b625906a9970..61a34e1c0f22 100644
--- a/arch/arm/mach-oxnas/Makefile
+++ b/arch/arm/mach-oxnas/Makefile
@@ -1,2 +1 @@
 obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
-obj-$(CONFIG_HOTPLUG_CPU) 	+= hotplug.o
diff --git a/arch/arm/mach-oxnas/hotplug.c b/arch/arm/mach-oxnas/hotplug.c
deleted file mode 100644
index 854f29b8cba6..000000000000
--- a/arch/arm/mach-oxnas/hotplug.c
+++ /dev/null
@@ -1,109 +0,0 @@
-/*
- *  Copyright (C) 2002 ARM Ltd.
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/smp.h>
-
-#include <asm/cp15.h>
-#include <asm/smp_plat.h>
-
-static inline void cpu_enter_lowpower(void)
-{
-	unsigned int v;
-
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
-	/*
-	 * Turn off coherency
-	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, #0x20\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	"	mrc	p15, 0, %0, c1, c0, 0\n"
-	"	bic	%0, %0, %2\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	  : "=&r" (v)
-	  : "r" (0), "Ir" (CR_C)
-	  : "cc");
-}
-
-static inline void cpu_leave_lowpower(void)
-{
-	unsigned int v;
-
-	asm volatile(	"mrc	p15, 0, %0, c1, c0, 0\n"
-	"	orr	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	orr	%0, %0, #0x20\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	  : "=&r" (v)
-	  : "Ir" (CR_C)
-	  : "cc");
-}
-
-static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
-{
-	/*
-	 * there is no power-control hardware on this platform, so all
-	 * we can do is put the core into WFI; this is safe as the calling
-	 * code will have already disabled interrupts
-	 */
-	for (;;) {
-		/*
-		 * here's the WFI
-		 */
-		asm(".word	0xe320f003\n"
-		    :
-		    :
-		    : "memory", "cc");
-
-		if (pen_release == cpu_logical_map(cpu)) {
-			/*
-			 * OK, proper wakeup, we're done
-			 */
-			break;
-		}
-
-		/*
-		 * Getting here, means that we have come out of WFI without
-		 * having been woken up - this shouldn't happen
-		 *
-		 * Just note it happening - when we're woken, we can report
-		 * its occurrence.
-		 */
-		(*spurious)++;
-	}
-}
-
-/*
- * platform-specific code to shutdown a CPU
- *
- * Called with IRQs disabled
- */
-void ox820_cpu_die(unsigned int cpu)
-{
-	int spurious = 0;
-
-	/*
-	 * we're ready for shutdown now, so do it
-	 */
-	cpu_enter_lowpower();
-	platform_do_lowpower(cpu, &spurious);
-
-	/*
-	 * bring this CPU back into the world of cache
-	 * coherency, and then restore interrupts
-	 */
-	cpu_leave_lowpower();
-
-	if (spurious)
-		pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
-}
diff --git a/arch/arm/mach-oxnas/platsmp.c b/arch/arm/mach-oxnas/platsmp.c
index 442cc8a2f7dc..735141c0e3a3 100644
--- a/arch/arm/mach-oxnas/platsmp.c
+++ b/arch/arm/mach-oxnas/platsmp.c
@@ -19,7 +19,6 @@
 #include <asm/smp_scu.h>
 
 extern void ox820_secondary_startup(void);
-extern void ox820_cpu_die(unsigned int cpu);
 
 static void __iomem *cpu_ctrl;
 static void __iomem *gic_cpu_ctrl;
@@ -94,9 +93,6 @@ static void __init ox820_smp_prepare_cpus(unsigned int max_cpus)
 static const struct smp_operations ox820_smp_ops __initconst = {
 	.smp_prepare_cpus	= ox820_smp_prepare_cpus,
 	.smp_boot_secondary	= ox820_boot_secondary,
-#ifdef CONFIG_HOTPLUG_CPU
-	.cpu_die		= ox820_cpu_die,
-#endif
 };
 
 CPU_METHOD_OF_DECLARE(ox820_smp, "oxsemi,ox820-smp", &ox820_smp_ops);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/9] ARM: sti: remove pen_release and boot_lock

[Russell King]

The pen_release implementation was created for Versatile platforms to
work around boot loaders that did not differentiate between the
various different secondary CPUs on this ARM development platform.
This should not be true of modern platforms where we send IPIs to
specific CPUs to wake them up.  Remove the pen_release stuff from
SoCs that make use of the per-CPU IPI mechanism.

The boot_lock is something that was required for ARM development
platforms to ensure that the delay calibration worked properly.  This
is not necessary for modern platforms that have better bus bandwidth
and do not need to calibrate the delay loop for secondary cores.
Remove the boot_lock entirely.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-sti/Makefile  |  2 +-
 arch/arm/mach-sti/headsmp.S | 43 -------------------------------
 arch/arm/mach-sti/platsmp.c | 62 ++-------------------------------------------
 3 files changed, 3 insertions(+), 104 deletions(-)
 delete mode 100644 arch/arm/mach-sti/headsmp.S

diff --git a/arch/arm/mach-sti/Makefile b/arch/arm/mach-sti/Makefile
index acb330916333..f85ff059cfba 100644
--- a/arch/arm/mach-sti/Makefile
+++ b/arch/arm/mach-sti/Makefile
@@ -1,2 +1,2 @@
-obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
+obj-$(CONFIG_SMP)		+= platsmp.o
 obj-$(CONFIG_ARCH_STI) 		+= board-dt.o
diff --git a/arch/arm/mach-sti/headsmp.S b/arch/arm/mach-sti/headsmp.S
deleted file mode 100644
index e0ad451700d5..000000000000
--- a/arch/arm/mach-sti/headsmp.S
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- *  arch/arm/mach-sti/headsmp.S
- *
- * Copyright (C) 2013 STMicroelectronics (R&D) Limited.
- *		http://www.st.com
- *
- * Cloned from linux/arch/arm/mach-vexpress/headsmp.S
- *
- *  Copyright (c) 2003 ARM Limited
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/linkage.h>
-#include <linux/init.h>
-
-/*
- * ST specific entry point for secondary CPUs.  This provides
- * a "holding pen" into which all secondary cores are held until we're
- * ready for them to initialise.
- */
-ENTRY(sti_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
-
-	/*
-	 * we've been released from the holding pen: secondary_stack
-	 * should now contain the SVC stack for this core
-	 */
-	b	secondary_startup
-ENDPROC(sti_secondary_startup)
-
-1:	.long	.
-	.long	pen_release
diff --git a/arch/arm/mach-sti/platsmp.c b/arch/arm/mach-sti/platsmp.c
index 231f19e17436..21668501c9bb 100644
--- a/arch/arm/mach-sti/platsmp.c
+++ b/arch/arm/mach-sti/platsmp.c
@@ -28,72 +28,15 @@
 
 #include "smp.h"
 
-static void write_pen_release(int val)
-{
-	pen_release = val;
-	smp_wmb();
-	sync_cache_w(&pen_release);
-}
-
-static DEFINE_SPINLOCK(boot_lock);
-
-static void sti_secondary_init(unsigned int cpu)
-{
-	/*
-	 * let the primary processor know we're out of the
-	 * pen, then head off into the C entry point
-	 */
-	write_pen_release(-1);
-
-	/*
-	 * Synchronise with the boot thread.
-	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
-}
-
 static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
-	unsigned long timeout;
-
-	/*
-	 * set synchronisation state between this boot processor
-	 * and the secondary one
-	 */
-	spin_lock(&boot_lock);
-
-	/*
-	 * 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.
-	 *
-	 * Note that "pen_release" is the hardware CPU ID, whereas
-	 * "cpu" is Linux's internal ID.
-	 */
-	write_pen_release(cpu_logical_map(cpu));
-
 	/*
 	 * Send the secondary CPU a soft interrupt, thereby causing
 	 * it to jump to the secondary entrypoint.
 	 */
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	timeout = jiffies + (1 * HZ);
-	while (time_before(jiffies, timeout)) {
-		smp_rmb();
-		if (pen_release == -1)
-			break;
-
-		udelay(10);
-	}
-
-	/*
-	 * now the secondary core is starting up let it run its
-	 * calibrations, then wait for it to finish
-	 */
-	spin_unlock(&boot_lock);
-
-	return pen_release != -1 ? -ENOSYS : 0;
+	return 0;
 }
 
 static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
@@ -103,7 +46,7 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
 	u32 __iomem *cpu_strt_ptr;
 	u32 release_phys;
 	int cpu;
-	unsigned long entry_pa = __pa_symbol(sti_secondary_startup);
+	unsigned long entry_pa = __pa_symbol(secondary_startup);
 
 	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
 
@@ -158,6 +101,5 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
 
 const struct smp_operations sti_smp_ops __initconst = {
 	.smp_prepare_cpus	= sti_smp_prepare_cpus,
-	.smp_secondary_init	= sti_secondary_init,
 	.smp_boot_secondary	= sti_boot_secondary,
 };
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/9] ARM: actions: remove boot_lock and pen_release

[Russell King]

The actions SMP implementation has several issues:

1. pen_release is only ever read and compared to -1, and is defined in
   arch/arm/kernel/smp.c to be -1.  This test will always succeed.

2. we are already guaranteed to be single threaded while bringing up a
   CPU, so the spinlock makes no sense, remove it.

3. owl_secondary_startup() is not referenced nor defined, the prototype
   is redundant, remove it.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-actions/platsmp.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/platsmp.c
index 3efaa10efc43..4fd479c948e6 100644
--- a/arch/arm/mach-actions/platsmp.c
+++ b/arch/arm/mach-actions/platsmp.c
@@ -39,10 +39,6 @@ static void __iomem *sps_base_addr;
 static void __iomem *timer_base_addr;
 static int ncores;
 
-static DEFINE_SPINLOCK(boot_lock);
-
-void owl_secondary_startup(void);
-
 static int s500_wakeup_secondary(unsigned int cpu)
 {
 	int ret;
@@ -84,7 +80,6 @@ static int s500_wakeup_secondary(unsigned int cpu)
 
 static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
 {
-	unsigned long timeout;
 	int ret;
 
 	ret = s500_wakeup_secondary(cpu);
@@ -93,21 +88,11 @@ static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 	udelay(10);
 
-	spin_lock(&boot_lock);
-
 	smp_send_reschedule(cpu);
 
-	timeout = jiffies + (1 * HZ);
-	while (time_before(jiffies, timeout)) {
-		if (pen_release == -1)
-			break;
-	}
-
 	writel(0, timer_base_addr + OWL_CPU1_ADDR + (cpu - 1) * 4);
 	writel(0, timer_base_addr + OWL_CPU1_FLAG + (cpu - 1) * 4);
 
-	spin_unlock(&boot_lock);
-
 	return 0;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/9] ARM: vexpress/realview: consolidate immitation CPU hotplug

[Russell King]

The only difference between the hotplug implementation for Realview
and Versatile Express are the bit in the auxiliary control register
to disable coherency.  Combine the two implentations accounting for
that difference.

Rename the functions to try to discourage cargo-cult copying of this
code.

Tested-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-realview/Makefile                    |   1 -
 arch/arm/mach-realview/hotplug.c                   | 111 ---------------------
 arch/arm/mach-realview/hotplug.h                   |   1 -
 arch/arm/mach-realview/platsmp-dt.c                |   8 +-
 arch/arm/mach-vexpress/Makefile                    |   1 -
 arch/arm/mach-vexpress/core.h                      |   2 -
 arch/arm/mach-vexpress/platsmp.c                   |   7 ++
 arch/arm/plat-versatile/Makefile                   |   1 +
 .../{mach-vexpress => plat-versatile}/hotplug.c    |  45 ++++-----
 arch/arm/plat-versatile/include/plat/platsmp.h     |   1 +
 10 files changed, 36 insertions(+), 142 deletions(-)
 delete mode 100644 arch/arm/mach-realview/hotplug.c
 delete mode 100644 arch/arm/mach-realview/hotplug.h
 rename arch/arm/{mach-vexpress => plat-versatile}/hotplug.c (58%)

diff --git a/arch/arm/mach-realview/Makefile b/arch/arm/mach-realview/Makefile
index adf39ad71cc3..6ca6400fa51e 100644
--- a/arch/arm/mach-realview/Makefile
+++ b/arch/arm/mach-realview/Makefile
@@ -5,4 +5,3 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/plat-versatile/inc
 
 obj-y					+= realview-dt.o
 obj-$(CONFIG_SMP)			+= platsmp-dt.o
-obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o
diff --git a/arch/arm/mach-realview/hotplug.c b/arch/arm/mach-realview/hotplug.c
deleted file mode 100644
index 968e2d1964f6..000000000000
--- a/arch/arm/mach-realview/hotplug.c
+++ /dev/null
@@ -1,111 +0,0 @@
-/*
- *  linux/arch/arm/mach-realview/hotplug.c
- *
- *  Copyright (C) 2002 ARM Ltd.
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/smp.h>
-
-#include <asm/cp15.h>
-#include <asm/smp_plat.h>
-
-static inline void cpu_enter_lowpower(void)
-{
-	unsigned int v;
-
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
-	/*
-	 * Turn off coherency
-	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, #0x20\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	"	mrc	p15, 0, %0, c1, c0, 0\n"
-	"	bic	%0, %0, %2\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	  : "=&r" (v)
-	  : "r" (0), "Ir" (CR_C)
-	  : "cc");
-}
-
-static inline void cpu_leave_lowpower(void)
-{
-	unsigned int v;
-
-	asm volatile(	"mrc	p15, 0, %0, c1, c0, 0\n"
-	"	orr	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	orr	%0, %0, #0x20\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	  : "=&r" (v)
-	  : "Ir" (CR_C)
-	  : "cc");
-}
-
-static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
-{
-	/*
-	 * there is no power-control hardware on this platform, so all
-	 * we can do is put the core into WFI; this is safe as the calling
-	 * code will have already disabled interrupts
-	 */
-	for (;;) {
-		/*
-		 * here's the WFI
-		 */
-		asm(".word	0xe320f003\n"
-		    :
-		    :
-		    : "memory", "cc");
-
-		if (pen_release == cpu_logical_map(cpu)) {
-			/*
-			 * OK, proper wakeup, we're done
-			 */
-			break;
-		}
-
-		/*
-		 * Getting here, means that we have come out of WFI without
-		 * having been woken up - this shouldn't happen
-		 *
-		 * Just note it happening - when we're woken, we can report
-		 * its occurrence.
-		 */
-		(*spurious)++;
-	}
-}
-
-/*
- * platform-specific code to shutdown a CPU
- *
- * Called with IRQs disabled
- */
-void realview_cpu_die(unsigned int cpu)
-{
-	int spurious = 0;
-
-	/*
-	 * we're ready for shutdown now, so do it
-	 */
-	cpu_enter_lowpower();
-	platform_do_lowpower(cpu, &spurious);
-
-	/*
-	 * bring this CPU back into the world of cache
-	 * coherency, and then restore interrupts
-	 */
-	cpu_leave_lowpower();
-
-	if (spurious)
-		pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
-}
diff --git a/arch/arm/mach-realview/hotplug.h b/arch/arm/mach-realview/hotplug.h
deleted file mode 100644
index eacd7a4dad2f..000000000000
--- a/arch/arm/mach-realview/hotplug.h
+++ /dev/null
@@ -1 +0,0 @@
-void realview_cpu_die(unsigned int cpu);
diff --git a/arch/arm/mach-realview/platsmp-dt.c b/arch/arm/mach-realview/platsmp-dt.c
index c242423bf8db..ce331b3dbf54 100644
--- a/arch/arm/mach-realview/platsmp-dt.c
+++ b/arch/arm/mach-realview/platsmp-dt.c
@@ -17,7 +17,6 @@
 #include <asm/smp_scu.h>
 
 #include <plat/platsmp.h>
-#include "hotplug.h"
 
 #define REALVIEW_SYS_FLAGSSET_OFFSET	0x30
 
@@ -79,6 +78,13 @@ static void __init realview_smp_prepare_cpus(unsigned int max_cpus)
 		     __pa_symbol(versatile_secondary_startup));
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+static void realview_cpu_die(unsigned int cpu)
+{
+	return versatile_immitation_cpu_die(cpu, 0x20);
+}
+#endif
+
 static const struct smp_operations realview_dt_smp_ops __initconst = {
 	.smp_prepare_cpus	= realview_smp_prepare_cpus,
 	.smp_secondary_init	= versatile_secondary_init,
diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile
index 51c35e2b737a..3651a1ed0f2b 100644
--- a/arch/arm/mach-vexpress/Makefile
+++ b/arch/arm/mach-vexpress/Makefile
@@ -15,6 +15,5 @@ obj-$(CONFIG_ARCH_VEXPRESS_TC2_PM)	+= tc2_pm.o
 CFLAGS_tc2_pm.o				+= -march=armv7-a
 CFLAGS_REMOVE_tc2_pm.o			= -pg
 obj-$(CONFIG_SMP)			+= platsmp.o
-obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o
 
 obj-$(CONFIG_ARCH_MPS2)			+= v2m-mps2.o
diff --git a/arch/arm/mach-vexpress/core.h b/arch/arm/mach-vexpress/core.h
index a162ab46ee02..f4a7519084f1 100644
--- a/arch/arm/mach-vexpress/core.h
+++ b/arch/arm/mach-vexpress/core.h
@@ -1,5 +1,3 @@
 bool vexpress_smp_init_ops(void);
 
 extern const struct smp_operations vexpress_smp_dt_ops;
-
-extern void vexpress_cpu_die(unsigned int cpu);
diff --git a/arch/arm/mach-vexpress/platsmp.c b/arch/arm/mach-vexpress/platsmp.c
index 742499bac6d0..af0113be5970 100644
--- a/arch/arm/mach-vexpress/platsmp.c
+++ b/arch/arm/mach-vexpress/platsmp.c
@@ -82,6 +82,13 @@ static void __init vexpress_smp_dt_prepare_cpus(unsigned int max_cpus)
 	vexpress_flags_set(__pa_symbol(versatile_secondary_startup));
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+static void vexpress_cpu_die(unsigned int cpu)
+{
+	versatile_immitation_cpu_die(cpu, 0x40);
+}
+#endif
+
 const struct smp_operations vexpress_smp_dt_ops __initconst = {
 	.smp_prepare_cpus	= vexpress_smp_dt_prepare_cpus,
 	.smp_secondary_init	= versatile_secondary_init,
diff --git a/arch/arm/plat-versatile/Makefile b/arch/arm/plat-versatile/Makefile
index bff3ba889882..b2f0ddfdc4cc 100644
--- a/arch/arm/plat-versatile/Makefile
+++ b/arch/arm/plat-versatile/Makefile
@@ -2,3 +2,4 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include
 
 obj-$(CONFIG_PLAT_VERSATILE_SCHED_CLOCK) += sched-clock.o
 obj-$(CONFIG_SMP) += headsmp.o platsmp.o
+obj-$(CONFIG_HOTPLUG_CPU)		+= hotplug.o
diff --git a/arch/arm/mach-vexpress/hotplug.c b/arch/arm/plat-versatile/hotplug.c
similarity index 58%
rename from arch/arm/mach-vexpress/hotplug.c
rename to arch/arm/plat-versatile/hotplug.c
index d8f1a05f5e87..e2d3e9035d0f 100644
--- a/arch/arm/mach-vexpress/hotplug.c
+++ b/arch/arm/plat-versatile/hotplug.c
@@ -1,12 +1,15 @@
 /*
- *  linux/arch/arm/mach-realview/hotplug.c
- *
  *  Copyright (C) 2002 ARM Ltd.
  *  All Rights Reserved
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
+ *
+ * This hotplug implementation is _specific_ to the situation found on
+ * ARM development platforms where there is _no_ possibility of actually
+ * taking a CPU offline, resetting it, or otherwise.  Real platforms must
+ * NOT copy this code.
  */
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -15,9 +18,7 @@
 #include <asm/smp_plat.h>
 #include <asm/cp15.h>
 
-#include "core.h"
-
-static inline void cpu_enter_lowpower(void)
+static inline void versatile_immitation_enter_lowpower(unsigned int actrl_mask)
 {
 	unsigned int v;
 
@@ -34,11 +35,11 @@ static inline void cpu_enter_lowpower(void)
 	"	bic	%0, %0, %2\n"
 	"	mcr	p15, 0, %0, c1, c0, 0\n"
 	  : "=&r" (v)
-	  : "r" (0), "Ir" (CR_C), "Ir" (0x40)
+	  : "r" (0), "Ir" (CR_C), "Ir" (actrl_mask)
 	  : "cc");
 }
 
-static inline void cpu_leave_lowpower(void)
+static inline void versatile_immitation_leave_lowpower(unsigned int actrl_mask)
 {
 	unsigned int v;
 
@@ -50,16 +51,18 @@ static inline void cpu_leave_lowpower(void)
 	"	orr	%0, %0, %2\n"
 	"	mcr	p15, 0, %0, c1, c0, 1\n"
 	  : "=&r" (v)
-	  : "Ir" (CR_C), "Ir" (0x40)
+	  : "Ir" (CR_C), "Ir" (actrl_mask)
 	  : "cc");
 }
 
-static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
+static inline void versatile_immitation_do_lowpower(unsigned int cpu, int *spurious)
 {
 	/*
 	 * there is no power-control hardware on this platform, so all
 	 * we can do is put the core into WFI; this is safe as the calling
-	 * code will have already disabled interrupts
+	 * code will have already disabled interrupts.
+	 *
+	 * This code should not be used outside Versatile platforms.
 	 */
 	for (;;) {
 		wfi();
@@ -83,25 +86,17 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
 }
 
 /*
- * platform-specific code to shutdown a CPU
- *
- * Called with IRQs disabled
+ * platform-specific code to shutdown a CPU.
+ * This code supports immitation-style CPU hotplug for Versatile/Realview/
+ * Versatile Express platforms that are unable to do real CPU hotplug.
  */
-void vexpress_cpu_die(unsigned int cpu)
+void versatile_immitation_cpu_die(unsigned int cpu, unsigned int actrl_mask)
 {
 	int spurious = 0;
 
-	/*
-	 * we're ready for shutdown now, so do it
-	 */
-	cpu_enter_lowpower();
-	platform_do_lowpower(cpu, &spurious);
-
-	/*
-	 * bring this CPU back into the world of cache
-	 * coherency, and then restore interrupts
-	 */
-	cpu_leave_lowpower();
+	versatile_immitation_enter_lowpower(actrl_mask);
+	versatile_immitation_do_lowpower(cpu, &spurious);
+	versatile_immitation_leave_lowpower(actrl_mask);
 
 	if (spurious)
 		pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
diff --git a/arch/arm/plat-versatile/include/plat/platsmp.h b/arch/arm/plat-versatile/include/plat/platsmp.h
index 50fb830192e0..9fff1f241c9e 100644
--- a/arch/arm/plat-versatile/include/plat/platsmp.h
+++ b/arch/arm/plat-versatile/include/plat/platsmp.h
@@ -12,3 +12,4 @@
 extern void versatile_secondary_startup(void);
 extern void versatile_secondary_init(unsigned int cpu);
 extern int  versatile_boot_secondary(unsigned int cpu, struct task_struct *idle);
+void versatile_immitation_cpu_die(unsigned int cpu, unsigned int actrl_mask);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/9] ARM: versatile: convert boot_lock to raw

[Russell King]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

The arm boot_lock is used by the secondary processor startup code.  The locking
task is the idle thread, which has idle->sched_class == &idle_sched_class.
idle_sched_class->enqueue_task == NULL, so if the idle task blocks on the
lock, the attempt to wake it when the lock becomes available will fail:

try_to_wake_up()
   ...
      activate_task()
         enqueue_task()
            p->sched_class->enqueue_task(rq, p, flags)

Fix by converting boot_lock to a raw spin lock.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Link: http://lkml.kernel.org/r/4E77B952.3010606@am.sony.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/plat-versatile/platsmp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-versatile/platsmp.c b/arch/arm/plat-versatile/platsmp.c
index c2366510187a..6b60f582b738 100644
--- a/arch/arm/plat-versatile/platsmp.c
+++ b/arch/arm/plat-versatile/platsmp.c
@@ -32,7 +32,7 @@ static void write_pen_release(int val)
 	sync_cache_w(&pen_release);
 }
 
-static DEFINE_SPINLOCK(boot_lock);
+static DEFINE_RAW_SPINLOCK(boot_lock);
 
 void versatile_secondary_init(unsigned int cpu)
 {
@@ -45,8 +45,8 @@ void versatile_secondary_init(unsigned int cpu)
 	/*
 	 * Synchronise with the boot thread.
 	 */
-	spin_lock(&boot_lock);
-	spin_unlock(&boot_lock);
+	raw_spin_lock(&boot_lock);
+	raw_spin_unlock(&boot_lock);
 }
 
 int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
@@ -57,7 +57,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 * Set synchronisation state between this boot processor
 	 * and the secondary one
 	 */
-	spin_lock(&boot_lock);
+	raw_spin_lock(&boot_lock);
 
 	/*
 	 * This is really belt and braces; we hold unintended secondary
@@ -87,7 +87,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 * now the secondary core is starting up let it run its
 	 * calibrations, then wait for it to finish
 	 */
-	spin_unlock(&boot_lock);
+	raw_spin_unlock(&boot_lock);
 
 	return pen_release != -1 ? -ENOSYS : 0;
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 8/9] ARM: versatile: rename and comment SMP implementation

[Russell King]

Rename pen_release and boot_lock in the Versatile specific SMP
implementation, describe why these exist and state clearly that they
should not be used in production implementations.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/plat-versatile/headsmp.S              |  2 +-
 arch/arm/plat-versatile/hotplug.c              |  4 ++-
 arch/arm/plat-versatile/include/plat/platsmp.h |  1 +
 arch/arm/plat-versatile/platsmp.c              | 47 ++++++++++++++++++--------
 4 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/arch/arm/plat-versatile/headsmp.S b/arch/arm/plat-versatile/headsmp.S
index 40f27e52de75..e99396dfa6f3 100644
--- a/arch/arm/plat-versatile/headsmp.S
+++ b/arch/arm/plat-versatile/headsmp.S
@@ -37,5 +37,5 @@ pen:	ldr	r7, [r6]
 
 	.align
 1:	.long	.
-	.long	pen_release
+	.long	versatile_cpu_release
 ENDPROC(versatile_secondary_startup)
diff --git a/arch/arm/plat-versatile/hotplug.c b/arch/arm/plat-versatile/hotplug.c
index e2d3e9035d0f..c974958417fe 100644
--- a/arch/arm/plat-versatile/hotplug.c
+++ b/arch/arm/plat-versatile/hotplug.c
@@ -18,6 +18,8 @@
 #include <asm/smp_plat.h>
 #include <asm/cp15.h>
 
+#include <plat/platsmp.h>
+
 static inline void versatile_immitation_enter_lowpower(unsigned int actrl_mask)
 {
 	unsigned int v;
@@ -67,7 +69,7 @@ static inline void versatile_immitation_do_lowpower(unsigned int cpu, int *spuri
 	for (;;) {
 		wfi();
 
-		if (pen_release == cpu_logical_map(cpu)) {
+		if (versatile_cpu_release == cpu_logical_map(cpu)) {
 			/*
 			 * OK, proper wakeup, we're done
 			 */
diff --git a/arch/arm/plat-versatile/include/plat/platsmp.h b/arch/arm/plat-versatile/include/plat/platsmp.h
index 9fff1f241c9e..1b087fbbc700 100644
--- a/arch/arm/plat-versatile/include/plat/platsmp.h
+++ b/arch/arm/plat-versatile/include/plat/platsmp.h
@@ -8,6 +8,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+extern volatile int versatile_cpu_release;
 
 extern void versatile_secondary_startup(void);
 extern void versatile_secondary_init(unsigned int cpu);
diff --git a/arch/arm/plat-versatile/platsmp.c b/arch/arm/plat-versatile/platsmp.c
index 6b60f582b738..6e2836243187 100644
--- a/arch/arm/plat-versatile/platsmp.c
+++ b/arch/arm/plat-versatile/platsmp.c
@@ -7,6 +7,11 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
+ *
+ * This code is specific to the hardware found on ARM Realview and
+ * Versatile Express platforms where the CPUs are unable to be individually
+ * woken, and where there is no way to hot-unplug CPUs.  Real platforms
+ * should not copy this code.
  */
 #include <linux/init.h>
 #include <linux/errno.h>
@@ -21,18 +26,32 @@
 #include <plat/platsmp.h>
 
 /*
- * Write pen_release in a way that is guaranteed to be visible to all
- * observers, irrespective of whether they're taking part in coherency
+ * versatile_cpu_release controls the release of CPUs from the holding
+ * pen in headsmp.S, which exists because we are not always able to
+ * control the release of individual CPUs from the board firmware.
+ * Production platforms do not need this.
+ */
+volatile int versatile_cpu_release = -1;
+
+/*
+ * Write versatile_cpu_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)
+static void versatile_write_cpu_release(int val)
 {
-	pen_release = val;
+	versatile_cpu_release = val;
 	smp_wmb();
-	sync_cache_w(&pen_release);
+	sync_cache_w(&versatile_cpu_release);
 }
 
-static DEFINE_RAW_SPINLOCK(boot_lock);
+/*
+ * versatile_lock exists to avoid running the loops_per_jiffy delay loop
+ * calibrations on the secondary CPU while the requesting CPU is using
+ * the limited-bandwidth bus - which affects the calibration value.
+ * Production platforms do not need this.
+ */
+static DEFINE_RAW_SPINLOCK(versatile_lock);
 
 void versatile_secondary_init(unsigned int cpu)
 {
@@ -40,13 +59,13 @@ void versatile_secondary_init(unsigned int cpu)
 	 * let the primary processor know we're out of the
 	 * pen, then head off into the C entry point
 	 */
-	write_pen_release(-1);
+	versatile_write_cpu_release(-1);
 
 	/*
 	 * Synchronise with the boot thread.
 	 */
-	raw_spin_lock(&boot_lock);
-	raw_spin_unlock(&boot_lock);
+	raw_spin_lock(&versatile_lock);
+	raw_spin_unlock(&versatile_lock);
 }
 
 int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
@@ -57,7 +76,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 * Set synchronisation state between this boot processor
 	 * and the secondary one
 	 */
-	raw_spin_lock(&boot_lock);
+	raw_spin_lock(&versatile_lock);
 
 	/*
 	 * This is really belt and braces; we hold unintended secondary
@@ -65,7 +84,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 * since we haven't sent them a soft interrupt, they shouldn't
 	 * be there.
 	 */
-	write_pen_release(cpu_logical_map(cpu));
+	versatile_write_cpu_release(cpu_logical_map(cpu));
 
 	/*
 	 * Send the secondary CPU a soft interrupt, thereby causing
@@ -77,7 +96,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	timeout = jiffies + (1 * HZ);
 	while (time_before(jiffies, timeout)) {
 		smp_rmb();
-		if (pen_release == -1)
+		if (versatile_cpu_release == -1)
 			break;
 
 		udelay(10);
@@ -87,7 +106,7 @@ int versatile_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 * now the secondary core is starting up let it run its
 	 * calibrations, then wait for it to finish
 	 */
-	raw_spin_unlock(&boot_lock);
+	raw_spin_unlock(&versatile_lock);
 
-	return pen_release != -1 ? -ENOSYS : 0;
+	return versatile_cpu_release != -1 ? -ENOSYS : 0;
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 9/9] ARM: smp: remove arch-provided "pen_release"

[Russell King]

Consolidating the "pen_release" stuff amongst the various SoC
implementations gives credence to having a CPU holding pen for
secondary CPUs.  However, this is far from the truth.

Many SoC implementations cargo-cult copied various bits of the pen
release implementation from the initial Realview/Versatile Express
implementation without understanding what it was or why it existed.
The reason it existed is because these are _development_ platforms,
and some board firmware is unable to individually control the
startup of secondary CPUs.  Moreover, they do not have a way to
power down or reset secondary CPUs for hot-unplug.  Hence, the
pen_release implementation was designed for ARM Ltd's development
platforms to provide a working implementation, even though it is
very far from what is required.

It was decided a while back to reduce the duplication by consolidating
the "pen_release" variable, but this only made the situation worse -
we have ended up with several implementations that read this variable
but do not write it - again, showing the cargo-cult mentality at work,
lack of proper review of new code, and in some cases a lack of testing.

While it would be preferable to remove pen_release entirely from the
kernel, this is not possible without help from the SoC maintainers,
which seems to be lacking.  However, I want to remove pen_release from
arch code to remove the credence that having it gives.

This patch removes pen_release from the arch code entirely, adding
private per-SoC definitions for it instead, and explicitly stating
that write_pen_release() is cargo-cult copied and should not be
copied any further.  Rename write_pen_release() in a similar fashion
as well.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/include/asm/smp.h     |  1 -
 arch/arm/kernel/smp.c          |  6 ------
 arch/arm/mach-exynos/headsmp.S |  2 +-
 arch/arm/mach-exynos/platsmp.c | 31 ++++++++++++++++++-------------
 arch/arm/mach-prima2/common.h  |  2 ++
 arch/arm/mach-prima2/headsmp.S |  2 +-
 arch/arm/mach-prima2/hotplug.c |  3 ++-
 arch/arm/mach-prima2/platsmp.c | 17 ++++++++++-------
 arch/arm/mach-spear/generic.h  |  2 ++
 arch/arm/mach-spear/headsmp.S  |  2 +-
 arch/arm/mach-spear/hotplug.c  |  4 +++-
 arch/arm/mach-spear/platsmp.c  | 27 ++++++++++++++++-----------
 12 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 709a55989cb0..451ae684aaf4 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -67,7 +67,6 @@ struct secondary_data {
 	void *stack;
 };
 extern struct secondary_data secondary_data;
-extern volatile int pen_release;
 extern void secondary_startup(void);
 extern void secondary_startup_arm(void);
 
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 0978282d5fc2..04aa1bcb8d98 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -61,12 +61,6 @@
  */
 struct secondary_data secondary_data;
 
-/*
- * control for which core is the next to come out of the secondary
- * boot "holding pen"
- */
-volatile int pen_release = -1;
-
 enum ipi_msg_type {
 	IPI_WAKEUP,
 	IPI_TIMER,
diff --git a/arch/arm/mach-exynos/headsmp.S b/arch/arm/mach-exynos/headsmp.S
index 005695c9bf40..0ac2cb9a7355 100644
--- a/arch/arm/mach-exynos/headsmp.S
+++ b/arch/arm/mach-exynos/headsmp.S
@@ -36,4 +36,4 @@ ENDPROC(exynos4_secondary_startup)
 
 	.align 2
 1:	.long	.
-	.long	pen_release
+	.long	exynos_pen_release
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 6a1e682371b3..76c3f05854e4 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -28,6 +28,9 @@
 
 extern void exynos4_secondary_startup(void);
 
+/* XXX exynos_pen_release is cargo culted code - DO NOT COPY XXX */
+volatile int exynos_pen_release = -1;
+
 #ifdef CONFIG_HOTPLUG_CPU
 static inline void cpu_leave_lowpower(u32 core_id)
 {
@@ -57,7 +60,7 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
 
 		wfi();
 
-		if (pen_release == core_id) {
+		if (exynos_pen_release == core_id) {
 			/*
 			 * OK, proper wakeup, we're done
 			 */
@@ -228,15 +231,17 @@ void exynos_core_restart(u32 core_id)
 }
 
 /*
- * Write pen_release in a way that is guaranteed to be visible to all
- * observers, irrespective of whether they're taking part in coherency
+ * XXX CARGO CULTED CODE - DO NOT COPY XXX
+ *
+ * Write exynos_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)
+static void exynos_write_pen_release(int val)
 {
-	pen_release = val;
+	exynos_pen_release = val;
 	smp_wmb();
-	sync_cache_w(&pen_release);
+	sync_cache_w(&exynos_pen_release);
 }
 
 static DEFINE_SPINLOCK(boot_lock);
@@ -247,7 +252,7 @@ static void exynos_secondary_init(unsigned int cpu)
 	 * let the primary processor know we're out of the
 	 * pen, then head off into the C entry point
 	 */
-	write_pen_release(-1);
+	exynos_write_pen_release(-1);
 
 	/*
 	 * Synchronise with the boot thread.
@@ -322,12 +327,12 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	/*
 	 * 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.
+	 * that it has been released by resetting exynos_pen_release.
 	 *
-	 * Note that "pen_release" is the hardware CPU core ID, whereas
+	 * Note that "exynos_pen_release" is the hardware CPU core ID, whereas
 	 * "cpu" is Linux's internal ID.
 	 */
-	write_pen_release(core_id);
+	exynos_write_pen_release(core_id);
 
 	if (!exynos_cpu_power_state(core_id)) {
 		exynos_cpu_power_up(core_id);
@@ -376,13 +381,13 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 		else
 			arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-		if (pen_release == -1)
+		if (exynos_pen_release == -1)
 			break;
 
 		udelay(10);
 	}
 
-	if (pen_release != -1)
+	if (exynos_pen_release != -1)
 		ret = -ETIMEDOUT;
 
 	/*
@@ -392,7 +397,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 fail:
 	spin_unlock(&boot_lock);
 
-	return pen_release != -1 ? ret : 0;
+	return exynos_pen_release != -1 ? ret : 0;
 }
 
 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
diff --git a/arch/arm/mach-prima2/common.h b/arch/arm/mach-prima2/common.h
index 6d77b622d168..457eb7b18160 100644
--- a/arch/arm/mach-prima2/common.h
+++ b/arch/arm/mach-prima2/common.h
@@ -15,6 +15,8 @@
 #include <asm/mach/time.h>
 #include <asm/exception.h>
 
+extern volatile int prima2_pen_release;
+
 extern const struct smp_operations sirfsoc_smp_ops;
 extern void sirfsoc_secondary_startup(void);
 extern void sirfsoc_cpu_die(unsigned int cpu);
diff --git a/arch/arm/mach-prima2/headsmp.S b/arch/arm/mach-prima2/headsmp.S
index 209d9fc5c16c..6cf4fc60347b 100644
--- a/arch/arm/mach-prima2/headsmp.S
+++ b/arch/arm/mach-prima2/headsmp.S
@@ -34,4 +34,4 @@ ENDPROC(sirfsoc_secondary_startup)
 
         .align
 1:      .long   .
-        .long   pen_release
+        .long   prima2_pen_release
diff --git a/arch/arm/mach-prima2/hotplug.c b/arch/arm/mach-prima2/hotplug.c
index a728c78b996f..b6cf1527e330 100644
--- a/arch/arm/mach-prima2/hotplug.c
+++ b/arch/arm/mach-prima2/hotplug.c
@@ -11,6 +11,7 @@
 #include <linux/smp.h>
 
 #include <asm/smp_plat.h>
+#include "common.h"
 
 static inline void platform_do_lowpower(unsigned int cpu)
 {
@@ -18,7 +19,7 @@ static inline void platform_do_lowpower(unsigned int cpu)
 	for (;;) {
 		__asm__ __volatile__("dsb\n\t" "wfi\n\t"
 			: : : "memory");
-		if (pen_release == cpu_logical_map(cpu)) {
+		if (prima2_pen_release == cpu_logical_map(cpu)) {
 			/*
 			 * OK, proper wakeup, we're done
 			 */
diff --git a/arch/arm/mach-prima2/platsmp.c b/arch/arm/mach-prima2/platsmp.c
index 75ef5d4be554..d1f8b5168083 100644
--- a/arch/arm/mach-prima2/platsmp.c
+++ b/arch/arm/mach-prima2/platsmp.c
@@ -24,13 +24,16 @@ static void __iomem *clk_base;
 
 static DEFINE_SPINLOCK(boot_lock);
 
+/* XXX prima2_pen_release is cargo culted code - DO NOT COPY XXX */
+volatile int prima2_pen_release = -1;
+
 static void sirfsoc_secondary_init(unsigned int cpu)
 {
 	/*
 	 * let the primary processor know we're out of the
 	 * pen, then head off into the C entry point
 	 */
-	pen_release = -1;
+	prima2_pen_release = -1;
 	smp_wmb();
 
 	/*
@@ -80,13 +83,13 @@ static int sirfsoc_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	/*
 	 * 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.
+	 * that it has been released by resetting prima2_pen_release.
 	 *
-	 * Note that "pen_release" is the hardware CPU ID, whereas
+	 * Note that "prima2_pen_release" is the hardware CPU ID, whereas
 	 * "cpu" is Linux's internal ID.
 	 */
-	pen_release = cpu_logical_map(cpu);
-	sync_cache_w(&pen_release);
+	prima2_pen_release = cpu_logical_map(cpu);
+	sync_cache_w(&prima2_pen_release);
 
 	/*
 	 * Send the secondary CPU SEV, thereby causing the boot monitor to read
@@ -97,7 +100,7 @@ static int sirfsoc_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	timeout = jiffies + (1 * HZ);
 	while (time_before(jiffies, timeout)) {
 		smp_rmb();
-		if (pen_release == -1)
+		if (prima2_pen_release == -1)
 			break;
 
 		udelay(10);
@@ -109,7 +112,7 @@ static int sirfsoc_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 */
 	spin_unlock(&boot_lock);
 
-	return pen_release != -1 ? -ENOSYS : 0;
+	return prima2_pen_release != -1 ? -ENOSYS : 0;
 }
 
 const struct smp_operations sirfsoc_smp_ops __initconst = {
diff --git a/arch/arm/mach-spear/generic.h b/arch/arm/mach-spear/generic.h
index 909b97c0b237..815333fc320c 100644
--- a/arch/arm/mach-spear/generic.h
+++ b/arch/arm/mach-spear/generic.h
@@ -20,6 +20,8 @@
 
 #include <asm/mach/time.h>
 
+extern volatile int prima2_pen_release;
+
 extern void spear13xx_timer_init(void);
 extern void spear3xx_timer_init(void);
 extern struct pl022_ssp_controller pl022_plat_data;
diff --git a/arch/arm/mach-spear/headsmp.S b/arch/arm/mach-spear/headsmp.S
index c52192dc3d9f..6e250b6c0aa2 100644
--- a/arch/arm/mach-spear/headsmp.S
+++ b/arch/arm/mach-spear/headsmp.S
@@ -43,5 +43,5 @@ pen:	ldr	r7, [r6]
 
 	.align
 1:	.long	.
-	.long	pen_release
+	.long	spear_pen_release
 ENDPROC(spear13xx_secondary_startup)
diff --git a/arch/arm/mach-spear/hotplug.c b/arch/arm/mach-spear/hotplug.c
index 12edd1cf8a12..0dd84f609627 100644
--- a/arch/arm/mach-spear/hotplug.c
+++ b/arch/arm/mach-spear/hotplug.c
@@ -16,6 +16,8 @@
 #include <asm/cp15.h>
 #include <asm/smp_plat.h>
 
+#include "generic.h"
+
 static inline void cpu_enter_lowpower(void)
 {
 	unsigned int v;
@@ -57,7 +59,7 @@ static inline void spear13xx_do_lowpower(unsigned int cpu, int *spurious)
 	for (;;) {
 		wfi();
 
-		if (pen_release == cpu) {
+		if (spear_pen_release == cpu) {
 			/*
 			 * OK, proper wakeup, we're done
 			 */
diff --git a/arch/arm/mach-spear/platsmp.c b/arch/arm/mach-spear/platsmp.c
index 39038a03836a..b1ff4bb86f6d 100644
--- a/arch/arm/mach-spear/platsmp.c
+++ b/arch/arm/mach-spear/platsmp.c
@@ -20,16 +20,21 @@
 #include <mach/spear.h>
 #include "generic.h"
 
+/* XXX spear_pen_release is cargo culted code - DO NOT COPY XXX */
+volatile int spear_pen_release = -1;
+
 /*
- * Write pen_release in a way that is guaranteed to be visible to all
- * observers, irrespective of whether they're taking part in coherency
+ * XXX CARGO CULTED CODE - DO NOT COPY XXX
+ *
+ * Write spear_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)
+static void spear_write_pen_release(int val)
 {
-	pen_release = val;
+	spear_pen_release = val;
 	smp_wmb();
-	sync_cache_w(&pen_release);
+	sync_cache_w(&spear_pen_release);
 }
 
 static DEFINE_SPINLOCK(boot_lock);
@@ -42,7 +47,7 @@ static void spear13xx_secondary_init(unsigned int cpu)
 	 * let the primary processor know we're out of the
 	 * pen, then head off into the C entry point
 	 */
-	write_pen_release(-1);
+	spear_write_pen_release(-1);
 
 	/*
 	 * Synchronise with the boot thread.
@@ -64,17 +69,17 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	/*
 	 * 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.
+	 * that it has been released by resetting spear_pen_release.
 	 *
-	 * Note that "pen_release" is the hardware CPU ID, whereas
+	 * Note that "spear_pen_release" is the hardware CPU ID, whereas
 	 * "cpu" is Linux's internal ID.
 	 */
-	write_pen_release(cpu);
+	spear_write_pen_release(cpu);
 
 	timeout = jiffies + (1 * HZ);
 	while (time_before(jiffies, timeout)) {
 		smp_rmb();
-		if (pen_release == -1)
+		if (spear_pen_release == -1)
 			break;
 
 		udelay(10);
@@ -86,7 +91,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 */
 	spin_unlock(&boot_lock);
 
-	return pen_release != -1 ? -ENOSYS : 0;
+	return spear_pen_release != -1 ? -ENOSYS : 0;
 }
 
 /*
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] ARM: smp: remove arch-provided "pen_release"

[Viresh Kumar]

On 13-12-18, 18:01, Russell King wrote:
> diff --git a/arch/arm/mach-spear/generic.h b/arch/arm/mach-spear/generic.h
> index 909b97c0b237..815333fc320c 100644
> --- a/arch/arm/mach-spear/generic.h
> +++ b/arch/arm/mach-spear/generic.h
> @@ -20,6 +20,8 @@
>  
>  #include <asm/mach/time.h>
>  
> +extern volatile int prima2_pen_release;

                       prima2 ?

I haven't tried but this may cause build regressions as well, I am
surprised on how this passed for you.

>  extern void spear13xx_timer_init(void);
>  extern void spear3xx_timer_init(void);
>  extern struct pl022_ssp_controller pl022_plat_data;
> diff --git a/arch/arm/mach-spear/headsmp.S b/arch/arm/mach-spear/headsmp.S
> index c52192dc3d9f..6e250b6c0aa2 100644
> --- a/arch/arm/mach-spear/headsmp.S
> +++ b/arch/arm/mach-spear/headsmp.S
> @@ -43,5 +43,5 @@ pen:	ldr	r7, [r6]
>  
>  	.align
>  1:	.long	.
> -	.long	pen_release
> +	.long	spear_pen_release
>  ENDPROC(spear13xx_secondary_startup)
> diff --git a/arch/arm/mach-spear/hotplug.c b/arch/arm/mach-spear/hotplug.c
> index 12edd1cf8a12..0dd84f609627 100644
> --- a/arch/arm/mach-spear/hotplug.c
> +++ b/arch/arm/mach-spear/hotplug.c
> @@ -16,6 +16,8 @@
>  #include <asm/cp15.h>
>  #include <asm/smp_plat.h>
>  
> +#include "generic.h"
> +
>  static inline void cpu_enter_lowpower(void)
>  {
>  	unsigned int v;
> @@ -57,7 +59,7 @@ static inline void spear13xx_do_lowpower(unsigned int cpu, int *spurious)
>  	for (;;) {
>  		wfi();
>  
> -		if (pen_release == cpu) {
> +		if (spear_pen_release == cpu) {
>  			/*
>  			 * OK, proper wakeup, we're done
>  			 */
> diff --git a/arch/arm/mach-spear/platsmp.c b/arch/arm/mach-spear/platsmp.c
> index 39038a03836a..b1ff4bb86f6d 100644
> --- a/arch/arm/mach-spear/platsmp.c
> +++ b/arch/arm/mach-spear/platsmp.c
> @@ -20,16 +20,21 @@
>  #include <mach/spear.h>
>  #include "generic.h"
>  
> +/* XXX spear_pen_release is cargo culted code - DO NOT COPY XXX */
> +volatile int spear_pen_release = -1;
> +
>  /*
> - * Write pen_release in a way that is guaranteed to be visible to all
> - * observers, irrespective of whether they're taking part in coherency
> + * XXX CARGO CULTED CODE - DO NOT COPY XXX
> + *
> + * Write spear_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)
> +static void spear_write_pen_release(int val)
>  {
> -	pen_release = val;
> +	spear_pen_release = val;
>  	smp_wmb();
> -	sync_cache_w(&pen_release);
> +	sync_cache_w(&spear_pen_release);
>  }
>  
>  static DEFINE_SPINLOCK(boot_lock);
> @@ -42,7 +47,7 @@ static void spear13xx_secondary_init(unsigned int cpu)
>  	 * let the primary processor know we're out of the
>  	 * pen, then head off into the C entry point
>  	 */
> -	write_pen_release(-1);
> +	spear_write_pen_release(-1);
>  
>  	/*
>  	 * Synchronise with the boot thread.
> @@ -64,17 +69,17 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
>  	/*
>  	 * 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.
> +	 * that it has been released by resetting spear_pen_release.
>  	 *
> -	 * Note that "pen_release" is the hardware CPU ID, whereas
> +	 * Note that "spear_pen_release" is the hardware CPU ID, whereas
>  	 * "cpu" is Linux's internal ID.
>  	 */
> -	write_pen_release(cpu);
> +	spear_write_pen_release(cpu);
>  
>  	timeout = jiffies + (1 * HZ);
>  	while (time_before(jiffies, timeout)) {
>  		smp_rmb();
> -		if (pen_release == -1)
> +		if (spear_pen_release == -1)
>  			break;
>  
>  		udelay(10);
> @@ -86,7 +91,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
>  	 */
>  	spin_unlock(&boot_lock);
>  
> -	return pen_release != -1 ? -ENOSYS : 0;
> +	return spear_pen_release != -1 ? -ENOSYS : 0;
>  }
>  
>  /*
> -- 
> 2.7.4

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] ARM: smp: remove arch-provided "pen_release"

[Russell King - ARM Linux]

On Fri, Dec 14, 2018 at 10:09:23AM +0530, Viresh Kumar wrote:
> On 13-12-18, 18:01, Russell King wrote:
> > diff --git a/arch/arm/mach-spear/generic.h b/arch/arm/mach-spear/generic.h
> > index 909b97c0b237..815333fc320c 100644
> > --- a/arch/arm/mach-spear/generic.h
> > +++ b/arch/arm/mach-spear/generic.h
> > @@ -20,6 +20,8 @@
> >  
> >  #include <asm/mach/time.h>
> >  
> > +extern volatile int prima2_pen_release;
> 
>                        prima2 ?
> 
> I haven't tried but this may cause build regressions as well, I am
> surprised on how this passed for you.

Thanks.

These patches aren't all build tested - I don't see the point of wasting
hours build testing when these platforms /really/ need something better.

This is not supposed to be a finished patch, but a patch to get folk
to do something about this issue.

Please fix this issue properly.  I can't give you a patch to do that,
you need to sort it yourself, but failure to do anything _will_ result
in an updated version of this patch being merged.

> >  extern void spear13xx_timer_init(void);
> >  extern void spear3xx_timer_init(void);
> >  extern struct pl022_ssp_controller pl022_plat_data;
> > diff --git a/arch/arm/mach-spear/headsmp.S b/arch/arm/mach-spear/headsmp.S
> > index c52192dc3d9f..6e250b6c0aa2 100644
> > --- a/arch/arm/mach-spear/headsmp.S
> > +++ b/arch/arm/mach-spear/headsmp.S
> > @@ -43,5 +43,5 @@ pen:	ldr	r7, [r6]
> >  
> >  	.align
> >  1:	.long	.
> > -	.long	pen_release
> > +	.long	spear_pen_release
> >  ENDPROC(spear13xx_secondary_startup)
> > diff --git a/arch/arm/mach-spear/hotplug.c b/arch/arm/mach-spear/hotplug.c
> > index 12edd1cf8a12..0dd84f609627 100644
> > --- a/arch/arm/mach-spear/hotplug.c
> > +++ b/arch/arm/mach-spear/hotplug.c
> > @@ -16,6 +16,8 @@
> >  #include <asm/cp15.h>
> >  #include <asm/smp_plat.h>
> >  
> > +#include "generic.h"
> > +
> >  static inline void cpu_enter_lowpower(void)
> >  {
> >  	unsigned int v;
> > @@ -57,7 +59,7 @@ static inline void spear13xx_do_lowpower(unsigned int cpu, int *spurious)
> >  	for (;;) {
> >  		wfi();
> >  
> > -		if (pen_release == cpu) {
> > +		if (spear_pen_release == cpu) {
> >  			/*
> >  			 * OK, proper wakeup, we're done
> >  			 */
> > diff --git a/arch/arm/mach-spear/platsmp.c b/arch/arm/mach-spear/platsmp.c
> > index 39038a03836a..b1ff4bb86f6d 100644
> > --- a/arch/arm/mach-spear/platsmp.c
> > +++ b/arch/arm/mach-spear/platsmp.c
> > @@ -20,16 +20,21 @@
> >  #include <mach/spear.h>
> >  #include "generic.h"
> >  
> > +/* XXX spear_pen_release is cargo culted code - DO NOT COPY XXX */
> > +volatile int spear_pen_release = -1;
> > +
> >  /*
> > - * Write pen_release in a way that is guaranteed to be visible to all
> > - * observers, irrespective of whether they're taking part in coherency
> > + * XXX CARGO CULTED CODE - DO NOT COPY XXX
> > + *
> > + * Write spear_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)
> > +static void spear_write_pen_release(int val)
> >  {
> > -	pen_release = val;
> > +	spear_pen_release = val;
> >  	smp_wmb();
> > -	sync_cache_w(&pen_release);
> > +	sync_cache_w(&spear_pen_release);
> >  }
> >  
> >  static DEFINE_SPINLOCK(boot_lock);
> > @@ -42,7 +47,7 @@ static void spear13xx_secondary_init(unsigned int cpu)
> >  	 * let the primary processor know we're out of the
> >  	 * pen, then head off into the C entry point
> >  	 */
> > -	write_pen_release(-1);
> > +	spear_write_pen_release(-1);
> >  
> >  	/*
> >  	 * Synchronise with the boot thread.
> > @@ -64,17 +69,17 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
> >  	/*
> >  	 * 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.
> > +	 * that it has been released by resetting spear_pen_release.
> >  	 *
> > -	 * Note that "pen_release" is the hardware CPU ID, whereas
> > +	 * Note that "spear_pen_release" is the hardware CPU ID, whereas
> >  	 * "cpu" is Linux's internal ID.
> >  	 */
> > -	write_pen_release(cpu);
> > +	spear_write_pen_release(cpu);
> >  
> >  	timeout = jiffies + (1 * HZ);
> >  	while (time_before(jiffies, timeout)) {
> >  		smp_rmb();
> > -		if (pen_release == -1)
> > +		if (spear_pen_release == -1)
> >  			break;
> >  
> >  		udelay(10);
> > @@ -86,7 +91,7 @@ static int spear13xx_boot_secondary(unsigned int cpu, struct task_struct *idle)
> >  	 */
> >  	spin_unlock(&boot_lock);
> >  
> > -	return pen_release != -1 ? -ENOSYS : 0;
> > +	return spear_pen_release != -1 ? -ENOSYS : 0;
> >  }
> >  
> >  /*
> > -- 
> > 2.7.4
> 
> -- 
> viresh
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 9/9] ARM: smp: remove arch-provided "pen_release"

[Viresh Kumar]

On 14-12-18, 13:12, Russell King - ARM Linux wrote:
> On Fri, Dec 14, 2018 at 10:09:23AM +0530, Viresh Kumar wrote:
> > On 13-12-18, 18:01, Russell King wrote:
> > > diff --git a/arch/arm/mach-spear/generic.h b/arch/arm/mach-spear/generic.h
> > > index 909b97c0b237..815333fc320c 100644
> > > --- a/arch/arm/mach-spear/generic.h
> > > +++ b/arch/arm/mach-spear/generic.h
> > > @@ -20,6 +20,8 @@
> > >  
> > >  #include <asm/mach/time.h>
> > >  
> > > +extern volatile int prima2_pen_release;
> > 
> >                        prima2 ?
> > 
> > I haven't tried but this may cause build regressions as well, I am
> > surprised on how this passed for you.
> 
> Thanks.
> 
> These patches aren't all build tested -

Ah okay.

> I don't see the point of wasting
> hours build testing when these platforms /really/ need something better.

I was expecting it to be build tested at least, to be honest. But anyway, we
will see build failures from bots if any such cases exist.

> This is not supposed to be a finished patch, but a patch to get folk
> to do something about this issue.
> 
> Please fix this issue properly.  I can't give you a patch to do that,
> you need to sort it yourself, but failure to do anything _will_ result
> in an updated version of this patch being merged.

Yeah, I know. I have seen the thread and it was lack of knowledge in the
beginning which made us copy code from rest of ARM platforms. That was stupid
and there can't be any excuse for that.

Unfortunately, I don't have access to hardware to test this stuff and so
wouldn't be possible to send a patch as well.

Lets get your patch merged at least.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/9] ARM: sti: remove pen_release and boot_lock

[Patrice CHOTARD]

Hi Russel

On 12/13/18 7:00 PM, Russell King wrote:
> The pen_release implementation was created for Versatile platforms to
> work around boot loaders that did not differentiate between the
> various different secondary CPUs on this ARM development platform.
> This should not be true of modern platforms where we send IPIs to
> specific CPUs to wake them up.  Remove the pen_release stuff from
> SoCs that make use of the per-CPU IPI mechanism.
> 
> The boot_lock is something that was required for ARM development
> platforms to ensure that the delay calibration worked properly.  This
> is not necessary for modern platforms that have better bus bandwidth
> and do not need to calibrate the delay loop for secondary cores.
> Remove the boot_lock entirely.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  arch/arm/mach-sti/Makefile  |  2 +-
>  arch/arm/mach-sti/headsmp.S | 43 -------------------------------
>  arch/arm/mach-sti/platsmp.c | 62 ++-------------------------------------------
>  3 files changed, 3 insertions(+), 104 deletions(-)
>  delete mode 100644 arch/arm/mach-sti/headsmp.S
> 
> diff --git a/arch/arm/mach-sti/Makefile b/arch/arm/mach-sti/Makefile
> index acb330916333..f85ff059cfba 100644
> --- a/arch/arm/mach-sti/Makefile
> +++ b/arch/arm/mach-sti/Makefile
> @@ -1,2 +1,2 @@
> -obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
> +obj-$(CONFIG_SMP)		+= platsmp.o
>  obj-$(CONFIG_ARCH_STI) 		+= board-dt.o
> diff --git a/arch/arm/mach-sti/headsmp.S b/arch/arm/mach-sti/headsmp.S
> deleted file mode 100644
> index e0ad451700d5..000000000000
> --- a/arch/arm/mach-sti/headsmp.S
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -/*
> - *  arch/arm/mach-sti/headsmp.S
> - *
> - * Copyright (C) 2013 STMicroelectronics (R&D) Limited.
> - *		http://www.st.com
> - *
> - * Cloned from linux/arch/arm/mach-vexpress/headsmp.S
> - *
> - *  Copyright (c) 2003 ARM Limited
> - *  All Rights Reserved
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> -#include <linux/linkage.h>
> -#include <linux/init.h>
> -
> -/*
> - * ST specific entry point for secondary CPUs.  This provides
> - * a "holding pen" into which all secondary cores are held until we're
> - * ready for them to initialise.
> - */
> -ENTRY(sti_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
> -
> -	/*
> -	 * we've been released from the holding pen: secondary_stack
> -	 * should now contain the SVC stack for this core
> -	 */
> -	b	secondary_startup
> -ENDPROC(sti_secondary_startup)
> -
> -1:	.long	.
> -	.long	pen_release
> diff --git a/arch/arm/mach-sti/platsmp.c b/arch/arm/mach-sti/platsmp.c
> index 231f19e17436..21668501c9bb 100644
> --- a/arch/arm/mach-sti/platsmp.c
> +++ b/arch/arm/mach-sti/platsmp.c
> @@ -28,72 +28,15 @@
>  
>  #include "smp.h"
>  
> -static void write_pen_release(int val)
> -{
> -	pen_release = val;
> -	smp_wmb();
> -	sync_cache_w(&pen_release);
> -}
> -
> -static DEFINE_SPINLOCK(boot_lock);
> -
> -static void sti_secondary_init(unsigned int cpu)
> -{
> -	/*
> -	 * let the primary processor know we're out of the
> -	 * pen, then head off into the C entry point
> -	 */
> -	write_pen_release(-1);
> -
> -	/*
> -	 * Synchronise with the boot thread.
> -	 */
> -	spin_lock(&boot_lock);
> -	spin_unlock(&boot_lock);
> -}
> -
>  static int sti_boot_secondary(unsigned int cpu, struct task_struct *idle)
>  {
> -	unsigned long timeout;
> -
> -	/*
> -	 * set synchronisation state between this boot processor
> -	 * and the secondary one
> -	 */
> -	spin_lock(&boot_lock);
> -
> -	/*
> -	 * 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.
> -	 *
> -	 * Note that "pen_release" is the hardware CPU ID, whereas
> -	 * "cpu" is Linux's internal ID.
> -	 */
> -	write_pen_release(cpu_logical_map(cpu));
> -
>  	/*
>  	 * Send the secondary CPU a soft interrupt, thereby causing
>  	 * it to jump to the secondary entrypoint.
>  	 */
>  	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>  
> -	timeout = jiffies + (1 * HZ);
> -	while (time_before(jiffies, timeout)) {
> -		smp_rmb();
> -		if (pen_release == -1)
> -			break;
> -
> -		udelay(10);
> -	}
> -
> -	/*
> -	 * now the secondary core is starting up let it run its
> -	 * calibrations, then wait for it to finish
> -	 */
> -	spin_unlock(&boot_lock);
> -
> -	return pen_release != -1 ? -ENOSYS : 0;
> +	return 0;
>  }
>  
>  static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
> @@ -103,7 +46,7 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
>  	u32 __iomem *cpu_strt_ptr;
>  	u32 release_phys;
>  	int cpu;
> -	unsigned long entry_pa = __pa_symbol(sti_secondary_startup);
> +	unsigned long entry_pa = __pa_symbol(secondary_startup);
>  
>  	np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>  
> @@ -158,6 +101,5 @@ static void __init sti_smp_prepare_cpus(unsigned int max_cpus)
>  
>  const struct smp_operations sti_smp_ops __initconst = {
>  	.smp_prepare_cpus	= sti_smp_prepare_cpus,
> -	.smp_secondary_init	= sti_secondary_init,
>  	.smp_boot_secondary	= sti_boot_secondary,
>  };
> 

Even if this patch is breaking the secondary CPU's bringup, it can be
merged as explained in the commit message.

I will send an additionnal patch to restore the secondary CPU's bringup.

Thanks for this clean-up.

Patrice
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/9] Clean up ARM SMP/CPU hotplug implementations

[Russell King - ARM Linux]

Since almost no one has responded, my intention is to queue up
patches 1-3,5-8 for the Christmas-time merge window through my
tree.  They will be in linux-next tonight.

On Thu, Dec 13, 2018 at 05:59:52PM +0000, Russell King - ARM Linux wrote:
> There is a lot of apparent copied code in arch/arm for handling SMP/
> CPU hotplug, much of which is inappropriate or plain buggy.  This
> seems to be a topic that occasionally comes up.
> 
> The "pen_release" thing was created for ARM Ltd development platforms
> where there was no way to individually control secondary CPUs leaving
> the boot loader - they all jumped to whatever physical code address
> was supplied at the same time.  This made it necessary for _these_
> platforms to have a "holding pen" for the CPUs while the kernel
> initialised.
> 
> The "boot_lock" thing was also created for ARM Ltd development
> platforms which had restricted bus bandwidth, and which used the
> loops_per_jiffy delay mechanism, which was calibrated for each
> secondary CPU.  With the restricted bus bandwidth, activity from the
> boot CPU would affect the delay calibration adversely.
> 
> Lastly, the Versatile CPU hotplug implementation is an entirely
> ficticious one - these platforms do _not_ support CPU hotplug as
> there is no way to actually disable any of the secondary CPUs, or
> reset them.  Such an implementation is not acceptable when supporting
> features such as suspend or kexec.  As the Versatile platforms are
> ARM development platforms which do not have suspend support, this is
> acceptable there, but not for production hardware.
> 
> None of these three facilities/implementations should be used on
> modern production hardware, yet we have a number of copies of this
> code.
> 
> This series addresses that by removing the inappropriate copies of
> some Realview/Versatile Express specific workarounds, and makes it
> (hopefully) more clear that introducing this code is really not
> acceptable.
> 
> To discourage copying the Versatile code, further comments are added
> and the functions renamed for CPU hotplug to be "immitation" to make
> it clear that it's not a real implementation.
> 
> We tried reducing the duplication in the past with ideas around
> consolidating the pen_release/boot_lock/immitation hotplug stuff,
> but I nacked that because it's not an acceptable implementation for
> production hardware.  However, we did decide to consolidate the
> "pen_release" definition.  In hind sight, that was a mistake,
> because that gave more credence to that way of doing things, and
> also gave rise to buggy implementations which only read from that
> variable - meaning it served no useful purpose.
> 
> There are some rather complex cases that remain, and those need the
> SoC folk to fix.
> 
> I have left the Actions Semi patch in place since following patches
> depend on it, but there is a five-patch series from Linus Walleij
> that address this platform which should replace this patch - with
> the patch concerned marked as "RFT" - request for testing.
> 
>  arch/arm/include/asm/smp.h                         |   1 -
>  arch/arm/kernel/smp.c                              |   6 --
>  arch/arm/mach-actions/platsmp.c                    |  15 ---
>  arch/arm/mach-exynos/headsmp.S                     |   2 +-
>  arch/arm/mach-exynos/platsmp.c                     |  31 +++---
>  arch/arm/mach-omap2/omap-smp.c                     |  20 ----
>  arch/arm/mach-oxnas/Makefile                       |   1 -
>  arch/arm/mach-oxnas/hotplug.c                      | 109 --------------------
>  arch/arm/mach-oxnas/platsmp.c                      |   4 -
>  arch/arm/mach-prima2/common.h                      |   2 +
>  arch/arm/mach-prima2/headsmp.S                     |   2 +-
>  arch/arm/mach-prima2/hotplug.c                     |   3 +-
>  arch/arm/mach-prima2/platsmp.c                     |  17 ++--
>  arch/arm/mach-qcom/platsmp.c                       |  26 -----
>  arch/arm/mach-realview/Makefile                    |   1 -
>  arch/arm/mach-realview/hotplug.c                   | 111 ---------------------
>  arch/arm/mach-realview/hotplug.h                   |   1 -
>  arch/arm/mach-realview/platsmp-dt.c                |   8 +-
>  arch/arm/mach-spear/generic.h                      |   2 +
>  arch/arm/mach-spear/headsmp.S                      |   2 +-
>  arch/arm/mach-spear/hotplug.c                      |   4 +-
>  arch/arm/mach-spear/platsmp.c                      |  27 +++--
>  arch/arm/mach-sti/Makefile                         |   2 +-
>  arch/arm/mach-sti/headsmp.S                        |  43 --------
>  arch/arm/mach-sti/platsmp.c                        |  62 +-----------
>  arch/arm/mach-vexpress/Makefile                    |   1 -
>  arch/arm/mach-vexpress/core.h                      |   2 -
>  arch/arm/mach-vexpress/platsmp.c                   |   7 ++
>  arch/arm/plat-versatile/Makefile                   |   1 +
>  arch/arm/plat-versatile/headsmp.S                  |   2 +-
>  .../{mach-vexpress => plat-versatile}/hotplug.c    |  47 ++++-----
>  arch/arm/plat-versatile/include/plat/platsmp.h     |   2 +
>  arch/arm/plat-versatile/platsmp.c                  |  47 ++++++---
>  33 files changed, 132 insertions(+), 479 deletions(-)
>  delete mode 100644 arch/arm/mach-oxnas/hotplug.c
>  delete mode 100644 arch/arm/mach-realview/hotplug.c
>  delete mode 100644 arch/arm/mach-realview/hotplug.h
>  delete mode 100644 arch/arm/mach-sti/headsmp.S
>  rename arch/arm/{mach-vexpress => plat-versatile}/hotplug.c (56%)
> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/9] Clean up ARM SMP/CPU hotplug implementations

[Krzysztof Kozlowski]

On Thu, 20 Dec 2018 at 11:11, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> Since almost no one has responded, my intention is to queue up
> patches 1-3,5-8 for the Christmas-time merge window through my
> tree.  They will be in linux-next tonight.

AFAIU, the patch 9 (or entire patchset) was not build tested, did not
compile and therefore was not in linux-next. Sending something, which
was not building, to linux-next just few days before Christmas merge
window makes the schedule really tight. Especially that during
Christmas some people might be offline.

I think it should sit in linux-next for few weeks... it should sit
there already so the auto-testers would try it. At least if it were in
linux-next, the booting and few simple tests were already done, e.g.
by my boards, without any additional effort.

So please give it a time after putting it into next.

Best regards,
Krzysztof



>
> On Thu, Dec 13, 2018 at 05:59:52PM +0000, Russell King - ARM Linux wrote:
> > There is a lot of apparent copied code in arch/arm for handling SMP/
> > CPU hotplug, much of which is inappropriate or plain buggy.  This
> > seems to be a topic that occasionally comes up.
> >
> > The "pen_release" thing was created for ARM Ltd development platforms
> > where there was no way to individually control secondary CPUs leaving
> > the boot loader - they all jumped to whatever physical code address
> > was supplied at the same time.  This made it necessary for _these_
> > platforms to have a "holding pen" for the CPUs while the kernel
> > initialised.
> >
> > The "boot_lock" thing was also created for ARM Ltd development
> > platforms which had restricted bus bandwidth, and which used the
> > loops_per_jiffy delay mechanism, which was calibrated for each
> > secondary CPU.  With the restricted bus bandwidth, activity from the
> > boot CPU would affect the delay calibration adversely.
> >
> > Lastly, the Versatile CPU hotplug implementation is an entirely
> > ficticious one - these platforms do _not_ support CPU hotplug as
> > there is no way to actually disable any of the secondary CPUs, or
> > reset them.  Such an implementation is not acceptable when supporting
> > features such as suspend or kexec.  As the Versatile platforms are
> > ARM development platforms which do not have suspend support, this is
> > acceptable there, but not for production hardware.
> >
> > None of these three facilities/implementations should be used on
> > modern production hardware, yet we have a number of copies of this
> > code.
> >
> > This series addresses that by removing the inappropriate copies of
> > some Realview/Versatile Express specific workarounds, and makes it
> > (hopefully) more clear that introducing this code is really not
> > acceptable.
> >
> > To discourage copying the Versatile code, further comments are added
> > and the functions renamed for CPU hotplug to be "immitation" to make
> > it clear that it's not a real implementation.
> >
> > We tried reducing the duplication in the past with ideas around
> > consolidating the pen_release/boot_lock/immitation hotplug stuff,
> > but I nacked that because it's not an acceptable implementation for
> > production hardware.  However, we did decide to consolidate the
> > "pen_release" definition.  In hind sight, that was a mistake,
> > because that gave more credence to that way of doing things, and
> > also gave rise to buggy implementations which only read from that
> > variable - meaning it served no useful purpose.
> >
> > There are some rather complex cases that remain, and those need the
> > SoC folk to fix.
> >
> > I have left the Actions Semi patch in place since following patches
> > depend on it, but there is a five-patch series from Linus Walleij
> > that address this platform which should replace this patch - with
> > the patch concerned marked as "RFT" - request for testing.
> >
> >  arch/arm/include/asm/smp.h                         |   1 -
> >  arch/arm/kernel/smp.c                              |   6 --
> >  arch/arm/mach-actions/platsmp.c                    |  15 ---
> >  arch/arm/mach-exynos/headsmp.S                     |   2 +-
> >  arch/arm/mach-exynos/platsmp.c                     |  31 +++---
> >  arch/arm/mach-omap2/omap-smp.c                     |  20 ----
> >  arch/arm/mach-oxnas/Makefile                       |   1 -
> >  arch/arm/mach-oxnas/hotplug.c                      | 109 --------------------
> >  arch/arm/mach-oxnas/platsmp.c                      |   4 -
> >  arch/arm/mach-prima2/common.h                      |   2 +
> >  arch/arm/mach-prima2/headsmp.S                     |   2 +-
> >  arch/arm/mach-prima2/hotplug.c                     |   3 +-
> >  arch/arm/mach-prima2/platsmp.c                     |  17 ++--
> >  arch/arm/mach-qcom/platsmp.c                       |  26 -----
> >  arch/arm/mach-realview/Makefile                    |   1 -
> >  arch/arm/mach-realview/hotplug.c                   | 111 ---------------------
> >  arch/arm/mach-realview/hotplug.h                   |   1 -
> >  arch/arm/mach-realview/platsmp-dt.c                |   8 +-
> >  arch/arm/mach-spear/generic.h                      |   2 +
> >  arch/arm/mach-spear/headsmp.S                      |   2 +-
> >  arch/arm/mach-spear/hotplug.c                      |   4 +-
> >  arch/arm/mach-spear/platsmp.c                      |  27 +++--
> >  arch/arm/mach-sti/Makefile                         |   2 +-
> >  arch/arm/mach-sti/headsmp.S                        |  43 --------
> >  arch/arm/mach-sti/platsmp.c                        |  62 +-----------
> >  arch/arm/mach-vexpress/Makefile                    |   1 -
> >  arch/arm/mach-vexpress/core.h                      |   2 -
> >  arch/arm/mach-vexpress/platsmp.c                   |   7 ++
> >  arch/arm/plat-versatile/Makefile                   |   1 +
> >  arch/arm/plat-versatile/headsmp.S                  |   2 +-
> >  .../{mach-vexpress => plat-versatile}/hotplug.c    |  47 ++++-----
> >  arch/arm/plat-versatile/include/plat/platsmp.h     |   2 +
> >  arch/arm/plat-versatile/platsmp.c                  |  47 ++++++---
> >  33 files changed, 132 insertions(+), 479 deletions(-)
> >  delete mode 100644 arch/arm/mach-oxnas/hotplug.c
> >  delete mode 100644 arch/arm/mach-realview/hotplug.c
> >  delete mode 100644 arch/arm/mach-realview/hotplug.h
> >  delete mode 100644 arch/arm/mach-sti/headsmp.S
> >  rename arch/arm/{mach-vexpress => plat-versatile}/hotplug.c (56%)
> >
> > --
> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/9] Clean up ARM SMP/CPU hotplug implementations

[Russell King - ARM Linux]

On Thu, Dec 20, 2018 at 11:23:57AM +0100, Krzysztof Kozlowski wrote:
> On Thu, 20 Dec 2018 at 11:11, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> >
> > Since almost no one has responded, my intention is to queue up
> > patches 1-3,5-8 for the Christmas-time merge window through my
> > tree.  They will be in linux-next tonight.
> 
> AFAIU, the patch 9 (or entire patchset) was not build tested, did not
> compile and therefore was not in linux-next. Sending something, which
> was not building, to linux-next just few days before Christmas merge
> window makes the schedule really tight. Especially that during
> Christmas some people might be offline.

If you're only testing linux-next, then you have a hole in your test
regime.

> I think it should sit in linux-next for few weeks... it should sit
> there already so the auto-testers would try it. At least if it were in
> linux-next, the booting and few simple tests were already done, e.g.
> by my boards, without any additional effort.

As I've said, I'm omitting patches 4 and 9.  Patch 9 is the troublesome
patch and can't be merged anyway since patch 4 is going via the STi
maintainer along with other changes.  The remainder of the patch set
are all specific to individual platforms, some of which have been
acked, but most platform maintainers have not responded in _any_ way.

You are aware that the autobuilders do build patches sent to the
mailing list, and then we have kernelci which builds my tree, builds
linux-next etc.  There's multiple stages of build test that such
stuff goes through.

Lastly, linux-next is not a general test tree, it is for integration
testing of code destined for the _NEXT_ merge window.  The clue is
in the name.  It is not for code destined for the following merge
window - only code for _this_ merge window should be in linux-next.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/9] ARM: oxnas: remove CPU hotplug implementation

[Neil Armstrong]

On 13/12/2018 19:00, Russell King wrote:
> The CPU hotplug implementation on this platform is cargo-culted from
> the plat-versatile implementation, and is buggy.  Once a CPU hits the
> "low power" loop, it will wait for pen_release to be set to the CPU
> number to wake up again - but nothing in this implementation does that.
> 
> So, once a CPU has entered cpu_die() it will never, ever leave.
> 
> Remove this useless cargo-culted implementation.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  arch/arm/mach-oxnas/Makefile  |   1 -
>  arch/arm/mach-oxnas/hotplug.c | 109 ------------------------------------------
>  arch/arm/mach-oxnas/platsmp.c |   4 --
>  3 files changed, 114 deletions(-)
>  delete mode 100644 arch/arm/mach-oxnas/hotplug.c
> 
> diff --git a/arch/arm/mach-oxnas/Makefile b/arch/arm/mach-oxnas/Makefile
> index b625906a9970..61a34e1c0f22 100644
> --- a/arch/arm/mach-oxnas/Makefile
> +++ b/arch/arm/mach-oxnas/Makefile
> @@ -1,2 +1 @@
>  obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
> -obj-$(CONFIG_HOTPLUG_CPU) 	+= hotplug.o
> diff --git a/arch/arm/mach-oxnas/hotplug.c b/arch/arm/mach-oxnas/hotplug.c
> deleted file mode 100644
> index 854f29b8cba6..000000000000
> --- a/arch/arm/mach-oxnas/hotplug.c
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -/*
> - *  Copyright (C) 2002 ARM Ltd.
> - *  All Rights Reserved
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> -#include <linux/kernel.h>
> -#include <linux/errno.h>
> -#include <linux/smp.h>
> -
> -#include <asm/cp15.h>
> -#include <asm/smp_plat.h>
> -
> -static inline void cpu_enter_lowpower(void)
> -{
> -	unsigned int v;
> -
> -	asm volatile(
> -	"	mcr	p15, 0, %1, c7, c5, 0\n"
> -	"	mcr	p15, 0, %1, c7, c10, 4\n"
> -	/*
> -	 * Turn off coherency
> -	 */
> -	"	mrc	p15, 0, %0, c1, c0, 1\n"
> -	"	bic	%0, %0, #0x20\n"
> -	"	mcr	p15, 0, %0, c1, c0, 1\n"
> -	"	mrc	p15, 0, %0, c1, c0, 0\n"
> -	"	bic	%0, %0, %2\n"
> -	"	mcr	p15, 0, %0, c1, c0, 0\n"
> -	  : "=&r" (v)
> -	  : "r" (0), "Ir" (CR_C)
> -	  : "cc");
> -}
> -
> -static inline void cpu_leave_lowpower(void)
> -{
> -	unsigned int v;
> -
> -	asm volatile(	"mrc	p15, 0, %0, c1, c0, 0\n"
> -	"	orr	%0, %0, %1\n"
> -	"	mcr	p15, 0, %0, c1, c0, 0\n"
> -	"	mrc	p15, 0, %0, c1, c0, 1\n"
> -	"	orr	%0, %0, #0x20\n"
> -	"	mcr	p15, 0, %0, c1, c0, 1\n"
> -	  : "=&r" (v)
> -	  : "Ir" (CR_C)
> -	  : "cc");
> -}
> -
> -static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> -{
> -	/*
> -	 * there is no power-control hardware on this platform, so all
> -	 * we can do is put the core into WFI; this is safe as the calling
> -	 * code will have already disabled interrupts
> -	 */
> -	for (;;) {
> -		/*
> -		 * here's the WFI
> -		 */
> -		asm(".word	0xe320f003\n"
> -		    :
> -		    :
> -		    : "memory", "cc");
> -
> -		if (pen_release == cpu_logical_map(cpu)) {
> -			/*
> -			 * OK, proper wakeup, we're done
> -			 */
> -			break;
> -		}
> -
> -		/*
> -		 * Getting here, means that we have come out of WFI without
> -		 * having been woken up - this shouldn't happen
> -		 *
> -		 * Just note it happening - when we're woken, we can report
> -		 * its occurrence.
> -		 */
> -		(*spurious)++;
> -	}
> -}
> -
> -/*
> - * platform-specific code to shutdown a CPU
> - *
> - * Called with IRQs disabled
> - */
> -void ox820_cpu_die(unsigned int cpu)
> -{
> -	int spurious = 0;
> -
> -	/*
> -	 * we're ready for shutdown now, so do it
> -	 */
> -	cpu_enter_lowpower();
> -	platform_do_lowpower(cpu, &spurious);
> -
> -	/*
> -	 * bring this CPU back into the world of cache
> -	 * coherency, and then restore interrupts
> -	 */
> -	cpu_leave_lowpower();
> -
> -	if (spurious)
> -		pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
> -}
> diff --git a/arch/arm/mach-oxnas/platsmp.c b/arch/arm/mach-oxnas/platsmp.c
> index 442cc8a2f7dc..735141c0e3a3 100644
> --- a/arch/arm/mach-oxnas/platsmp.c
> +++ b/arch/arm/mach-oxnas/platsmp.c
> @@ -19,7 +19,6 @@
>  #include <asm/smp_scu.h>
>  
>  extern void ox820_secondary_startup(void);
> -extern void ox820_cpu_die(unsigned int cpu);
>  
>  static void __iomem *cpu_ctrl;
>  static void __iomem *gic_cpu_ctrl;
> @@ -94,9 +93,6 @@ static void __init ox820_smp_prepare_cpus(unsigned int max_cpus)
>  static const struct smp_operations ox820_smp_ops __initconst = {
>  	.smp_prepare_cpus	= ox820_smp_prepare_cpus,
>  	.smp_boot_secondary	= ox820_boot_secondary,
> -#ifdef CONFIG_HOTPLUG_CPU
> -	.cpu_die		= ox820_cpu_die,
> -#endif
>  };
>  
>  CPU_METHOD_OF_DECLARE(ox820_smp, "oxsemi,ox820-smp", &ox820_smp_ops);
> 

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Linus Walleij]

On Thu, Dec 13, 2018 at 7:00 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:

> The boot_lock is something that was required for ARM development
> platforms to ensure that the delay calibration worked properly.  This
> is not necessary for modern platforms that have better bus bandwidth
> and do not need to calibrate the delay loop for secondary cores.
> Remove the boot_lock entirely.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Andy have you had a chance to test/apply this patch?

I think the majority of Qcom systems use CLKSRC_QCOM
which registers an arch delay timer and swiftly bypasses the
calibration anyways so this code is pretty much pointless.

There is a question about MSM8974 though. This appears
to not use CLKSRC_QCOM but rather HAVE_ARM_ARCH_TIMER.
drivers/clocksource/arm_arch_timer.c seems to
not register a delay timer, which is probably something that
should be fixed?

Mark/Marc: is there any specific reason to why the ARM arch
timer does not register an arch delay timer?

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Stephen Boyd]

Quoting Linus Walleij (2019-01-10 04:50:56)
> On Thu, Dec 13, 2018 at 7:00 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> 
> > The boot_lock is something that was required for ARM development
> > platforms to ensure that the delay calibration worked properly.  This
> > is not necessary for modern platforms that have better bus bandwidth
> > and do not need to calibrate the delay loop for secondary cores.
> > Remove the boot_lock entirely.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Andy have you had a chance to test/apply this patch?
> 
> I think the majority of Qcom systems use CLKSRC_QCOM
> which registers an arch delay timer and swiftly bypasses the
> calibration anyways so this code is pretty much pointless.
> 
> There is a question about MSM8974 though. This appears
> to not use CLKSRC_QCOM but rather HAVE_ARM_ARCH_TIMER.
> drivers/clocksource/arm_arch_timer.c seems to
> not register a delay timer, which is probably something that
> should be fixed?
> 
> Mark/Marc: is there any specific reason to why the ARM arch
> timer does not register an arch delay timer?
> 

Doesn't the ARM arch timer get registered in
arch/arm/kernel/arch_timer.c via arch_timer_delay_timer_register()?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Linus Walleij]

On Thu, Jan 10, 2019 at 9:56 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Linus Walleij (2019-01-10 04:50:56)

> > Mark/Marc: is there any specific reason to why the ARM arch
> > timer does not register an arch delay timer?
>
> Doesn't the ARM arch timer get registered in
> arch/arm/kernel/arch_timer.c via arch_timer_delay_timer_register()?

Ah indeed it does, OK then, this patch should be fine from that
point of view.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Stephen Boyd]

Quoting Russell King (2018-12-13 10:00:46)
> The boot_lock is something that was required for ARM development
> platforms to ensure that the delay calibration worked properly.  This
> is not necessary for modern platforms that have better bus bandwidth
> and do not need to calibrate the delay loop for secondary cores.
> Remove the boot_lock entirely.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Russell King - ARM Linux]

On Thu, Jan 10, 2019 at 01:50:56PM +0100, Linus Walleij wrote:
> On Thu, Dec 13, 2018 at 7:00 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> 
> > The boot_lock is something that was required for ARM development
> > platforms to ensure that the delay calibration worked properly.  This
> > is not necessary for modern platforms that have better bus bandwidth
> > and do not need to calibrate the delay loop for secondary cores.
> > Remove the boot_lock entirely.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Andy have you had a chance to test/apply this patch?

I would rather folk do not _apply_ these patches - the final patch
is dependent on all the preceeding patches, and if they're applied
to individual trees, it will mean that the final patch has to be
delayed by a complete kernel development cycle to avoid bisect
breakage.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] ARM: qcom: remove unnecessary boot_lock

[Linus Walleij]

On Fri, Jan 11, 2019 at 4:09 PM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Thu, Jan 10, 2019 at 01:50:56PM +0100, Linus Walleij wrote:
> > On Thu, Dec 13, 2018 at 7:00 PM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > > The boot_lock is something that was required for ARM development
> > > platforms to ensure that the delay calibration worked properly.  This
> > > is not necessary for modern platforms that have better bus bandwidth
> > > and do not need to calibrate the delay loop for secondary cores.
> > > Remove the boot_lock entirely.
> > >
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >
> > Andy have you had a chance to test/apply this patch?
>
> I would rather folk do not _apply_ these patches - the final patch
> is dependent on all the preceeding patches, and if they're applied
> to individual trees, it will mean that the final patch has to be
> delayed by a complete kernel development cycle to avoid bisect
> breakage.

Ah good point. It's way better that you queue it up indeed.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel