linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
@ 2011-02-02 18:53 Dave Martin
  2011-02-02 19:33 ` Nicolas Pitre
  0 siblings, 1 reply; 12+ messages in thread
From: Dave Martin @ 2011-02-02 18:53 UTC (permalink / raw)
  To: linux-arm-kernel

The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
a fault by attempting to store to address zero.

This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
allow negative register offsets at all for loads and stores.

The patch loads 0 into r2 and uses that as a base register
instead for the Thumb-2 case: r2 seems non-live at every
instance of this problem.

The ARM case is unaffected.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/lib/bitops.h |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
index f8a2bd3..87d6b17 100644
--- a/arch/arm/lib/bitops.h
+++ b/arch/arm/lib/bitops.h
@@ -1,7 +1,9 @@
 #if __LINUX_ARM_ARCH__ >= 6
 	.macro	bitop, instr
 	tst	r1, #3
-	strne	r1, [r1, -r1]		@ assert word-aligned
+ ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
+ THUMB(	movne	r2, #0		)
+ THUMB(	strne	r1, [r2]	)
 	mov	r2, #1
 	and	r3, r0, #31		@ Get bit offset
 	mov	r0, r0, lsr #5
@@ -17,7 +19,9 @@
 
 	.macro	testop, instr, store
 	tst	r1, #3
-	strne	r1, [r1, -r1]		@ assert word-aligned
+ ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
+ THUMB(	movne	r2, #0		)
+ THUMB(	strne	r1, [r2]	)
 	mov	r2, #1
 	and	r3, r0, #31		@ Get bit offset
 	mov	r0, r0, lsr #5
@@ -38,7 +42,9 @@
 #else
 	.macro	bitop, instr
 	tst	r1, #3
-	strne	r1, [r1, -r1]		@ assert word-aligned
+ ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
+ THUMB(	movne	r2, #0		)
+ THUMB(	strne	r1, [r2]	)
 	and	r2, r0, #31
 	mov	r0, r0, lsr #5
 	mov	r3, #1
@@ -61,7 +67,9 @@
  */
 	.macro	testop, instr, store
 	tst	r1, #3
-	strne	r1, [r1, -r1]		@ assert word-aligned
+ ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
+ THUMB(	movne	r2, #0		)
+ THUMB(	strne	r1, [r2]	)
 	and	r3, r0, #31
 	mov	r0, r0, lsr #5
 	save_and_disable_irqs ip
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 18:53 [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible Dave Martin
@ 2011-02-02 19:33 ` Nicolas Pitre
  2011-02-02 19:50   ` Russell King - ARM Linux
  2011-02-02 21:24   ` Russell King - ARM Linux
  0 siblings, 2 replies; 12+ messages in thread
From: Nicolas Pitre @ 2011-02-02 19:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2 Feb 2011, Dave Martin wrote:

> The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
> a fault by attempting to store to address zero.
> 
> This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
> allow negative register offsets at all for loads and stores.
> 
> The patch loads 0 into r2 and uses that as a base register
> instead for the Thumb-2 case: r2 seems non-live at every
> instance of this problem.
> 
> The ARM case is unaffected.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

Russell proposed a better solution already:

http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home

No idea why this wasn't folded in his series yet though.


Nicolas


> ---
>  arch/arm/lib/bitops.h |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
> index f8a2bd3..87d6b17 100644
> --- a/arch/arm/lib/bitops.h
> +++ b/arch/arm/lib/bitops.h
> @@ -1,7 +1,9 @@
>  #if __LINUX_ARM_ARCH__ >= 6
>  	.macro	bitop, instr
>  	tst	r1, #3
> -	strne	r1, [r1, -r1]		@ assert word-aligned
> + ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
> + THUMB(	movne	r2, #0		)
> + THUMB(	strne	r1, [r2]	)
>  	mov	r2, #1
>  	and	r3, r0, #31		@ Get bit offset
>  	mov	r0, r0, lsr #5
> @@ -17,7 +19,9 @@
>  
>  	.macro	testop, instr, store
>  	tst	r1, #3
> -	strne	r1, [r1, -r1]		@ assert word-aligned
> + ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
> + THUMB(	movne	r2, #0		)
> + THUMB(	strne	r1, [r2]	)
>  	mov	r2, #1
>  	and	r3, r0, #31		@ Get bit offset
>  	mov	r0, r0, lsr #5
> @@ -38,7 +42,9 @@
>  #else
>  	.macro	bitop, instr
>  	tst	r1, #3
> -	strne	r1, [r1, -r1]		@ assert word-aligned
> + ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
> + THUMB(	movne	r2, #0		)
> + THUMB(	strne	r1, [r2]	)
>  	and	r2, r0, #31
>  	mov	r0, r0, lsr #5
>  	mov	r3, #1
> @@ -61,7 +67,9 @@
>   */
>  	.macro	testop, instr, store
>  	tst	r1, #3
> -	strne	r1, [r1, -r1]		@ assert word-aligned
> + ARM(	strne	r1, [r1, -r1]	)	@ assert word-aligned
> + THUMB(	movne	r2, #0		)
> + THUMB(	strne	r1, [r2]	)
>  	and	r3, r0, #31
>  	mov	r0, r0, lsr #5
>  	save_and_disable_irqs ip
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 19:33 ` Nicolas Pitre
@ 2011-02-02 19:50   ` Russell King - ARM Linux
  2011-02-02 20:39     ` Nicolas Pitre
  2011-02-02 21:24   ` Russell King - ARM Linux
  1 sibling, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux @ 2011-02-02 19:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 02, 2011 at 02:33:55PM -0500, Nicolas Pitre wrote:
> On Wed, 2 Feb 2011, Dave Martin wrote:
> 
> > The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
> > a fault by attempting to store to address zero.
> > 
> > This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
> > allow negative register offsets at all for loads and stores.
> > 
> > The patch loads 0 into r2 and uses that as a base register
> > instead for the Thumb-2 case: r2 seems non-live at every
> > instance of this problem.
> > 
> > The ARM case is unaffected.
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> 
> Russell proposed a better solution already:
> 
> http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
> 
> No idea why this wasn't folded in his series yet though.

Because I haven't got a round tuit.

http://www.quantumenterprises.co.uk/roundtuit/traditional_round_tuits.htm

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 19:50   ` Russell King - ARM Linux
@ 2011-02-02 20:39     ` Nicolas Pitre
  2011-02-02 20:47       ` Russell King - ARM Linux
  0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Pitre @ 2011-02-02 20:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2 Feb 2011, Russell King - ARM Linux wrote:

> On Wed, Feb 02, 2011 at 02:33:55PM -0500, Nicolas Pitre wrote:
> > On Wed, 2 Feb 2011, Dave Martin wrote:
> > 
> > > The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
> > > a fault by attempting to store to address zero.
> > > 
> > > This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
> > > allow negative register offsets at all for loads and stores.
> > > 
> > > The patch loads 0 into r2 and uses that as a base register
> > > instead for the Thumb-2 case: r2 seems non-live at every
> > > instance of this problem.
> > > 
> > > The ARM case is unaffected.
> > > 
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > 
> > Russell proposed a better solution already:
> > 
> > http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
> > 
> > No idea why this wasn't folded in his series yet though.
> 
> Because I haven't got a round tuit.
> 
> http://www.quantumenterprises.co.uk/roundtuit/traditional_round_tuits.htm

LOL!  No problem, here's a virtual one.  :-)


Nicolas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 20:39     ` Nicolas Pitre
@ 2011-02-02 20:47       ` Russell King - ARM Linux
  0 siblings, 0 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2011-02-02 20:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 02, 2011 at 03:39:13PM -0500, Nicolas Pitre wrote:
> On Wed, 2 Feb 2011, Russell King - ARM Linux wrote:
> 
> > On Wed, Feb 02, 2011 at 02:33:55PM -0500, Nicolas Pitre wrote:
> > > On Wed, 2 Feb 2011, Dave Martin wrote:
> > > 
> > > > The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
> > > > a fault by attempting to store to address zero.
> > > > 
> > > > This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
> > > > allow negative register offsets at all for loads and stores.
> > > > 
> > > > The patch loads 0 into r2 and uses that as a base register
> > > > instead for the Thumb-2 case: r2 seems non-live at every
> > > > instance of this problem.
> > > > 
> > > > The ARM case is unaffected.
> > > > 
> > > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > > 
> > > Russell proposed a better solution already:
> > > 
> > > http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
> > > 
> > > No idea why this wasn't folded in his series yet though.
> > 
> > Because I haven't got a round tuit.
> > 
> > http://www.quantumenterprises.co.uk/roundtuit/traditional_round_tuits.htm
> 
> LOL!  No problem, here's a virtual one.  :-)

While you're waiting, you might like to get involved with the firestorm
which David Brown has started with GregKH over the visibility of my git
tree.  I'm beyond participating in such arguments.

Oh, and if I follow what GregKH is saying, I have to refuse to fold this
change in.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 19:33 ` Nicolas Pitre
  2011-02-02 19:50   ` Russell King - ARM Linux
@ 2011-02-02 21:24   ` Russell King - ARM Linux
  2011-02-02 21:54     ` Nicolas Pitre
                       ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2011-02-02 21:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 02, 2011 at 02:33:55PM -0500, Nicolas Pitre wrote:
> On Wed, 2 Feb 2011, Dave Martin wrote:
> 
> > The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
> > a fault by attempting to store to address zero.
> > 
> > This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
> > allow negative register offsets at all for loads and stores.
> > 
> > The patch loads 0 into r2 and uses that as a base register
> > instead for the Thumb-2 case: r2 seems non-live at every
> > instance of this problem.
> > 
> > The ARM case is unaffected.
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> 
> Russell proposed a better solution already:
> 
> http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
> 
> No idea why this wasn't folded in his series yet though.

Now added, and original tested-by's dropped.  It now needs re-testing.

8<----
Subject: [PATCH] ARM: bitops: ensure set/clear/change bitops take a word-aligned pointer

Add additional instructions to our assembly bitops functions to ensure
that they only operate on word-aligned pointers.  This will be necessary
when we switch these operations to use the word-based exclusive
operations.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/lib/bitops.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
index d422529..bd00551 100644
--- a/arch/arm/lib/bitops.h
+++ b/arch/arm/lib/bitops.h
@@ -1,6 +1,8 @@
 
 #if __LINUX_ARM_ARCH__ >= 6 && defined(CONFIG_CPU_32v6K)
 	.macro	bitop, instr
+	ands	ip, r1, #3
+	strneb	r1, [ip]		@ assert word-aligned
 	mov	r2, #1
 	and	r3, r0, #7		@ Get bit offset
 	add	r1, r1, r0, lsr #3	@ Get byte offset
@@ -14,6 +16,8 @@
 	.endm
 
 	.macro	testop, instr, store
+	ands	ip, r1, #3
+	strneb	r1, [ip]		@ assert word-aligned
 	and	r3, r0, #7		@ Get bit offset
 	mov	r2, #1
 	add	r1, r1, r0, lsr #3	@ Get byte offset
@@ -32,6 +36,8 @@
 	.endm
 #else
 	.macro	bitop, instr
+	ands	ip, r1, #3
+	strneb	r1, [ip]		@ assert word-aligned
 	and	r2, r0, #7
 	mov	r3, #1
 	mov	r3, r3, lsl r2
@@ -52,6 +58,8 @@
  * to avoid dirtying the data cache.
  */
 	.macro	testop, instr, store
+	ands	ip, r1, #3
+	strneb	r1, [ip]		@ assert word-aligned
 	add	r1, r1, r0, lsr #3
 	and	r3, r0, #7
 	mov	r0, #1
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 21:24   ` Russell King - ARM Linux
@ 2011-02-02 21:54     ` Nicolas Pitre
  2011-02-03 10:08     ` Dave Martin
  2011-02-03 10:44     ` [PATCH] ARM: bitops: Use BX instead of MOV PC,LR Dave Martin
  2 siblings, 0 replies; 12+ messages in thread
From: Nicolas Pitre @ 2011-02-02 21:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2 Feb 2011, Russell King - ARM Linux wrote:

> On Wed, Feb 02, 2011 at 02:33:55PM -0500, Nicolas Pitre wrote:
> > On Wed, 2 Feb 2011, Dave Martin wrote:
> > 
> > > The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
> > > a fault by attempting to store to address zero.
> > > 
> > > This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
> > > allow negative register offsets at all for loads and stores.
> > > 
> > > The patch loads 0 into r2 and uses that as a base register
> > > instead for the Thumb-2 case: r2 seems non-live at every
> > > instance of this problem.
> > > 
> > > The ARM case is unaffected.
> > > 
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > 
> > Russell proposed a better solution already:
> > 
> > http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
> > 
> > No idea why this wasn't folded in his series yet though.
> 
> Now added, and original tested-by's dropped.  It now needs re-testing.
> 
> 8<----
> Subject: [PATCH] ARM: bitops: ensure set/clear/change bitops take a word-aligned pointer
> 
> Add additional instructions to our assembly bitops functions to ensure
> that they only operate on word-aligned pointers.  This will be necessary
> when we switch these operations to use the word-based exclusive
> operations.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Tested-by: Nicolas Pitre <nicolas.pitre@linaro.org>

And this has been tested by many people already as this is the version 
I've been carrying in the Linaro kernel tree for a while.




> ---
>  arch/arm/lib/bitops.h |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
> index d422529..bd00551 100644
> --- a/arch/arm/lib/bitops.h
> +++ b/arch/arm/lib/bitops.h
> @@ -1,6 +1,8 @@
>  
>  #if __LINUX_ARM_ARCH__ >= 6 && defined(CONFIG_CPU_32v6K)
>  	.macro	bitop, instr
> +	ands	ip, r1, #3
> +	strneb	r1, [ip]		@ assert word-aligned
>  	mov	r2, #1
>  	and	r3, r0, #7		@ Get bit offset
>  	add	r1, r1, r0, lsr #3	@ Get byte offset
> @@ -14,6 +16,8 @@
>  	.endm
>  
>  	.macro	testop, instr, store
> +	ands	ip, r1, #3
> +	strneb	r1, [ip]		@ assert word-aligned
>  	and	r3, r0, #7		@ Get bit offset
>  	mov	r2, #1
>  	add	r1, r1, r0, lsr #3	@ Get byte offset
> @@ -32,6 +36,8 @@
>  	.endm
>  #else
>  	.macro	bitop, instr
> +	ands	ip, r1, #3
> +	strneb	r1, [ip]		@ assert word-aligned
>  	and	r2, r0, #7
>  	mov	r3, #1
>  	mov	r3, r3, lsl r2
> @@ -52,6 +58,8 @@
>   * to avoid dirtying the data cache.
>   */
>  	.macro	testop, instr, store
> +	ands	ip, r1, #3
> +	strneb	r1, [ip]		@ assert word-aligned
>  	add	r1, r1, r0, lsr #3
>  	and	r3, r0, #7
>  	mov	r0, #1
> -- 
> 1.6.2.5
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible
  2011-02-02 21:24   ` Russell King - ARM Linux
  2011-02-02 21:54     ` Nicolas Pitre
@ 2011-02-03 10:08     ` Dave Martin
  2011-02-03 10:44     ` [PATCH] ARM: bitops: Use BX instead of MOV PC,LR Dave Martin
  2 siblings, 0 replies; 12+ messages in thread
From: Dave Martin @ 2011-02-03 10:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 2, 2011 at 9:24 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 02, 2011 at 02:33:55PM -0500, Nicolas Pitre wrote:
>> On Wed, 2 Feb 2011, Dave Martin wrote:
>>
>> > The new bitops code cunningly str <Rt>, [r1, -r1] to trigger
>> > a fault by attempting to store to address zero.
>> >
>> > This code doesn't assemble in Thumb-2, since Thumb-2 doesn't
>> > allow negative register offsets at all for loads and stores.
>> >
>> > The patch loads 0 into r2 and uses that as a base register
>> > instead for the Thumb-2 case: r2 seems non-live at every
>> > instance of this problem.
>> >
>> > The ARM case is unaffected.
>> >
>> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
>>
>> Russell proposed a better solution already:
>>
>> http://mid.gmane.org/alpine.LFD.2.00.1101181312360.8580 at xanadu.home
>>
>> No idea why this wasn't folded in his series yet though.
>
> Now added, and original tested-by's dropped. ?It now needs re-testing.
>
> 8<----
> Subject: [PATCH] ARM: bitops: ensure set/clear/change bitops take a word-aligned pointer
>
> Add additional instructions to our assembly bitops functions to ensure
> that they only operate on word-aligned pointers. ?This will be necessary
> when we switch these operations to use the word-based exclusive
> operations.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> ?arch/arm/lib/bitops.h | ? ?8 ++++++++
> ?1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
> index d422529..bd00551 100644
> --- a/arch/arm/lib/bitops.h
> +++ b/arch/arm/lib/bitops.h
> @@ -1,6 +1,8 @@
>
> ?#if __LINUX_ARM_ARCH__ >= 6 && defined(CONFIG_CPU_32v6K)
> ? ? ? ?.macro ?bitop, instr
> + ? ? ? ands ? ?ip, r1, #3
> + ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned
> ? ? ? ?mov ? ? r2, #1
> ? ? ? ?and ? ? r3, r0, #7 ? ? ? ? ? ? ?@ Get bit offset
> ? ? ? ?add ? ? r1, r1, r0, lsr #3 ? ? ?@ Get byte offset
> @@ -14,6 +16,8 @@
> ? ? ? ?.endm
>
> ? ? ? ?.macro ?testop, instr, store
> + ? ? ? ands ? ?ip, r1, #3
> + ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned
> ? ? ? ?and ? ? r3, r0, #7 ? ? ? ? ? ? ?@ Get bit offset
> ? ? ? ?mov ? ? r2, #1
> ? ? ? ?add ? ? r1, r1, r0, lsr #3 ? ? ?@ Get byte offset
> @@ -32,6 +36,8 @@
> ? ? ? ?.endm
> ?#else
> ? ? ? ?.macro ?bitop, instr
> + ? ? ? ands ? ?ip, r1, #3
> + ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned
> ? ? ? ?and ? ? r2, r0, #7
> ? ? ? ?mov ? ? r3, #1
> ? ? ? ?mov ? ? r3, r3, lsl r2
> @@ -52,6 +58,8 @@
> ?* to avoid dirtying the data cache.
> ?*/
> ? ? ? ?.macro ?testop, instr, store
> + ? ? ? ands ? ?ip, r1, #3
> + ? ? ? strneb ?r1, [ip] ? ? ? ? ? ? ? ?@ assert word-aligned
> ? ? ? ?add ? ? r1, r1, r0, lsr #3
> ? ? ? ?and ? ? r3, r0, #7
> ? ? ? ?mov ? ? r0, #1
> --
> 1.6.2.5
>
>

Heh, yes, that's a nicer fix.  I'll give it a try.

Thanks
---Dave

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ARM: bitops: Use BX instead of MOV PC,LR
  2011-02-02 21:24   ` Russell King - ARM Linux
  2011-02-02 21:54     ` Nicolas Pitre
  2011-02-03 10:08     ` Dave Martin
@ 2011-02-03 10:44     ` Dave Martin
  2011-02-03 10:47       ` [PATCH REPOST] " Dave Martin
  2 siblings, 1 reply; 12+ messages in thread
From: Dave Martin @ 2011-02-03 10:44 UTC (permalink / raw)
  To: linux-arm-kernel

The kernel doesn't officially need to interwork, but using BX
wherever appropriate will help educate people into good assembler
coding habits.

BX is appropriate here because this code is predicated on

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---

A bit of a pedantic one, but is there any chance of switching
to using BX here?

I spend a significant amount of my life trying to educate people
not to do things like mov pc,lr ... though in interworking userspace
is admittedly a different world from the kernel.

Cheers
Dave

 arch/arm/lib/bitops.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
index a9d9d15..5527e23 100644
--- a/arch/arm/lib/bitops.h
+++ b/arch/arm/lib/bitops.h
@@ -12,7 +12,7 @@
 	strex	r0, r2, [r1]
 	cmp	r0, #0
 	bne	1b
-	mov	pc, lr
+	bx	lr
 	.endm
 
 	.macro	testop, instr, store
@@ -33,7 +33,7 @@
 	smp_dmb
 	cmp	r0, #0
 	movne	r0, #1
-2:	mov	pc, lr
+2:	bx	lr
 	.endm
 #else
 	.macro	bitop, instr
@@ -48,7 +48,7 @@
 	\instr	r2, r2, r3
 	str	r2, [r1, r0, lsl #2]
 	restore_irqs ip
-	mov	pc, lr
+	bx	lr
 	.endm
 
 /**
@@ -72,6 +72,6 @@
 	\store	r2, [r1]
 	moveq	r0, #0
 	restore_irqs ip
-	mov	pc, lr
+	bx	lr
 	.endm
 #endif
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH REPOST] ARM: bitops: Use BX instead of MOV PC,LR
  2011-02-03 10:44     ` [PATCH] ARM: bitops: Use BX instead of MOV PC,LR Dave Martin
@ 2011-02-03 10:47       ` Dave Martin
  2011-02-03 16:27         ` Rabin Vincent
  0 siblings, 1 reply; 12+ messages in thread
From: Dave Martin @ 2011-02-03 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

The kernel doesn't officially need to interwork, but using BX
wherever appropriate will help educate people into good assembler
coding habits.

BX is appropriate here because this code is predicated on
__LINUX_ARM_ARCH__ >= 6.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---

Reposting ... git ate my commit message :/

 arch/arm/lib/bitops.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
index a9d9d15..5527e23 100644
--- a/arch/arm/lib/bitops.h
+++ b/arch/arm/lib/bitops.h
@@ -12,7 +12,7 @@
 	strex	r0, r2, [r1]
 	cmp	r0, #0
 	bne	1b
-	mov	pc, lr
+	bx	lr
 	.endm
 
 	.macro	testop, instr, store
@@ -33,7 +33,7 @@
 	smp_dmb
 	cmp	r0, #0
 	movne	r0, #1
-2:	mov	pc, lr
+2:	bx	lr
 	.endm
 #else
 	.macro	bitop, instr
@@ -48,7 +48,7 @@
 	\instr	r2, r2, r3
 	str	r2, [r1, r0, lsl #2]
 	restore_irqs ip
-	mov	pc, lr
+	bx	lr
 	.endm
 
 /**
@@ -72,6 +72,6 @@
 	\store	r2, [r1]
 	moveq	r0, #0
 	restore_irqs ip
-	mov	pc, lr
+	bx	lr
 	.endm
 #endif
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH REPOST] ARM: bitops: Use BX instead of MOV PC,LR
  2011-02-03 10:47       ` [PATCH REPOST] " Dave Martin
@ 2011-02-03 16:27         ` Rabin Vincent
  2011-02-03 18:06           ` Dave Martin
  0 siblings, 1 reply; 12+ messages in thread
From: Rabin Vincent @ 2011-02-03 16:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 3, 2011 at 16:17, Dave Martin <dave.martin@linaro.org> wrote:
> BX is appropriate here because this code is predicated on
> __LINUX_ARM_ARCH__ >= 6.

But you're modifying the #else portion too?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH REPOST] ARM: bitops: Use BX instead of MOV PC,LR
  2011-02-03 16:27         ` Rabin Vincent
@ 2011-02-03 18:06           ` Dave Martin
  0 siblings, 0 replies; 12+ messages in thread
From: Dave Martin @ 2011-02-03 18:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 3, 2011 at 4:27 PM, Rabin Vincent <rabin@rab.in> wrote:
> On Thu, Feb 3, 2011 at 16:17, Dave Martin <dave.martin@linaro.org> wrote:
>> BX is appropriate here because this code is predicated on
>> __LINUX_ARM_ARCH__ >= 6.
>
> But you're modifying the #else portion too?
>

Darn, you're quite right... thanks for spotting that!

Will repost.

Cheers
---Dave

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-02-03 18:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-02 18:53 [PATCH] ARM: bitops: Fix low-level code to be Thumb-2 compatible Dave Martin
2011-02-02 19:33 ` Nicolas Pitre
2011-02-02 19:50   ` Russell King - ARM Linux
2011-02-02 20:39     ` Nicolas Pitre
2011-02-02 20:47       ` Russell King - ARM Linux
2011-02-02 21:24   ` Russell King - ARM Linux
2011-02-02 21:54     ` Nicolas Pitre
2011-02-03 10:08     ` Dave Martin
2011-02-03 10:44     ` [PATCH] ARM: bitops: Use BX instead of MOV PC,LR Dave Martin
2011-02-03 10:47       ` [PATCH REPOST] " Dave Martin
2011-02-03 16:27         ` Rabin Vincent
2011-02-03 18:06           ` Dave Martin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).