Patchwork [1/2] ARM: build ssi-fiq.S in ARM mode to prevent CONFIG_THUMB2_KERNEL build breakage

login
register
mail settings
Submitter Dave Martin
Date Aug. 9, 2012, 10:24 a.m.
Message ID <20120809102414.GA17588@linaro.org>
Download mbox | patch
Permalink /patch/176048/
State New
Headers show

Comments

Dave Martin - Aug. 9, 2012, 10:24 a.m.
On Wed, Aug 08, 2012 at 12:32:39PM -0500, Matt Sealey wrote:

[...]
 
> I'm going to do a trapse through and find where Russell nacked Dave's
> thumb-aware
> rewrite.. would you mind if you have any of these boards seeing if it
> really DOES

There was no NAK because I didn't get as far as posting the patch,
mostly because of the doubt about whether this code is ever relevant
on Thumb2-capable hardware.

If somebody with some old imx hardware that definitely relies on this
wants to pick it up and test it, it might still be useful.

I don't know whether I still have the original patch, but I think the
only think I did was to swap the roles of r13 and r12.  In the original
code r12 is only used as a base register, which is permissible usage
for r13 in Thumb-2.  Those two registers appear private to the fiq
handler, so I don't think the change will break anything, but I'd be
more confident is somebody is able to test it.

Cheers
---Dave
Matt Sealey - Aug. 9, 2012, 2:32 p.m.
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.


On Thu, Aug 9, 2012 at 5:24 AM, Dave Martin <dave.martin@linaro.org> wrote:
> On Wed, Aug 08, 2012 at 12:32:39PM -0500, Matt Sealey wrote:
>
> [...]
>
>> I'm going to do a trapse through and find where Russell nacked Dave's
>> thumb-aware
>> rewrite.. would you mind if you have any of these boards seeing if it
>> really DOES
>
> There was no NAK because I didn't get as far as posting the patch,

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

You did, twice :)

> mostly because of the doubt about whether this code is ever relevant
> on Thumb2-capable hardware.

It's not, but it does break the build, and without disabling phyCORE boards for
Thumb2 it will always break the build..

> If somebody with some old imx hardware that definitely relies on this
> wants to pick it up and test it, it might still be useful.
>
> I don't know whether I still have the original patch, but I think the
> only think I did was to swap the roles of r13 and r12.  In the original
> code r12 is only used as a base register, which is permissible usage
> for r13 in Thumb-2.  Those two registers appear private to the fiq
> handler, so I don't think the change will break anything, but I'd be
> more confident is somebody is able to test it.
>
> Cheers
> ---Dave
>
> diff --git a/arch/arm/plat-mxc/ssi-fiq.S b/arch/arm/plat-mxc/ssi-fiq.S
> index 8397a2d..3589afb 100644
> --- a/arch/arm/plat-mxc/ssi-fiq.S
> +++ b/arch/arm/plat-mxc/ssi-fiq.S
> @@ -35,19 +35,19 @@
>                 .global imx_ssi_fiq_tx_buffer
>
>  imx_ssi_fiq_start:
> -               ldr r12, imx_ssi_fiq_base
> +               ldr r13, imx_ssi_fiq_base
>
>                 /* TX */
>                 ldr r11, imx_ssi_fiq_tx_buffer
>
>                 /* shall we send? */
> -               ldr r13, [r12, #SSI_SIER]
> -               tst r13, #SSI_SIER_TFE0_EN
> +               ldr r12, [r13, #SSI_SIER]
> +               tst r12, #SSI_SIER_TFE0_EN
>                 beq 1f
>
>                 /* TX FIFO empty? */
> -               ldr r13, [r12, #SSI_SISR]
> -               tst r13, #SSI_SISR_TFE0
> +               ldr r12, [r13, #SSI_SISR]
> +               tst r12, #SSI_SISR_TFE0
>                 beq 1f
>
>                 mov r10, #0x10000
> @@ -56,34 +56,34 @@ imx_ssi_fiq_start:
>
>                 add r11, r11, r10
>
> -               ldrh r13, [r11]
> -               strh r13, [r12, #SSI_STX0]
> +               ldrh r12, [r11]
> +               strh r12, [r13, #SSI_STX0]
>
> -               ldrh r13, [r11, #2]
> -               strh r13, [r12, #SSI_STX0]
> +               ldrh r12, [r11, #2]
> +               strh r12, [r13, #SSI_STX0]
>
> -               ldrh r13, [r11, #4]
> -               strh r13, [r12, #SSI_STX0]
> +               ldrh r12, [r11, #4]
> +               strh r12, [r13, #SSI_STX0]
>
> -               ldrh r13, [r11, #6]
> -               strh r13, [r12, #SSI_STX0]
> +               ldrh r12, [r11, #6]
> +               strh r12, [r13, #SSI_STX0]
>
>                 add r10, #8
> -               lsr r13, r8, #16        /* r13: buffer size */
> -               cmp r10, r13
> -               lslgt r8, r13, #16
> +               lsr r12, r8, #16        /* r12: buffer size */
> +               cmp r10, r12
> +               lslgt r8, r12, #16
>                 addle r8, #8
>  1:
>                 /* RX */
>
>                 /* shall we receive? */
> -               ldr r13, [r12, #SSI_SIER]
> -               tst r13, #SSI_SIER_RFF0_EN
> +               ldr r12, [r13, #SSI_SIER]
> +               tst r12, #SSI_SIER_RFF0_EN
>                 beq 1f
>
>                 /* RX FIFO full? */
> -               ldr r13, [r12, #SSI_SISR]
> -               tst r13, #SSI_SISR_RFF0
> +               ldr r12, [r13, #SSI_SISR]
> +               tst r12, #SSI_SISR_RFF0
>                 beq 1f
>
>                 ldr r11, imx_ssi_fiq_rx_buffer
> @@ -94,31 +94,31 @@ imx_ssi_fiq_start:
>
>                 add r11, r11, r10
>
> -               ldr r13, [r12, #SSI_SACNT]
> -               tst r13, #SSI_SACNT_AC97EN
> +               ldr r12, [r13, #SSI_SACNT]
> +               tst r12, #SSI_SACNT_AC97EN
>
> -               ldr r13, [r12, #SSI_SRX0]
> -               strh r13, [r11]
> +               ldr r12, [r13, #SSI_SRX0]
> +               strh r12, [r11]
>
> -               ldr r13, [r12, #SSI_SRX0]
> -               strh r13, [r11, #2]
> +               ldr r12, [r13, #SSI_SRX0]
> +               strh r12, [r11, #2]
>
>                 /* dummy read to skip slot 12 */
> -               ldrne r13, [r12, #SSI_SRX0]
> +               ldrne r12, [r13, #SSI_SRX0]
>
> -               ldr r13, [r12, #SSI_SRX0]
> -               strh r13, [r11, #4]
> +               ldr r12, [r13, #SSI_SRX0]
> +               strh r12, [r11, #4]
>
> -               ldr r13, [r12, #SSI_SRX0]
> -               strh r13, [r11, #6]
> +               ldr r12, [r13, #SSI_SRX0]
> +               strh r12, [r11, #6]
>
>                 /* dummy read to skip slot 12 */
> -               ldrne r13, [r12, #SSI_SRX0]
> +               ldrne r12, [r13, #SSI_SRX0]
>
>                 add r10, #8
> -               lsr r13, r9, #16        /* r13: buffer size */
> -               cmp r10, r13
> -               lslgt r9, r13, #16
> +               lsr r12, r9, #16        /* r12: buffer size */
> +               cmp r10, r12
> +               lslgt r9, r12, #16
>                 addle r9, #8
>
>  1:
>
Dave Martin - Aug. 9, 2012, 2:50 p.m.
On Thu, Aug 09, 2012 at 09:32:59AM -0500, Matt Sealey wrote:
> Matt Sealey <matt@genesi-usa.com>
> Product Development Analyst, Genesi USA, Inc.
> 
> 
> On Thu, Aug 9, 2012 at 5:24 AM, Dave Martin <dave.martin@linaro.org> wrote:
> > On Wed, Aug 08, 2012 at 12:32:39PM -0500, Matt Sealey wrote:
> >
> > [...]
> >
> >> I'm going to do a trapse through and find where Russell nacked Dave's
> >> thumb-aware
> >> rewrite.. would you mind if you have any of these boards seeing if it
> >> really DOES
> >
> > There was no NAK because I didn't get as far as posting the patch,
> 
> http://lists.arm.linux.org.uk/lurker/message/20111202.133911.393b6e28.en.html
> 
> You did, twice :)

Well, I meant that I had not posted it as a stand-alone patch for
inclusion; I only posted it when commenting on other threads.

> > mostly because of the doubt about whether this code is ever relevant
> > on Thumb2-capable hardware.
> 
> It's not, but it does break the build, and without disabling phyCORE boards for
> Thumb2 it will always break the build..

If you want me to push it, let me know.  I don't think this should get
merged without some Tested-bys for hardware where the FIQ stuff actually
gets used.

The older post would be the one to use, since that at least got a
reasonable level of build testing.

Cheers
---Dave
Matt Sealey - Aug. 9, 2012, 4:05 p.m.
On Thu, Aug 9, 2012 at 9:50 AM, Dave Martin <dave.martin@linaro.org> wrote:
> On Thu, Aug 09, 2012 at 09:32:59AM -0500, Matt Sealey wrote:
>> Matt Sealey <matt@genesi-usa.com>
>> Product Development Analyst, Genesi USA, Inc.
>>
>>
>> On Thu, Aug 9, 2012 at 5:24 AM, Dave Martin <dave.martin@linaro.org> wrote:
>> > On Wed, Aug 08, 2012 at 12:32:39PM -0500, Matt Sealey wrote:
>> >
>> > [...]
>> >
>> >> I'm going to do a trapse through and find where Russell nacked Dave's
>> >> thumb-aware
>> >> rewrite.. would you mind if you have any of these boards seeing if it
>> >> really DOES
>> >
>> > There was no NAK because I didn't get as far as posting the patch,
>>
>> http://lists.arm.linux.org.uk/lurker/message/20111202.133911.393b6e28.en.html
>>
>> You did, twice :)
>
> Well, I meant that I had not posted it as a stand-alone patch for
> inclusion; I only posted it when commenting on other threads.
>
>> > mostly because of the doubt about whether this code is ever relevant
>> > on Thumb2-capable hardware.
>>
>> It's not, but it does break the build, and without disabling phyCORE boards for
>> Thumb2 it will always break the build..
>
> If you want me to push it, let me know.  I don't think this should get
> merged without some Tested-bys for hardware where the FIQ stuff actually
> gets used.

Please, then all the Tested-by's should flood in, right? :)

> The older post would be the one to use, since that at least got a
> reasonable level of build testing.
Dave Martin - Aug. 9, 2012, 4:51 p.m.
On Thu, Aug 09, 2012 at 11:05:47AM -0500, Matt Sealey wrote:
> On Thu, Aug 9, 2012 at 9:50 AM, Dave Martin <dave.martin@linaro.org> wrote:
> > On Thu, Aug 09, 2012 at 09:32:59AM -0500, Matt Sealey wrote:
> >> Matt Sealey <matt@genesi-usa.com>
> >> Product Development Analyst, Genesi USA, Inc.
> >>
> >>
> >> On Thu, Aug 9, 2012 at 5:24 AM, Dave Martin <dave.martin@linaro.org> wrote:
> >> > On Wed, Aug 08, 2012 at 12:32:39PM -0500, Matt Sealey wrote:
> >> >
> >> > [...]
> >> >
> >> >> I'm going to do a trapse through and find where Russell nacked Dave's
> >> >> thumb-aware
> >> >> rewrite.. would you mind if you have any of these boards seeing if it
> >> >> really DOES
> >> >
> >> > There was no NAK because I didn't get as far as posting the patch,
> >>
> >> http://lists.arm.linux.org.uk/lurker/message/20111202.133911.393b6e28.en.html
> >>
> >> You did, twice :)
> >
> > Well, I meant that I had not posted it as a stand-alone patch for
> > inclusion; I only posted it when commenting on other threads.
> >
> >> > mostly because of the doubt about whether this code is ever relevant
> >> > on Thumb2-capable hardware.
> >>
> >> It's not, but it does break the build, and without disabling phyCORE boards for
> >> Thumb2 it will always break the build..
> >
> > If you want me to push it, let me know.  I don't think this should get
> > merged without some Tested-bys for hardware where the FIQ stuff actually
> > gets used.
> 
> Please, then all the Tested-by's should flood in, right? :)

Do you have any suggestions for who I should CC to speed things up?

Cheers
---Dave

Patch

diff --git a/arch/arm/plat-mxc/ssi-fiq.S b/arch/arm/plat-mxc/ssi-fiq.S
index 8397a2d..3589afb 100644
--- a/arch/arm/plat-mxc/ssi-fiq.S
+++ b/arch/arm/plat-mxc/ssi-fiq.S
@@ -35,19 +35,19 @@ 
 		.global imx_ssi_fiq_tx_buffer
 
 imx_ssi_fiq_start:
-		ldr r12, imx_ssi_fiq_base
+		ldr r13, imx_ssi_fiq_base
 
 		/* TX */
 		ldr r11, imx_ssi_fiq_tx_buffer
 
 		/* shall we send? */
-		ldr r13, [r12, #SSI_SIER]
-		tst r13, #SSI_SIER_TFE0_EN
+		ldr r12, [r13, #SSI_SIER]
+		tst r12, #SSI_SIER_TFE0_EN
 		beq 1f
 
 		/* TX FIFO empty? */
-		ldr r13, [r12, #SSI_SISR]
-		tst r13, #SSI_SISR_TFE0
+		ldr r12, [r13, #SSI_SISR]
+		tst r12, #SSI_SISR_TFE0
 		beq 1f
 
 		mov r10, #0x10000
@@ -56,34 +56,34 @@  imx_ssi_fiq_start:
 
 		add r11, r11, r10
 
-		ldrh r13, [r11]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r12, [r11]
+		strh r12, [r13, #SSI_STX0]
 
-		ldrh r13, [r11, #2]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r12, [r11, #2]
+		strh r12, [r13, #SSI_STX0]
 
-		ldrh r13, [r11, #4]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r12, [r11, #4]
+		strh r12, [r13, #SSI_STX0]
 
-		ldrh r13, [r11, #6]
-		strh r13, [r12, #SSI_STX0]
+		ldrh r12, [r11, #6]
+		strh r12, [r13, #SSI_STX0]
 
 		add r10, #8
-		lsr r13, r8, #16	/* r13: buffer size */
-		cmp r10, r13
-		lslgt r8, r13, #16
+		lsr r12, r8, #16	/* r12: buffer size */
+		cmp r10, r12
+		lslgt r8, r12, #16
 		addle r8, #8
 1:
 		/* RX */
 
 		/* shall we receive? */
-		ldr r13, [r12, #SSI_SIER]
-		tst r13, #SSI_SIER_RFF0_EN
+		ldr r12, [r13, #SSI_SIER]
+		tst r12, #SSI_SIER_RFF0_EN
 		beq 1f
 
 		/* RX FIFO full? */
-		ldr r13, [r12, #SSI_SISR]
-		tst r13, #SSI_SISR_RFF0
+		ldr r12, [r13, #SSI_SISR]
+		tst r12, #SSI_SISR_RFF0
 		beq 1f
 
 		ldr r11, imx_ssi_fiq_rx_buffer
@@ -94,31 +94,31 @@  imx_ssi_fiq_start:
 
 		add r11, r11, r10
 
-		ldr r13, [r12, #SSI_SACNT]
-		tst r13, #SSI_SACNT_AC97EN
+		ldr r12, [r13, #SSI_SACNT]
+		tst r12, #SSI_SACNT_AC97EN
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11]
+		ldr r12, [r13, #SSI_SRX0]
+		strh r12, [r11]
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11, #2]
+		ldr r12, [r13, #SSI_SRX0]
+		strh r12, [r11, #2]
 
 		/* dummy read to skip slot 12 */
-		ldrne r13, [r12, #SSI_SRX0]
+		ldrne r12, [r13, #SSI_SRX0]
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11, #4]
+		ldr r12, [r13, #SSI_SRX0]
+		strh r12, [r11, #4]
 
-		ldr r13, [r12, #SSI_SRX0]
-		strh r13, [r11, #6]
+		ldr r12, [r13, #SSI_SRX0]
+		strh r12, [r11, #6]
 
 		/* dummy read to skip slot 12 */
-		ldrne r13, [r12, #SSI_SRX0]
+		ldrne r12, [r13, #SSI_SRX0]
 
 		add r10, #8
-		lsr r13, r9, #16	/* r13: buffer size */
-		cmp r10, r13
-		lslgt r9, r13, #16
+		lsr r12, r9, #16	/* r12: buffer size */
+		cmp r10, r12
+		lslgt r9, r12, #16
 		addle r9, #8
 
 1: