* [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions
@ 2013-01-17 2:53 Rob Herring
2013-01-17 2:53 ` [PATCH 2/3] ARM: convert platform hotplug inline assembly to C Rob Herring
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Rob Herring @ 2013-01-17 2:53 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@calxeda.com>
Move the private set_auxcr/get_auxcr functions from
drivers/cpuidle/cpuidle-calxeda.c so they can be used across platforms.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Russell King <linux@arm.linux.org.uk>
---
arch/arm/include/asm/cp15.h | 14 ++++++++++++++
drivers/cpuidle/cpuidle-calxeda.c | 14 --------------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
index 5ef4d80..ef0094a 100644
--- a/arch/arm/include/asm/cp15.h
+++ b/arch/arm/include/asm/cp15.h
@@ -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
diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
index e1aab38..ece83d6 100644
--- a/drivers/cpuidle/cpuidle-calxeda.c
+++ b/drivers/cpuidle/cpuidle-calxeda.c
@@ -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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] ARM: convert platform hotplug inline assembly to C
2013-01-17 2:53 [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Rob Herring
@ 2013-01-17 2:53 ` Rob Herring
2013-01-17 4:40 ` Nicolas Pitre
2013-01-17 2:53 ` [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read Rob Herring
2013-01-17 4:34 ` [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Nicolas Pitre
2 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2013-01-17 2:53 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@calxeda.com>
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.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Shiraz Hashim <shiraz.hashim@st.com>
---
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(-)
diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index c3f825b..5548fa3 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -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)
diff --git a/arch/arm/mach-imx/hotplug.c b/arch/arm/mach-imx/hotplug.c
index 3dec962..1470c75 100644
--- a/arch/arm/mach-imx/hotplug.c
+++ b/arch/arm/mach-imx/hotplug.c
@@ -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);
}
/*
diff --git a/arch/arm/mach-realview/hotplug.c b/arch/arm/mach-realview/hotplug.c
index 53818e5..c17c8c0 100644
--- a/arch/arm/mach-realview/hotplug.c
+++ b/arch/arm/mach-realview/hotplug.c
@@ -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)
diff --git a/arch/arm/mach-spear13xx/hotplug.c b/arch/arm/mach-spear13xx/hotplug.c
index a7d2dd1..a38a087 100644
--- a/arch/arm/mach-spear13xx/hotplug.c
+++ b/arch/arm/mach-spear13xx/hotplug.c
@@ -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)
diff --git a/arch/arm/mach-vexpress/hotplug.c b/arch/arm/mach-vexpress/hotplug.c
index a141b98..74a9eb5 100644
--- a/arch/arm/mach-vexpress/hotplug.c
+++ b/arch/arm/mach-vexpress/hotplug.c
@@ -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)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read
2013-01-17 2:53 [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Rob Herring
2013-01-17 2:53 ` [PATCH 2/3] ARM: convert platform hotplug inline assembly to C Rob Herring
@ 2013-01-17 2:53 ` Rob Herring
2013-01-17 4:41 ` Nicolas Pitre
2013-01-17 8:30 ` Santosh Shilimkar
2013-01-17 4:34 ` [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Nicolas Pitre
2 siblings, 2 replies; 12+ messages in thread
From: Rob Herring @ 2013-01-17 2:53 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@calxeda.com>
Use get_auxcr instead of inline assembly to read the CP15 aux ctrl
register.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/pm34xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 7be3622..bef8ef6 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -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));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions
2013-01-17 2:53 [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Rob Herring
2013-01-17 2:53 ` [PATCH 2/3] ARM: convert platform hotplug inline assembly to C Rob Herring
2013-01-17 2:53 ` [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read Rob Herring
@ 2013-01-17 4:34 ` Nicolas Pitre
2 siblings, 0 replies; 12+ messages in thread
From: Nicolas Pitre @ 2013-01-17 4:34 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 16 Jan 2013, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Move the private set_auxcr/get_auxcr functions from
> drivers/cpuidle/cpuidle-calxeda.c so they can be used across platforms.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/include/asm/cp15.h | 14 ++++++++++++++
> drivers/cpuidle/cpuidle-calxeda.c | 14 --------------
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index 5ef4d80..ef0094a 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -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
> diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
> index e1aab38..ece83d6 100644
> --- a/drivers/cpuidle/cpuidle-calxeda.c
> +++ b/drivers/cpuidle/cpuidle-calxeda.c
> @@ -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);
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] ARM: convert platform hotplug inline assembly to C
2013-01-17 2:53 ` [PATCH 2/3] ARM: convert platform hotplug inline assembly to C Rob Herring
@ 2013-01-17 4:40 ` Nicolas Pitre
2013-01-17 14:18 ` Rob Herring
0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Pitre @ 2013-01-17 4:40 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 16 Jan 2013, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> 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
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Viresh Kumar <viresh.linux@gmail.com>
> Cc: Shiraz Hashim <shiraz.hashim@st.com>
> ---
> 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(-)
>
> diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> index c3f825b..5548fa3 100644
> --- a/arch/arm/mach-exynos/hotplug.c
> +++ b/arch/arm/mach-exynos/hotplug.c
> @@ -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)
> diff --git a/arch/arm/mach-imx/hotplug.c b/arch/arm/mach-imx/hotplug.c
> index 3dec962..1470c75 100644
> --- a/arch/arm/mach-imx/hotplug.c
> +++ b/arch/arm/mach-imx/hotplug.c
> @@ -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);
> }
>
> /*
> diff --git a/arch/arm/mach-realview/hotplug.c b/arch/arm/mach-realview/hotplug.c
> index 53818e5..c17c8c0 100644
> --- a/arch/arm/mach-realview/hotplug.c
> +++ b/arch/arm/mach-realview/hotplug.c
> @@ -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)
> diff --git a/arch/arm/mach-spear13xx/hotplug.c b/arch/arm/mach-spear13xx/hotplug.c
> index a7d2dd1..a38a087 100644
> --- a/arch/arm/mach-spear13xx/hotplug.c
> +++ b/arch/arm/mach-spear13xx/hotplug.c
> @@ -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)
> diff --git a/arch/arm/mach-vexpress/hotplug.c b/arch/arm/mach-vexpress/hotplug.c
> index a141b98..74a9eb5 100644
> --- a/arch/arm/mach-vexpress/hotplug.c
> +++ b/arch/arm/mach-vexpress/hotplug.c
> @@ -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)
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read
2013-01-17 2:53 ` [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read Rob Herring
@ 2013-01-17 4:41 ` Nicolas Pitre
2013-01-17 8:30 ` Santosh Shilimkar
1 sibling, 0 replies; 12+ messages in thread
From: Nicolas Pitre @ 2013-01-17 4:41 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 16 Jan 2013, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Use get_auxcr instead of inline assembly to read the CP15 aux ctrl
> register.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
Looks trivial enough.
Acked-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/mach-omap2/pm34xx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 7be3622..bef8ef6 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -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));
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read
2013-01-17 2:53 ` [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read Rob Herring
2013-01-17 4:41 ` Nicolas Pitre
@ 2013-01-17 8:30 ` Santosh Shilimkar
2013-01-17 17:07 ` Tony Lindgren
1 sibling, 1 reply; 12+ messages in thread
From: Santosh Shilimkar @ 2013-01-17 8:30 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 17 January 2013 08:23 AM, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Use get_auxcr instead of inline assembly to read the CP15 aux ctrl
> register.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> ---
This patch and the rest of the series looks fine to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] ARM: convert platform hotplug inline assembly to C
2013-01-17 4:40 ` Nicolas Pitre
@ 2013-01-17 14:18 ` Rob Herring
2013-01-17 15:42 ` Nicolas Pitre
0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2013-01-17 14:18 UTC (permalink / raw)
To: linux-arm-kernel
On 01/16/2013 10:40 PM, Nicolas Pitre wrote:
> On Wed, 16 Jan 2013, Rob Herring wrote:
>
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> 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
>
>
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Kukjin Kim <kgene.kim@samsung.com>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Cc: Sascha Hauer <kernel@pengutronix.de>
>> Cc: Viresh Kumar <viresh.linux@gmail.com>
>> Cc: Shiraz Hashim <shiraz.hashim@st.com>
>> ---
>> 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(-)
>>
>> diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
>> index c3f825b..5548fa3 100644
>> --- a/arch/arm/mach-exynos/hotplug.c
>> +++ b/arch/arm/mach-exynos/hotplug.c
>> @@ -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)
>> diff --git a/arch/arm/mach-imx/hotplug.c b/arch/arm/mach-imx/hotplug.c
>> index 3dec962..1470c75 100644
>> --- a/arch/arm/mach-imx/hotplug.c
>> +++ b/arch/arm/mach-imx/hotplug.c
>> @@ -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);
>> }
>>
>> /*
>> diff --git a/arch/arm/mach-realview/hotplug.c b/arch/arm/mach-realview/hotplug.c
>> index 53818e5..c17c8c0 100644
>> --- a/arch/arm/mach-realview/hotplug.c
>> +++ b/arch/arm/mach-realview/hotplug.c
>> @@ -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)
>> diff --git a/arch/arm/mach-spear13xx/hotplug.c b/arch/arm/mach-spear13xx/hotplug.c
>> index a7d2dd1..a38a087 100644
>> --- a/arch/arm/mach-spear13xx/hotplug.c
>> +++ b/arch/arm/mach-spear13xx/hotplug.c
>> @@ -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)
>> diff --git a/arch/arm/mach-vexpress/hotplug.c b/arch/arm/mach-vexpress/hotplug.c
>> index a141b98..74a9eb5 100644
>> --- a/arch/arm/mach-vexpress/hotplug.c
>> +++ b/arch/arm/mach-vexpress/hotplug.c
>> @@ -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)
>> --
>> 1.7.10.4
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] ARM: convert platform hotplug inline assembly to C
2013-01-17 14:18 ` Rob Herring
@ 2013-01-17 15:42 ` Nicolas Pitre
2013-01-18 3:34 ` Rob Herring
0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Pitre @ 2013-01-17 15:42 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 17 Jan 2013, Rob Herring wrote:
> On 01/16/2013 10:40 PM, Nicolas Pitre wrote:
> > On Wed, 16 Jan 2013, Rob Herring wrote:
> >
> >> From: Rob Herring <rob.herring@calxeda.com>
> >>
> >> 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.
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read
2013-01-17 8:30 ` Santosh Shilimkar
@ 2013-01-17 17:07 ` Tony Lindgren
0 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2013-01-17 17:07 UTC (permalink / raw)
To: linux-arm-kernel
* Santosh Shilimkar <santosh.shilimkar@ti.com> [130117 00:33]:
> On Thursday 17 January 2013 08:23 AM, Rob Herring wrote:
> >From: Rob Herring <rob.herring@calxeda.com>
> >
> >Use get_auxcr instead of inline assembly to read the CP15 aux ctrl
> >register.
> >
> >Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> >Cc: Kevin Hilman <khilman@ti.com>
> >Cc: Tony Lindgren <tony@atomide.com>
> >---
> This patch and the rest of the series looks fine to me.
>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
The series looks good to me too:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] ARM: convert platform hotplug inline assembly to C
2013-01-17 15:42 ` Nicolas Pitre
@ 2013-01-18 3:34 ` Rob Herring
2013-01-18 4:56 ` Nicolas Pitre
0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2013-01-18 3:34 UTC (permalink / raw)
To: linux-arm-kernel
On 01/17/2013 09:42 AM, Nicolas Pitre wrote:
> On Thu, 17 Jan 2013, Rob Herring wrote:
>
>> On 01/16/2013 10:40 PM, Nicolas Pitre wrote:
>>> On Wed, 16 Jan 2013, Rob Herring wrote:
>>>
>>>> From: Rob Herring <rob.herring@calxeda.com>
>>>>
>>>> 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.
>
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] ARM: convert platform hotplug inline assembly to C
2013-01-18 3:34 ` Rob Herring
@ 2013-01-18 4:56 ` Nicolas Pitre
0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Pitre @ 2013-01-18 4:56 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 17 Jan 2013, Rob Herring wrote:
> On 01/17/2013 09:42 AM, Nicolas Pitre wrote:
> > On Thu, 17 Jan 2013, Rob Herring wrote:
> >
> >> On 01/16/2013 10:40 PM, Nicolas Pitre wrote:
> >>> On Wed, 16 Jan 2013, Rob Herring wrote:
> >>>
> >>>> From: Rob Herring <rob.herring@calxeda.com>
> >>>>
> >>>> 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.
> >
> > 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.
> 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.
_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.
> 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();
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-01-18 4:56 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-17 2:53 [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Rob Herring
2013-01-17 2:53 ` [PATCH 2/3] ARM: convert platform hotplug inline assembly to C Rob Herring
2013-01-17 4:40 ` Nicolas Pitre
2013-01-17 14:18 ` Rob Herring
2013-01-17 15:42 ` Nicolas Pitre
2013-01-18 3:34 ` Rob Herring
2013-01-18 4:56 ` Nicolas Pitre
2013-01-17 2:53 ` [PATCH 3/3] ARM: omap2: use get_auxcr for aux ctrl register read Rob Herring
2013-01-17 4:41 ` Nicolas Pitre
2013-01-17 8:30 ` Santosh Shilimkar
2013-01-17 17:07 ` Tony Lindgren
2013-01-17 4:34 ` [PATCH 1/3] ARM: introduce common set_auxcr/get_auxcr functions Nicolas Pitre
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).