linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c
@ 2014-08-05 10:43 Krzysztof Kozlowski
  2014-08-05 10:43 ` [PATCH v5 2/3] ARM: EXYNOS: Remove unneeded __ref annotation for cpu_die function Krzysztof Kozlowski
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-08-05 10:43 UTC (permalink / raw)
  To: linux-arm-kernel

Cleanup a little the SMP/hotplug code for Exynos by:
1. Moving completely all functions from hotplug.c into the platsmp.c;
2. Deleting the hotplug.c file.

After recent cleanups (e.g. 75ad2ab28f0f "ARM: EXYNOS: use
v7_exit_coherency_flush macro for cache disabling") there was only CPU
power down related code in hotplug.c file. Keeping this file does not
give any benefits.

The commit only moves code around with one additional observable change:
the hotplug.c was compiled with custom CFLAGS (-march=armv7-a). These
CFLAGS are not necessary any more.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>

---
Changes since v4:
1. Rebase on linux-next-20140804.
2. Add Reviewed-by Tomasz Figa.
---
 arch/arm/mach-exynos/Makefile  |  3 --
 arch/arm/mach-exynos/common.h  |  2 -
 arch/arm/mach-exynos/hotplug.c | 91 ------------------------------------------
 arch/arm/mach-exynos/platsmp.c | 74 ++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 96 deletions(-)
 delete mode 100644 arch/arm/mach-exynos/hotplug.c

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 788f26d21141..0095de99d703 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -21,9 +21,6 @@ obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
 
 obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
 
-obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
-CFLAGS_hotplug.o		+= -march=armv7-a
-
 plus_sec := $(call as-instr,.arch_extension sec,+sec)
 AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-a$(plus_sec)
 
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 47b904b3b973..3d3e6af9d015 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -130,8 +130,6 @@ extern void exynos_cpu_resume(void);
 
 extern struct smp_operations exynos_smp_ops;
 
-extern void exynos_cpu_die(unsigned int cpu);
-
 /* PMU(Power Management Unit) support */
 
 #define PMU_TABLE_END	(-1U)
diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
deleted file mode 100644
index 4d86961a7957..000000000000
--- a/arch/arm/mach-exynos/hotplug.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- *  Cloned from 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 <linux/io.h>
-
-#include <asm/cacheflush.h>
-#include <asm/cp15.h>
-#include <asm/smp_plat.h>
-
-#include "common.h"
-#include "regs-pmu.h"
-
-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, %2\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	  : "=&r" (v)
-	  : "Ir" (CR_C), "Ir" (0x40)
-	  : "cc");
-}
-
-static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
-{
-	u32 mpidr = cpu_logical_map(cpu);
-	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
-
-	for (;;) {
-
-		/* Turn the CPU off on next WFI instruction. */
-		exynos_cpu_power_down(core_id);
-
-		wfi();
-
-		if (pen_release == core_id) {
-			/*
-			 * 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 __ref exynos_cpu_die(unsigned int cpu)
-{
-	int spurious = 0;
-
-	v7_exit_coherency_flush(louis);
-
-	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-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index a9f1cf759949..a809f76c8160 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -22,6 +22,7 @@
 #include <linux/of_address.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cp15.h>
 #include <asm/smp_plat.h>
 #include <asm/smp_scu.h>
 #include <asm/firmware.h>
@@ -33,6 +34,54 @@
 
 extern void exynos4_secondary_startup(void);
 
+#ifdef CONFIG_HOTPLUG_CPU
+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, %2\n"
+	"	mcr	p15, 0, %0, c1, c0, 1\n"
+	  : "=&r" (v)
+	  : "Ir" (CR_C), "Ir" (0x40)
+	  : "cc");
+}
+
+static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
+{
+	u32 mpidr = cpu_logical_map(cpu);
+	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+
+	for (;;) {
+
+		/* Turn the CPU off on next WFI instruction. */
+		exynos_cpu_power_down(core_id);
+
+		wfi();
+
+		if (pen_release == core_id) {
+			/*
+			 * 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)++;
+	}
+}
+#endif /* CONFIG_HOTPLUG_CPU */
+
 /**
  * exynos_core_power_down : power down the specified cpu
  * @cpu : the cpu to power down
@@ -318,6 +367,31 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
+#ifdef CONFIG_HOTPLUG_CPU
+/*
+ * platform-specific code to shutdown a CPU
+ *
+ * Called with IRQs disabled
+ */
+static void __ref exynos_cpu_die(unsigned int cpu)
+{
+	int spurious = 0;
+
+	v7_exit_coherency_flush(louis);
+
+	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);
+}
+#endif /* CONFIG_HOTPLUG_CPU */
+
 struct smp_operations exynos_smp_ops __initdata = {
 	.smp_init_cpus		= exynos_smp_init_cpus,
 	.smp_prepare_cpus	= exynos_smp_prepare_cpus,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/3] ARM: EXYNOS: Remove unneeded __ref annotation for cpu_die function
  2014-08-05 10:43 [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Krzysztof Kozlowski
@ 2014-08-05 10:43 ` Krzysztof Kozlowski
  2014-08-05 10:43 ` [PATCH v5 3/3] ARM: exynos4: hotplug: Fix CPU idle clock down after CPU off Krzysztof Kozlowski
  2014-08-05 14:46 ` [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Daniel Lezcano
  2 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-08-05 10:43 UTC (permalink / raw)
  To: linux-arm-kernel

The __ref annotation for exynos_cpu_die() is not needed because the
function does not reference any __init/__exit symbol or call.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>

---
Changes since v4:
1. Rebase on linux-next-20140804.
2. Add Reviewed-by Tomasz Figa.
---
 arch/arm/mach-exynos/platsmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index a809f76c8160..83b20a38a598 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -373,7 +373,7 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
  *
  * Called with IRQs disabled
  */
-static void __ref exynos_cpu_die(unsigned int cpu)
+static void exynos_cpu_die(unsigned int cpu)
 {
 	int spurious = 0;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 3/3] ARM: exynos4: hotplug: Fix CPU idle clock down after CPU off
  2014-08-05 10:43 [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Krzysztof Kozlowski
  2014-08-05 10:43 ` [PATCH v5 2/3] ARM: EXYNOS: Remove unneeded __ref annotation for cpu_die function Krzysztof Kozlowski
@ 2014-08-05 10:43 ` Krzysztof Kozlowski
  2014-08-05 14:46 ` [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Daniel Lezcano
  2 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-08-05 10:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Exynos4 USE_DELAYED_RESET_ASSERTION must be set in
ARM_COREx_OPTION register during CPU power down. This is the proper way
of powering down CPU on Exynos4.

Additionally on Exynos4212 without this the CPU clock down feature won't
work after powering down some CPU and the online CPUs will work at full
frequency chosen by CPUfreq governor.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>

---
Changes since v4:
1. Rebase on linux-next-20140804.
2. Add Reviewed-by Tomasz Figa.

Changes since v3:
1. Add patches moving hotplug code to platsmp.c file (patches 1 and 2).
   Suggested by Tomasz Figa.
2. Remove declaration of exynos_clear_delayed_reset_assertion() in
   header because hotplug code is integrated into platsmp.c file.
   Suggested by Tomasz Figa.
3. Replace 'clear' helper with 'set' helper. Suggested by Tomasz Figa.

Changes since v2:
1. Add missing disable of the "use delayed reset assertion" feature
   when starting secondary CPU (suggested by Tomasz Figa).

Changes since v1:
1. Use delayed reset assertion on all Exynos4 family and all cores, not
   only on core 1 of Exynos4212.
2. Rebase on Tomasz Figa's patch:
   ARM: EXYNOS: Fix core ID used by platsmp and hotplug code
   http://www.spinics.net/lists/linux-samsung-soc/msg31604.html
   Tomasz's patch is currently applied to Kukjin's v3.16-samsung-fixes-4
---
 arch/arm/mach-exynos/platsmp.c  | 43 +++++++++++++++++++++++++++++++++++++++--
 arch/arm/mach-exynos/regs-pmu.h |  3 +++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 83b20a38a598..99c151bf30ad 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -34,8 +34,32 @@
 
 extern void exynos4_secondary_startup(void);
 
+/*
+ * Set or clear the USE_DELAYED_RESET_ASSERTION option, set on Exynos4 SoCs
+ * during hot-(un)plugging CPUx.
+ *
+ * The feature can be cleared safely during first boot of secondary CPU.
+ *
+ * Exynos4 SoCs require setting USE_DELAYED_RESET_ASSERTION during powering
+ * down a CPU so the CPU idle clock down feature could properly detect global
+ * idle state when CPUx is off.
+ */
+static void exynos_set_delayed_reset_assertion(u32 core_id, bool enable)
+{
+	if (soc_is_exynos4()) {
+		unsigned int tmp;
+
+		tmp = pmu_raw_readl(EXYNOS_ARM_CORE_OPTION(core_id));
+		if (enable)
+			tmp |= S5P_USE_DELAYED_RESET_ASSERTION;
+		else
+			tmp &= ~(S5P_USE_DELAYED_RESET_ASSERTION);
+		pmu_raw_writel(tmp, EXYNOS_ARM_CORE_OPTION(core_id));
+	}
+}
+
 #ifdef CONFIG_HOTPLUG_CPU
-static inline void cpu_leave_lowpower(void)
+static inline void cpu_leave_lowpower(u32 core_id)
 {
 	unsigned int v;
 
@@ -49,6 +73,8 @@ static inline void cpu_leave_lowpower(void)
 	  : "=&r" (v)
 	  : "Ir" (CR_C), "Ir" (0x40)
 	  : "cc");
+
+	 exynos_set_delayed_reset_assertion(core_id, false);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
@@ -61,6 +87,14 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
 		/* Turn the CPU off on next WFI instruction. */
 		exynos_cpu_power_down(core_id);
 
+		/*
+		 * Exynos4 SoCs require setting
+		 * USE_DELAYED_RESET_ASSERTION so the CPU idle
+		 * clock down feature could properly detect
+		 * global idle state when CPUx is off.
+		 */
+		exynos_set_delayed_reset_assertion(core_id, true);
+
 		wfi();
 
 		if (pen_release == core_id) {
@@ -286,6 +320,9 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
 		udelay(10);
 	}
 
+	/* No harm if this is called during first boot of secondary CPU */
+	exynos_set_delayed_reset_assertion(core_id, false);
+
 	/*
 	 * now the secondary core is starting up let it run its
 	 * calibrations, then wait for it to finish
@@ -376,6 +413,8 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
 static void exynos_cpu_die(unsigned int cpu)
 {
 	int spurious = 0;
+	u32 mpidr = cpu_logical_map(cpu);
+	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 
 	v7_exit_coherency_flush(louis);
 
@@ -385,7 +424,7 @@ static void exynos_cpu_die(unsigned int cpu)
 	 * bring this CPU back into the world of cache
 	 * coherency, and then restore interrupts
 	 */
-	cpu_leave_lowpower();
+	cpu_leave_lowpower(core_id);
 
 	if (spurious)
 		pr_warn("CPU%u: %u spurious wakeup calls\n", cpu, spurious);
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 96a1569262b5..4e9b4440e2bd 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -20,6 +20,7 @@
 
 #define S5P_USE_STANDBY_WFI0			(1 << 16)
 #define S5P_USE_STANDBY_WFE0			(1 << 24)
+#define S5P_USE_DELAYED_RESET_ASSERTION		BIT(12)
 
 #define EXYNOS_SWRESET				0x0400
 #define EXYNOS5440_SWRESET			0x00C4
@@ -106,6 +107,8 @@
 			(EXYNOS_ARM_CORE0_CONFIGURATION + (0x80 * (_nr)))
 #define EXYNOS_ARM_CORE_STATUS(_nr)		\
 			(EXYNOS_ARM_CORE_CONFIGURATION(_nr) + 0x4)
+#define EXYNOS_ARM_CORE_OPTION(_nr)		\
+			(EXYNOS_ARM_CORE_CONFIGURATION(_nr) + 0x8)
 
 #define EXYNOS_ARM_COMMON_CONFIGURATION		0x2500
 #define EXYNOS_COMMON_CONFIGURATION(_nr)	\
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c
  2014-08-05 10:43 [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Krzysztof Kozlowski
  2014-08-05 10:43 ` [PATCH v5 2/3] ARM: EXYNOS: Remove unneeded __ref annotation for cpu_die function Krzysztof Kozlowski
  2014-08-05 10:43 ` [PATCH v5 3/3] ARM: exynos4: hotplug: Fix CPU idle clock down after CPU off Krzysztof Kozlowski
@ 2014-08-05 14:46 ` Daniel Lezcano
  2014-08-05 15:03   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2014-08-05 14:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/05/2014 12:43 PM, Krzysztof Kozlowski wrote:
> Cleanup a little the SMP/hotplug code for Exynos by:
> 1. Moving completely all functions from hotplug.c into the platsmp.c;
> 2. Deleting the hotplug.c file.
>
> After recent cleanups (e.g. 75ad2ab28f0f "ARM: EXYNOS: use
> v7_exit_coherency_flush macro for cache disabling") there was only CPU
> power down related code in hotplug.c file. Keeping this file does not
> give any benefits.
>
> The commit only moves code around with one additional observable change:
> the hotplug.c was compiled with custom CFLAGS (-march=armv7-a). These
> CFLAGS are not necessary any more.

What is the benefit of moving this code around ?

Usually the cpu hotplug code is located in hotplug.c for most of the 
platforms, so why moving pm code inside platsmp ?

> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
>
> ---
> Changes since v4:
> 1. Rebase on linux-next-20140804.
> 2. Add Reviewed-by Tomasz Figa.
> ---
>   arch/arm/mach-exynos/Makefile  |  3 --
>   arch/arm/mach-exynos/common.h  |  2 -
>   arch/arm/mach-exynos/hotplug.c | 91 ------------------------------------------
>   arch/arm/mach-exynos/platsmp.c | 74 ++++++++++++++++++++++++++++++++++
>   4 files changed, 74 insertions(+), 96 deletions(-)
>   delete mode 100644 arch/arm/mach-exynos/hotplug.c
>
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 788f26d21141..0095de99d703 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -21,9 +21,6 @@ obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>
>   obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
>
> -obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
> -CFLAGS_hotplug.o		+= -march=armv7-a
> -
>   plus_sec := $(call as-instr,.arch_extension sec,+sec)
>   AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-a$(plus_sec)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 47b904b3b973..3d3e6af9d015 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -130,8 +130,6 @@ extern void exynos_cpu_resume(void);
>
>   extern struct smp_operations exynos_smp_ops;
>
> -extern void exynos_cpu_die(unsigned int cpu);
> -
>   /* PMU(Power Management Unit) support */
>
>   #define PMU_TABLE_END	(-1U)
> diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> deleted file mode 100644
> index 4d86961a7957..000000000000
> --- a/arch/arm/mach-exynos/hotplug.c
> +++ /dev/null
> @@ -1,91 +0,0 @@
> -/*
> - *  Cloned from 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 <linux/io.h>
> -
> -#include <asm/cacheflush.h>
> -#include <asm/cp15.h>
> -#include <asm/smp_plat.h>
> -
> -#include "common.h"
> -#include "regs-pmu.h"
> -
> -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, %2\n"
> -	"	mcr	p15, 0, %0, c1, c0, 1\n"
> -	  : "=&r" (v)
> -	  : "Ir" (CR_C), "Ir" (0x40)
> -	  : "cc");
> -}
> -
> -static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> -{
> -	u32 mpidr = cpu_logical_map(cpu);
> -	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> -
> -	for (;;) {
> -
> -		/* Turn the CPU off on next WFI instruction. */
> -		exynos_cpu_power_down(core_id);
> -
> -		wfi();
> -
> -		if (pen_release == core_id) {
> -			/*
> -			 * 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 __ref exynos_cpu_die(unsigned int cpu)
> -{
> -	int spurious = 0;
> -
> -	v7_exit_coherency_flush(louis);
> -
> -	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-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index a9f1cf759949..a809f76c8160 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -22,6 +22,7 @@
>   #include <linux/of_address.h>
>
>   #include <asm/cacheflush.h>
> +#include <asm/cp15.h>
>   #include <asm/smp_plat.h>
>   #include <asm/smp_scu.h>
>   #include <asm/firmware.h>
> @@ -33,6 +34,54 @@
>
>   extern void exynos4_secondary_startup(void);
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +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, %2\n"
> +	"	mcr	p15, 0, %0, c1, c0, 1\n"
> +	  : "=&r" (v)
> +	  : "Ir" (CR_C), "Ir" (0x40)
> +	  : "cc");
> +}
> +
> +static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> +{
> +	u32 mpidr = cpu_logical_map(cpu);
> +	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +
> +	for (;;) {
> +
> +		/* Turn the CPU off on next WFI instruction. */
> +		exynos_cpu_power_down(core_id);
> +
> +		wfi();
> +
> +		if (pen_release == core_id) {
> +			/*
> +			 * 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)++;
> +	}
> +}
> +#endif /* CONFIG_HOTPLUG_CPU */
> +
>   /**
>    * exynos_core_power_down : power down the specified cpu
>    * @cpu : the cpu to power down
> @@ -318,6 +367,31 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
>   	}
>   }
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +/*
> + * platform-specific code to shutdown a CPU
> + *
> + * Called with IRQs disabled
> + */
> +static void __ref exynos_cpu_die(unsigned int cpu)
> +{
> +	int spurious = 0;
> +
> +	v7_exit_coherency_flush(louis);
> +
> +	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);
> +}
> +#endif /* CONFIG_HOTPLUG_CPU */
> +
>   struct smp_operations exynos_smp_ops __initdata = {
>   	.smp_init_cpus		= exynos_smp_init_cpus,
>   	.smp_prepare_cpus	= exynos_smp_prepare_cpus,
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c
  2014-08-05 14:46 ` [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Daniel Lezcano
@ 2014-08-05 15:03   ` Krzysztof Kozlowski
  2014-08-05 19:53     ` Tomasz Figa
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2014-08-05 15:03 UTC (permalink / raw)
  To: linux-arm-kernel

On wto, 2014-08-05 at 16:46 +0200, Daniel Lezcano wrote:
> On 08/05/2014 12:43 PM, Krzysztof Kozlowski wrote:
> > Cleanup a little the SMP/hotplug code for Exynos by:
> > 1. Moving completely all functions from hotplug.c into the platsmp.c;
> > 2. Deleting the hotplug.c file.
> >
> > After recent cleanups (e.g. 75ad2ab28f0f "ARM: EXYNOS: use
> > v7_exit_coherency_flush macro for cache disabling") there was only CPU
> > power down related code in hotplug.c file. Keeping this file does not
> > give any benefits.
> >
> > The commit only moves code around with one additional observable change:
> > the hotplug.c was compiled with custom CFLAGS (-march=armv7-a). These
> > CFLAGS are not necessary any more.
> 
> What is the benefit of moving this code around ?
> 
> Usually the cpu hotplug code is located in hotplug.c for most of the 
> platforms, so why moving pm code inside platsmp ?

The benefits are:

1. Less dependencies, cleaner code because platsmp.c is the only user of
code located in hotplug.c. So the exynos_cpu_die() had to be declared in
common.h.

2. Less dependencies between modules after adding patch 3/3 [1] which
introduces exynos_set_delayed_reset_assertion() function. The
exynos_set_delayed_reset_assertion() is called by:
 - cpu_leave_power (hotplug.c)
 - platform_do_lowpower (hotplug.c)
 - exynos_boot_secondary (platsmp.c)

so merging everything into one file seemed simpler and cleaner.

However I do not insist on such solution. I'm merely interested in
getting the third patch into the kernel :) .

[1] http://www.spinics.net/lists/arm-kernel/msg352890.html

Best regards,
Krzysztof

> 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> >
> > ---
> > Changes since v4:
> > 1. Rebase on linux-next-20140804.
> > 2. Add Reviewed-by Tomasz Figa.
> > ---
> >   arch/arm/mach-exynos/Makefile  |  3 --
> >   arch/arm/mach-exynos/common.h  |  2 -
> >   arch/arm/mach-exynos/hotplug.c | 91 ------------------------------------------
> >   arch/arm/mach-exynos/platsmp.c | 74 ++++++++++++++++++++++++++++++++++
> >   4 files changed, 74 insertions(+), 96 deletions(-)
> >   delete mode 100644 arch/arm/mach-exynos/hotplug.c
> >
> > diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> > index 788f26d21141..0095de99d703 100644
> > --- a/arch/arm/mach-exynos/Makefile
> > +++ b/arch/arm/mach-exynos/Makefile
> > @@ -21,9 +21,6 @@ obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
> >
> >   obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
> >
> > -obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
> > -CFLAGS_hotplug.o		+= -march=armv7-a
> > -
> >   plus_sec := $(call as-instr,.arch_extension sec,+sec)
> >   AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-a$(plus_sec)
> >
> > diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> > index 47b904b3b973..3d3e6af9d015 100644
> > --- a/arch/arm/mach-exynos/common.h
> > +++ b/arch/arm/mach-exynos/common.h
> > @@ -130,8 +130,6 @@ extern void exynos_cpu_resume(void);
> >
> >   extern struct smp_operations exynos_smp_ops;
> >
> > -extern void exynos_cpu_die(unsigned int cpu);
> > -
> >   /* PMU(Power Management Unit) support */
> >
> >   #define PMU_TABLE_END	(-1U)
> > diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> > deleted file mode 100644
> > index 4d86961a7957..000000000000
> > --- a/arch/arm/mach-exynos/hotplug.c
> > +++ /dev/null
> > @@ -1,91 +0,0 @@
> > -/*
> > - *  Cloned from 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 <linux/io.h>
> > -
> > -#include <asm/cacheflush.h>
> > -#include <asm/cp15.h>
> > -#include <asm/smp_plat.h>
> > -
> > -#include "common.h"
> > -#include "regs-pmu.h"
> > -
> > -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, %2\n"
> > -	"	mcr	p15, 0, %0, c1, c0, 1\n"
> > -	  : "=&r" (v)
> > -	  : "Ir" (CR_C), "Ir" (0x40)
> > -	  : "cc");
> > -}
> > -
> > -static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> > -{
> > -	u32 mpidr = cpu_logical_map(cpu);
> > -	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > -
> > -	for (;;) {
> > -
> > -		/* Turn the CPU off on next WFI instruction. */
> > -		exynos_cpu_power_down(core_id);
> > -
> > -		wfi();
> > -
> > -		if (pen_release == core_id) {
> > -			/*
> > -			 * 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 __ref exynos_cpu_die(unsigned int cpu)
> > -{
> > -	int spurious = 0;
> > -
> > -	v7_exit_coherency_flush(louis);
> > -
> > -	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-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> > index a9f1cf759949..a809f76c8160 100644
> > --- a/arch/arm/mach-exynos/platsmp.c
> > +++ b/arch/arm/mach-exynos/platsmp.c
> > @@ -22,6 +22,7 @@
> >   #include <linux/of_address.h>
> >
> >   #include <asm/cacheflush.h>
> > +#include <asm/cp15.h>
> >   #include <asm/smp_plat.h>
> >   #include <asm/smp_scu.h>
> >   #include <asm/firmware.h>
> > @@ -33,6 +34,54 @@
> >
> >   extern void exynos4_secondary_startup(void);
> >
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +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, %2\n"
> > +	"	mcr	p15, 0, %0, c1, c0, 1\n"
> > +	  : "=&r" (v)
> > +	  : "Ir" (CR_C), "Ir" (0x40)
> > +	  : "cc");
> > +}
> > +
> > +static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> > +{
> > +	u32 mpidr = cpu_logical_map(cpu);
> > +	u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > +
> > +	for (;;) {
> > +
> > +		/* Turn the CPU off on next WFI instruction. */
> > +		exynos_cpu_power_down(core_id);
> > +
> > +		wfi();
> > +
> > +		if (pen_release == core_id) {
> > +			/*
> > +			 * 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)++;
> > +	}
> > +}
> > +#endif /* CONFIG_HOTPLUG_CPU */
> > +
> >   /**
> >    * exynos_core_power_down : power down the specified cpu
> >    * @cpu : the cpu to power down
> > @@ -318,6 +367,31 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
> >   	}
> >   }
> >
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +/*
> > + * platform-specific code to shutdown a CPU
> > + *
> > + * Called with IRQs disabled
> > + */
> > +static void __ref exynos_cpu_die(unsigned int cpu)
> > +{
> > +	int spurious = 0;
> > +
> > +	v7_exit_coherency_flush(louis);
> > +
> > +	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);
> > +}
> > +#endif /* CONFIG_HOTPLUG_CPU */
> > +
> >   struct smp_operations exynos_smp_ops __initdata = {
> >   	.smp_init_cpus		= exynos_smp_init_cpus,
> >   	.smp_prepare_cpus	= exynos_smp_prepare_cpus,
> >
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c
  2014-08-05 15:03   ` Krzysztof Kozlowski
@ 2014-08-05 19:53     ` Tomasz Figa
  0 siblings, 0 replies; 6+ messages in thread
From: Tomasz Figa @ 2014-08-05 19:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 05.08.2014 17:03, Krzysztof Kozlowski wrote:
> On wto, 2014-08-05 at 16:46 +0200, Daniel Lezcano wrote:
>> On 08/05/2014 12:43 PM, Krzysztof Kozlowski wrote:
>>> Cleanup a little the SMP/hotplug code for Exynos by:
>>> 1. Moving completely all functions from hotplug.c into the platsmp.c;
>>> 2. Deleting the hotplug.c file.
>>>
>>> After recent cleanups (e.g. 75ad2ab28f0f "ARM: EXYNOS: use
>>> v7_exit_coherency_flush macro for cache disabling") there was only CPU
>>> power down related code in hotplug.c file. Keeping this file does not
>>> give any benefits.
>>>
>>> The commit only moves code around with one additional observable change:
>>> the hotplug.c was compiled with custom CFLAGS (-march=armv7-a). These
>>> CFLAGS are not necessary any more.
>>
>> What is the benefit of moving this code around ?
>>
>> Usually the cpu hotplug code is located in hotplug.c for most of the 
>> platforms, so why moving pm code inside platsmp ?
> 
> The benefits are:
> 
> 1. Less dependencies, cleaner code because platsmp.c is the only user of
> code located in hotplug.c. So the exynos_cpu_die() had to be declared in
> common.h.
> 
> 2. Less dependencies between modules after adding patch 3/3 [1] which
> introduces exynos_set_delayed_reset_assertion() function. The
> exynos_set_delayed_reset_assertion() is called by:
>  - cpu_leave_power (hotplug.c)
>  - platform_do_lowpower (hotplug.c)
>  - exynos_boot_secondary (platsmp.c)
> 
> so merging everything into one file seemed simpler and cleaner.

mach-exynos/hotplug.c does not have too much code right now and as
Krzysztof noticed the only user of functions from it is platsmp.c
anyway. We don't need to follow other machs in something that makes no
sense, so

Acked-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-08-05 19:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-05 10:43 [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Krzysztof Kozlowski
2014-08-05 10:43 ` [PATCH v5 2/3] ARM: EXYNOS: Remove unneeded __ref annotation for cpu_die function Krzysztof Kozlowski
2014-08-05 10:43 ` [PATCH v5 3/3] ARM: exynos4: hotplug: Fix CPU idle clock down after CPU off Krzysztof Kozlowski
2014-08-05 14:46 ` [PATCH v5 1/3] ARM: EXYNOS: Move code from hotplug.c to platsmp.c Daniel Lezcano
2014-08-05 15:03   ` Krzysztof Kozlowski
2014-08-05 19:53     ` Tomasz Figa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).