Skip to contentSkip to conversation

ARM: iwmmxt/pj4 fixes for v7/Thumb-2

RFC PATCH Superseded Dave Martin 7 participants 19 messages Sep 7, 2011 – Sep 8, 2011 Arm (32-bit)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover ARM: iwmmxt/pj4 fixes for v7/Thumb-2

The iwmmxt and pj4 support code contains some code which isn't compatible with Thumb-2 and looks possibly inappropriate for v7.

There is also an issue with Makefile rules causing iwmmxt.S to be built for the wrong architecture.

This series attempts to fix those issues. However, I'm not totally happy with it -- if someone with relevant boards and/or experience can test or comment, that would be appreciated.

In particular, the current makefile rules are still not really right. To avoid excessive pain and disruption, simpler but somewhat incorrect changes are made in the Makefile, and iwmmxt.S is changed for Thumb-2 kernels only. This may be adequete, but I don't kave the required knowledge to be sure.

If someone with some platform knowledge can comment on whether it appears valid to replace the old instruction sequences with ISBs, that would be appreciated too. In this series I'm currently making assumptions based on the core architecture.

Dave Martin (3):
ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2
ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2
ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

 arch/arm/kernel/Makefile  |    7 +++++++
 arch/arm/kernel/iwmmxt.S  |    9 +++++++++
 arch/arm/kernel/pj4-cp0.c |    4 ++++
 3 files changed, 20 insertions(+), 0 deletions(-)

-- 1.7.4.1

#1 lore raw

RFC PATCH 1/3 ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2

Because gcc/gas have no sane way to turn on individual CPU extensions from the command-line, iwmmxt.S was previously built with -mcpu=iwmmxt. Unfortunately, this also downgrades the CPU to v5, with the result that this file fails to build for a Thumb-2 kernel.

New versions of the tools support -march=<base arch>+iwmmxt, and it seems reasonable to require up-to-date tools when building in Thumb-2. So, this patch uses -march=armv7-a+iwmmxt for CONFIG_THUMB2_KERNEL=y.

---
 arch/arm/kernel/Makefile |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
arch/arm/kernel/Makefile +7 -0
index f7887dc..8a58339 100644
@@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4)		+= pj4-cp0.o
 obj-$(CONFIG_IWMMXT)		+= iwmmxt.o
 obj-$(CONFIG_CPU_HAS_PMU)	+= pmu.o
 obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event.o
+
+# When enough people have binutils which support -march=...+iwmmxt, this
+# should change to something like if __LINUX_ARM_ARCH__ < 7.
+ifdef CONFIG_THUMB2_KERNEL
+AFLAGS_iwmmxt.o			:= -Wa,-march=armv7-a+iwmmxt
+else
 AFLAGS_iwmmxt.o			:= -Wa,-mcpu=iwmmxt
+endif
 
 ifneq ($(CONFIG_ARCH_EBSA110),y)
   obj-y		+= io.o
Signature · 1 line
1.7.4.1
#2 lore raw

RFC PATCH 1/3 ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2

Quoted 30 lines · Dave Martin

Because gcc/gas have no sane way to turn on individual CPU extensions from the command-line, iwmmxt.S was previously built with -mcpu=iwmmxt. ?Unfortunately, this also downgrades the CPU to v5, with the result that this file fails to build for a Thumb-2 kernel.

New versions of the tools support -march=<base arch>+iwmmxt, and it seems reasonable to require up-to-date tools when building in Thumb-2. ?So, this patch uses -march=armv7-a+iwmmxt for CONFIG_THUMB2_KERNEL=y.

---
?arch/arm/kernel/Makefile | ? ?7 +++++++
?1 files changed, 7 insertions(+), 0 deletions(-)

arch/arm/kernel/Makefile +0 -0
index f7887dc..8a58339 100644
@@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4) ? ? ? ? ? ? ? ?+= pj4-cp0.o

?obj-$(CONFIG_IWMMXT) ? ? ? ? ? += iwmmxt.o
?obj-$(CONFIG_CPU_HAS_PMU) ? ? ?+= pmu.o
?obj-$(CONFIG_HW_PERF_EVENTS) ? += perf_event.o

arch/arm/kernel/Makefile +6 -0
+
+# When enough people have binutils which support -march=...+iwmmxt, this
+# should change to something like if __LINUX_ARM_ARCH__ < 7.
+ifdef CONFIG_THUMB2_KERNEL
+AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-march=armv7-a+iwmmxt
+else
Quoted 2 lines

?AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-mcpu=iwmmxt +endif

It looks more like the switch should depend on the compiler version. Unless there is a clear way to decide if gcc supports this switch, I think it's reasonable to have the change like above.

Quoted 7 lines

?ifneq ($(CONFIG_ARCH_EBSA110),y)
? obj-y ? ? ? ? ? ? ? ?+= io.o
--
1.7.4.1

#3 lore raw

RFC PATCH 1/3 ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2

Quoted 29 lines · Eric Miao
Quoted 28 lines · Dave Martin

Because gcc/gas have no sane way to turn on individual CPU extensions from the command-line, iwmmxt.S was previously built with -mcpu=iwmmxt. ?Unfortunately, this also downgrades the CPU to v5, with the result that this file fails to build for a Thumb-2 kernel.

New versions of the tools support -march=<base arch>+iwmmxt, and it seems reasonable to require up-to-date tools when building in Thumb-2. ?So, this patch uses -march=armv7-a+iwmmxt for CONFIG_THUMB2_KERNEL=y.

---
?arch/arm/kernel/Makefile | ? ?7 +++++++
?1 files changed, 7 insertions(+), 0 deletions(-)

arch/arm/kernel/Makefile +0 -0
index f7887dc..8a58339 100644
@@ -65,7 +65,14 @@ obj-$(CONFIG_CPU_PJ4) ? ? ? ? ? ? ? ?+= pj4-cp0.o

?obj-$(CONFIG_IWMMXT) ? ? ? ? ? += iwmmxt.o
?obj-$(CONFIG_CPU_HAS_PMU) ? ? ?+= pmu.o
?obj-$(CONFIG_HW_PERF_EVENTS) ? += perf_event.o

arch/arm/kernel/Makefile +4 -0
+
+# When enough people have binutils which support -march=...+iwmmxt, this
+# should change to something like if __LINUX_ARM_ARCH__ < 7.
+ifdef CONFIG_THUMB2_KERNEL
Quoted 8 lines
Quoted 4 lines
arch/arm/kernel/Makefile +2 -0
+AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-march=armv7-a+iwmmxt
+else

?AFLAGS_iwmmxt.o ? ? ? ? ? ? ? ? ? ? ? ?:= -Wa,-mcpu=iwmmxt +endif

It looks more like the switch should depend on the compiler version. Unless there is a clear way to decide if gcc supports this switch, I think it's reasonable to have the change like above.

Normally the way to go with gcc version dependent alternatives is to use something like:

AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>)

This will test if <the_new_flag> is supported by the used gcc, and use the fallback otherwise.

Nicolas

#4 lore raw

RFC PATCH 1/3 ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2

Quoted 13 lines · On Wednesday 07 September 2011 13:18:21 Nicolas Pitre wrote:
Quoted 4 lines
Quoted 4 lines
arch/arm/kernel/Makefile +4 -0
+
+# When enough people have binutils which support -march=...+iwmmxt, this
+# should change to something like if __LINUX_ARM_ARCH__ < 7.
+ifdef CONFIG_THUMB2_KERNEL
Quoted 8 lines

arch/arm/kernel/Makefile obj-$(CONFIG_CPU_PJ4) += pj4-cp0.o · line 75

Quoted 4 lines
arch/arm/kernel/Makefile +3 -0
+AFLAGS_iwmmxt.o                        := -Wa,-march=armv7-a+iwmmxt
+else
 AFLAGS_iwmmxt.o                        := -Wa,-mcpu=iwmmxt
+endif

It looks more like the switch should depend on the compiler version. Unless there is a clear way to decide if gcc supports this switch, I think it's reasonable to have the change like above.

Quoted 7 lines

Normally the way to go with gcc version dependent alternatives is to use something like:

AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>)

This will test if <the_new_flag> is supported by the used gcc, and use the fallback otherwise.

Yes, that's possible here, but it's not actually correct either, because the CPU core that we are running on is either a v5 XScale with iwmmxt or a v7 pj4 with iwmmxt. Now, it should not really matter if we build the code with flags for a different more complex instruction set, but it can potentially hide bugs.

I think the simple solution that Dave posted is actually more appropriate. The three possible cases are:

v5+iwmmxt: always use -Wa,-mcpu=iwmmxt as we've always done, and it's correct v7+iwmmxt+arm: still use -Wa,-mcpu=iwmmxt, not correct but close enough and
is known to build the file with all existing toolchaings
v7+iwmmxt+thumb2: always use -Wa,-march=armv7-a+iwmmxt, which is correct and
the only possible way to build this file anyway. Old toolchains will fail and there is nothing we can do about it.

Arnd

#5 lore raw

RFC PATCH 1/3 ARM: iwmmxt: Fix Makefile rules for building iwmmxt for Thumb-2

Quoted 31 lines · Arnd Bergmann
Quoted 13 lines · On Wednesday 07 September 2011 13:18:21 Nicolas Pitre wrote:
Quoted 4 lines
Quoted 4 lines
arch/arm/kernel/Makefile +4 -0
+
+# When enough people have binutils which support -march=...+iwmmxt, this
+# should change to something like if __LINUX_ARM_ARCH__ < 7.
+ifdef CONFIG_THUMB2_KERNEL
Quoted 8 lines

arch/arm/kernel/Makefile obj-$(CONFIG_CPU_PJ4) += pj4-cp0.o · line 75

Quoted 4 lines
arch/arm/kernel/Makefile +3 -0
+AFLAGS_iwmmxt.o                        := -Wa,-march=armv7-a+iwmmxt
+else
 AFLAGS_iwmmxt.o                        := -Wa,-mcpu=iwmmxt
+endif

It looks more like the switch should depend on the compiler version. Unless there is a clear way to decide if gcc supports this switch, I think it's reasonable to have the change like above.

Quoted 7 lines

Normally the way to go with gcc version dependent alternatives is to use something like:

AFLAGS_foo.o := $(call cc-option,<the_new_flag>,<the_fallback_flag>)

This will test if <the_new_flag> is supported by the used gcc, and use the fallback otherwise.

Yes, that's possible here, but it's not actually correct either, because the CPU core that we are running on is either a v5 XScale with iwmmxt or a v7 pj4 with iwmmxt. Now, it should not really matter if we build the code with flags for a different more complex instruction set, but it can potentially hide bugs.

I think the simple solution that Dave posted is actually more appropriate. The three possible cases are:

Quoted 6 lines

v5+iwmmxt: always use -Wa,-mcpu=iwmmxt as we've always done, and it's correct v7+iwmmxt+arm: still use -Wa,-mcpu=iwmmxt, not correct but close enough and
is known to build the file with all existing toolchaings
v7+iwmmxt+thumb2: always use -Wa,-march=armv7-a+iwmmxt, which is correct and
the only possible way to build this file anyway. Old toolchains will fail and there is nothing we can do about it.

There is another option, which is to use cc-option and then check the result in AFLAGS_iwmmxt.o, throwing an error from the Makefile as necessary.

I'll have a go at that if nobody has any objections.

There doesn't seem to be any cleaner way to catch this error -- compiler version issues are invisible to Kconfig.

Cheers
---Dave

#6 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

In reply to Dave Martin

The iwmmxt code contains some code to implement a pseudo-ISB, but this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one for Thumb-2 kernels.

---
 arch/arm/kernel/iwmmxt.S |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
arch/arm/kernel/iwmmxt.S +9 -0
index a087838..bee8a74 100644
@@ -319,8 +319,17 @@ ENTRY(iwmmxt_task_switch)
 	PJ4(eor r1, r1, #0xf)
 	PJ4(mcr	p15, 0, r1, c1, c0, 2)
 
+/*
+ * When enough people have binutils which support -march=...+iwmmxt, this
+ * should change to #if __LINUX_ARM_ARCH__ < 7.
+ */
+#ifndef CONFIG_THUMB2_KERNEL
 	mrc	p15, 0, r1, c2, c0, 0
 	sub	pc, lr, r1, lsr #32		@ cpwait and return
+#else
+	isb					@ ISB needed instead on ARMv7
+	mov	pc, lr
+#endif
 
 /*
  * Remove Concan ownership of given task
Signature · 1 line
1.7.4.1
#7 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

Quoted 26 lines · Dave Martin

The iwmmxt code contains some code to implement a pseudo-ISB, but this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one for Thumb-2 kernels.

---
?arch/arm/kernel/iwmmxt.S | ? ?9 +++++++++
?1 files changed, 9 insertions(+), 0 deletions(-)

arch/arm/kernel/iwmmxt.S +5 -0
index a087838..bee8a74 100644
@@ -319,8 +319,17 @@ ENTRY(iwmmxt_task_switch)
 ? ? ? ?PJ4(eor r1, r1, #0xf)
 ? ? ? ?PJ4(mcr p15, 0, r1, c1, c0, 2)
 
+/*
+ * When enough people have binutils which support -march=...+iwmmxt, this
+ * should change to #if __LINUX_ARM_ARCH__ < 7.
+ */
+#ifndef CONFIG_THUMB2_KERNEL

? ? ? ?mrc ? ? p15, 0, r1, c2, c0, 0
? ? ? ?sub ? ? pc, lr, r1, lsr #32 ? ? ? ? ? ? @ cpwait and return

arch/arm/kernel/iwmmxt.S ENTRY(iwmmxt_task_switch) · line 332

Quoted 4 lines
arch/arm/kernel/iwmmxt.S +4 -0
+#else
+ ? ? ? isb ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? @ ISB needed instead on ARMv7
+ ? ? ? mov ? ? pc, lr
+#endif

Or maybe using ARM() and THUMB() will be a better fit here?

#8 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

Quoted 14 lines · On Wednesday 07 September 2011 09:13:38 Eric Miao wrote:

arch/arm/kernel/iwmmxt.S ENTRY(iwmmxt_task_switch) · line 332

Quoted 12 lines
arch/arm/kernel/iwmmxt.S +9 -0
+/*
+ * When enough people have binutils which support -march=...+iwmmxt, this
+ * should change to #if __LINUX_ARM_ARCH__ < 7.
+ */
+#ifndef CONFIG_THUMB2_KERNEL
       mrc     p15, 0, r1, c2, c0, 0
       sub     pc, lr, r1, lsr #32             @ cpwait and return
+#else
+       isb                                     @ ISB needed instead on ARMv7
+       mov     pc, lr
+#endif

Or maybe using ARM() and THUMB() will be a better fit here?

I think it would be logical to use XSC() and PJ4() instead, as those are used elsehere in this file. However, that doesn't work with the only binutils.

If the !THUMB code above is actually correct for v7 in ARM mode (i.e. it performs the equivalent of an ISB), then I think we should leave it.

It does look like it's whitespace broken though.

Arnd

#9 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

Quoted 16 lines · Arnd Bergmann
Quoted 14 lines · On Wednesday 07 September 2011 09:13:38 Eric Miao wrote:

arch/arm/kernel/iwmmxt.S ENTRY(iwmmxt_task_switch) · line 332

Quoted 12 lines
arch/arm/kernel/iwmmxt.S +9 -0
+/*
+ * When enough people have binutils which support -march=...+iwmmxt, this
+ * should change to #if __LINUX_ARM_ARCH__ < 7.
+ */
+#ifndef CONFIG_THUMB2_KERNEL
       mrc     p15, 0, r1, c2, c0, 0
       sub     pc, lr, r1, lsr #32             @ cpwait and return
+#else
+       isb                                     @ ISB needed instead on ARMv7
+       mov     pc, lr
+#endif

Or maybe using ARM() and THUMB() will be a better fit here?

Quoted 2 lines

I think it would be logical to use XSC() and PJ4() instead, as those are used elsehere in this file. However, that doesn't work with the only binutils.

I take pj4 and older platforms can never be built in the same kernel? Otherwise, this approach is totally broken anyway-- we'll generate code which won't run on some of the kernel's supported platforms.

Quoted 2 lines

If the !THUMB code above is actually correct for v7 in ARM mode (i.e. it performs the equivalent of an ISB), then I think we should leave it.

With the proposed change to use cc-option in the Makefile, we have the option of using the v7 code whenever binutils supports it, and printing a warning otherwise.

Quoted 1 line

It does look like it's whitespace broken though.

I'll double-check that.

Cheers
---Dave

#10 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

Quoted 3 lines · Dave Martin

I take pj4 and older platforms can never be built in the same kernel? Otherwise, this approach is totally broken anyway-- we'll generate code which won't run on some of the kernel's supported platforms.

We have a well enforced split between ARMv5 and older CPUs, and ARMv6 and later CPUs.

#11 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Dave Martin

I take pj4 and older platforms can never be built in the same kernel? Otherwise, this approach is totally broken anyway-- we'll generate code which won't run on some of the kernel's supported platforms.

We have a well enforced split between ARMv5 and older CPUs, and ARMv6 and later CPUs.

Good point -- OK, I guess we don't need to worry about that scenario.

Cheers
---Dave

#12 lore raw

RFC PATCH 2/3 ARM: iwmmxt: Port problematic iwmmxt support code to v7/Thumb-2

Quoted 10 lines · Dave Martin
Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Dave Martin

I take pj4 and older platforms can never be built in the same kernel? Otherwise, this approach is totally broken anyway-- we'll generate code which won't run on some of the kernel's supported platforms.

We have a well enforced split between ARMv5 and older CPUs, and ARMv6 and later CPUs.

Good point -- OK, I guess we don't need to worry about that scenario.

I actually have a patch that enforces this rule in Kconfig for pxa. Right now, you can happily select any combination of boards, which blows up very early during the build phase if you combine v5 and v7.

Arnd

#13 lore raw

RFC PATCH 3/3 ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

In reply to Dave Martin

The iwmmxt code contains some code to implement a pseudo-ISB, but this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one on v7 and above.

---
 arch/arm/kernel/pj4-cp0.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/kernel/pj4-cp0.c +4 -0
index a4b1b07..5117d9d 100644
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 
 	__asm__ __volatile__ (
 		"mcr	p15, 0, %1, c1, c0, 2\n\t"
+#ifdef __LINUX_ARM_ARCH__ >= 7
+		"isb\n\t"
+#else
 		"mrc	p15, 0, %0, c1, c0, 2\n\t"
 		"mov	%0, %0\n\t"
 		"sub	pc, pc, #4\n\t"
+#endif
 		: "=r" (temp) : "r" (value));
 }
 
Signature · 1 line
1.7.4.1
#14 lore raw

RFC PATCH 3/3 ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

Quoted 21 lines · Dave Martin

The iwmmxt code contains some code to implement a pseudo-ISB, but this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one on v7 and above.

---
?arch/arm/kernel/pj4-cp0.c | ? ?4 ++++
?1 files changed, 4 insertions(+), 0 deletions(-)

arch/arm/kernel/pj4-cp0.c +0 -0
index a4b1b07..5117d9d 100644
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 

? ? ? ?__asm__ __volatile__ (
? ? ? ? ? ? ? ?"mcr ? ?p15, 0, %1, c1, c0, 2\n\t"

arch/arm/kernel/pj4-cp0.c +3 -0
+#ifdef __LINUX_ARM_ARCH__ >= 7
+ ? ? ? ? ? ? ? "isb\n\t"
+#else
Quoted 4 lines

? ? ? ? ? ? ? ?"mrc ? ?p15, 0, %0, c1, c0, 2\n\t"
? ? ? ? ? ? ? ?"mov ? ?%0, %0\n\t"
? ? ? ? ? ? ? ?"sub ? ?pc, pc, #4\n\t"
+#endif

Haojian,

Could you check internally if isb() will work here as PJ4 is both v6/v7 compatible, and if it's in v7 mode, I guess isb() can be safely used here?

Quoted 7 lines

? ? ? ? ? ? ? ?: "=r" (temp) : "r" (value));
?}

--
1.7.4.1

#15 lore raw

RFC PATCH 3/3 ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

Quoted 18 lines · On Wednesday 07 September 2011 09:18:15 Eric Miao wrote:
Quoted 5 lines
arch/arm/kernel/pj4-cp0.c +0 -0
index a4b1b07..5117d9d 100644

arch/arm/kernel/pj4-cp0.c static void __init pj4_cp_access_write(u32 value) · line 75

Quoted 10 lines
arch/arm/kernel/pj4-cp0.c +4 -0
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 
       __asm__ __volatile__ (
               "mcr    p15, 0, %1, c1, c0, 2\n\t"
+#ifdef __LINUX_ARM_ARCH__ >= 7
+               "isb\n\t"
+#else
               "mrc    p15, 0, %0, c1, c0, 2\n\t"
               "mov    %0, %0\n\t"
               "sub    pc, pc, #4\n\t"
+#endif

Haojian,

Quoted 2 lines

Could you check internally if isb() will work here as PJ4 is both v6/v7 compatible, and if it's in v7 mode, I guess isb() can be safely used here?

I thought we only support pj4 in v7 mode in Linux anyway.

Arnd

#16 lore raw

RFC PATCH 3/3 ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

Quoted 19 lines · Arnd Bergmann
Quoted 18 lines · On Wednesday 07 September 2011 09:18:15 Eric Miao wrote:
Quoted 5 lines
arch/arm/kernel/pj4-cp0.c +0 -0
index a4b1b07..5117d9d 100644

arch/arm/kernel/pj4-cp0.c static void __init pj4_cp_access_write(u32 value) · line 75

Quoted 10 lines
arch/arm/kernel/pj4-cp0.c +4 -0
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 
       __asm__ __volatile__ (
               "mcr    p15, 0, %1, c1, c0, 2\n\t"
+#ifdef __LINUX_ARM_ARCH__ >= 7
+               "isb\n\t"
+#else
               "mrc    p15, 0, %0, c1, c0, 2\n\t"
               "mov    %0, %0\n\t"
               "sub    pc, pc, #4\n\t"
+#endif

Haojian,

Quoted 3 lines
Quoted 2 lines

Could you check internally if isb() will work here as PJ4 is both v6/v7 compatible, and if it's in v7 mode, I guess isb() can be safely used here?

PJ4 v6 mode is abandoned in real usage. So we only need to cover v7 mode.

Quoted 3 lines

I thought we only support pj4 in v7 mode in Linux anyway.

Arnd

#17 lore raw

RFC PATCH 3/3 ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

In reply to Dave Martin

Hello.

Quoted 15 lines · Dave Martin

The iwmmxt code contains some code to implement a pseudo-ISB, but this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one on v7 and above.

---
  arch/arm/kernel/pj4-cp0.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/kernel/pj4-cp0.c +0 -0
index a4b1b07..5117d9d 100644

arch/arm/kernel/pj4-cp0.c static void __init pj4_cp_access_write(u32 value) · line 69

Quoted 4 lines
arch/arm/kernel/pj4-cp0.c +1 -0
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 
  	__asm__ __volatile__ (
  		"mcr	p15, 0, %1, c1, c0, 2\n\t"
+#ifdef __LINUX_ARM_ARCH__ >= 7

Maybe #if?

WBR, Sergei

#18 lore raw

RFC PATCH 3/3 ARM: pxa/pj4: Port problematic pj4 support code to v7/Thumb-2

Quoted 20 lines · Sergei Shtylyov

Hello.

Quoted 16 lines · Dave Martin

The iwmmxt code contains some code to implement a pseudo-ISB, but this is not buildable for Thumb-2.

This patch replaces the pseudo-ISB with a real one on v7 and above.

---
 arch/arm/kernel/pj4-cp0.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/kernel/pj4-cp0.c +0 -0
index a4b1b07..5117d9d 100644
@@ -66,9 +66,13 @@ static void __init pj4_cp_access_write(u32 value)
 
Quoted 7 lines

arch/arm/kernel/pj4-cp0.c static void __init pj4_cp_access_write(u32 value) · line 69

Quoted 3 lines
arch/arm/kernel/pj4-cp0.c +1 -0
 	__asm__ __volatile__ (
 		"mcr	p15, 0, %1, c1, c0, 2\n\t"
+#ifdef __LINUX_ARM_ARCH__ >= 7

Maybe #if?

WBR, Sergei

Hmmm, yes.

With a bit of luck, we may be able to get rid of the conditional anyway.

Cheers
---Dave