Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Eric Miao]

Hi Dave & Sascha,

I checked the log of this file, found a THUMB2 related changes, yet
I'm still having the failure below, can you help do a quick check?


AS      arch/arm/plat-mxc/ssi-fiq.o
arch/arm/plat-mxc/ssi-fiq.S: Assembler messages:
arch/arm/plat-mxc/ssi-fiq.S:45: Error: r13 not allowed here -- `tst r13,#(1<<0)'
arch/arm/plat-mxc/ssi-fiq.S:50: Error: r13 not allowed here -- `tst r13,#(1<<0)'
arch/arm/plat-mxc/ssi-fiq.S:59: Error: r13 not allowed here -- `ldrh r13,[r11]'
arch/arm/plat-mxc/ssi-fiq.S:60: Error: r13 not allowed here -- `strh
r13,[r12,#0x00]'
arch/arm/plat-mxc/ssi-fiq.S:62: Error: r13 not allowed here -- `ldrh
r13,[r11,#2]'
arch/arm/plat-mxc/ssi-fiq.S:63: Error: r13 not allowed here -- `strh
r13,[r12,#0x00]'
arch/arm/plat-mxc/ssi-fiq.S:65: Error: r13 not allowed here -- `ldrh
r13,[r11,#4]'
arch/arm/plat-mxc/ssi-fiq.S:66: Error: r13 not allowed here -- `strh
r13,[r12,#0x00]'
arch/arm/plat-mxc/ssi-fiq.S:68: Error: r13 not allowed here -- `ldrh
r13,[r11,#6]'
arch/arm/plat-mxc/ssi-fiq.S:69: Error: r13 not allowed here -- `strh
r13,[r12,#0x00]'
arch/arm/plat-mxc/ssi-fiq.S:72: Error: r13 not allowed here -- `lsr r13,r8,#16'
arch/arm/plat-mxc/ssi-fiq.S:74: Error: r13 not allowed here -- `lslgt
r8,r13,#16'
arch/arm/plat-mxc/ssi-fiq.S:81: Error: r13 not allowed here -- `tst r13,#(1<<2)'
arch/arm/plat-mxc/ssi-fiq.S:86: Error: r13 not allowed here -- `tst r13,#(1<<2)'
arch/arm/plat-mxc/ssi-fiq.S:98: Error: r13 not allowed here -- `tst r13,#(1<<0)'
arch/arm/plat-mxc/ssi-fiq.S:101: Error: r13 not allowed here -- `strh r13,[r11]'
arch/arm/plat-mxc/ssi-fiq.S:104: Error: r13 not allowed here -- `strh
r13,[r11,#2]'
arch/arm/plat-mxc/ssi-fiq.S:110: Error: r13 not allowed here -- `strh
r13,[r11,#4]'
arch/arm/plat-mxc/ssi-fiq.S:113: Error: r13 not allowed here -- `strh
r13,[r11,#6]'
arch/arm/plat-mxc/ssi-fiq.S:119: Error: r13 not allowed here -- `lsr r13,r9,#16'
arch/arm/plat-mxc/ssi-fiq.S:121: Error: r13 not allowed here -- `lslgt
r9,r13,#16'
make[2]: *** [arch/arm/plat-mxc/ssi-fiq.o] Error 1
make[1]: *** [arch/arm/plat-mxc] Error 2
make[1]: *** Waiting for unfinished jobs....

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Uwe Kleine-König]

On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> Hi Dave & Sascha,
> 
> I checked the log of this file, found a THUMB2 related changes, yet
> I'm still having the failure below, can you help do a quick check?
the problem is that in Thumb most commands don't work with r8-r15
because there are only three bits used to encode them. This is
unfortunate as the other registers are not banked for FIQ.

The best possibility I see is to save some unbanked registers to the
banked registers on entry and restore them on exception exit.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Dave Martin]

On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> > Hi Dave & Sascha,
> > 
> > I checked the log of this file, found a THUMB2 related changes, yet
> > I'm still having the failure below, can you help do a quick check?
> the problem is that in Thumb most commands don't work with r8-r15
> because there are only three bits used to encode them. This is
> unfortunate as the other registers are not banked for FIQ.

That really only applies to Thumb-1 (i.e., prior to ARMv6T2).  The
real problem here is that the code (ab)uses r13: this register is no
longer fully general purpose in the Thumb-2 instruction set.  Instead,
it can only be used as a base register in LDM/STM/PUSH/POP and the LDR/
STR family of instructions, and as a simple operand in ADD/SUB/MOV type
instructions.

It just so happens that the use of r11 in this file _does_ fit those
constraints.

I hit this issue some time ago, and worked around the build problem by
simply swapping the roles of r11 and r13, both of which are banked for
FIQ mode.  I was never able to test it though, and I remain unsure
whether this code applies to new platforms.

Is this code still used on ARMv7 and above?  FIQ is not normally
available for Linux interrupts on hardware which makes use of the
TrustZone security extensions.  Plus, on modern hardware with a deep
memory hierarchy, FIQ is may not be all that "fast" either, due to
cache/TLB effects.



Here's my original patch, which also tries to fixes some other issues
which cause problems with some tool versions.  If someone is in a
position to test it, that would be great.

Cheers
---Dave


>From 014db9cd2b542cd9331c79cff6f38dad3026a32a Mon Sep 17 00:00:00 2001
From: Dave Martin <dave.martin@linaro.org>
Date: Wed, 6 Apr 2011 16:25:25 +0100
Subject: [PATCH] ARM: mxc: ssi-fiq: Make ssi-fiq.S Thumb-2 compatible

References to locally-defined global symbols with adr and ldr
may not be accepted by the assembler in Thumb-2.  Local shadow
symbols are added to work around this.

The code contains use of r13 (sp) which isn't allowed in Thumb-2.
r11 and r13 have been swapped throughout the file to work around
this.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/plat-mxc/ssi-fiq.S |   89 ++++++++++++++++++++++++-------------------
 1 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/arch/arm/plat-mxc/ssi-fiq.S b/arch/arm/plat-mxc/ssi-fiq.S
index 8397a2d..a8b93c5 100644
--- a/arch/arm/plat-mxc/ssi-fiq.S
+++ b/arch/arm/plat-mxc/ssi-fiq.S
@@ -34,91 +34,98 @@
 		.global imx_ssi_fiq_rx_buffer
 		.global imx_ssi_fiq_tx_buffer
 
+/*
+ * imx_ssi_fiq_start is _intentionally_ not marked as a function symbol
+ * using ENDPROC().  imx_ssi_fiq_start and imx_ssi_fiq_end are used to
+ * mark the function body so that it can be copied to the FIQ vector in
+ * the vectors page.  imx_ssi_fiq_start should only be called as the result
+ * of an FIQ: calling it directly will not work.
+ */
 imx_ssi_fiq_start:
-		ldr r12, imx_ssi_fiq_base
+		ldr r12, .L_imx_ssi_fiq_base
 
 		/* TX */
-		ldr r11, imx_ssi_fiq_tx_buffer
+		ldr r13, .L_imx_ssi_fiq_tx_buffer
 
 		/* shall we send? */
-		ldr r13, [r12, #SSI_SIER]
-		tst r13, #SSI_SIER_TFE0_EN
+		ldr r11, [r12, #SSI_SIER]
+		tst r11, #SSI_SIER_TFE0_EN
 		beq 1f
 
 		/* TX FIFO empty? */
-		ldr r13, [r12, #SSI_SISR]
-		tst r13, #SSI_SISR_TFE0
+		ldr r11, [r12, #SSI_SISR]
+		tst r11, #SSI_SISR_TFE0
 		beq 1f
 
 		mov r10, #0x10000
 		sub r10, #1
 		and r10, r10, r8	/* r10: current buffer offset */
 
-		add r11, r11, r10
+		add r13, r13, r10
 
-		ldrh r13, [r11]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r11, [r13]
+		strh r11, [r12, #SSI_STX0]
 
-		ldrh r13, [r11, #2]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r11, [r13, #2]
+		strh r11, [r12, #SSI_STX0]
 
-		ldrh r13, [r11, #4]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r11, [r13, #4]
+		strh r11, [r12, #SSI_STX0]
 
-		ldrh r13, [r11, #6]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r11, [r13, #6]
+		strh r11, [r12, #SSI_STX0]
 
 		add r10, #8
-		lsr r13, r8, #16	/* r13: buffer size */
-		cmp r10, r13
-		lslgt r8, r13, #16
+		lsr r11, r8, #16	/* r11: buffer size */
+		cmp r10, r11
+		lslgt r8, r11, #16
 		addle r8, #8
 1:
 		/* RX */
 
 		/* shall we receive? */
-		ldr r13, [r12, #SSI_SIER]
-		tst r13, #SSI_SIER_RFF0_EN
+		ldr r11, [r12, #SSI_SIER]
+		tst r11, #SSI_SIER_RFF0_EN
 		beq 1f
 
 		/* RX FIFO full? */
-		ldr r13, [r12, #SSI_SISR]
-		tst r13, #SSI_SISR_RFF0
+		ldr r11, [r12, #SSI_SISR]
+		tst r11, #SSI_SISR_RFF0
 		beq 1f
 
-		ldr r11, imx_ssi_fiq_rx_buffer
+		ldr r13, .L_imx_ssi_fiq_rx_buffer
 
 		mov r10, #0x10000
 		sub r10, #1
 		and r10, r10, r9	/* r10: current buffer offset */
 
-		add r11, r11, r10
+		add r13, r13, r10
 
-		ldr r13, [r12, #SSI_SACNT]
-		tst r13, #SSI_SACNT_AC97EN
+		ldr r11, [r12, #SSI_SACNT]
+		tst r11, #SSI_SACNT_AC97EN
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11]
+		ldr r11, [r12, #SSI_SRX0]
+		strh r11, [r13]
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11, #2]
+		ldr r11, [r12, #SSI_SRX0]
+		strh r11, [r13, #2]
 
 		/* dummy read to skip slot 12 */
-		ldrne r13, [r12, #SSI_SRX0]
+		ldrne r11, [r12, #SSI_SRX0]
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11, #4]
+		ldr r11, [r12, #SSI_SRX0]
+		strh r11, [r13, #4]
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11, #6]
+		ldr r11, [r12, #SSI_SRX0]
+		strh r11, [r13, #6]
 
 		/* dummy read to skip slot 12 */
-		ldrne r13, [r12, #SSI_SRX0]
+		ldrne r11, [r12, #SSI_SRX0]
 
 		add r10, #8
-		lsr r13, r9, #16	/* r13: buffer size */
-		cmp r10, r13
-		lslgt r9, r13, #16
+		lsr r11, r9, #16	/* r11: buffer size */
+		cmp r10, r11
+		lslgt r9, r11, #16
 		addle r9, #8
 
 1:
@@ -126,11 +133,15 @@ imx_ssi_fiq_start:
 		subs	pc, lr, #4
 
 		.align
+.L_imx_ssi_fiq_base:
 imx_ssi_fiq_base:
 		.word 0x0
+.L_imx_ssi_fiq_rx_buffer:
 imx_ssi_fiq_rx_buffer:
 		.word 0x0
+.L_imx_ssi_fiq_tx_buffer:
 imx_ssi_fiq_tx_buffer:
 		.word 0x0
+.L_imx_ssi_fiq_end:
 imx_ssi_fiq_end:
 
-- 
1.7.4.1

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Dave Martin]

Hi Eric, Sascha

Do you know whether anyone was planning to merge the patch discussed
below?  Matt reported he's still hitting problems which this patch may
help with.


You can get the original raw post here:

http://lists.arm.linux.org.uk/lurker/message/20111202.133911.393b6e28.en.html


Cheers
---Dave


On Fri, Dec 02, 2011 at 01:39:11PM +0000, Dave Martin wrote:
> On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> > > Hi Dave & Sascha,
> > > 
> > > I checked the log of this file, found a THUMB2 related changes, yet
> > > I'm still having the failure below, can you help do a quick check?
> > the problem is that in Thumb most commands don't work with r8-r15
> > because there are only three bits used to encode them. This is
> > unfortunate as the other registers are not banked for FIQ.
> 
> That really only applies to Thumb-1 (i.e., prior to ARMv6T2).  The
> real problem here is that the code (ab)uses r13: this register is no
> longer fully general purpose in the Thumb-2 instruction set.  Instead,
> it can only be used as a base register in LDM/STM/PUSH/POP and the LDR/
> STR family of instructions, and as a simple operand in ADD/SUB/MOV type
> instructions.
> 
> It just so happens that the use of r11 in this file _does_ fit those
> constraints.
> 
> I hit this issue some time ago, and worked around the build problem by
> simply swapping the roles of r11 and r13, both of which are banked for
> FIQ mode.  I was never able to test it though, and I remain unsure
> whether this code applies to new platforms.
> 
> Is this code still used on ARMv7 and above?  FIQ is not normally
> available for Linux interrupts on hardware which makes use of the
> TrustZone security extensions.  Plus, on modern hardware with a deep
> memory hierarchy, FIQ is may not be all that "fast" either, due to
> cache/TLB effects.
> 
> 
> 
> Here's my original patch, which also tries to fixes some other issues
> which cause problems with some tool versions.  If someone is in a
> position to test it, that would be great.
> 
> Cheers
> ---Dave
> 
> 
> From 014db9cd2b542cd9331c79cff6f38dad3026a32a Mon Sep 17 00:00:00 2001
> From: Dave Martin <dave.martin@linaro.org>
> Date: Wed, 6 Apr 2011 16:25:25 +0100
> Subject: [PATCH] ARM: mxc: ssi-fiq: Make ssi-fiq.S Thumb-2 compatible
> 
> References to locally-defined global symbols with adr and ldr
> may not be accepted by the assembler in Thumb-2.  Local shadow
> symbols are added to work around this.
> 
> The code contains use of r13 (sp) which isn't allowed in Thumb-2.
> r11 and r13 have been swapped throughout the file to work around
> this.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
>  arch/arm/plat-mxc/ssi-fiq.S |   89 ++++++++++++++++++++++++-------------------
>  1 files changed, 50 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/ssi-fiq.S b/arch/arm/plat-mxc/ssi-fiq.S
> index 8397a2d..a8b93c5 100644
> --- a/arch/arm/plat-mxc/ssi-fiq.S
> +++ b/arch/arm/plat-mxc/ssi-fiq.S
> @@ -34,91 +34,98 @@
>  		.global imx_ssi_fiq_rx_buffer
>  		.global imx_ssi_fiq_tx_buffer
>  
> +/*
> + * imx_ssi_fiq_start is _intentionally_ not marked as a function symbol
> + * using ENDPROC().  imx_ssi_fiq_start and imx_ssi_fiq_end are used to
> + * mark the function body so that it can be copied to the FIQ vector in
> + * the vectors page.  imx_ssi_fiq_start should only be called as the result
> + * of an FIQ: calling it directly will not work.
> + */
>  imx_ssi_fiq_start:
> -		ldr r12, imx_ssi_fiq_base
> +		ldr r12, .L_imx_ssi_fiq_base
>  
>  		/* TX */
> -		ldr r11, imx_ssi_fiq_tx_buffer
> +		ldr r13, .L_imx_ssi_fiq_tx_buffer
>  
>  		/* shall we send? */
> -		ldr r13, [r12, #SSI_SIER]
> -		tst r13, #SSI_SIER_TFE0_EN
> +		ldr r11, [r12, #SSI_SIER]
> +		tst r11, #SSI_SIER_TFE0_EN
>  		beq 1f
>  
>  		/* TX FIFO empty? */
> -		ldr r13, [r12, #SSI_SISR]
> -		tst r13, #SSI_SISR_TFE0
> +		ldr r11, [r12, #SSI_SISR]
> +		tst r11, #SSI_SISR_TFE0
>  		beq 1f
>  
>  		mov r10, #0x10000
>  		sub r10, #1
>  		and r10, r10, r8	/* r10: current buffer offset */
>  
> -		add r11, r11, r10
> +		add r13, r13, r10
>  
> -		ldrh r13, [r11]
> -		strh r13, [r12, #SSI_STX0]
> +		ldrh r11, [r13]
> +		strh r11, [r12, #SSI_STX0]
>  
> -		ldrh r13, [r11, #2]
> -		strh r13, [r12, #SSI_STX0]
> +		ldrh r11, [r13, #2]
> +		strh r11, [r12, #SSI_STX0]
>  
> -		ldrh r13, [r11, #4]
> -		strh r13, [r12, #SSI_STX0]
> +		ldrh r11, [r13, #4]
> +		strh r11, [r12, #SSI_STX0]
>  
> -		ldrh r13, [r11, #6]
> -		strh r13, [r12, #SSI_STX0]
> +		ldrh r11, [r13, #6]
> +		strh r11, [r12, #SSI_STX0]
>  
>  		add r10, #8
> -		lsr r13, r8, #16	/* r13: buffer size */
> -		cmp r10, r13
> -		lslgt r8, r13, #16
> +		lsr r11, r8, #16	/* r11: buffer size */
> +		cmp r10, r11
> +		lslgt r8, r11, #16
>  		addle r8, #8
>  1:
>  		/* RX */
>  
>  		/* shall we receive? */
> -		ldr r13, [r12, #SSI_SIER]
> -		tst r13, #SSI_SIER_RFF0_EN
> +		ldr r11, [r12, #SSI_SIER]
> +		tst r11, #SSI_SIER_RFF0_EN
>  		beq 1f
>  
>  		/* RX FIFO full? */
> -		ldr r13, [r12, #SSI_SISR]
> -		tst r13, #SSI_SISR_RFF0
> +		ldr r11, [r12, #SSI_SISR]
> +		tst r11, #SSI_SISR_RFF0
>  		beq 1f
>  
> -		ldr r11, imx_ssi_fiq_rx_buffer
> +		ldr r13, .L_imx_ssi_fiq_rx_buffer
>  
>  		mov r10, #0x10000
>  		sub r10, #1
>  		and r10, r10, r9	/* r10: current buffer offset */
>  
> -		add r11, r11, r10
> +		add r13, r13, r10
>  
> -		ldr r13, [r12, #SSI_SACNT]
> -		tst r13, #SSI_SACNT_AC97EN
> +		ldr r11, [r12, #SSI_SACNT]
> +		tst r11, #SSI_SACNT_AC97EN
>  
> -		ldr r13, [r12, #SSI_SRX0]
> -		strh r13, [r11]
> +		ldr r11, [r12, #SSI_SRX0]
> +		strh r11, [r13]
>  
> -		ldr r13, [r12, #SSI_SRX0]
> -		strh r13, [r11, #2]
> +		ldr r11, [r12, #SSI_SRX0]
> +		strh r11, [r13, #2]
>  
>  		/* dummy read to skip slot 12 */
> -		ldrne r13, [r12, #SSI_SRX0]
> +		ldrne r11, [r12, #SSI_SRX0]
>  
> -		ldr r13, [r12, #SSI_SRX0]
> -		strh r13, [r11, #4]
> +		ldr r11, [r12, #SSI_SRX0]
> +		strh r11, [r13, #4]
>  
> -		ldr r13, [r12, #SSI_SRX0]
> -		strh r13, [r11, #6]
> +		ldr r11, [r12, #SSI_SRX0]
> +		strh r11, [r13, #6]
>  
>  		/* dummy read to skip slot 12 */
> -		ldrne r13, [r12, #SSI_SRX0]
> +		ldrne r11, [r12, #SSI_SRX0]
>  
>  		add r10, #8
> -		lsr r13, r9, #16	/* r13: buffer size */
> -		cmp r10, r13
> -		lslgt r9, r13, #16
> +		lsr r11, r9, #16	/* r11: buffer size */
> +		cmp r10, r11
> +		lslgt r9, r11, #16
>  		addle r9, #8
>  
>  1:
> @@ -126,11 +133,15 @@ imx_ssi_fiq_start:
>  		subs	pc, lr, #4
>  
>  		.align
> +.L_imx_ssi_fiq_base:
>  imx_ssi_fiq_base:
>  		.word 0x0
> +.L_imx_ssi_fiq_rx_buffer:
>  imx_ssi_fiq_rx_buffer:
>  		.word 0x0
> +.L_imx_ssi_fiq_tx_buffer:
>  imx_ssi_fiq_tx_buffer:
>  		.word 0x0
> +.L_imx_ssi_fiq_end:
>  imx_ssi_fiq_end:
>  
> -- 
> 1.7.4.1
> 

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Russell King - ARM Linux]

On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> > Hi Dave & Sascha,
> > 
> > I checked the log of this file, found a THUMB2 related changes, yet
> > I'm still having the failure below, can you help do a quick check?
> the problem is that in Thumb most commands don't work with r8-r15
> because there are only three bits used to encode them. This is
> unfortunate as the other registers are not banked for FIQ.
> 
> The best possibility I see is to save some unbanked registers to the
> banked registers on entry and restore them on exception exit.

Or always build this as ARM code.

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Russell King - ARM Linux]

On Fri, Dec 02, 2011 at 01:39:11PM +0000, Dave Martin wrote:
> Is this code still used on ARMv7 and above?  FIQ is not normally
> available for Linux interrupts on hardware which makes use of the
> TrustZone security extensions.  Plus, on modern hardware with a deep
> memory hierarchy, FIQ is may not be all that "fast" either, due to
> cache/TLB effects.

It's worth pointing out that people end up using FIQs for certain things
because the hardware requires you to do it.  So if a platform is using
them, they're probably not doing it out of choice, but are doing it
because it's a baseline requirement to get something working.

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Dave Martin]

On Fri, Jan 20, 2012 at 11:52:17AM +0000, Russell King - ARM Linux wrote:
> On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> > > Hi Dave & Sascha,
> > > 
> > > I checked the log of this file, found a THUMB2 related changes, yet
> > > I'm still having the failure below, can you help do a quick check?
> > the problem is that in Thumb most commands don't work with r8-r15
> > because there are only three bits used to encode them. This is
> > unfortunate as the other registers are not banked for FIQ.
> > 
> > The best possibility I see is to save some unbanked registers to the
> > banked registers on entry and restore them on exception exit.
> 
> Or always build this as ARM code.

That should work, sure -- I prefer to keep the number of instances where
we need to do this to a minimum if we can, though.

Cheers
---Dave

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Dave Martin]

On Fri, Jan 20, 2012 at 11:56:06AM +0000, Russell King - ARM Linux wrote:
> On Fri, Dec 02, 2011 at 01:39:11PM +0000, Dave Martin wrote:
> > Is this code still used on ARMv7 and above?  FIQ is not normally
> > available for Linux interrupts on hardware which makes use of the
> > TrustZone security extensions.  Plus, on modern hardware with a deep
> > memory hierarchy, FIQ is may not be all that "fast" either, due to
> > cache/TLB effects.
> 
> It's worth pointing out that people end up using FIQs for certain things
> because the hardware requires you to do it.  So if a platform is using
> them, they're probably not doing it out of choice, but are doing it
> because it's a baseline requirement to get something working.

Agreed -- but I'm wondering whether this code is actually non-relevant
to newer platforms, and is only being built at all because of a
non-relevant driver being included in mx5_defconfig, or due to some
Kconfig anomaly.


If this is really non-relevant to any >= v7 platform, we just need
for fix Kconfig : this code should never ever be built into a Thumb-2
kernel in that case.


Unfotunately I don't have the i.MX hardware knowledge to answer that
"if" though... it's just a hunch on my part.

One of the i.MX guys will have to comment on that.

Cheers
---Dave

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Sascha Hauer]

On Fri, Jan 20, 2012 at 12:00:03PM +0000, Dave Martin wrote:
> On Fri, Jan 20, 2012 at 11:52:17AM +0000, Russell King - ARM Linux wrote:
> > On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> > > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> > > > Hi Dave & Sascha,
> > > > 
> > > > I checked the log of this file, found a THUMB2 related changes, yet
> > > > I'm still having the failure below, can you help do a quick check?
> > > the problem is that in Thumb most commands don't work with r8-r15
> > > because there are only three bits used to encode them. This is
> > > unfortunate as the other registers are not banked for FIQ.
> > > 
> > > The best possibility I see is to save some unbanked registers to the
> > > banked registers on entry and restore them on exception exit.
> > 
> > Or always build this as ARM code.
> 
> That should work, sure -- I prefer to keep the number of instances where
> we need to do this to a minimum if we can, though.

I vote for compiling this in ARM mode since it's a straight forward
solution.

Some background:

The original reason for creating this code was that we did not have SDMA
support at the time I wanted audio support. The FIQ code was a simple
way to seperate audio and DMA support and to push them to mainline
seperately. There is another reason for having this code though. The
i.MX SSI port is quite broken in AC97 mode. Some AC97 codecs like the
WM9712 always send data in slot 12 (IIRC it contains gpio status). The
SSI unit always puts all valid slots into the data stream, so we end up
having the audio rx data interleaved with the gpio status data. The FIQ
code is used to skip the gpio status data (see /* dummy read to skip
slot 12 */ in ssi-fiq.S).

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Matt Sealey]

On Mon, Jan 23, 2012 at 5:59 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Jan 20, 2012 at 12:00:03PM +0000, Dave Martin wrote:
>> On Fri, Jan 20, 2012 at 11:52:17AM +0000, Russell King - ARM Linux wrote:
>> > On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
>> > > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
>> > > > Hi Dave & Sascha,
>> > > >
>> > > > I checked the log of this file, found a THUMB2 related changes, yet
>> > > > I'm still having the failure below, can you help do a quick check?
>> > > the problem is that in Thumb most commands don't work with r8-r15
>> > > because there are only three bits used to encode them. This is
>> > > unfortunate as the other registers are not banked for FIQ.
>> > >
>> > > The best possibility I see is to save some unbanked registers to the
>> > > banked registers on entry and restore them on exception exit.
>> >
>> > Or always build this as ARM code.
>>
>> That should work, sure -- I prefer to keep the number of instances where
>> we need to do this to a minimum if we can, though.
>
> I vote for compiling this in ARM mode since it's a straight forward
> solution.
>
> Some background:
>
> The original reason for creating this code was that we did not have SDMA
> support at the time I wanted audio support. The FIQ code was a simple
> way to seperate audio and DMA support and to push them to mainline
> seperately. There is another reason for having this code though. The
> i.MX SSI port is quite broken in AC97 mode. Some AC97 codecs like the
> WM9712 always send data in slot 12 (IIRC it contains gpio status). The
> SSI unit always puts all valid slots into the data stream, so we end up
> having the audio rx data interleaved with the gpio status data. The FIQ
> code is used to skip the gpio status data (see /* dummy read to skip
> slot 12 */ in ssi-fiq.S).

Okay so let's start with the thumb-isation of that code since it's
breaking thumb2 kernel builds for i.MX with
SSI audio (regardless of AC97 requirement).

Freescale's older BSPs implemented AC97 and I2S audio mode seperately
(duplicating code but.. that can
be fixed) to set up different DAIs so it's not impossible to consider
that the different SSI modes could be
made fairly modular based on some kind of internal core subsystem..?
Later though, right? :)

I can officially say that I don't care, as long as it builds in Thumb2
mode, and audio plays.. for now. Dave's
patch seems to work, can we upstream it?

-- 
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Russell King - ARM Linux]

On Wed, Jan 25, 2012 at 08:06:03AM -0600, Matt Sealey wrote:
> On Mon, Jan 23, 2012 at 5:59 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Fri, Jan 20, 2012 at 12:00:03PM +0000, Dave Martin wrote:
> >> On Fri, Jan 20, 2012 at 11:52:17AM +0000, Russell King - ARM Linux wrote:
> >> > On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> >> > > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> >> > > > Hi Dave & Sascha,
> >> > > >
> >> > > > I checked the log of this file, found a THUMB2 related changes, yet
> >> > > > I'm still having the failure below, can you help do a quick check?
> >> > > the problem is that in Thumb most commands don't work with r8-r15
> >> > > because there are only three bits used to encode them. This is
> >> > > unfortunate as the other registers are not banked for FIQ.
> >> > >
> >> > > The best possibility I see is to save some unbanked registers to the
> >> > > banked registers on entry and restore them on exception exit.
> >> >
> >> > Or always build this as ARM code.
> >>
> >> That should work, sure -- I prefer to keep the number of instances where
> >> we need to do this to a minimum if we can, though.
> >
> > I vote for compiling this in ARM mode since it's a straight forward
> > solution.
> >
> > Some background:
> >
> > The original reason for creating this code was that we did not have SDMA
> > support at the time I wanted audio support. The FIQ code was a simple
> > way to seperate audio and DMA support and to push them to mainline
> > seperately. There is another reason for having this code though. The
> > i.MX SSI port is quite broken in AC97 mode. Some AC97 codecs like the
> > WM9712 always send data in slot 12 (IIRC it contains gpio status). The
> > SSI unit always puts all valid slots into the data stream, so we end up
> > having the audio rx data interleaved with the gpio status data. The FIQ
> > code is used to skip the gpio status data (see /* dummy read to skip
> > slot 12 */ in ssi-fiq.S).
> 
> Okay so let's start with the thumb-isation of that code since it's
> breaking thumb2 kernel builds for i.MX with
> SSI audio (regardless of AC97 requirement).

Well, there's two solutions to this:

- we declare that it's obsolete now that SDMA support is in place.
- we always build the code in ARM mode.

I don't think the code should be converted to thumb, because that's going
to involve finding some way to allocate some place for it to save registers
and all that kind of thing - which for relocatable code isn't going to be
nice to do.  It's just easier to stick a .arm in there and be done with it.
(Remembering that it won't run on T2-only CPUs.  But then I doubt that it
would get run on such CPUs.)

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Sascha Hauer]

On Wed, Jan 25, 2012 at 02:34:18PM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 25, 2012 at 08:06:03AM -0600, Matt Sealey wrote:
> > On Mon, Jan 23, 2012 at 5:59 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > On Fri, Jan 20, 2012 at 12:00:03PM +0000, Dave Martin wrote:
> > >> On Fri, Jan 20, 2012 at 11:52:17AM +0000, Russell King - ARM Linux wrote:
> > >> > On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
> > >> > > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
> > >> > > > Hi Dave & Sascha,
> > >> > > >
> > >> > > > I checked the log of this file, found a THUMB2 related changes, yet
> > >> > > > I'm still having the failure below, can you help do a quick check?
> > >> > > the problem is that in Thumb most commands don't work with r8-r15
> > >> > > because there are only three bits used to encode them. This is
> > >> > > unfortunate as the other registers are not banked for FIQ.
> > >> > >
> > >> > > The best possibility I see is to save some unbanked registers to the
> > >> > > banked registers on entry and restore them on exception exit.
> > >> >
> > >> > Or always build this as ARM code.
> > >>
> > >> That should work, sure -- I prefer to keep the number of instances where
> > >> we need to do this to a minimum if we can, though.
> > >
> > > I vote for compiling this in ARM mode since it's a straight forward
> > > solution.
> > >
> > > Some background:
> > >
> > > The original reason for creating this code was that we did not have SDMA
> > > support at the time I wanted audio support. The FIQ code was a simple
> > > way to seperate audio and DMA support and to push them to mainline
> > > seperately. There is another reason for having this code though. The
> > > i.MX SSI port is quite broken in AC97 mode. Some AC97 codecs like the
> > > WM9712 always send data in slot 12 (IIRC it contains gpio status). The
> > > SSI unit always puts all valid slots into the data stream, so we end up
> > > having the audio rx data interleaved with the gpio status data. The FIQ
> > > code is used to skip the gpio status data (see /* dummy read to skip
> > > slot 12 */ in ssi-fiq.S).
> > 
> > Okay so let's start with the thumb-isation of that code since it's
> > breaking thumb2 kernel builds for i.MX with
> > SSI audio (regardless of AC97 requirement).
> 
> Well, there's two solutions to this:
> 
> - we declare that it's obsolete now that SDMA support is in place.
> - we always build the code in ARM mode.

The latter please. As mentioned it is still necessary for AC97 mode and
used at least on the pcm043 board.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Building of arch/arm/plat-mxc/ssi-fiq.S failed w/ THUMB2 enabled?

[Dave Martin]

On Wed, Jan 25, 2012 at 2:34 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 25, 2012 at 08:06:03AM -0600, Matt Sealey wrote:
>> On Mon, Jan 23, 2012 at 5:59 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Fri, Jan 20, 2012 at 12:00:03PM +0000, Dave Martin wrote:
>> >> On Fri, Jan 20, 2012 at 11:52:17AM +0000, Russell King - ARM Linux wrote:
>> >> > On Fri, Dec 02, 2011 at 11:24:18AM +0100, Uwe Kleine-K?nig wrote:
>> >> > > On Fri, Dec 02, 2011 at 06:01:08PM +0800, Eric Miao wrote:
>> >> > > > Hi Dave & Sascha,
>> >> > > >
>> >> > > > I checked the log of this file, found a THUMB2 related changes, yet
>> >> > > > I'm still having the failure below, can you help do a quick check?
>> >> > > the problem is that in Thumb most commands don't work with r8-r15
>> >> > > because there are only three bits used to encode them. This is
>> >> > > unfortunate as the other registers are not banked for FIQ.
>> >> > >
>> >> > > The best possibility I see is to save some unbanked registers to the
>> >> > > banked registers on entry and restore them on exception exit.
>> >> >
>> >> > Or always build this as ARM code.
>> >>
>> >> That should work, sure -- I prefer to keep the number of instances where
>> >> we need to do this to a minimum if we can, though.
>> >
>> > I vote for compiling this in ARM mode since it's a straight forward
>> > solution.
>> >
>> > Some background:
>> >
>> > The original reason for creating this code was that we did not have SDMA
>> > support at the time I wanted audio support. The FIQ code was a simple
>> > way to seperate audio and DMA support and to push them to mainline
>> > seperately. There is another reason for having this code though. The
>> > i.MX SSI port is quite broken in AC97 mode. Some AC97 codecs like the
>> > WM9712 always send data in slot 12 (IIRC it contains gpio status). The
>> > SSI unit always puts all valid slots into the data stream, so we end up
>> > having the audio rx data interleaved with the gpio status data. The FIQ
>> > code is used to skip the gpio status data (see /* dummy read to skip
>> > slot 12 */ in ssi-fiq.S).
>>
>> Okay so let's start with the thumb-isation of that code since it's
>> breaking thumb2 kernel builds for i.MX with
>> SSI audio (regardless of AC97 requirement).
>
> Well, there's two solutions to this:
>
> - we declare that it's obsolete now that SDMA support is in place.
> - we always build the code in ARM mode.
>
> I don't think the code should be converted to thumb, because that's going
> to involve finding some way to allocate some place for it to save registers
> and all that kind of thing - which for relocatable code isn't going to be
> nice to do. ?It's just easier to stick a .arm in there and be done with it.
> (Remembering that it won't run on T2-only CPUs. ?But then I doubt that it
> would get run on such CPUs.)

The Thumb-2 problems are not problems of register pressure -- it's
just down to uses of sp which are illegal in Thumb-2.  Fixing this is
straightforward: we just need to swap some registers.  The resulting
should function exactly the same in ARM as the original code.  Because
this only effects code running in FIQ mode, there is not ABI impact to
this.

However, if the code is inapplicable to v7 platform or later, I
suggest we just make is depend on !CONFIG_THUMB2_KERNEL, since v7
kernels cannot possibly include support for pre-v7 platforms anyway.
In that case, "fixing" this code would just be unnecessary churn.

I suggest the right thing is either to just add .arm (as per Russell's
suggestion) or to make this functionality depend on
!CONFIG_THUMB2_KERNEL and fix the imx Kconfigs not to erroneously
enable it on newer boards where it is inapplicable.  The latter route
might be beneficial in provoking people into fixing their Kconfigs not
to include that dead code for new boards.

Cheers
---Dave