diff mbox series

ARM: pxa: fix building with clang

Message ID 20221215160747.2173998-1-arnd@kernel.org (mailing list archive)
State Superseded
Headers show
Series ARM: pxa: fix building with clang | expand

Commit Message

Arnd Bergmann Dec. 15, 2022, 4:07 p.m. UTC 
From: Arnd Bergmann <arnd@arndb.de>

The integrated assembler in clang does not understand the xscale
specific mra/mar instructions:

arch/arm/mach-pxa/pxa27x.c:136:15: error: unsupported architectural extension: xscale
        asm volatile(".arch_extension xscale\n\t"
arch/arm/mach-pxa/pxa27x.c:136:40: error: invalid instruction, did you mean: mcr, mla, mrc, mrs, msr?
        mra r2, r3, acc0

Since these are coprocessor features, the same can be expressed using
mrrc/mcrr, so use that for builds with IAS.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-pxa/pxa27x.c |  8 ++++++++
 arch/arm/mach-pxa/pxa3xx.c | 16 +++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

Comments

Robert Jarzmik Dec. 17, 2022, 8:45 p.m. UTC | #1 
Arnd Bergmann <arnd@kernel.org> writes:

> From: Arnd Bergmann <arnd@arndb.de>
>
> The integrated assembler in clang does not understand the xscale
> specific mra/mar instructions:
>
> arch/arm/mach-pxa/pxa27x.c:136:15: error: unsupported 
> architectural extension: xscale
>         asm volatile(".arch_extension xscale\n\t"
> arch/arm/mach-pxa/pxa27x.c:136:40: error: invalid instruction, 
> did you mean: mcr, mla, mrc, mrs, msr?
>         mra r2, r3, acc0
>
> Since these are coprocessor features, the same can be expressed 
> using
> mrrc/mcrr, so use that for builds with IAS.
Ok Arnd, but "mrrc" is an ARMv6 instruction if I'm not mistaken.
Yet PXA27X is an ARMv5 XScale instruction set IP.

Is that patch correct therefore, or is it just to make clang happy 
even if it's
not correct ?

Cheers.

--
Robert
Arnd Bergmann Dec. 18, 2022, 11 a.m. UTC | #2 
On Sat, Dec 17, 2022, at 21:45, Robert Jarzmik wrote:
> Arnd Bergmann <arnd@kernel.org> writes:
>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The integrated assembler in clang does not understand the xscale
>> specific mra/mar instructions:
>>
>> arch/arm/mach-pxa/pxa27x.c:136:15: error: unsupported 
>> architectural extension: xscale
>>         asm volatile(".arch_extension xscale\n\t"
>> arch/arm/mach-pxa/pxa27x.c:136:40: error: invalid instruction, 
>> did you mean: mcr, mla, mrc, mrs, msr?
>>         mra r2, r3, acc0
>>
>> Since these are coprocessor features, the same can be expressed 
>> using
>> mrrc/mcrr, so use that for builds with IAS.
> Ok Arnd, but "mrrc" is an ARMv6 instruction if I'm not mistaken.
> Yet PXA27X is an ARMv5 XScale instruction set IP.
>
> Is that patch correct therefore, or is it just to make clang happy 
> even if it's not correct ?

According to

https://developer.arm.com/documentation/dui0231/b/arm-instruction-reference/coprocessor-instructions/mrrc-and-mrrc2?lang=en

"MRRC is available in ARMv6 and above, and E variants of ARMv5
excluding xP variants". I'm not entire sure what "xP variants"
means, but we do build for ARMv5E, so I think this is actually
correct.

    Arnd
Robert Jarzmik Dec. 18, 2022, 11:11 a.m. UTC | #3 
"Arnd Bergmann" <arnd@arndb.de> writes:
> "MRRC is available in ARMv6 and above, and E variants of ARMv5
> excluding xP variants". I'm not entire sure what "xP variants"
> means, but we do build for ARMv5E, so I think this is actually
> correct.
Ah yes, and XScale is ARMv5TE.

Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

--
Robert
diff mbox series

Patch

diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index afbf6ace954f..eea507fd5095 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -133,8 +133,12 @@  void pxa27x_cpu_pm_enter(suspend_state_t state)
 #ifndef CONFIG_IWMMXT
 	u64 acc0;
 
+#ifndef CONFIG_AS_IS_LLVM
 	asm volatile(".arch_extension xscale\n\t"
 		     "mra %Q0, %R0, acc0" : "=r" (acc0));
+#else
+	asm volatile("mrrc p0, 0, %Q0, %R0, c0" : "=r" (acc0));
+#endif
 #endif
 
 	/* ensure voltage-change sequencer not initiated, which hangs */
@@ -153,8 +157,12 @@  void pxa27x_cpu_pm_enter(suspend_state_t state)
 	case PM_SUSPEND_MEM:
 		cpu_suspend(pwrmode, pxa27x_finish_suspend);
 #ifndef CONFIG_IWMMXT
+#ifndef CONFIG_AS_IS_LLVM
 		asm volatile(".arch_extension xscale\n\t"
 			     "mar acc0, %Q0, %R0" : "=r" (acc0));
+#else
+		asm volatile("mcrr p0, 0, %Q0, %R0, c0" :: "r" (acc0));
+#endif
 #endif
 		break;
 	}
diff --git a/arch/arm/mach-pxa/pxa3xx.c b/arch/arm/mach-pxa/pxa3xx.c
index 4637c54dd6a1..b26f00fc75d5 100644
--- a/arch/arm/mach-pxa/pxa3xx.c
+++ b/arch/arm/mach-pxa/pxa3xx.c
@@ -108,11 +108,12 @@  static void pxa3xx_cpu_pm_suspend(void)
 #ifndef CONFIG_IWMMXT
 	u64 acc0;
 
-	asm volatile(
 #ifdef CONFIG_CC_IS_GCC
-		     ".arch_extension xscale\n\t"
-#endif
+	asm volatile(".arch_extension xscale\n\t"
 		     "mra %Q0, %R0, acc0" : "=r" (acc0));
+#else
+	asm volatile("mrrc p0, 0, %Q0, %R0, c0" : "=r" (acc0));
+#endif
 #endif
 
 	/* resuming from D2 requires the HSIO2/BOOT/TPM clocks enabled */
@@ -140,11 +141,12 @@  static void pxa3xx_cpu_pm_suspend(void)
 	AD3ER = 0;
 
 #ifndef CONFIG_IWMMXT
-	asm volatile(
-#ifdef CONFIG_CC_IS_GCC
-		     ".arch_extension xscale\n\t"
-#endif
+#ifndef CONFIG_AS_IS_LLVM
+	asm volatile(".arch_extension xscale\n\t"
 		     "mar acc0, %Q0, %R0" : "=r" (acc0));
+#else
+	asm volatile("mcrr p0, 0, %Q0, %R0, c0" :: "r" (acc0));
+#endif
 #endif
 }