Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ARM: smp: remove arch-provided "pen_release"

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>

+56 -43
-1
arch/arm/include/asm/smp.h
··· 67 67 void *stack; 68 68 }; 69 69 extern struct secondary_data secondary_data; 70 - extern volatile int pen_release; 71 70 extern void secondary_startup(void); 72 71 extern void secondary_startup_arm(void); 73 72
-6
arch/arm/kernel/smp.c
··· 62 62 */ 63 63 struct secondary_data secondary_data; 64 64 65 - /* 66 - * control for which core is the next to come out of the secondary 67 - * boot "holding pen" 68 - */ 69 - volatile int pen_release = -1; 70 - 71 65 enum ipi_msg_type { 72 66 IPI_WAKEUP, 73 67 IPI_TIMER,
+1 -1
arch/arm/mach-exynos/headsmp.S
··· 36 36 37 37 .align 2 38 38 1: .long . 39 - .long pen_release 39 + .long exynos_pen_release
+18 -13
arch/arm/mach-exynos/platsmp.c
··· 28 28 29 29 extern void exynos4_secondary_startup(void); 30 30 31 + /* XXX exynos_pen_release is cargo culted code - DO NOT COPY XXX */ 32 + volatile int exynos_pen_release = -1; 33 + 31 34 #ifdef CONFIG_HOTPLUG_CPU 32 35 static inline void cpu_leave_lowpower(u32 core_id) 33 36 { ··· 60 57 61 58 wfi(); 62 59 63 - if (pen_release == core_id) { 60 + if (exynos_pen_release == core_id) { 64 61 /* 65 62 * OK, proper wakeup, we're done 66 63 */ ··· 231 228 } 232 229 233 230 /* 234 - * Write pen_release in a way that is guaranteed to be visible to all 235 - * observers, irrespective of whether they're taking part in coherency 231 + * XXX CARGO CULTED CODE - DO NOT COPY XXX 232 + * 233 + * Write exynos_pen_release in a way that is guaranteed to be visible to 234 + * all observers, irrespective of whether they're taking part in coherency 236 235 * or not. This is necessary for the hotplug code to work reliably. 237 236 */ 238 - static void write_pen_release(int val) 237 + static void exynos_write_pen_release(int val) 239 238 { 240 - pen_release = val; 239 + exynos_pen_release = val; 241 240 smp_wmb(); 242 - sync_cache_w(&pen_release); 241 + sync_cache_w(&exynos_pen_release); 243 242 } 244 243 245 244 static DEFINE_SPINLOCK(boot_lock); ··· 252 247 * let the primary processor know we're out of the 253 248 * pen, then head off into the C entry point 254 249 */ 255 - write_pen_release(-1); 250 + exynos_write_pen_release(-1); 256 251 257 252 /* 258 253 * Synchronise with the boot thread. ··· 327 322 /* 328 323 * The secondary processor is waiting to be released from 329 324 * the holding pen - release it, then wait for it to flag 330 - * that it has been released by resetting pen_release. 325 + * that it has been released by resetting exynos_pen_release. 331 326 * 332 - * Note that "pen_release" is the hardware CPU core ID, whereas 327 + * Note that "exynos_pen_release" is the hardware CPU core ID, whereas 333 328 * "cpu" is Linux's internal ID. 334 329 */ 335 - write_pen_release(core_id); 330 + exynos_write_pen_release(core_id); 336 331 337 332 if (!exynos_cpu_power_state(core_id)) { 338 333 exynos_cpu_power_up(core_id); ··· 381 376 else 382 377 arch_send_wakeup_ipi_mask(cpumask_of(cpu)); 383 378 384 - if (pen_release == -1) 379 + if (exynos_pen_release == -1) 385 380 break; 386 381 387 382 udelay(10); 388 383 } 389 384 390 - if (pen_release != -1) 385 + if (exynos_pen_release != -1) 391 386 ret = -ETIMEDOUT; 392 387 393 388 /* ··· 397 392 fail: 398 393 spin_unlock(&boot_lock); 399 394 400 - return pen_release != -1 ? ret : 0; 395 + return exynos_pen_release != -1 ? ret : 0; 401 396 } 402 397 403 398 static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
+2
arch/arm/mach-prima2/common.h
··· 15 15 #include <asm/mach/time.h> 16 16 #include <asm/exception.h> 17 17 18 + extern volatile int prima2_pen_release; 19 + 18 20 extern const struct smp_operations sirfsoc_smp_ops; 19 21 extern void sirfsoc_secondary_startup(void); 20 22 extern void sirfsoc_cpu_die(unsigned int cpu);
+1 -1
arch/arm/mach-prima2/headsmp.S
··· 34 34 35 35 .align 36 36 1: .long . 37 - .long pen_release 37 + .long prima2_pen_release
+2 -1
arch/arm/mach-prima2/hotplug.c
··· 11 11 #include <linux/smp.h> 12 12 13 13 #include <asm/smp_plat.h> 14 + #include "common.h" 14 15 15 16 static inline void platform_do_lowpower(unsigned int cpu) 16 17 { ··· 19 18 for (;;) { 20 19 __asm__ __volatile__("dsb\n\t" "wfi\n\t" 21 20 : : : "memory"); 22 - if (pen_release == cpu_logical_map(cpu)) { 21 + if (prima2_pen_release == cpu_logical_map(cpu)) { 23 22 /* 24 23 * OK, proper wakeup, we're done 25 24 */
+10 -7
arch/arm/mach-prima2/platsmp.c
··· 24 24 25 25 static DEFINE_SPINLOCK(boot_lock); 26 26 27 + /* XXX prima2_pen_release is cargo culted code - DO NOT COPY XXX */ 28 + volatile int prima2_pen_release = -1; 29 + 27 30 static void sirfsoc_secondary_init(unsigned int cpu) 28 31 { 29 32 /* 30 33 * let the primary processor know we're out of the 31 34 * pen, then head off into the C entry point 32 35 */ 33 - pen_release = -1; 36 + prima2_pen_release = -1; 34 37 smp_wmb(); 35 38 36 39 /* ··· 83 80 /* 84 81 * The secondary processor is waiting to be released from 85 82 * the holding pen - release it, then wait for it to flag 86 - * that it has been released by resetting pen_release. 83 + * that it has been released by resetting prima2_pen_release. 87 84 * 88 - * Note that "pen_release" is the hardware CPU ID, whereas 85 + * Note that "prima2_pen_release" is the hardware CPU ID, whereas 89 86 * "cpu" is Linux's internal ID. 90 87 */ 91 - pen_release = cpu_logical_map(cpu); 92 - sync_cache_w(&pen_release); 88 + prima2_pen_release = cpu_logical_map(cpu); 89 + sync_cache_w(&prima2_pen_release); 93 90 94 91 /* 95 92 * Send the secondary CPU SEV, thereby causing the boot monitor to read ··· 100 97 timeout = jiffies + (1 * HZ); 101 98 while (time_before(jiffies, timeout)) { 102 99 smp_rmb(); 103 - if (pen_release == -1) 100 + if (prima2_pen_release == -1) 104 101 break; 105 102 106 103 udelay(10); ··· 112 109 */ 113 110 spin_unlock(&boot_lock); 114 111 115 - return pen_release != -1 ? -ENOSYS : 0; 112 + return prima2_pen_release != -1 ? -ENOSYS : 0; 116 113 } 117 114 118 115 const struct smp_operations sirfsoc_smp_ops __initconst = {
+2
arch/arm/mach-spear/generic.h
··· 20 20 21 21 #include <asm/mach/time.h> 22 22 23 + extern volatile int spear_pen_release; 24 + 23 25 extern void spear13xx_timer_init(void); 24 26 extern void spear3xx_timer_init(void); 25 27 extern struct pl022_ssp_controller pl022_plat_data;
+1 -1
arch/arm/mach-spear/headsmp.S
··· 43 43 44 44 .align 45 45 1: .long . 46 - .long pen_release 46 + .long spear_pen_release 47 47 ENDPROC(spear13xx_secondary_startup)
+3 -1
arch/arm/mach-spear/hotplug.c
··· 16 16 #include <asm/cp15.h> 17 17 #include <asm/smp_plat.h> 18 18 19 + #include "generic.h" 20 + 19 21 static inline void cpu_enter_lowpower(void) 20 22 { 21 23 unsigned int v; ··· 59 57 for (;;) { 60 58 wfi(); 61 59 62 - if (pen_release == cpu) { 60 + if (spear_pen_release == cpu) { 63 61 /* 64 62 * OK, proper wakeup, we're done 65 63 */
+16 -11
arch/arm/mach-spear/platsmp.c
··· 20 20 #include <mach/spear.h> 21 21 #include "generic.h" 22 22 23 + /* XXX spear_pen_release is cargo culted code - DO NOT COPY XXX */ 24 + volatile int spear_pen_release = -1; 25 + 23 26 /* 24 - * Write pen_release in a way that is guaranteed to be visible to all 25 - * observers, irrespective of whether they're taking part in coherency 27 + * XXX CARGO CULTED CODE - DO NOT COPY XXX 28 + * 29 + * Write spear_pen_release in a way that is guaranteed to be visible to 30 + * all observers, irrespective of whether they're taking part in coherency 26 31 * or not. This is necessary for the hotplug code to work reliably. 27 32 */ 28 - static void write_pen_release(int val) 33 + static void spear_write_pen_release(int val) 29 34 { 30 - pen_release = val; 35 + spear_pen_release = val; 31 36 smp_wmb(); 32 - sync_cache_w(&pen_release); 37 + sync_cache_w(&spear_pen_release); 33 38 } 34 39 35 40 static DEFINE_SPINLOCK(boot_lock); ··· 47 42 * let the primary processor know we're out of the 48 43 * pen, then head off into the C entry point 49 44 */ 50 - write_pen_release(-1); 45 + spear_write_pen_release(-1); 51 46 52 47 /* 53 48 * Synchronise with the boot thread. ··· 69 64 /* 70 65 * The secondary processor is waiting to be released from 71 66 * the holding pen - release it, then wait for it to flag 72 - * that it has been released by resetting pen_release. 67 + * that it has been released by resetting spear_pen_release. 73 68 * 74 - * Note that "pen_release" is the hardware CPU ID, whereas 69 + * Note that "spear_pen_release" is the hardware CPU ID, whereas 75 70 * "cpu" is Linux's internal ID. 76 71 */ 77 - write_pen_release(cpu); 72 + spear_write_pen_release(cpu); 78 73 79 74 timeout = jiffies + (1 * HZ); 80 75 while (time_before(jiffies, timeout)) { 81 76 smp_rmb(); 82 - if (pen_release == -1) 77 + if (spear_pen_release == -1) 83 78 break; 84 79 85 80 udelay(10); ··· 91 86 */ 92 87 spin_unlock(&boot_lock); 93 88 94 - return pen_release != -1 ? -ENOSYS : 0; 89 + return spear_pen_release != -1 ? -ENOSYS : 0; 95 90 } 96 91 97 92 /*