* ARM: CPU hotplug: fix hard-coded control register constants [not found] <4D303758.4060608@st.com> @ 2011-01-14 11:50 ` viresh kumar 2011-01-14 12:04 ` Russell King - ARM Linux 0 siblings, 1 reply; 5+ messages in thread From: viresh kumar @ 2011-01-14 11:50 UTC (permalink / raw) To: linux-arm-kernel Russell, I was just tracking latest changes in realview, for preparing SPEAr support V4. I found below patch and didn't understood it very well. > Subject: [PATCH] ARM: CPU hotplug: fix hard-coded control register constants > > Use the definition we've provided in asm/system.h rather than > numeric constants. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > arch/arm/mach-realview/hotplug.c | 8 ++++---- > arch/arm/mach-s5pv310/hotplug.c | 8 ++++---- > arch/arm/mach-tegra/hotplug.c | 8 ++++---- > 3 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/mach-realview/hotplug.c b/arch/arm/mach-realview/hotplug.c > index b6387cf..a87523d 100644 > --- a/arch/arm/mach-realview/hotplug.c > +++ b/arch/arm/mach-realview/hotplug.c > @@ -31,10 +31,10 @@ static inline void cpu_enter_lowpower(void) > " bic %0, %0, #0x20\n" > " mcr p15, 0, %0, c1, c0, 1\n" > " mrc p15, 0, %0, c1, c0, 0\n" > - " bic %0, %0, #0x04\n" > + " bic %0, %0, %2\n" > " mcr p15, 0, %0, c1, c0, 0\n" > : "=&r" (v) > - : "r" (0) > + : "r" (0), "Ir" (CR_C) > : "cc"); > } This looks fine as you have replaced 0x04 with CR_C, which itself is (1 << 2). > > @@ -43,13 +43,13 @@ static inline void cpu_leave_lowpower(void) > unsigned int v; > > asm volatile( "mrc p15, 0, %0, c1, c0, 0\n" > - " orr %0, %0, #0x04\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"); > } fine here also. > > diff --git a/arch/arm/mach-s5pv310/hotplug.c b/arch/arm/mach-s5pv310/hotplug.c > index 951ba6d..afa5392 100644 > --- a/arch/arm/mach-s5pv310/hotplug.c > +++ b/arch/arm/mach-s5pv310/hotplug.c > @@ -30,13 +30,13 @@ static inline void cpu_enter_lowpower(void) > * Turn off coherency > */ > " mrc p15, 0, %0, c1, c0, 1\n" > - " bic %0, %0, #0x20\n" > + " bic %0, %0, %2\n" but why replace 0x20 with CR_C instead of CR_D > " mcr p15, 0, %0, c1, c0, 1\n" > " mrc p15, 0, %0, c1, c0, 0\n" > " bic %0, %0, #0x04\n" > " mcr p15, 0, %0, c1, c0, 0\n" > : "=&r" (v) > - : "r" (0) > + : "r" (0), "Ir" (CR_C) > : "cc"); > } > > @@ -46,13 +46,13 @@ static inline void cpu_leave_lowpower(void) > > asm volatile( > "mrc p15, 0, %0, c1, c0, 0\n" > - " orr %0, %0, #0x04\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"); > } > > diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c > index 17faf77..a5cb1ce 100644 > --- a/arch/arm/mach-tegra/hotplug.c > +++ b/arch/arm/mach-tegra/hotplug.c > @@ -26,13 +26,13 @@ static inline void cpu_enter_lowpower(void) > * Turn off coherency > */ > " mrc p15, 0, %0, c1, c0, 1\n" > - " bic %0, %0, #0x20\n" > + " bic %0, %0, %2\n" here also. Was this code wrong earlier?? -- viresh ^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: CPU hotplug: fix hard-coded control register constants 2011-01-14 11:50 ` ARM: CPU hotplug: fix hard-coded control register constants viresh kumar @ 2011-01-14 12:04 ` Russell King - ARM Linux 2011-01-14 12:08 ` Russell King - ARM Linux 2011-01-14 18:40 ` Stephen Warren 0 siblings, 2 replies; 5+ messages in thread From: Russell King - ARM Linux @ 2011-01-14 12:04 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jan 14, 2011 at 05:20:03PM +0530, viresh kumar wrote: > > diff --git a/arch/arm/mach-s5pv310/hotplug.c b/arch/arm/mach-s5pv310/hotplug.c > > index 951ba6d..afa5392 100644 > > --- a/arch/arm/mach-s5pv310/hotplug.c > > +++ b/arch/arm/mach-s5pv310/hotplug.c > > @@ -30,13 +30,13 @@ static inline void cpu_enter_lowpower(void) > > * Turn off coherency > > */ > > " mrc p15, 0, %0, c1, c0, 1\n" > > - " bic %0, %0, #0x20\n" > > + " bic %0, %0, %2\n" > > but why replace 0x20 with CR_C instead of CR_D Looks like a mistake - it should've been the one below. > > " mcr p15, 0, %0, c1, c0, 1\n" > > " mrc p15, 0, %0, c1, c0, 0\n" > > " bic %0, %0, #0x04\n" > > " mcr p15, 0, %0, c1, c0, 0\n" > > : "=&r" (v) > > - : "r" (0) > > + : "r" (0), "Ir" (CR_C) > > : "cc"); In any case, this code is probably doing the wrong thing - if s5pv310 is not an ARMv6 MPCore the its fiddling with the auxillary control register is already broken. > > diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c > > index 17faf77..a5cb1ce 100644 > > --- a/arch/arm/mach-tegra/hotplug.c > > +++ b/arch/arm/mach-tegra/hotplug.c > > @@ -26,13 +26,13 @@ static inline void cpu_enter_lowpower(void) > > * Turn off coherency > > */ > > " mrc p15, 0, %0, c1, c0, 1\n" > > - " bic %0, %0, #0x20\n" > > + " bic %0, %0, %2\n" > > here also. Same comment. ^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: CPU hotplug: fix hard-coded control register constants 2011-01-14 12:04 ` Russell King - ARM Linux @ 2011-01-14 12:08 ` Russell King - ARM Linux 2011-01-15 14:35 ` viresh kumar 2011-01-14 18:40 ` Stephen Warren 1 sibling, 1 reply; 5+ messages in thread From: Russell King - ARM Linux @ 2011-01-14 12:08 UTC (permalink / raw) To: linux-arm-kernel Here's the fix which restores the old (but probably still broken) behaviour: Subject: [PATCH] ARM: fix wrongly patched constants e3d9c625 (ARM: CPU hotplug: fix hard-coded control register constants) changed the wrong constants in the hotplug assembly code. Fix this. Reported-by: viresh kumar <viresh.kumar@st.com> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/mach-s5pv310/hotplug.c | 4 ++-- arch/arm/mach-tegra/hotplug.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-s5pv310/hotplug.c b/arch/arm/mach-s5pv310/hotplug.c index afa5392..c24235c 100644 --- a/arch/arm/mach-s5pv310/hotplug.c +++ b/arch/arm/mach-s5pv310/hotplug.c @@ -30,10 +30,10 @@ static inline void cpu_enter_lowpower(void) * Turn off coherency */ " mrc p15, 0, %0, c1, c0, 1\n" - " bic %0, %0, %2\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, #0x04\n" + " bic %0, %0, %2\n" " mcr p15, 0, %0, c1, c0, 0\n" : "=&r" (v) : "r" (0), "Ir" (CR_C) diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c index a5cb1ce..f329404 100644 --- a/arch/arm/mach-tegra/hotplug.c +++ b/arch/arm/mach-tegra/hotplug.c @@ -26,10 +26,10 @@ static inline void cpu_enter_lowpower(void) * Turn off coherency */ " mrc p15, 0, %0, c1, c0, 1\n" - " bic %0, %0, %2\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, #0x04\n" + " bic %0, %0, %2\n" " mcr p15, 0, %0, c1, c0, 0\n" : "=&r" (v) : "r" (0), "Ir" (CR_C) -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* ARM: CPU hotplug: fix hard-coded control register constants 2011-01-14 12:08 ` Russell King - ARM Linux @ 2011-01-15 14:35 ` viresh kumar 0 siblings, 0 replies; 5+ messages in thread From: viresh kumar @ 2011-01-15 14:35 UTC (permalink / raw) To: linux-arm-kernel On 1/14/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > Subject: [PATCH] ARM: fix wrongly patched constants > > e3d9c625 (ARM: CPU hotplug: fix hard-coded control register constants) > changed the wrong constants in the hotplug assembly code. Fix this. > > Reported-by: viresh kumar <viresh.kumar@st.com> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > arch/arm/mach-s5pv310/hotplug.c | 4 ++-- > arch/arm/mach-tegra/hotplug.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-s5pv310/hotplug.c > b/arch/arm/mach-s5pv310/hotplug.c > index afa5392..c24235c 100644 > --- a/arch/arm/mach-s5pv310/hotplug.c > +++ b/arch/arm/mach-s5pv310/hotplug.c > @@ -30,10 +30,10 @@ static inline void cpu_enter_lowpower(void) > * Turn off coherency > */ > " mrc p15, 0, %0, c1, c0, 1\n" > - " bic %0, %0, %2\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, #0x04\n" > + " bic %0, %0, %2\n" > " mcr p15, 0, %0, c1, c0, 0\n" > : "=&r" (v) > : "r" (0), "Ir" (CR_C) > diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c > index a5cb1ce..f329404 100644 > --- a/arch/arm/mach-tegra/hotplug.c > +++ b/arch/arm/mach-tegra/hotplug.c > @@ -26,10 +26,10 @@ static inline void cpu_enter_lowpower(void) > * Turn off coherency > */ > " mrc p15, 0, %0, c1, c0, 1\n" > - " bic %0, %0, %2\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, #0x04\n" > + " bic %0, %0, %2\n" > " mcr p15, 0, %0, c1, c0, 0\n" > : "=&r" (v) > : "r" (0), "Ir" (CR_C) Acked-by: Viresh Kumar <viresh.kumar@st.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* ARM: CPU hotplug: fix hard-coded control register constants 2011-01-14 12:04 ` Russell King - ARM Linux 2011-01-14 12:08 ` Russell King - ARM Linux @ 2011-01-14 18:40 ` Stephen Warren 1 sibling, 0 replies; 5+ messages in thread From: Stephen Warren @ 2011-01-14 18:40 UTC (permalink / raw) To: linux-arm-kernel Russell King wrote: > Sent: Friday, January 14, 2011 5:05 AM > > On Fri, Jan 14, 2011 at 05:20:03PM +0530, viresh kumar wrote: > ... > In any case, this code is probably doing the wrong thing - if s5pv310 > is not an ARMv6 MPCore the its fiddling with the auxillary control > register is already broken. > > > > diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c > > > index 17faf77..a5cb1ce 100644 > > > --- a/arch/arm/mach-tegra/hotplug.c > > > +++ b/arch/arm/mach-tegra/hotplug.c > > > @@ -26,13 +26,13 @@ static inline void cpu_enter_lowpower(void) > > > * Turn off coherency > > > */ > > > " mrc p15, 0, %0, c1, c0, 1\n" > > > - " bic %0, %0, #0x20\n" > > > + " bic %0, %0, %2\n" > > > > here also. > > Same comment. Just as an FYI, in various downstream trees, that file (for mach-tegra) has been removed and replaced with completely different code. However, the replacement isn't in the process of being upstreamed right yet. Specifically: git://android.git.kernel.org/kernel/tegra.git linux-tegra-2.6.37 cb8e70bd4fa41ea7ac66f01f7425943db99ac7d3 ARM: tegra: Add suspend and hotplug support Hope this helps. -- nvpublic ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-01-15 14:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4D303758.4060608@st.com>
2011-01-14 11:50 ` ARM: CPU hotplug: fix hard-coded control register constants viresh kumar
2011-01-14 12:04 ` Russell King - ARM Linux
2011-01-14 12:08 ` Russell King - ARM Linux
2011-01-15 14:35 ` viresh kumar
2011-01-14 18:40 ` Stephen Warren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox