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 at 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 at 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
>