Skip to contentSkip to conversation

ARM: introduce common set_auxcr/get_auxcr functions

PATCH New Rob Herring 4 participants 12 messages Jan 17, 2013 – Jan 18, 2013

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 ARM: introduce common set_auxcr/get_auxcr functions

From: Rob Herring <[email protected]>

Move the private set_auxcr/get_auxcr functions from drivers/cpuidle/cpuidle-calxeda.c so they can be used across platforms.

---
 arch/arm/include/asm/cp15.h       |   14 ++++++++++++++
 drivers/cpuidle/cpuidle-calxeda.c |   14 --------------
 2 files changed, 14 insertions(+), 14 deletions(-)
arch/arm/include/asm/cp15.h +14 -0
index 5ef4d80..ef0094a 100644
@@ -59,6 +59,20 @@ static inline void set_cr(unsigned int val)
 	isb();
 }
 
+static inline unsigned int get_auxcr(void)
+{
+	unsigned int val;
+	asm("mrc p15, 0, %0, c1, c0, 1	@ get AUXCR" : "=r" (val) : : "cc");
+	return val;
+}
+
+static inline void set_auxcr(unsigned int val)
+{
+	asm volatile("mcr p15, 0, %0, c1, c0, 1	@ set AUXCR"
+	  : : "r" (val) : "cc");
+	isb();
+}
+
 #ifndef CONFIG_SMP
 extern void adjust_cr(unsigned long mask, unsigned long set);
 #endif
drivers/cpuidle/cpuidle-calxeda.c +0 -14
index e1aab38..ece83d6 100644
@@ -37,20 +37,6 @@ extern void *scu_base_addr;
 
 static struct cpuidle_device __percpu *calxeda_idle_cpuidle_devices;
 
-static inline unsigned int get_auxcr(void)
-{
-	unsigned int val;
-	asm("mrc p15, 0, %0, c1, c0, 1	@ get AUXCR" : "=r" (val) : : "cc");
-	return val;
-}
-
-static inline void set_auxcr(unsigned int val)
-{
-	asm volatile("mcr p15, 0, %0, c1, c0, 1	@ set AUXCR"
-	  : : "r" (val) : "cc");
-	isb();
-}
-
 static noinline void calxeda_idle_restore(void)
 {
 	set_cr(get_cr() | CR_C);
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 2/3 ARM: convert platform hotplug inline assembly to C

From: Rob Herring <[email protected]>

With the addition of set_auxcr/get_auxcr, all the hotplug inline assembly code for exynos, imx, realview, spear13xx and vexpress can be converted to C code.

Cc Kukjin Kim <[email protected]>
Cc Russell King <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Viresh Kumar <[email protected]>
Cc Shiraz Hashim <[email protected]>
---
 arch/arm/mach-exynos/hotplug.c    |   57 ++++++-------------------------------
 arch/arm/mach-imx/hotplug.c       |   19 ++++---------
 arch/arm/mach-realview/hotplug.c  |   32 +++++----------------
 arch/arm/mach-spear13xx/hotplug.c |   32 +++++----------------
 arch/arm/mach-vexpress/hotplug.c  |   33 +++++----------------
 5 files changed, 35 insertions(+), 138 deletions(-)
arch/arm/mach-exynos/hotplug.c +9 -48
index c3f825b..5548fa3 100644
@@ -26,69 +26,30 @@
 
 static inline void cpu_enter_lowpower_a9(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 static inline void cpu_enter_lowpower_a15(void)
 {
-	unsigned int v;
-
-	asm volatile(
-	"	mrc	p15, 0, %0, c1, c0, 0\n"
-	"	bic	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	  : "=&r" (v)
-	  : "Ir" (CR_C)
-	  : "cc");
-
+	set_cr(get_cr() & ~CR_C);
 	flush_cache_louis();
+	set_auxcr(get_auxcr() & ~0x40);
 
-	asm volatile(
-	/*
-	* Turn off coherency
-	*/
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	: "=&r" (v)
-	: "Ir" (0x40)
-	: "cc");
-
-	isb();
 	dsb();
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-imx/hotplug.c +5 -14
index 3dec962..1470c75 100644
@@ -18,24 +18,15 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-		"mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 /*
arch/arm/mach-realview/hotplug.c +7 -25
index 53818e5..c17c8c0 100644
@@ -18,39 +18,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * 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");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-spear13xx/hotplug.c +7 -25
index a7d2dd1..a38a087 100644
@@ -19,39 +19,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	dsb\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * 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", "memory");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void spear13xx_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-vexpress/hotplug.c +7 -26
index a141b98..74a9eb5 100644
@@ -18,40 +18,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-		"mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH 2/3 ARM: convert platform hotplug inline assembly to C

Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the addition of set_auxcr/get_auxcr, all the hotplug inline assembly code for exynos, imx, realview, spear13xx and vexpress can be converted to C code.

That might not be all safe. Please see
http://article.gmane.org/gmane.linux.ports.arm.kernel/209584

Quoted 289 lines
Cc Kukjin Kim <[email protected]>
Cc Russell King <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Viresh Kumar <[email protected]>
Cc Shiraz Hashim <[email protected]>
---
 arch/arm/mach-exynos/hotplug.c    |   57 ++++++-------------------------------
 arch/arm/mach-imx/hotplug.c       |   19 ++++---------
 arch/arm/mach-realview/hotplug.c  |   32 +++++----------------
 arch/arm/mach-spear13xx/hotplug.c |   32 +++++----------------
 arch/arm/mach-vexpress/hotplug.c  |   33 +++++----------------
 5 files changed, 35 insertions(+), 138 deletions(-)
arch/arm/mach-exynos/hotplug.c +9 -48
index c3f825b..5548fa3 100644
@@ -26,69 +26,30 @@
 
 static inline void cpu_enter_lowpower_a9(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 static inline void cpu_enter_lowpower_a15(void)
 {
-	unsigned int v;
-
-	asm volatile(
-	"	mrc	p15, 0, %0, c1, c0, 0\n"
-	"	bic	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	  : "=&r" (v)
-	  : "Ir" (CR_C)
-	  : "cc");
-
+	set_cr(get_cr() & ~CR_C);
 	flush_cache_louis();
+	set_auxcr(get_auxcr() & ~0x40);
 
-	asm volatile(
-	/*
-	* Turn off coherency
-	*/
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	: "=&r" (v)
-	: "Ir" (0x40)
-	: "cc");
-
-	isb();
 	dsb();
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-imx/hotplug.c +5 -14
index 3dec962..1470c75 100644
@@ -18,24 +18,15 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-		"mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 /*
arch/arm/mach-realview/hotplug.c +7 -25
index 53818e5..c17c8c0 100644
@@ -18,39 +18,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * 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");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-spear13xx/hotplug.c +7 -25
index a7d2dd1..a38a087 100644
@@ -19,39 +19,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	dsb\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * 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", "memory");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void spear13xx_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-vexpress/hotplug.c +7 -26
index a141b98..74a9eb5 100644
@@ -18,40 +18,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-		"mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH 2/3 ARM: convert platform hotplug inline assembly to C

Quoted 10 lines · Nicolas Pitre
Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the addition of set_auxcr/get_auxcr, all the hotplug inline assembly code for exynos, imx, realview, spear13xx and vexpress can be converted to C code.

That might not be all safe. Please see
http://article.gmane.org/gmane.linux.ports.arm.kernel/209584

Other than the OR/AND operations, it's all just inline assembly functions that are called, so it gets compiled to the same code. Perhaps I should put noinline on the functions so they stay of limited complexity. If you don't think doing this in C is okay, then there is probably no point in having the set_auxcr/get_auxcr functions.

Alternatively, we could create common enter/exit coherency functions.

Rob

Quoted 291 lines
Quoted 291 lines
Cc Kukjin Kim <[email protected]>
Cc Russell King <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Viresh Kumar <[email protected]>
Cc Shiraz Hashim <[email protected]>
---
 arch/arm/mach-exynos/hotplug.c    |   57 ++++++-------------------------------
 arch/arm/mach-imx/hotplug.c       |   19 ++++---------
 arch/arm/mach-realview/hotplug.c  |   32 +++++----------------
 arch/arm/mach-spear13xx/hotplug.c |   32 +++++----------------
 arch/arm/mach-vexpress/hotplug.c  |   33 +++++----------------
 5 files changed, 35 insertions(+), 138 deletions(-)
arch/arm/mach-exynos/hotplug.c +9 -48
index c3f825b..5548fa3 100644
@@ -26,69 +26,30 @@
 
 static inline void cpu_enter_lowpower_a9(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 static inline void cpu_enter_lowpower_a15(void)
 {
-	unsigned int v;
-
-	asm volatile(
-	"	mrc	p15, 0, %0, c1, c0, 0\n"
-	"	bic	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 0\n"
-	  : "=&r" (v)
-	  : "Ir" (CR_C)
-	  : "cc");
-
+	set_cr(get_cr() & ~CR_C);
 	flush_cache_louis();
+	set_auxcr(get_auxcr() & ~0x40);
 
-	asm volatile(
-	/*
-	* Turn off coherency
-	*/
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %1\n"
-	"	mcr	p15, 0, %0, c1, c0, 1\n"
-	: "=&r" (v)
-	: "Ir" (0x40)
-	: "cc");
-
-	isb();
 	dsb();
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-imx/hotplug.c +5 -14
index 3dec962..1470c75 100644
@@ -18,24 +18,15 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-		"mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 /*
arch/arm/mach-realview/hotplug.c +7 -25
index 53818e5..c17c8c0 100644
@@ -18,39 +18,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * 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");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-spear13xx/hotplug.c +7 -25
index a7d2dd1..a38a087 100644
@@ -19,39 +19,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-	"	mcr	p15, 0, %1, c7, c5, 0\n"
-	"	dsb\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * 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", "memory");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void spear13xx_do_lowpower(unsigned int cpu, int *spurious)
arch/arm/mach-vexpress/hotplug.c +7 -26
index a141b98..74a9eb5 100644
@@ -18,40 +18,21 @@
 
 static inline void cpu_enter_lowpower(void)
 {
-	unsigned int v;
-
 	flush_cache_all();
-	asm volatile(
-		"mcr	p15, 0, %1, c7, c5, 0\n"
-	"	mcr	p15, 0, %1, c7, c10, 4\n"
+	__flush_icache_all();
+	dsb();
+
 	/*
 	 * Turn off coherency
 	 */
-	"	mrc	p15, 0, %0, c1, c0, 1\n"
-	"	bic	%0, %0, %3\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), "Ir" (0x40)
-	  : "cc");
+	set_auxcr(get_auxcr() & ~0x40);
+	set_cr(get_cr() & ~CR_C);
 }
 
 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");
+	set_cr(get_cr() | CR_C);
+	set_auxcr(get_auxcr() | 0x40);
 }
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH 2/3 ARM: convert platform hotplug inline assembly to C

Quoted 9 lines · Rob Herring
Quoted 8 lines · Nicolas Pitre
Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the addition of set_auxcr/get_auxcr, all the hotplug inline assembly code for exynos, imx, realview, spear13xx and vexpress can be converted to C code.

Quoted 8 lines
Quoted 2 lines

That might not be all safe. Please see
http://article.gmane.org/gmane.linux.ports.arm.kernel/209584

Other than the OR/AND operations, it's all just inline assembly functions that are called, so it gets compiled to the same code. Perhaps I should put noinline on the functions so they stay of limited complexity. If you don't think doing this in C is okay, then there is probably no point in having the set_auxcr/get_auxcr functions.

I think set_auxcr/get_auxcr is fine.

But your patch goes beyond simply converting those. You also converted the cache flush and disable from assembly to C, which on a Cortex A9 might be unsafe if the stack is modified in the sequence according to the discussion I referenced.

Nicolas

#5 lore raw

PATCH 2/3 ARM: convert platform hotplug inline assembly to C

Quoted 20 lines · Nicolas Pitre
Quoted 9 lines · Rob Herring
Quoted 8 lines · Nicolas Pitre
Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the addition of set_auxcr/get_auxcr, all the hotplug inline assembly code for exynos, imx, realview, spear13xx and vexpress can be converted to C code.

Quoted 8 lines
Quoted 2 lines

That might not be all safe. Please see
http://article.gmane.org/gmane.linux.ports.arm.kernel/209584

Other than the OR/AND operations, it's all just inline assembly functions that are called, so it gets compiled to the same code. Perhaps I should put noinline on the functions so they stay of limited complexity. If you don't think doing this in C is okay, then there is probably no point in having the set_auxcr/get_auxcr functions.

Quoted 6 lines

I think set_auxcr/get_auxcr is fine.

But your patch goes beyond simply converting those. You also converted the cache flush and disable from assembly to C, which on a Cortex A9 might be unsafe if the stack is modified in the sequence according to the discussion I referenced.

The referenced discussion is mainly for the A15, not the A9, right? It seems the sequences are a bit different. So this is the code for A9 (spear13xx):

	flush_cache_all();
	__flush_icache_all();
	dsb();

	/*
	 * Turn off coherency
	 */
	set_auxcr(get_auxcr() & ~0x40);
	set_cr(get_cr() & ~CR_C);

which generates this:

c027f36c:       ebf648d2        bl      c00116bc <v7_flush_kern_cache_all>
c027f370:       e3a03000        mov     r3, #0
c027f374:       ee073f11        mcr     15, 0, r3, cr7, cr1, {0}
c027f378:       f57ff04f        dsb     sy
c027f37c:       ee113f30        mrc     15, 0, r3, cr1, cr0, {1}
c027f380:       e3c33040        bic     r3, r3, #64     ; 0x40
c027f384:       ee013f30        mcr     15, 0, r3, cr1, cr0, {1}
c027f388:       f57ff06f        isb     sy
c027f38c:       ee113f10        mrc     15, 0, r3, cr1, cr0, {0}
c027f390:       e3c33004        bic     r3, r3, #4
c027f394:       ee013f10        mcr     15, 0, r3, cr1, cr0, {0}
c027f398:       f57ff06f        isb     sy
c027f39c:       e320f003        wfi

v7_flush_kern_cache_all will generate stack accesses, but I didn't change that fact. The I cache invalidate changed from invalidate all to invalidate inner-shareable. I believe that should be equivalent. And the mcr version of dsb changed to a dsb instruction. Some isb's are inserted as well.

So I don't follow your concern. You can't guarantee that the compiler wouldn't insert a data access in the middle, but then you could not guarantee that here either (exynos A15):

        asm volatile(
        "       mrc     p15, 0, %0, c1, c0, 0\n"
        "       bic     %0, %0, %1\n"
        "       mcr     p15, 0, %0, c1, c0, 0\n"
          : "=&r" (v)
          : "Ir" (CR_C)
          : "cc");

        flush_cache_louis();

        asm volatile(
        /*
        * Turn off coherency
        */
        "       mrc     p15, 0, %0, c1, c0, 1\n"
        "       bic     %0, %0, %1\n"
        "       mcr     p15, 0, %0, c1, c0, 1\n"
        : "=&r" (v)
        : "Ir" (0x40)
        : "cc");

The only thing that guarantees this code works is flush_cache_louis does not touch the stack and you rely that compiler doesn't do anything like register save/restore around the function call.

Rob

#6 lore raw

PATCH 2/3 ARM: convert platform hotplug inline assembly to C

Quoted 21 lines · Rob Herring
Quoted 20 lines · Nicolas Pitre
Quoted 17 lines · Rob Herring
Quoted 10 lines · Nicolas Pitre
Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

With the addition of set_auxcr/get_auxcr, all the hotplug inline assembly code for exynos, imx, realview, spear13xx and vexpress can be converted to C code.

That might not be all safe. Please see
http://article.gmane.org/gmane.linux.ports.arm.kernel/209584

Other than the OR/AND operations, it's all just inline assembly functions that are called, so it gets compiled to the same code. Perhaps I should put noinline on the functions so they stay of limited complexity. If you don't think doing this in C is okay, then there is probably no point in having the set_auxcr/get_auxcr functions.

Quoted 8 lines
Quoted 6 lines

I think set_auxcr/get_auxcr is fine.

But your patch goes beyond simply converting those. You also converted the cache flush and disable from assembly to C, which on a Cortex A9 might be unsafe if the stack is modified in the sequence according to the discussion I referenced.

The referenced discussion is mainly for the A15, not the A9, right?

Yes, however Santosh and Lorenzo were concerned by the fact that some people could look at the A15 code where accessing the stack is fine and copy it for some A9 where it apparently it is not. See that message I directed you to and the responses to it.

Quoted 29 lines

It seems the sequences are a bit different. So this is the code for A9 (spear13xx):

	flush_cache_all();
	__flush_icache_all();
	dsb();

	/*
	 * Turn off coherency
	 */
	set_auxcr(get_auxcr() & ~0x40);
	set_cr(get_cr() & ~CR_C);

which generates this:

c027f36c:       ebf648d2        bl      c00116bc <v7_flush_kern_cache_all>
c027f370:       e3a03000        mov     r3, #0
c027f374:       ee073f11        mcr     15, 0, r3, cr7, cr1, {0}
c027f378:       f57ff04f        dsb     sy
c027f37c:       ee113f30        mrc     15, 0, r3, cr1, cr0, {1}
c027f380:       e3c33040        bic     r3, r3, #64     ; 0x40
c027f384:       ee013f30        mcr     15, 0, r3, cr1, cr0, {1}
c027f388:       f57ff06f        isb     sy
c027f38c:       ee113f10        mrc     15, 0, r3, cr1, cr0, {0}
c027f390:       e3c33004        bic     r3, r3, #4
c027f394:       ee013f10        mcr     15, 0, r3, cr1, cr0, {0}
c027f398:       f57ff06f        isb     sy
c027f39c:       e320f003        wfi
Quoted 6 lines

v7_flush_kern_cache_all will generate stack accesses, but I didn't change that fact. The I cache invalidate changed from invalidate all to invalidate inner-shareable. I believe that should be equivalent. And the mcr version of dsb changed to a dsb instruction. Some isb's are inserted as well.

So I don't follow your concern.

_Their_ concern. I did not stop to think about the alleged implications myself yet. And that doesn't mean the existing code is correct either.

Quoted 4 lines

You can't guarantee that the compiler wouldn't
insert a data access in the middle, but then you could not guarantee that here either (exynos A15):

Quoted 9 lines
        asm volatile(
        "       mrc     p15, 0, %0, c1, c0, 0\n"
        "       bic     %0, %0, %1\n"
        "       mcr     p15, 0, %0, c1, c0, 0\n"
          : "=&r" (v)
          : "Ir" (CR_C)
          : "cc");

        flush_cache_louis();

Hence Lorenzo's argument that this should be done from assembly in a single stretch without memory access in the middle.

So, given the uncertainty around the correctness of the cache flush on a Cortex-A9, I'd suggest you steer clear of the debate and only convert accesses to the aux ctrl register.

Nicolas

#7 lore raw

PATCH 3/3 ARM: omap2: use get_auxcr for aux ctrl register read

In reply to Rob Herring

From: Rob Herring <[email protected]>

Use get_auxcr instead of inline assembly to read the CP15 aux ctrl register.

Cc Kevin Hilman <[email protected]>
Cc Tony Lindgren <[email protected]>
---
 arch/arm/mach-omap2/pm34xx.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/pm34xx.c +2 -2
index 7be3622..bef8ef6 100644
@@ -33,6 +33,7 @@
 
 #include <trace/events/power.h>
 
+#include <asm/cp15.h>
 #include <asm/fncpy.h>
 #include <asm/suspend.h>
 #include <asm/system_misc.h>
@@ -216,9 +217,8 @@ static void omap34xx_save_context(u32 *save)
 	u32 val;
 
 	/* Read Auxiliary Control Register */
-	asm("mrc p15, 0, %0, c1, c0, 1" : "=r" (val));
 	*save++ = 1;
-	*save++ = val;
+	*save++ = get_auxcr();
 
 	/* Read L2 AUX ctrl register */
 	asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
Signature · 1 line
1.7.10.4
#8 lore raw

PATCH 3/3 ARM: omap2: use get_auxcr for aux ctrl register read

Quoted 8 lines · Rob Herring

From: Rob Herring <[email protected]>

Use get_auxcr instead of inline assembly to read the CP15 aux ctrl register.

Cc Kevin Hilman <[email protected]>
Cc Tony Lindgren <[email protected]>

Looks trivial enough.

Quoted 30 lines
---
 arch/arm/mach-omap2/pm34xx.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/pm34xx.c +2 -2
index 7be3622..bef8ef6 100644
@@ -33,6 +33,7 @@
 
 #include <trace/events/power.h>
 
+#include <asm/cp15.h>
 #include <asm/fncpy.h>
 #include <asm/suspend.h>
 #include <asm/system_misc.h>
@@ -216,9 +217,8 @@ static void omap34xx_save_context(u32 *save)
 	u32 val;
 
 	/* Read Auxiliary Control Register */
-	asm("mrc p15, 0, %0, c1, c0, 1" : "=r" (val));
 	*save++ = 1;
-	*save++ = val;
+	*save++ = get_auxcr();
 
 	/* Read L2 AUX ctrl register */
 	asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (val));
Signature · 1 line
1.7.10.4
#9 lore raw

PATCH 3/3 ARM: omap2: use get_auxcr for aux ctrl register read

In reply to Rob Herring
Quoted 9 lines · Rob Herring

From: Rob Herring <[email protected]>

Use get_auxcr instead of inline assembly to read the CP15 aux ctrl register.

Cc Kevin Hilman <[email protected]>
Cc Tony Lindgren <[email protected]>

---

This patch and the rest of the series looks fine to me.

maintainer
#10 lore raw

PATCH 3/3 ARM: omap2: use get_auxcr for aux ctrl register read

Quoted 6 lines · Santosh Shilimkar
Quoted 5 lines · Rob Herring

From: Rob Herring <[email protected]>

Use get_auxcr instead of inline assembly to read the CP15 aux ctrl register.

Quoted 7 lines
Quoted 4 lines
Cc Kevin Hilman <[email protected]>
Cc Tony Lindgren <[email protected]>

---

This patch and the rest of the series looks fine to me.

The series looks good to me too:

#11 lore raw

PATCH 1/3 ARM: introduce common set_auxcr/get_auxcr functions

In reply to Rob Herring
Quoted 7 lines · Rob Herring

From: Rob Herring <[email protected]>

Move the private set_auxcr/get_auxcr functions from drivers/cpuidle/cpuidle-calxeda.c so they can be used across platforms.

Quoted 58 lines
---
 arch/arm/include/asm/cp15.h       |   14 ++++++++++++++
 drivers/cpuidle/cpuidle-calxeda.c |   14 --------------
 2 files changed, 14 insertions(+), 14 deletions(-)
arch/arm/include/asm/cp15.h +14 -0
index 5ef4d80..ef0094a 100644
@@ -59,6 +59,20 @@ static inline void set_cr(unsigned int val)
 	isb();
 }
 
+static inline unsigned int get_auxcr(void)
+{
+	unsigned int val;
+	asm("mrc p15, 0, %0, c1, c0, 1	@ get AUXCR" : "=r" (val) : : "cc");
+	return val;
+}
+
+static inline void set_auxcr(unsigned int val)
+{
+	asm volatile("mcr p15, 0, %0, c1, c0, 1	@ set AUXCR"
+	  : : "r" (val) : "cc");
+	isb();
+}
+
 #ifndef CONFIG_SMP
 extern void adjust_cr(unsigned long mask, unsigned long set);
 #endif
drivers/cpuidle/cpuidle-calxeda.c +0 -14
index e1aab38..ece83d6 100644
@@ -37,20 +37,6 @@ extern void *scu_base_addr;
 
 static struct cpuidle_device __percpu *calxeda_idle_cpuidle_devices;
 
-static inline unsigned int get_auxcr(void)
-{
-	unsigned int val;
-	asm("mrc p15, 0, %0, c1, c0, 1	@ get AUXCR" : "=r" (val) : : "cc");
-	return val;
-}
-
-static inline void set_auxcr(unsigned int val)
-{
-	asm volatile("mcr p15, 0, %0, c1, c0, 1	@ set AUXCR"
-	  : : "r" (val) : "cc");
-	isb();
-}
-
 static noinline void calxeda_idle_restore(void)
 {
 	set_cr(get_cr() | CR_C);
Signature · 1 line
1.7.10.4