All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-09 15:01 ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Dave Martin, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

This set of patches, along with some other patches under
discussion on alkml, should enable omap3 and omap4 kernels to be
built with CONFIG_THUMB2_KERNEL.

This patch set builds on recent cleanup done by the omap
maintainers.

All affected low-level code is now built in Thumb-2.  At least some
of this code definitely works, most features have been tested
successfully.  Further testing, especially of Thumb-2 behaviour, is
still welcome.

Seems to work with CONFIG_SMP_ON_UP and CONFIG_THUMB2_KERNEL
enabled on Beagle xM A2 and Panda A1.

Thanks also to Santosh Shilimkar <santosh.shilimkar@ti.com>
for his help with testing.

v3:
    * make SMC instruction syntax more consistent
    * remove do_wfi() in favour of the generic wfi() macro

v4:
    * revert to the OMAP-specific do_wfi() implementation, pending
      discussion of how generic macros can/should be provided

Details below.

Cheers,
Dave


The patches can be found, along with a buildable working tree,
in the following repo:

git://git.linaro.org/people/dmart/linux-2.6-arm.git

 * arm/omap-thumb2: has the patches proposed here
 * arm/omap-thumb2+merged: additionally has some patches cherry-
        picked from other trees which are needed in order for the
        patches on arm/omap-thumb2 to work usefully.
 * dirty/arm/omap-thumb2+merged: buildable test tree, which adds
        2 local patches to work around a toolchain bug.

A working kernel config for this tree is here:
http://people.linaro.org/~dmart/arm_omap-thumb2+v2_config :

  CONFIG_SMP_ON_UP=y
  CONFIG_THUMB2_KERNEL=y
  CONFIG_SERIAL_OMAP=n (to avoid garbage on xM; for Panda use console=ttyS2)

(The config is derived from the linaro omap config and so turns on
loads of modules -- don't feel you have to build them all...)


Cherry-picked patches originated from Russell's devel tree
and Tony Lindgren's omap-testing tree:

http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/linux-2.6-arm.git devel

git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6.git omap-testing

This series also depends on one un-merged patch to generic-ise
the wfi() macro:

git://git.linaro.org/people/dmart/linux-2.6-arm.git arm/wfi-macro

Dave Martin (5):
  ARM: omap4: Provide do_wfi() for Thumb-2
  ARM: omap4: Convert END() to ENDPROC() for correct linkage with
    CONFIG_THUMB2_KERNEL
  ARM: omap3: Remove hand-encoded SMC instructions
  ARM: omap3: Thumb-2 compatibility for sram34xx.S
  ARM: omap3: Thumb-2 compatibility for sleep34xx.S

 arch/arm/mach-omap2/include/mach/omap4-common.h |    4 ++
 arch/arm/mach-omap2/omap-headsmp.S              |    2 +-
 arch/arm/mach-omap2/omap44xx-smc.S              |    8 ++--
 arch/arm/mach-omap2/sleep34xx.S                 |   56 ++++++++++++++++-------
 arch/arm/mach-omap2/sram34xx.S                  |   28 ++++++++---
 5 files changed, 68 insertions(+), 30 deletions(-)


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-09 15:01 ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

This set of patches, along with some other patches under
discussion on alkml, should enable omap3 and omap4 kernels to be
built with CONFIG_THUMB2_KERNEL.

This patch set builds on recent cleanup done by the omap
maintainers.

All affected low-level code is now built in Thumb-2.  At least some
of this code definitely works, most features have been tested
successfully.  Further testing, especially of Thumb-2 behaviour, is
still welcome.

Seems to work with CONFIG_SMP_ON_UP and CONFIG_THUMB2_KERNEL
enabled on Beagle xM A2 and Panda A1.

Thanks also to Santosh Shilimkar <santosh.shilimkar@ti.com>
for his help with testing.

v3:
    * make SMC instruction syntax more consistent
    * remove do_wfi() in favour of the generic wfi() macro

v4:
    * revert to the OMAP-specific do_wfi() implementation, pending
      discussion of how generic macros can/should be provided

Details below.

Cheers,
Dave


The patches can be found, along with a buildable working tree,
in the following repo:

git://git.linaro.org/people/dmart/linux-2.6-arm.git

 * arm/omap-thumb2: has the patches proposed here
 * arm/omap-thumb2+merged: additionally has some patches cherry-
        picked from other trees which are needed in order for the
        patches on arm/omap-thumb2 to work usefully.
 * dirty/arm/omap-thumb2+merged: buildable test tree, which adds
        2 local patches to work around a toolchain bug.

A working kernel config for this tree is here:
http://people.linaro.org/~dmart/arm_omap-thumb2+v2_config :

  CONFIG_SMP_ON_UP=y
  CONFIG_THUMB2_KERNEL=y
  CONFIG_SERIAL_OMAP=n (to avoid garbage on xM; for Panda use console=ttyS2)

(The config is derived from the linaro omap config and so turns on
loads of modules -- don't feel you have to build them all...)


Cherry-picked patches originated from Russell's devel tree
and Tony Lindgren's omap-testing tree:

http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/linux-2.6-arm.git devel

git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6.git omap-testing

This series also depends on one un-merged patch to generic-ise
the wfi() macro:

git://git.linaro.org/people/dmart/linux-2.6-arm.git arm/wfi-macro

Dave Martin (5):
  ARM: omap4: Provide do_wfi() for Thumb-2
  ARM: omap4: Convert END() to ENDPROC() for correct linkage with
    CONFIG_THUMB2_KERNEL
  ARM: omap3: Remove hand-encoded SMC instructions
  ARM: omap3: Thumb-2 compatibility for sram34xx.S
  ARM: omap3: Thumb-2 compatibility for sleep34xx.S

 arch/arm/mach-omap2/include/mach/omap4-common.h |    4 ++
 arch/arm/mach-omap2/omap-headsmp.S              |    2 +-
 arch/arm/mach-omap2/omap44xx-smc.S              |    8 ++--
 arch/arm/mach-omap2/sleep34xx.S                 |   56 ++++++++++++++++-------
 arch/arm/mach-omap2/sram34xx.S                  |   28 ++++++++---
 5 files changed, 68 insertions(+), 30 deletions(-)

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 1/5] ARM: omap4: Provide do_wfi() for Thumb-2
  2011-02-09 15:01 ` Dave Martin
@ 2011-02-09 15:01   ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Dave Martin, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

For CONFIG_THUMB2_KERNEL, the existing definition of do_wfi() will
insert invalid code into the instruction stream.

Any assembler which can assemble Thumb-2 is guaranteed to accept
the "wfi" mnemonic, so for the Thumb-2 case, just use the mnemonic.

The ARM case is left as-is.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/mach-omap2/include/mach/omap4-common.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h b/arch/arm/mach-omap2/include/mach/omap4-common.h
index 5b0270b..de441c0 100644
--- a/arch/arm/mach-omap2/include/mach/omap4-common.h
+++ b/arch/arm/mach-omap2/include/mach/omap4-common.h
@@ -17,8 +17,12 @@
  * wfi used in low power code. Directly opcode is used instead
  * of instruction to avoid mulit-omap build break
  */
+#ifdef CONFIG_THUMB2_KERNEL
+#define do_wfi() __asm__ __volatile__ ("wfi" : : : "memory")
+#else
 #define do_wfi()			\
 		__asm__ __volatile__ (".word	0xe320f003" : : : "memory")
+#endif
 
 #ifdef CONFIG_CACHE_L2X0
 extern void __iomem *l2cache_base;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 1/5] ARM: omap4: Provide do_wfi() for Thumb-2
@ 2011-02-09 15:01   ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

For CONFIG_THUMB2_KERNEL, the existing definition of do_wfi() will
insert invalid code into the instruction stream.

Any assembler which can assemble Thumb-2 is guaranteed to accept
the "wfi" mnemonic, so for the Thumb-2 case, just use the mnemonic.

The ARM case is left as-is.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/mach-omap2/include/mach/omap4-common.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h b/arch/arm/mach-omap2/include/mach/omap4-common.h
index 5b0270b..de441c0 100644
--- a/arch/arm/mach-omap2/include/mach/omap4-common.h
+++ b/arch/arm/mach-omap2/include/mach/omap4-common.h
@@ -17,8 +17,12 @@
  * wfi used in low power code. Directly opcode is used instead
  * of instruction to avoid mulit-omap build break
  */
+#ifdef CONFIG_THUMB2_KERNEL
+#define do_wfi() __asm__ __volatile__ ("wfi" : : : "memory")
+#else
 #define do_wfi()			\
 		__asm__ __volatile__ (".word	0xe320f003" : : : "memory")
+#endif
 
 #ifdef CONFIG_CACHE_L2X0
 extern void __iomem *l2cache_base;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 2/5] ARM: omap4: Convert END() to ENDPROC() for correct linkage with CONFIG_THUMB2_KERNEL
  2011-02-09 15:01 ` Dave Martin
@ 2011-02-09 15:01   ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Dave Martin, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

Code marked with ENTRY() also needs a matching ENDPROC() directive,
in order to ensure that the type and instruction set of the
symbol are correctly annotated.

ENDPROC() tags the affected symbol as a function symbol, which will
ensure that link-time fixups don't accidentally switch to the
wrong instruction set.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap-headsmp.S |    2 +-
 arch/arm/mach-omap2/omap44xx-smc.S |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-headsmp.S b/arch/arm/mach-omap2/omap-headsmp.S
index 6ae937a..4ee6aec 100644
--- a/arch/arm/mach-omap2/omap-headsmp.S
+++ b/arch/arm/mach-omap2/omap-headsmp.S
@@ -45,5 +45,5 @@ hold:	ldr	r12,=0x103
 	 * should now contain the SVC stack for this core
 	 */
 	b	secondary_startup
-END(omap_secondary_startup)
+ENDPROC(omap_secondary_startup)
 
diff --git a/arch/arm/mach-omap2/omap44xx-smc.S b/arch/arm/mach-omap2/omap44xx-smc.S
index 1980dc3..e69d37d 100644
--- a/arch/arm/mach-omap2/omap44xx-smc.S
+++ b/arch/arm/mach-omap2/omap44xx-smc.S
@@ -29,7 +29,7 @@ ENTRY(omap_smc1)
 	dsb
 	smc	#0
 	ldmfd   sp!, {r2-r12, pc}
-END(omap_smc1)
+ENDPROC(omap_smc1)
 
 ENTRY(omap_modify_auxcoreboot0)
 	stmfd   sp!, {r1-r12, lr}
@@ -37,7 +37,7 @@ ENTRY(omap_modify_auxcoreboot0)
 	dsb
 	smc	#0
 	ldmfd   sp!, {r1-r12, pc}
-END(omap_modify_auxcoreboot0)
+ENDPROC(omap_modify_auxcoreboot0)
 
 ENTRY(omap_auxcoreboot_addr)
 	stmfd   sp!, {r2-r12, lr}
@@ -45,7 +45,7 @@ ENTRY(omap_auxcoreboot_addr)
 	dsb
 	smc	#0
 	ldmfd   sp!, {r2-r12, pc}
-END(omap_auxcoreboot_addr)
+ENDPROC(omap_auxcoreboot_addr)
 
 ENTRY(omap_read_auxcoreboot0)
 	stmfd   sp!, {r2-r12, lr}
@@ -54,4 +54,4 @@ ENTRY(omap_read_auxcoreboot0)
 	smc	#0
 	mov	r0, r0, lsr #9
 	ldmfd   sp!, {r2-r12, pc}
-END(omap_read_auxcoreboot0)
+ENDPROC(omap_read_auxcoreboot0)
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 2/5] ARM: omap4: Convert END() to ENDPROC() for correct linkage with CONFIG_THUMB2_KERNEL
@ 2011-02-09 15:01   ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

Code marked with ENTRY() also needs a matching ENDPROC() directive,
in order to ensure that the type and instruction set of the
symbol are correctly annotated.

ENDPROC() tags the affected symbol as a function symbol, which will
ensure that link-time fixups don't accidentally switch to the
wrong instruction set.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap-headsmp.S |    2 +-
 arch/arm/mach-omap2/omap44xx-smc.S |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-headsmp.S b/arch/arm/mach-omap2/omap-headsmp.S
index 6ae937a..4ee6aec 100644
--- a/arch/arm/mach-omap2/omap-headsmp.S
+++ b/arch/arm/mach-omap2/omap-headsmp.S
@@ -45,5 +45,5 @@ hold:	ldr	r12,=0x103
 	 * should now contain the SVC stack for this core
 	 */
 	b	secondary_startup
-END(omap_secondary_startup)
+ENDPROC(omap_secondary_startup)
 
diff --git a/arch/arm/mach-omap2/omap44xx-smc.S b/arch/arm/mach-omap2/omap44xx-smc.S
index 1980dc3..e69d37d 100644
--- a/arch/arm/mach-omap2/omap44xx-smc.S
+++ b/arch/arm/mach-omap2/omap44xx-smc.S
@@ -29,7 +29,7 @@ ENTRY(omap_smc1)
 	dsb
 	smc	#0
 	ldmfd   sp!, {r2-r12, pc}
-END(omap_smc1)
+ENDPROC(omap_smc1)
 
 ENTRY(omap_modify_auxcoreboot0)
 	stmfd   sp!, {r1-r12, lr}
@@ -37,7 +37,7 @@ ENTRY(omap_modify_auxcoreboot0)
 	dsb
 	smc	#0
 	ldmfd   sp!, {r1-r12, pc}
-END(omap_modify_auxcoreboot0)
+ENDPROC(omap_modify_auxcoreboot0)
 
 ENTRY(omap_auxcoreboot_addr)
 	stmfd   sp!, {r2-r12, lr}
@@ -45,7 +45,7 @@ ENTRY(omap_auxcoreboot_addr)
 	dsb
 	smc	#0
 	ldmfd   sp!, {r2-r12, pc}
-END(omap_auxcoreboot_addr)
+ENDPROC(omap_auxcoreboot_addr)
 
 ENTRY(omap_read_auxcoreboot0)
 	stmfd   sp!, {r2-r12, lr}
@@ -54,4 +54,4 @@ ENTRY(omap_read_auxcoreboot0)
 	smc	#0
 	mov	r0, r0, lsr #9
 	ldmfd   sp!, {r2-r12, pc}
-END(omap_read_auxcoreboot0)
+ENDPROC(omap_read_auxcoreboot0)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 3/5] ARM: omap3: Remove hand-encoded SMC instructions
  2011-02-09 15:01 ` Dave Martin
@ 2011-02-09 15:01   ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Dave Martin, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

For various reasons, Linux now only officially supports being built
with tools which are new enough to understand the SMC instruction.

Replacing the hand-encoded instructions when the mnemonic also
allows for correct assembly in Thumb-2 (otherwise, the result is
random data in the middle of the code).

The Makefile already ensures that this file is built with a high
enough gcc -march= flag (armv7-a).

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/sleep34xx.S |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 98d8232..a05c348 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -133,7 +133,7 @@ ENTRY(save_secure_ram_context)
 	mov	r6, #0xff
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 	nop
 	nop
 	nop
@@ -408,7 +408,7 @@ skipl2dis:
 	adr	r3, l2_inv_api_params	@ r3 points to dummy parameters
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 	/* Write to Aux control register to set some bits */
 	mov	r0, #42			@ set service ID for PPA
 	mov	r12, r0			@ copy secure Service ID in r12
@@ -419,7 +419,7 @@ skipl2dis:
 	ldr	r3, [r4, #0xBC]		@ r3 points to parameters
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 
 #ifdef CONFIG_OMAP3_L2_AUX_SECURE_SAVE_RESTORE
 	/* Restore L2 aux control register */
@@ -434,7 +434,7 @@ skipl2dis:
 	adds	r3, r3, #8		@ r3 points to parameters
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 #endif
 	b	logic_l1_restore
 
@@ -443,18 +443,18 @@ l2_inv_api_params:
 l2_inv_gp:
 	/* Execute smi to invalidate L2 cache */
 	mov r12, #0x1			@ set up to invalidate L2
-	.word 0xE1600070		@ Call SMI monitor (smieq)
+	smc	#0			@ Call SMI monitor (smieq)
 	/* Write to Aux control register to set some bits */
 	ldr	r4, scratchpad_base
 	ldr	r3, [r4,#0xBC]
 	ldr	r0, [r3,#4]
 	mov	r12, #0x3
-	.word	0xE1600070		@ Call SMI monitor (smieq)
+	smc	#0			@ Call SMI monitor (smieq)
 	ldr	r4, scratchpad_base
 	ldr	r3, [r4,#0xBC]
 	ldr	r0, [r3,#12]
 	mov	r12, #0x2
-	.word	0xE1600070		@ Call SMI monitor (smieq)
+	smc	#0			@ Call SMI monitor (smieq)
 logic_l1_restore:
 	ldr	r1, l2dis_3630
 	cmp	r1, #0x1		@ Test if L2 re-enable needed on 3630
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 3/5] ARM: omap3: Remove hand-encoded SMC instructions
@ 2011-02-09 15:01   ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

For various reasons, Linux now only officially supports being built
with tools which are new enough to understand the SMC instruction.

Replacing the hand-encoded instructions when the mnemonic also
allows for correct assembly in Thumb-2 (otherwise, the result is
random data in the middle of the code).

The Makefile already ensures that this file is built with a high
enough gcc -march= flag (armv7-a).

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/sleep34xx.S |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 98d8232..a05c348 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -133,7 +133,7 @@ ENTRY(save_secure_ram_context)
 	mov	r6, #0xff
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 	nop
 	nop
 	nop
@@ -408,7 +408,7 @@ skipl2dis:
 	adr	r3, l2_inv_api_params	@ r3 points to dummy parameters
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 	/* Write to Aux control register to set some bits */
 	mov	r0, #42			@ set service ID for PPA
 	mov	r12, r0			@ copy secure Service ID in r12
@@ -419,7 +419,7 @@ skipl2dis:
 	ldr	r3, [r4, #0xBC]		@ r3 points to parameters
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 
 #ifdef CONFIG_OMAP3_L2_AUX_SECURE_SAVE_RESTORE
 	/* Restore L2 aux control register */
@@ -434,7 +434,7 @@ skipl2dis:
 	adds	r3, r3, #8		@ r3 points to parameters
 	mcr	p15, 0, r0, c7, c10, 4	@ data write barrier
 	mcr	p15, 0, r0, c7, c10, 5	@ data memory barrier
-	.word	0xE1600071		@ call SMI monitor (smi #1)
+	smc	#1			@ call SMI monitor (smi #1)
 #endif
 	b	logic_l1_restore
 
@@ -443,18 +443,18 @@ l2_inv_api_params:
 l2_inv_gp:
 	/* Execute smi to invalidate L2 cache */
 	mov r12, #0x1			@ set up to invalidate L2
-	.word 0xE1600070		@ Call SMI monitor (smieq)
+	smc	#0			@ Call SMI monitor (smieq)
 	/* Write to Aux control register to set some bits */
 	ldr	r4, scratchpad_base
 	ldr	r3, [r4,#0xBC]
 	ldr	r0, [r3,#4]
 	mov	r12, #0x3
-	.word	0xE1600070		@ Call SMI monitor (smieq)
+	smc	#0			@ Call SMI monitor (smieq)
 	ldr	r4, scratchpad_base
 	ldr	r3, [r4,#0xBC]
 	ldr	r0, [r3,#12]
 	mov	r12, #0x2
-	.word	0xE1600070		@ Call SMI monitor (smieq)
+	smc	#0			@ Call SMI monitor (smieq)
 logic_l1_restore:
 	ldr	r1, l2dis_3630
 	cmp	r1, #0x1		@ Test if L2 re-enable needed on 3630
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 4/5] ARM: omap3: Thumb-2 compatibility for sram34xx.S
  2011-02-09 15:01 ` Dave Martin
@ 2011-02-09 15:01   ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Dave Martin, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

 * Remove deprecated/undefined PC-relative stores
 * Add the required ENDPROC() directive for each ENTRY().
 * .align before data words

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/sram34xx.S |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 7f893a2..829d235 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -116,24 +116,34 @@ ENTRY(omap3_sram_configure_core_dpll)
 
 					@ pull the extra args off the stack
 					@  and store them in SRAM
+
+@ PC-relative stores lead to undefined behaviour in Thumb-2: use a r7 as a
+@ base instead.
+@ Be careful not to clobber r7 when maintaing this file.
+ THUMB(	adr	r7, omap3_sram_configure_core_dpll			)
+	.macro strtext Rt:req, label:req
+ ARM(	str	\Rt, \label						)
+ THUMB(	str	\Rt, [r7, \label - omap3_sram_configure_core_dpll]	)
+	.endm
+
 	ldr	r4, [sp, #52]
-	str     r4, omap_sdrc_rfr_ctrl_0_val
+	strtext	r4, omap_sdrc_rfr_ctrl_0_val
 	ldr	r4, [sp, #56]
-	str     r4, omap_sdrc_actim_ctrl_a_0_val
+	strtext	r4, omap_sdrc_actim_ctrl_a_0_val
 	ldr	r4, [sp, #60]
-	str     r4, omap_sdrc_actim_ctrl_b_0_val
+	strtext	r4, omap_sdrc_actim_ctrl_b_0_val
 	ldr	r4, [sp, #64]
-	str     r4, omap_sdrc_mr_0_val
+	strtext	r4, omap_sdrc_mr_0_val
 	ldr	r4, [sp, #68]
-	str     r4, omap_sdrc_rfr_ctrl_1_val
+	strtext	r4, omap_sdrc_rfr_ctrl_1_val
 	cmp	r4, #0			@ if SDRC_RFR_CTRL_1 is 0,
 	beq	skip_cs1_params		@  do not use cs1 params
 	ldr	r4, [sp, #72]
-	str     r4, omap_sdrc_actim_ctrl_a_1_val
+	strtext	r4, omap_sdrc_actim_ctrl_a_1_val
 	ldr	r4, [sp, #76]
-	str     r4, omap_sdrc_actim_ctrl_b_1_val
+	strtext	r4, omap_sdrc_actim_ctrl_b_1_val
 	ldr	r4, [sp, #80]
-	str     r4, omap_sdrc_mr_1_val
+	strtext	r4, omap_sdrc_mr_1_val
 skip_cs1_params:
 	mrc	p15, 0, r8, c1, c0, 0	@ read ctrl register
 	bic	r10, r8, #0x800		@ clear Z-bit, disable branch prediction
@@ -271,6 +281,7 @@ skip_cs1_prog:
 	ldr	r12, [r11]		@ posted-write barrier for SDRC
 	bx	lr
 
+	.align
 omap3_sdrc_power:
 	.word OMAP34XX_SDRC_REGADDR(SDRC_POWER)
 omap3_cm_clksel1_pll:
@@ -319,6 +330,7 @@ omap3_sdrc_dlla_ctrl:
 	.word OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
 core_m2_mask_val:
 	.word 0x07FFFFFF
+ENDPROC(omap3_sram_configure_core_dpll)
 
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 4/5] ARM: omap3: Thumb-2 compatibility for sram34xx.S
@ 2011-02-09 15:01   ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

 * Remove deprecated/undefined PC-relative stores
 * Add the required ENDPROC() directive for each ENTRY().
 * .align before data words

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/sram34xx.S |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 7f893a2..829d235 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -116,24 +116,34 @@ ENTRY(omap3_sram_configure_core_dpll)
 
 					@ pull the extra args off the stack
 					@  and store them in SRAM
+
+@ PC-relative stores lead to undefined behaviour in Thumb-2: use a r7 as a
+@ base instead.
+@ Be careful not to clobber r7 when maintaing this file.
+ THUMB(	adr	r7, omap3_sram_configure_core_dpll			)
+	.macro strtext Rt:req, label:req
+ ARM(	str	\Rt, \label						)
+ THUMB(	str	\Rt, [r7, \label - omap3_sram_configure_core_dpll]	)
+	.endm
+
 	ldr	r4, [sp, #52]
-	str     r4, omap_sdrc_rfr_ctrl_0_val
+	strtext	r4, omap_sdrc_rfr_ctrl_0_val
 	ldr	r4, [sp, #56]
-	str     r4, omap_sdrc_actim_ctrl_a_0_val
+	strtext	r4, omap_sdrc_actim_ctrl_a_0_val
 	ldr	r4, [sp, #60]
-	str     r4, omap_sdrc_actim_ctrl_b_0_val
+	strtext	r4, omap_sdrc_actim_ctrl_b_0_val
 	ldr	r4, [sp, #64]
-	str     r4, omap_sdrc_mr_0_val
+	strtext	r4, omap_sdrc_mr_0_val
 	ldr	r4, [sp, #68]
-	str     r4, omap_sdrc_rfr_ctrl_1_val
+	strtext	r4, omap_sdrc_rfr_ctrl_1_val
 	cmp	r4, #0			@ if SDRC_RFR_CTRL_1 is 0,
 	beq	skip_cs1_params		@  do not use cs1 params
 	ldr	r4, [sp, #72]
-	str     r4, omap_sdrc_actim_ctrl_a_1_val
+	strtext	r4, omap_sdrc_actim_ctrl_a_1_val
 	ldr	r4, [sp, #76]
-	str     r4, omap_sdrc_actim_ctrl_b_1_val
+	strtext	r4, omap_sdrc_actim_ctrl_b_1_val
 	ldr	r4, [sp, #80]
-	str     r4, omap_sdrc_mr_1_val
+	strtext	r4, omap_sdrc_mr_1_val
 skip_cs1_params:
 	mrc	p15, 0, r8, c1, c0, 0	@ read ctrl register
 	bic	r10, r8, #0x800		@ clear Z-bit, disable branch prediction
@@ -271,6 +281,7 @@ skip_cs1_prog:
 	ldr	r12, [r11]		@ posted-write barrier for SDRC
 	bx	lr
 
+	.align
 omap3_sdrc_power:
 	.word OMAP34XX_SDRC_REGADDR(SDRC_POWER)
 omap3_cm_clksel1_pll:
@@ -319,6 +330,7 @@ omap3_sdrc_dlla_ctrl:
 	.word OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL)
 core_m2_mask_val:
 	.word 0x07FFFFFF
+ENDPROC(omap3_sram_configure_core_dpll)
 
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 5/5] ARM: omap3: Thumb-2 compatibility for sleep34xx.S
  2011-02-09 15:01 ` Dave Martin
@ 2011-02-09 15:01   ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Dave Martin, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

 * Use BSYM() to get the correct Thumb branch address
   for adr <Rd>, <label>

 * Fix an out-of-range ADR when building for ARM

 * Correctly call es3_sdrc_fix as Thumb when copied to SRAM.

 * Remove deprecated/undefined PC-relative stores

 * Add the required ENDPROC() directive for each ENTRY().

 * .align before data words

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/sleep34xx.S |   42 +++++++++++++++++++++++++++++---------
 1 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index a05c348..a204c78 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -80,8 +80,10 @@
 /* Function call to get the restore pointer for resume from OFF */
 ENTRY(get_restore_pointer)
 	stmfd	sp!, {lr}	@ save registers on stack
-	adr	r0, restore
+	adr	r0, BSYM(restore)
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(get_restore_pointer)
+	.align
 ENTRY(get_restore_pointer_sz)
 	.word	. - get_restore_pointer
 
@@ -89,8 +91,10 @@ ENTRY(get_restore_pointer_sz)
 /* Function call to get the restore pointer for 3630 resume from OFF */
 ENTRY(get_omap3630_restore_pointer)
 	stmfd	sp!, {lr}	@ save registers on stack
-	adr	r0, restore_3630
+	adr	r0, BSYM(restore_3630)
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(get_omap3630_restore_pointer)
+	.align
 ENTRY(get_omap3630_restore_pointer_sz)
 	.word	. - get_omap3630_restore_pointer
 
@@ -98,8 +102,10 @@ ENTRY(get_omap3630_restore_pointer_sz)
 /* Function call to get the restore pointer for ES3 to resume from OFF */
 ENTRY(get_es3_restore_pointer)
 	stmfd	sp!, {lr}	@ save registers on stack
-	adr	r0, restore_es3
+	adr	r0, BSYM(restore_es3)
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(get_es3_restore_pointer)
+	.align
 ENTRY(get_es3_restore_pointer_sz)
 	.word	. - get_es3_restore_pointer
 
@@ -113,8 +119,11 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	stmfd	sp!, {lr}	@ save registers on stack
 	/* Setup so that we will disable and enable l2 */
 	mov	r1, #0x1
-	str	r1, l2dis_3630
+ ARM(	adrl	r2, l2dis_3630	)	@ may be out of range for adr in ARM
+ THUMB(	adr	r2, l2dis_3630	)	@ Thumb has more range, but not adrl
+	str	r1, [r2]
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(enable_omap3630_toggle_l2_on_restore)
 
 	.text
 /* Function to call rom code to save secure ram context */
@@ -139,12 +148,14 @@ ENTRY(save_secure_ram_context)
 	nop
 	nop
 	ldmfd	sp!, {r1-r12, pc}
+	.align
 sram_phy_addr_mask:
 	.word	SRAM_BASE_P
 high_mask:
 	.word	0xffff
 api_params:
 	.word	0x4, 0x0, 0x0, 0x1, 0x1
+ENDPROC(save_secure_ram_context)
 ENTRY(save_secure_ram_context_sz)
 	.word	. - save_secure_ram_context
 
@@ -279,8 +290,7 @@ clean_l2:
 	 *  - 'might' have to copy address, load and jump to it
 	 */
 	ldr	r1, kernel_flush
-	mov	lr, pc
-	bx	r1
+	blx	r1
 
 omap3_do_wfi:
 	ldr	r4, sdrc_power		@ read the SDRC_POWER register
@@ -346,8 +356,8 @@ restore_es3:
 	and	r4, r4, #0x3
 	cmp	r4, #0x0	@ Check if previous power state of CORE is OFF
 	bne	restore
-	adr	r0, es3_sdrc_fix
-	ldr	r1, sram_base
+	adr	r0, es3_sdrc_fix	@ Not using BSYM clears the Thumb bit.
+	ldr	r1, sram_base	@ Must be 8-byte aligned to preserve alignment.
 	ldr	r2, es3_sdrc_fix_sz
 	mov	r2, r2, ror #2
 copy_to_sram:
@@ -356,6 +366,7 @@ copy_to_sram:
 	subs	r2, r2, #0x1	@ num_words--
 	bne	copy_to_sram
 	ldr	r1, sram_base
+ THUMB(	orr	r1, r1, #BSYM(es3_sdrc_fix) & 1 )
 	blx	r1
 	b	restore
 
@@ -438,6 +449,7 @@ skipl2dis:
 #endif
 	b	logic_l1_restore
 
+	.align
 l2_inv_api_params:
 	.word	0x1, 0x00
 l2_inv_gp:
@@ -607,6 +619,7 @@ usettbr0:
 
 /* This function implements the erratum ID i443 WA, applies to 34xx >= ES3.0 */
 	.text
+	.align	3
 ENTRY(es3_sdrc_fix)
 	ldr	r4, sdrc_syscfg		@ get config addr
 	ldr	r5, [r4]		@ get value
@@ -634,6 +647,7 @@ ENTRY(es3_sdrc_fix)
 	str	r5, [r4]		@ kick off refreshes
 	bx	lr
 
+	.align
 sdrc_syscfg:
 	.word	SDRC_SYSCONFIG_P
 sdrc_mr_0:
@@ -648,6 +662,7 @@ sdrc_emr2_1:
 	.word	SDRC_EMR2_1_P
 sdrc_manual_1:
 	.word	SDRC_MANUAL_1_P
+ENDPROC(es3_sdrc_fix)
 ENTRY(es3_sdrc_fix_sz)
 	.word	. - es3_sdrc_fix
 
@@ -682,6 +697,10 @@ wait_sdrc_ready:
 	bic	r5, r5, #0x40
 	str	r5, [r4]
 
+@ PC-relative stores lead to undefined behaviour in Thumb-2: use a r7 as a
+@ base instead.
+@ Be careful not to clobber r7 when maintaing this code.
+
 is_dll_in_lock_mode:
 	/* Is dll in lock mode? */
 	ldr	r4, sdrc_dlla_ctrl
@@ -689,10 +708,11 @@ is_dll_in_lock_mode:
 	tst	r5, #0x4
 	bxne	lr			@ Return if locked
 	/* wait till dll locks */
+	adr	r7, kick_counter
 wait_dll_lock_timed:
 	ldr	r4, wait_dll_lock_counter
 	add	r4, r4, #1
-	str	r4, wait_dll_lock_counter
+	str	r4, [r7, #wait_dll_lock_counter - kick_counter]
 	ldr	r4, sdrc_dlla_status
 	/* Wait 20uS for lock */
 	mov	r6, #8
@@ -718,9 +738,10 @@ kick_dll:
 	dsb
 	ldr	r4, kick_counter
 	add	r4, r4, #1
-	str	r4, kick_counter
+	str	r4, [r7]		@ kick_counter
 	b	wait_dll_lock_timed
 
+	.align
 cm_idlest1_core:
 	.word	CM_IDLEST1_CORE_V
 cm_idlest_ckgen:
@@ -763,6 +784,7 @@ kick_counter:
 	.word	0
 wait_dll_lock_counter:
 	.word	0
+ENDPROC(omap34xx_cpu_suspend)
 
 ENTRY(omap34xx_cpu_suspend_sz)
 	.word	. - omap34xx_cpu_suspend
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 5/5] ARM: omap3: Thumb-2 compatibility for sleep34xx.S
@ 2011-02-09 15:01   ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-09 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

 * Use BSYM() to get the correct Thumb branch address
   for adr <Rd>, <label>

 * Fix an out-of-range ADR when building for ARM

 * Correctly call es3_sdrc_fix as Thumb when copied to SRAM.

 * Remove deprecated/undefined PC-relative stores

 * Add the required ENDPROC() directive for each ENTRY().

 * .align before data words

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Tested-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/sleep34xx.S |   42 +++++++++++++++++++++++++++++---------
 1 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index a05c348..a204c78 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -80,8 +80,10 @@
 /* Function call to get the restore pointer for resume from OFF */
 ENTRY(get_restore_pointer)
 	stmfd	sp!, {lr}	@ save registers on stack
-	adr	r0, restore
+	adr	r0, BSYM(restore)
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(get_restore_pointer)
+	.align
 ENTRY(get_restore_pointer_sz)
 	.word	. - get_restore_pointer
 
@@ -89,8 +91,10 @@ ENTRY(get_restore_pointer_sz)
 /* Function call to get the restore pointer for 3630 resume from OFF */
 ENTRY(get_omap3630_restore_pointer)
 	stmfd	sp!, {lr}	@ save registers on stack
-	adr	r0, restore_3630
+	adr	r0, BSYM(restore_3630)
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(get_omap3630_restore_pointer)
+	.align
 ENTRY(get_omap3630_restore_pointer_sz)
 	.word	. - get_omap3630_restore_pointer
 
@@ -98,8 +102,10 @@ ENTRY(get_omap3630_restore_pointer_sz)
 /* Function call to get the restore pointer for ES3 to resume from OFF */
 ENTRY(get_es3_restore_pointer)
 	stmfd	sp!, {lr}	@ save registers on stack
-	adr	r0, restore_es3
+	adr	r0, BSYM(restore_es3)
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(get_es3_restore_pointer)
+	.align
 ENTRY(get_es3_restore_pointer_sz)
 	.word	. - get_es3_restore_pointer
 
@@ -113,8 +119,11 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	stmfd	sp!, {lr}	@ save registers on stack
 	/* Setup so that we will disable and enable l2 */
 	mov	r1, #0x1
-	str	r1, l2dis_3630
+ ARM(	adrl	r2, l2dis_3630	)	@ may be out of range for adr in ARM
+ THUMB(	adr	r2, l2dis_3630	)	@ Thumb has more range, but not adrl
+	str	r1, [r2]
 	ldmfd	sp!, {pc}	@ restore regs and return
+ENDPROC(enable_omap3630_toggle_l2_on_restore)
 
 	.text
 /* Function to call rom code to save secure ram context */
@@ -139,12 +148,14 @@ ENTRY(save_secure_ram_context)
 	nop
 	nop
 	ldmfd	sp!, {r1-r12, pc}
+	.align
 sram_phy_addr_mask:
 	.word	SRAM_BASE_P
 high_mask:
 	.word	0xffff
 api_params:
 	.word	0x4, 0x0, 0x0, 0x1, 0x1
+ENDPROC(save_secure_ram_context)
 ENTRY(save_secure_ram_context_sz)
 	.word	. - save_secure_ram_context
 
@@ -279,8 +290,7 @@ clean_l2:
 	 *  - 'might' have to copy address, load and jump to it
 	 */
 	ldr	r1, kernel_flush
-	mov	lr, pc
-	bx	r1
+	blx	r1
 
 omap3_do_wfi:
 	ldr	r4, sdrc_power		@ read the SDRC_POWER register
@@ -346,8 +356,8 @@ restore_es3:
 	and	r4, r4, #0x3
 	cmp	r4, #0x0	@ Check if previous power state of CORE is OFF
 	bne	restore
-	adr	r0, es3_sdrc_fix
-	ldr	r1, sram_base
+	adr	r0, es3_sdrc_fix	@ Not using BSYM clears the Thumb bit.
+	ldr	r1, sram_base	@ Must be 8-byte aligned to preserve alignment.
 	ldr	r2, es3_sdrc_fix_sz
 	mov	r2, r2, ror #2
 copy_to_sram:
@@ -356,6 +366,7 @@ copy_to_sram:
 	subs	r2, r2, #0x1	@ num_words--
 	bne	copy_to_sram
 	ldr	r1, sram_base
+ THUMB(	orr	r1, r1, #BSYM(es3_sdrc_fix) & 1 )
 	blx	r1
 	b	restore
 
@@ -438,6 +449,7 @@ skipl2dis:
 #endif
 	b	logic_l1_restore
 
+	.align
 l2_inv_api_params:
 	.word	0x1, 0x00
 l2_inv_gp:
@@ -607,6 +619,7 @@ usettbr0:
 
 /* This function implements the erratum ID i443 WA, applies to 34xx >= ES3.0 */
 	.text
+	.align	3
 ENTRY(es3_sdrc_fix)
 	ldr	r4, sdrc_syscfg		@ get config addr
 	ldr	r5, [r4]		@ get value
@@ -634,6 +647,7 @@ ENTRY(es3_sdrc_fix)
 	str	r5, [r4]		@ kick off refreshes
 	bx	lr
 
+	.align
 sdrc_syscfg:
 	.word	SDRC_SYSCONFIG_P
 sdrc_mr_0:
@@ -648,6 +662,7 @@ sdrc_emr2_1:
 	.word	SDRC_EMR2_1_P
 sdrc_manual_1:
 	.word	SDRC_MANUAL_1_P
+ENDPROC(es3_sdrc_fix)
 ENTRY(es3_sdrc_fix_sz)
 	.word	. - es3_sdrc_fix
 
@@ -682,6 +697,10 @@ wait_sdrc_ready:
 	bic	r5, r5, #0x40
 	str	r5, [r4]
 
+@ PC-relative stores lead to undefined behaviour in Thumb-2: use a r7 as a
+@ base instead.
+@ Be careful not to clobber r7 when maintaing this code.
+
 is_dll_in_lock_mode:
 	/* Is dll in lock mode? */
 	ldr	r4, sdrc_dlla_ctrl
@@ -689,10 +708,11 @@ is_dll_in_lock_mode:
 	tst	r5, #0x4
 	bxne	lr			@ Return if locked
 	/* wait till dll locks */
+	adr	r7, kick_counter
 wait_dll_lock_timed:
 	ldr	r4, wait_dll_lock_counter
 	add	r4, r4, #1
-	str	r4, wait_dll_lock_counter
+	str	r4, [r7, #wait_dll_lock_counter - kick_counter]
 	ldr	r4, sdrc_dlla_status
 	/* Wait 20uS for lock */
 	mov	r6, #8
@@ -718,9 +738,10 @@ kick_dll:
 	dsb
 	ldr	r4, kick_counter
 	add	r4, r4, #1
-	str	r4, kick_counter
+	str	r4, [r7]		@ kick_counter
 	b	wait_dll_lock_timed
 
+	.align
 cm_idlest1_core:
 	.word	CM_IDLEST1_CORE_V
 cm_idlest_ckgen:
@@ -763,6 +784,7 @@ kick_counter:
 	.word	0
 wait_dll_lock_counter:
 	.word	0
+ENDPROC(omap34xx_cpu_suspend)
 
 ENTRY(omap34xx_cpu_suspend_sz)
 	.word	. - omap34xx_cpu_suspend
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/5] ARM: omap4: Provide do_wfi() for Thumb-2
  2011-02-09 15:01   ` Dave Martin
@ 2011-02-09 16:19     ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-02-09 16:19 UTC (permalink / raw)
  To: Dave Martin
  Cc: linux-arm-kernel, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap

On Wed, 9 Feb 2011, Dave Martin wrote:

> For CONFIG_THUMB2_KERNEL, the existing definition of do_wfi() will
> insert invalid code into the instruction stream.
> 
> Any assembler which can assemble Thumb-2 is guaranteed to accept
> the "wfi" mnemonic, so for the Thumb-2 case, just use the mnemonic.
> 
> The ARM case is left as-is.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
>  arch/arm/mach-omap2/include/mach/omap4-common.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h b/arch/arm/mach-omap2/include/mach/omap4-common.h
> index 5b0270b..de441c0 100644
> --- a/arch/arm/mach-omap2/include/mach/omap4-common.h
> +++ b/arch/arm/mach-omap2/include/mach/omap4-common.h
> @@ -17,8 +17,12 @@
>   * wfi used in low power code. Directly opcode is used instead
>   * of instruction to avoid mulit-omap build break

Might be a good idea to fix the typo above while at it.


> +#ifdef CONFIG_THUMB2_KERNEL
> +#define do_wfi() __asm__ __volatile__ ("wfi" : : : "memory")
> +#else
>  #define do_wfi()			\
>  		__asm__ __volatile__ (".word	0xe320f003" : : : "memory")
> +#endif
>  
>  #ifdef CONFIG_CACHE_L2X0
>  extern void __iomem *l2cache_base;
> -- 
> 1.7.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 1/5] ARM: omap4: Provide do_wfi() for Thumb-2
@ 2011-02-09 16:19     ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-02-09 16:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 9 Feb 2011, Dave Martin wrote:

> For CONFIG_THUMB2_KERNEL, the existing definition of do_wfi() will
> insert invalid code into the instruction stream.
> 
> Any assembler which can assemble Thumb-2 is guaranteed to accept
> the "wfi" mnemonic, so for the Thumb-2 case, just use the mnemonic.
> 
> The ARM case is left as-is.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> ---
>  arch/arm/mach-omap2/include/mach/omap4-common.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/include/mach/omap4-common.h b/arch/arm/mach-omap2/include/mach/omap4-common.h
> index 5b0270b..de441c0 100644
> --- a/arch/arm/mach-omap2/include/mach/omap4-common.h
> +++ b/arch/arm/mach-omap2/include/mach/omap4-common.h
> @@ -17,8 +17,12 @@
>   * wfi used in low power code. Directly opcode is used instead
>   * of instruction to avoid mulit-omap build break

Might be a good idea to fix the typo above while at it.


> +#ifdef CONFIG_THUMB2_KERNEL
> +#define do_wfi() __asm__ __volatile__ ("wfi" : : : "memory")
> +#else
>  #define do_wfi()			\
>  		__asm__ __volatile__ (".word	0xe320f003" : : : "memory")
> +#endif
>  
>  #ifdef CONFIG_CACHE_L2X0
>  extern void __iomem *l2cache_base;
> -- 
> 1.7.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-09 15:01 ` Dave Martin
@ 2011-02-11 23:31   ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-02-11 23:31 UTC (permalink / raw)
  To: Dave Martin
  Cc: linux-arm-kernel, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

Hi Dave,

Dave Martin <dave.martin@linaro.org> writes:

> This set of patches, along with some other patches under
> discussion on alkml, should enable omap3 and omap4 kernels to be
> built with CONFIG_THUMB2_KERNEL.

OK, I tried some more testing with your 'dirty' branch merged with my PM
branch.

Compiled in ARM mode, everything worked as expected on my 3530/omap3evm,
including off-mode (in suspend and idle).  My 3630 (Zoom3) also can't do
CORE off due to i583, but MPU, PER etc. all can hit off.

To rebuild in Thumb-2 mode, I disabled OMAP2 support and added Thumb-2
mode:

   CONFIG_ARCH_OMAP2=n
   CONFIG_THUMB2_KERNEL=y

then tested on 3530/omap3evm.  Testing suspend/resume to retention
seemed to work fine.  However, enabling retention during idle[1] hung
someplace (didn't debug further.)

I also tried off-mode, and suspend/resume to off didn't even work.

I didn't have time to debug this any further, so this is just to report
raw test results.

Hope that helps,

Kevin


[1] Here's what's needed to attempt low-power states during idle

# UART timeouts: omap-serial (4th UART only on OMAP36xx and OMAP4)
echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout 
echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout 
echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout 
echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout 

# enable low-power states during idle
echo 1 > /debug/pm_debug/sleep_while_idle  


[2] to enable off-mode

echo 1 > /debug/pm_debug/enable_off_mode

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-11 23:31   ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-02-11 23:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dave,

Dave Martin <dave.martin@linaro.org> writes:

> This set of patches, along with some other patches under
> discussion on alkml, should enable omap3 and omap4 kernels to be
> built with CONFIG_THUMB2_KERNEL.

OK, I tried some more testing with your 'dirty' branch merged with my PM
branch.

Compiled in ARM mode, everything worked as expected on my 3530/omap3evm,
including off-mode (in suspend and idle).  My 3630 (Zoom3) also can't do
CORE off due to i583, but MPU, PER etc. all can hit off.

To rebuild in Thumb-2 mode, I disabled OMAP2 support and added Thumb-2
mode:

   CONFIG_ARCH_OMAP2=n
   CONFIG_THUMB2_KERNEL=y

then tested on 3530/omap3evm.  Testing suspend/resume to retention
seemed to work fine.  However, enabling retention during idle[1] hung
someplace (didn't debug further.)

I also tried off-mode, and suspend/resume to off didn't even work.

I didn't have time to debug this any further, so this is just to report
raw test results.

Hope that helps,

Kevin


[1] Here's what's needed to attempt low-power states during idle

# UART timeouts: omap-serial (4th UART only on OMAP36xx and OMAP4)
echo 5 > /sys/devices/platform/omap/omap_uart.0/sleep_timeout 
echo 5 > /sys/devices/platform/omap/omap_uart.1/sleep_timeout 
echo 5 > /sys/devices/platform/omap/omap_uart.2/sleep_timeout 
echo 5 > /sys/devices/platform/omap/omap_uart.3/sleep_timeout 

# enable low-power states during idle
echo 1 > /debug/pm_debug/sleep_while_idle  


[2] to enable off-mode

echo 1 > /debug/pm_debug/enable_off_mode

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-11 23:31   ` Kevin Hilman
@ 2011-02-14 13:17     ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-14 13:17 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-arm-kernel, Tony Lindgren, Santosh Shilimkar, Jean Pihet,
	linux-omap, Nicolas Pitre

On Fri, Feb 11, 2011 at 03:31:20PM -0800, Kevin Hilman wrote:
> Hi Dave,
> 
> Dave Martin <dave.martin@linaro.org> writes:
> 
> > This set of patches, along with some other patches under
> > discussion on alkml, should enable omap3 and omap4 kernels to be
> > built with CONFIG_THUMB2_KERNEL.
> 
> OK, I tried some more testing with your 'dirty' branch merged with my PM
> branch.
> 
> Compiled in ARM mode, everything worked as expected on my 3530/omap3evm,
> including off-mode (in suspend and idle).  My 3630 (Zoom3) also can't do
> CORE off due to i583, but MPU, PER etc. all can hit off.
> 
> To rebuild in Thumb-2 mode, I disabled OMAP2 support and added Thumb-2
> mode:
> 
>    CONFIG_ARCH_OMAP2=n
>    CONFIG_THUMB2_KERNEL=y
> 
> then tested on 3530/omap3evm.  Testing suspend/resume to retention
> seemed to work fine.  However, enabling retention during idle[1] hung
> someplace (didn't debug further.)
> 
> I also tried off-mode, and suspend/resume to off didn't even work.
> 
> I didn't have time to debug this any further, so this is just to report
> raw test results.

Thanks, that's a useful step forward anyhow.

A possibility is that the Secure firmware can't cope with
interoperating with Thumb-2 code in the kernel, so that wakeup
entry points the SMC call sites may need to be ARM code.

If you get a moment, if would be interesting to see if this
patch makes a difference ... in the meantime, I'll see if
I can get my hands on an EVM from somewhere.

Cheers
---Dave

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index a204c78..ee1edb1 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -32,6 +32,14 @@
 #include "sdrc.h"
 #include "control.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 /*
  * Registers access definitions
  */
@@ -289,8 +297,20 @@ clean_l2:
 	 *  - should be faster and will change with kernel
 	 *  - 'might' have to copy address, load and jump to it
 	 */
+#ifdef CONFIG_THUMB2_KERNEL
+	/* kernel is non-interworking : must do this from Thumb */
+	adr	r1, . + 1
+	bx	r1
+	.thumb
+#endif
 	ldr	r1, kernel_flush
 	blx	r1
+#ifdef CONFIG_THUMB2_KERNEL
+	.align
+	bx	pc
+	nop
+	.arm
+#endif
 
 omap3_do_wfi:
 	ldr	r4, sdrc_power		@ read the SDRC_POWER register
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 829d235..64faab8 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -34,6 +34,14 @@
 #include "sdrc.h"
 #include "cm2xxx_3xxx.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 	.text
 
 /* r1 parameters */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-14 13:17     ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-14 13:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 11, 2011 at 03:31:20PM -0800, Kevin Hilman wrote:
> Hi Dave,
> 
> Dave Martin <dave.martin@linaro.org> writes:
> 
> > This set of patches, along with some other patches under
> > discussion on alkml, should enable omap3 and omap4 kernels to be
> > built with CONFIG_THUMB2_KERNEL.
> 
> OK, I tried some more testing with your 'dirty' branch merged with my PM
> branch.
> 
> Compiled in ARM mode, everything worked as expected on my 3530/omap3evm,
> including off-mode (in suspend and idle).  My 3630 (Zoom3) also can't do
> CORE off due to i583, but MPU, PER etc. all can hit off.
> 
> To rebuild in Thumb-2 mode, I disabled OMAP2 support and added Thumb-2
> mode:
> 
>    CONFIG_ARCH_OMAP2=n
>    CONFIG_THUMB2_KERNEL=y
> 
> then tested on 3530/omap3evm.  Testing suspend/resume to retention
> seemed to work fine.  However, enabling retention during idle[1] hung
> someplace (didn't debug further.)
> 
> I also tried off-mode, and suspend/resume to off didn't even work.
> 
> I didn't have time to debug this any further, so this is just to report
> raw test results.

Thanks, that's a useful step forward anyhow.

A possibility is that the Secure firmware can't cope with
interoperating with Thumb-2 code in the kernel, so that wakeup
entry points the SMC call sites may need to be ARM code.

If you get a moment, if would be interesting to see if this
patch makes a difference ... in the meantime, I'll see if
I can get my hands on an EVM from somewhere.

Cheers
---Dave

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index a204c78..ee1edb1 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -32,6 +32,14 @@
 #include "sdrc.h"
 #include "control.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 /*
  * Registers access definitions
  */
@@ -289,8 +297,20 @@ clean_l2:
 	 *  - should be faster and will change with kernel
 	 *  - 'might' have to copy address, load and jump to it
 	 */
+#ifdef CONFIG_THUMB2_KERNEL
+	/* kernel is non-interworking : must do this from Thumb */
+	adr	r1, . + 1
+	bx	r1
+	.thumb
+#endif
 	ldr	r1, kernel_flush
 	blx	r1
+#ifdef CONFIG_THUMB2_KERNEL
+	.align
+	bx	pc
+	nop
+	.arm
+#endif
 
 omap3_do_wfi:
 	ldr	r4, sdrc_power		@ read the SDRC_POWER register
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 829d235..64faab8 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -34,6 +34,14 @@
 #include "sdrc.h"
 #include "cm2xxx_3xxx.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 	.text
 
 /* r1 parameters */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-14 13:17     ` Dave Martin
@ 2011-02-14 15:00       ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-02-14 15:00 UTC (permalink / raw)
  To: Dave Martin
  Cc: Kevin Hilman, linux-arm-kernel, Tony Lindgren, Santosh Shilimkar,
	Jean Pihet, linux-omap

On Mon, 14 Feb 2011, Dave Martin wrote:

> @@ -289,8 +297,20 @@ clean_l2:
>  	 *  - should be faster and will change with kernel
>  	 *  - 'might' have to copy address, load and jump to it
>  	 */
> +#ifdef CONFIG_THUMB2_KERNEL
> +	/* kernel is non-interworking : must do this from Thumb */
> +	adr	r1, . + 1
> +	bx	r1
> +	.thumb
> +#endif
>  	ldr	r1, kernel_flush

Didn't you mean this instead:

	/* kernel is non-interworking : must do this from Thumb */
	adr	r1, 1f + 1
	bx	r1
	.thumb
1:	ldr	r1, kernel_flush
	...

?

>  	blx	r1
> +#ifdef CONFIG_THUMB2_KERNEL
> +	.align
> +	bx	pc
> +	nop
> +	.arm

Also here, the .align has the potential to introduce a zero halfword in 
the instruction stream before the bx.  What about:

	adr	r3, 1f
	bx	r3
	.align
	.arm
1:	...


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-14 15:00       ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-02-14 15:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 14 Feb 2011, Dave Martin wrote:

> @@ -289,8 +297,20 @@ clean_l2:
>  	 *  - should be faster and will change with kernel
>  	 *  - 'might' have to copy address, load and jump to it
>  	 */
> +#ifdef CONFIG_THUMB2_KERNEL
> +	/* kernel is non-interworking : must do this from Thumb */
> +	adr	r1, . + 1
> +	bx	r1
> +	.thumb
> +#endif
>  	ldr	r1, kernel_flush

Didn't you mean this instead:

	/* kernel is non-interworking : must do this from Thumb */
	adr	r1, 1f + 1
	bx	r1
	.thumb
1:	ldr	r1, kernel_flush
	...

?

>  	blx	r1
> +#ifdef CONFIG_THUMB2_KERNEL
> +	.align
> +	bx	pc
> +	nop
> +	.arm

Also here, the .align has the potential to introduce a zero halfword in 
the instruction stream before the bx.  What about:

	adr	r3, 1f
	bx	r3
	.align
	.arm
1:	...


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-14 15:00       ` Nicolas Pitre
@ 2011-02-14 15:37         ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-14 15:37 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Kevin Hilman, linux-arm-kernel, Tony Lindgren, Santosh Shilimkar,
	Jean Pihet, linux-omap

On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
> On Mon, 14 Feb 2011, Dave Martin wrote:
> 
> > @@ -289,8 +297,20 @@ clean_l2:
> >  	 *  - should be faster and will change with kernel
> >  	 *  - 'might' have to copy address, load and jump to it
> >  	 */
> > +#ifdef CONFIG_THUMB2_KERNEL
> > +	/* kernel is non-interworking : must do this from Thumb */
> > +	adr	r1, . + 1
> > +	bx	r1
> > +	.thumb
> > +#endif
> >  	ldr	r1, kernel_flush
> 
> Didn't you mean this instead:
> 
> 	/* kernel is non-interworking : must do this from Thumb */
> 	adr	r1, 1f + 1
> 	bx	r1
> 	.thumb
> 1:	ldr	r1, kernel_flush
> 	...

Note that this is intended as an experimental hack, not a real patch
(apologies if I didn't make that clear...)

Well, actually I meant "add r1, pc, #1" ... which means I was too
busy trying to be clever... oops!

That is of course exactly equivalent to your code...

> 
> ?
> 
> >  	blx	r1
> > +#ifdef CONFIG_THUMB2_KERNEL
> > +	.align
> > +	bx	pc
> > +	nop
> > +	.arm
> 
> Also here, the .align has the potential to introduce a zero halfword in 
> the instruction stream before the bx.  What about:
> 
> 	adr	r3, 1f
> 	bx	r3
> 	.align
> 	.arm
> 1:	...

.align inserts a 16-bit nop when misaligned in Thumb in a text section,
and a word-aligned bx pc is a specific architecturally allowed way
to do an inline switch to ARM.  The linker uses this trick for PLT
veneers etc.

A nicer fix for doing this sort of call from low-level code which
might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.

Generally, we can do this for all arches >= v5, without any
incompatibility.  However, since the need for it will be rare and it
will generate patch noise for not much real benefit,
I haven't proposed this.

Updated patch below.

Cheers
---Dave

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index a204c78..6ae8a92 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -32,6 +32,14 @@
 #include "sdrc.h"
 #include "control.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 /*
  * Registers access definitions
  */
@@ -289,8 +297,20 @@ clean_l2:
 	 *  - should be faster and will change with kernel
 	 *  - 'might' have to copy address, load and jump to it
 	 */
-	ldr	r1, kernel_flush
+#ifdef CONFIG_THUMB2_KERNEL
+	/* kernel is non-interworking : must do this from Thumb */
+	adr	r1, 1f + 1
+	bx	r1
+	.thumb
+#endif
+1:	ldr	r1, kernel_flush
 	blx	r1
+#ifdef CONFIG_THUMB2_KERNEL
+	.align
+	bx	pc
+	nop
+	.arm
+#endif
 
 omap3_do_wfi:
 	ldr	r4, sdrc_power		@ read the SDRC_POWER register
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 829d235..64faab8 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -34,6 +34,14 @@
 #include "sdrc.h"
 #include "cm2xxx_3xxx.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 	.text
 
 /* r1 parameters */
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-14 15:37         ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-14 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
> On Mon, 14 Feb 2011, Dave Martin wrote:
> 
> > @@ -289,8 +297,20 @@ clean_l2:
> >  	 *  - should be faster and will change with kernel
> >  	 *  - 'might' have to copy address, load and jump to it
> >  	 */
> > +#ifdef CONFIG_THUMB2_KERNEL
> > +	/* kernel is non-interworking : must do this from Thumb */
> > +	adr	r1, . + 1
> > +	bx	r1
> > +	.thumb
> > +#endif
> >  	ldr	r1, kernel_flush
> 
> Didn't you mean this instead:
> 
> 	/* kernel is non-interworking : must do this from Thumb */
> 	adr	r1, 1f + 1
> 	bx	r1
> 	.thumb
> 1:	ldr	r1, kernel_flush
> 	...

Note that this is intended as an experimental hack, not a real patch
(apologies if I didn't make that clear...)

Well, actually I meant "add r1, pc, #1" ... which means I was too
busy trying to be clever... oops!

That is of course exactly equivalent to your code...

> 
> ?
> 
> >  	blx	r1
> > +#ifdef CONFIG_THUMB2_KERNEL
> > +	.align
> > +	bx	pc
> > +	nop
> > +	.arm
> 
> Also here, the .align has the potential to introduce a zero halfword in 
> the instruction stream before the bx.  What about:
> 
> 	adr	r3, 1f
> 	bx	r3
> 	.align
> 	.arm
> 1:	...

.align inserts a 16-bit nop when misaligned in Thumb in a text section,
and a word-aligned bx pc is a specific architecturally allowed way
to do an inline switch to ARM.  The linker uses this trick for PLT
veneers etc.

A nicer fix for doing this sort of call from low-level code which
might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.

Generally, we can do this for all arches >= v5, without any
incompatibility.  However, since the need for it will be rare and it
will generate patch noise for not much real benefit,
I haven't proposed this.

Updated patch below.

Cheers
---Dave

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index a204c78..6ae8a92 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -32,6 +32,14 @@
 #include "sdrc.h"
 #include "control.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 /*
  * Registers access definitions
  */
@@ -289,8 +297,20 @@ clean_l2:
 	 *  - should be faster and will change with kernel
 	 *  - 'might' have to copy address, load and jump to it
 	 */
-	ldr	r1, kernel_flush
+#ifdef CONFIG_THUMB2_KERNEL
+	/* kernel is non-interworking : must do this from Thumb */
+	adr	r1, 1f + 1
+	bx	r1
+	.thumb
+#endif
+1:	ldr	r1, kernel_flush
 	blx	r1
+#ifdef CONFIG_THUMB2_KERNEL
+	.align
+	bx	pc
+	nop
+	.arm
+#endif
 
 omap3_do_wfi:
 	ldr	r4, sdrc_power		@ read the SDRC_POWER register
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 829d235..64faab8 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -34,6 +34,14 @@
 #include "sdrc.h"
 #include "cm2xxx_3xxx.h"
 
+#undef ARM
+#undef THUMB
+#undef BSYM
+#define ARM(x...) x
+#define THUMB(x...)
+#define BSYM(x) (x)
+	.arm
+
 	.text
 
 /* r1 parameters */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-14 15:37         ` Dave Martin
@ 2011-02-14 20:10           ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-02-14 20:10 UTC (permalink / raw)
  To: Dave Martin
  Cc: Kevin Hilman, linux-arm-kernel, Tony Lindgren, Santosh Shilimkar,
	Jean Pihet, linux-omap

On Mon, 14 Feb 2011, Dave Martin wrote:

> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
> > > +#ifdef CONFIG_THUMB2_KERNEL
> > > +	.align
> > > +	bx	pc
> > > +	nop
> > > +	.arm
> > 
> > Also here, the .align has the potential to introduce a zero halfword in 
> > the instruction stream before the bx.  What about:
> > 
> > 	adr	r3, 1f
> > 	bx	r3
> > 	.align
> > 	.arm
> > 1:	...
> 
> .align inserts a 16-bit nop when misaligned in Thumb in a text section,

Ah, OK then.  I didn't know about that. In ARM mode the all-zero bits 
decode to an instruction that sort of does nothing, but this isn't an 
eleguant nop, and that's what .align (used to?) insert as padding.

> and a word-aligned bx pc is a specific architecturally allowed way
> to do an inline switch to ARM.  The linker uses this trick for PLT
> veneers etc.

Yep.  It's just that with my suggestion the bx wasn't necessarily 
aligned.

> A nicer fix for doing this sort of call from low-level code which
> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
> 
> Generally, we can do this for all arches >= v5, without any
> incompatibility.  However, since the need for it will be rare and it
> will generate patch noise for not much real benefit,
> I haven't proposed this.

This can be done in those places where this might be needed without 
having to convert them all.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-14 20:10           ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-02-14 20:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 14 Feb 2011, Dave Martin wrote:

> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
> > > +#ifdef CONFIG_THUMB2_KERNEL
> > > +	.align
> > > +	bx	pc
> > > +	nop
> > > +	.arm
> > 
> > Also here, the .align has the potential to introduce a zero halfword in 
> > the instruction stream before the bx.  What about:
> > 
> > 	adr	r3, 1f
> > 	bx	r3
> > 	.align
> > 	.arm
> > 1:	...
> 
> .align inserts a 16-bit nop when misaligned in Thumb in a text section,

Ah, OK then.  I didn't know about that. In ARM mode the all-zero bits 
decode to an instruction that sort of does nothing, but this isn't an 
eleguant nop, and that's what .align (used to?) insert as padding.

> and a word-aligned bx pc is a specific architecturally allowed way
> to do an inline switch to ARM.  The linker uses this trick for PLT
> veneers etc.

Yep.  It's just that with my suggestion the bx wasn't necessarily 
aligned.

> A nicer fix for doing this sort of call from low-level code which
> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
> 
> Generally, we can do this for all arches >= v5, without any
> incompatibility.  However, since the need for it will be rare and it
> will generate patch noise for not much real benefit,
> I haven't proposed this.

This can be done in those places where this might be needed without 
having to convert them all.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-14 15:37         ` Dave Martin
@ 2011-02-14 23:15           ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-02-14 23:15 UTC (permalink / raw)
  To: Dave Martin
  Cc: Nicolas Pitre, linux-arm-kernel, Tony Lindgren,
	Santosh Shilimkar, Jean Pihet, linux-omap

Hi Dave,

Dave Martin <dave.martin@linaro.org> writes:

> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
>> On Mon, 14 Feb 2011, Dave Martin wrote:
>> 
>> > @@ -289,8 +297,20 @@ clean_l2:
>> >  	 *  - should be faster and will change with kernel
>> >  	 *  - 'might' have to copy address, load and jump to it
>> >  	 */
>> > +#ifdef CONFIG_THUMB2_KERNEL
>> > +	/* kernel is non-interworking : must do this from Thumb */
>> > +	adr	r1, . + 1
>> > +	bx	r1
>> > +	.thumb
>> > +#endif
>> >  	ldr	r1, kernel_flush
>> 
>> Didn't you mean this instead:
>> 
>> 	/* kernel is non-interworking : must do this from Thumb */
>> 	adr	r1, 1f + 1
>> 	bx	r1
>> 	.thumb
>> 1:	ldr	r1, kernel_flush
>> 	...
>
> Note that this is intended as an experimental hack, not a real patch
> (apologies if I didn't make that clear...)
>
> Well, actually I meant "add r1, pc, #1" ... which means I was too
> busy trying to be clever... oops!
>
> That is of course exactly equivalent to your code...
>
>> 
>> ?
>> 
>> >  	blx	r1
>> > +#ifdef CONFIG_THUMB2_KERNEL
>> > +	.align
>> > +	bx	pc
>> > +	nop
>> > +	.arm
>> 
>> Also here, the .align has the potential to introduce a zero halfword in 
>> the instruction stream before the bx.  What about:
>> 
>> 	adr	r3, 1f
>> 	bx	r3
>> 	.align
>> 	.arm
>> 1:	...
>
> .align inserts a 16-bit nop when misaligned in Thumb in a text section,
> and a word-aligned bx pc is a specific architecturally allowed way
> to do an inline switch to ARM.  The linker uses this trick for PLT
> veneers etc.
>
> A nicer fix for doing this sort of call from low-level code which
> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
>
> Generally, we can do this for all arches >= v5, without any
> incompatibility.  However, since the need for it will be rare and it
> will generate patch noise for not much real benefit,
> I haven't proposed this.
>
> Updated patch below.

I tested the updated patch on top of your "dirty" branch I tested with
last week, and now see off-mode working just fine.

Kevin

> Cheers
> ---Dave
>
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index a204c78..6ae8a92 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -32,6 +32,14 @@
>  #include "sdrc.h"
>  #include "control.h"
>  
> +#undef ARM
> +#undef THUMB
> +#undef BSYM
> +#define ARM(x...) x
> +#define THUMB(x...)
> +#define BSYM(x) (x)
> +	.arm
> +
>  /*
>   * Registers access definitions
>   */
> @@ -289,8 +297,20 @@ clean_l2:
>  	 *  - should be faster and will change with kernel
>  	 *  - 'might' have to copy address, load and jump to it
>  	 */
> -	ldr	r1, kernel_flush
> +#ifdef CONFIG_THUMB2_KERNEL
> +	/* kernel is non-interworking : must do this from Thumb */
> +	adr	r1, 1f + 1
> +	bx	r1
> +	.thumb
> +#endif
> +1:	ldr	r1, kernel_flush
>  	blx	r1
> +#ifdef CONFIG_THUMB2_KERNEL
> +	.align
> +	bx	pc
> +	nop
> +	.arm
> +#endif
>  
>  omap3_do_wfi:
>  	ldr	r4, sdrc_power		@ read the SDRC_POWER register
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index 829d235..64faab8 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -34,6 +34,14 @@
>  #include "sdrc.h"
>  #include "cm2xxx_3xxx.h"
>  
> +#undef ARM
> +#undef THUMB
> +#undef BSYM
> +#define ARM(x...) x
> +#define THUMB(x...)
> +#define BSYM(x) (x)
> +	.arm
> +
>  	.text
>  
>  /* r1 parameters */

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-14 23:15           ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-02-14 23:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dave,

Dave Martin <dave.martin@linaro.org> writes:

> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
>> On Mon, 14 Feb 2011, Dave Martin wrote:
>> 
>> > @@ -289,8 +297,20 @@ clean_l2:
>> >  	 *  - should be faster and will change with kernel
>> >  	 *  - 'might' have to copy address, load and jump to it
>> >  	 */
>> > +#ifdef CONFIG_THUMB2_KERNEL
>> > +	/* kernel is non-interworking : must do this from Thumb */
>> > +	adr	r1, . + 1
>> > +	bx	r1
>> > +	.thumb
>> > +#endif
>> >  	ldr	r1, kernel_flush
>> 
>> Didn't you mean this instead:
>> 
>> 	/* kernel is non-interworking : must do this from Thumb */
>> 	adr	r1, 1f + 1
>> 	bx	r1
>> 	.thumb
>> 1:	ldr	r1, kernel_flush
>> 	...
>
> Note that this is intended as an experimental hack, not a real patch
> (apologies if I didn't make that clear...)
>
> Well, actually I meant "add r1, pc, #1" ... which means I was too
> busy trying to be clever... oops!
>
> That is of course exactly equivalent to your code...
>
>> 
>> ?
>> 
>> >  	blx	r1
>> > +#ifdef CONFIG_THUMB2_KERNEL
>> > +	.align
>> > +	bx	pc
>> > +	nop
>> > +	.arm
>> 
>> Also here, the .align has the potential to introduce a zero halfword in 
>> the instruction stream before the bx.  What about:
>> 
>> 	adr	r3, 1f
>> 	bx	r3
>> 	.align
>> 	.arm
>> 1:	...
>
> .align inserts a 16-bit nop when misaligned in Thumb in a text section,
> and a word-aligned bx pc is a specific architecturally allowed way
> to do an inline switch to ARM.  The linker uses this trick for PLT
> veneers etc.
>
> A nicer fix for doing this sort of call from low-level code which
> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
>
> Generally, we can do this for all arches >= v5, without any
> incompatibility.  However, since the need for it will be rare and it
> will generate patch noise for not much real benefit,
> I haven't proposed this.
>
> Updated patch below.

I tested the updated patch on top of your "dirty" branch I tested with
last week, and now see off-mode working just fine.

Kevin

> Cheers
> ---Dave
>
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index a204c78..6ae8a92 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -32,6 +32,14 @@
>  #include "sdrc.h"
>  #include "control.h"
>  
> +#undef ARM
> +#undef THUMB
> +#undef BSYM
> +#define ARM(x...) x
> +#define THUMB(x...)
> +#define BSYM(x) (x)
> +	.arm
> +
>  /*
>   * Registers access definitions
>   */
> @@ -289,8 +297,20 @@ clean_l2:
>  	 *  - should be faster and will change with kernel
>  	 *  - 'might' have to copy address, load and jump to it
>  	 */
> -	ldr	r1, kernel_flush
> +#ifdef CONFIG_THUMB2_KERNEL
> +	/* kernel is non-interworking : must do this from Thumb */
> +	adr	r1, 1f + 1
> +	bx	r1
> +	.thumb
> +#endif
> +1:	ldr	r1, kernel_flush
>  	blx	r1
> +#ifdef CONFIG_THUMB2_KERNEL
> +	.align
> +	bx	pc
> +	nop
> +	.arm
> +#endif
>  
>  omap3_do_wfi:
>  	ldr	r4, sdrc_power		@ read the SDRC_POWER register
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index 829d235..64faab8 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -34,6 +34,14 @@
>  #include "sdrc.h"
>  #include "cm2xxx_3xxx.h"
>  
> +#undef ARM
> +#undef THUMB
> +#undef BSYM
> +#define ARM(x...) x
> +#define THUMB(x...)
> +#define BSYM(x) (x)
> +	.arm
> +
>  	.text
>  
>  /* r1 parameters */

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-14 23:15           ` Kevin Hilman
@ 2011-02-15 10:45             ` Dave Martin
  -1 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-15 10:45 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Nicolas Pitre, linux-arm-kernel, Tony Lindgren,
	Santosh Shilimkar, Jean Pihet, linux-omap

On Mon, Feb 14, 2011 at 11:15 PM, Kevin Hilman <khilman@ti.com> wrote:
> Hi Dave,
>
> Dave Martin <dave.martin@linaro.org> writes:
>
>> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
>>> On Mon, 14 Feb 2011, Dave Martin wrote:
>>>
>>> > @@ -289,8 +297,20 @@ clean_l2:
>>> >     *  - should be faster and will change with kernel
>>> >     *  - 'might' have to copy address, load and jump to it
>>> >     */
>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>> > +  /* kernel is non-interworking : must do this from Thumb */
>>> > +  adr     r1, . + 1
>>> > +  bx      r1
>>> > +  .thumb
>>> > +#endif
>>> >    ldr     r1, kernel_flush
>>>
>>> Didn't you mean this instead:
>>>
>>>      /* kernel is non-interworking : must do this from Thumb */
>>>      adr     r1, 1f + 1
>>>      bx      r1
>>>      .thumb
>>> 1:   ldr     r1, kernel_flush
>>>      ...
>>
>> Note that this is intended as an experimental hack, not a real patch
>> (apologies if I didn't make that clear...)
>>
>> Well, actually I meant "add r1, pc, #1" ... which means I was too
>> busy trying to be clever... oops!
>>
>> That is of course exactly equivalent to your code...
>>
>>>
>>> ?
>>>
>>> >    blx     r1
>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>> > +  .align
>>> > +  bx      pc
>>> > +  nop
>>> > +  .arm
>>>
>>> Also here, the .align has the potential to introduce a zero halfword in
>>> the instruction stream before the bx.  What about:
>>>
>>>      adr     r3, 1f
>>>      bx      r3
>>>      .align
>>>      .arm
>>> 1:   ...
>>
>> .align inserts a 16-bit nop when misaligned in Thumb in a text section,
>> and a word-aligned bx pc is a specific architecturally allowed way
>> to do an inline switch to ARM.  The linker uses this trick for PLT
>> veneers etc.
>>
>> A nicer fix for doing this sort of call from low-level code which
>> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
>>
>> Generally, we can do this for all arches >= v5, without any
>> incompatibility.  However, since the need for it will be rare and it
>> will generate patch noise for not much real benefit,
>> I haven't proposed this.
>>
>> Updated patch below.
>
> I tested the updated patch on top of your "dirty" branch I tested with
> last week, and now see off-mode working just fine.

Thanks for testing-- that's great news.

I will have a think about whether the patch can be tidied up to revert
most of the code back to Thumb, though that isn't essential.  If I've
understood what's going on correctly, I think only the restore entry
points, SMC call sites and the entry to omap3_sram_configure_core_dpll
could need to be ARM; the rest shouldn't really make any difference...

Cheers
---Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-15 10:45             ` Dave Martin
  0 siblings, 0 replies; 30+ messages in thread
From: Dave Martin @ 2011-02-15 10:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 14, 2011 at 11:15 PM, Kevin Hilman <khilman@ti.com> wrote:
> Hi Dave,
>
> Dave Martin <dave.martin@linaro.org> writes:
>
>> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
>>> On Mon, 14 Feb 2011, Dave Martin wrote:
>>>
>>> > @@ -289,8 +297,20 @@ clean_l2:
>>> > ? ? * ?- should be faster and will change with kernel
>>> > ? ? * ?- 'might' have to copy address, load and jump to it
>>> > ? ? */
>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>> > + ?/* kernel is non-interworking : must do this from Thumb */
>>> > + ?adr ? ? r1, . + 1
>>> > + ?bx ? ? ?r1
>>> > + ?.thumb
>>> > +#endif
>>> > ? ?ldr ? ? r1, kernel_flush
>>>
>>> Didn't you mean this instead:
>>>
>>> ? ? ?/* kernel is non-interworking : must do this from Thumb */
>>> ? ? ?adr ? ? r1, 1f + 1
>>> ? ? ?bx ? ? ?r1
>>> ? ? ?.thumb
>>> 1: ? ldr ? ? r1, kernel_flush
>>> ? ? ?...
>>
>> Note that this is intended as an experimental hack, not a real patch
>> (apologies if I didn't make that clear...)
>>
>> Well, actually I meant "add r1, pc, #1" ... which means I was too
>> busy trying to be clever... oops!
>>
>> That is of course exactly equivalent to your code...
>>
>>>
>>> ?
>>>
>>> > ? ?blx ? ? r1
>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>> > + ?.align
>>> > + ?bx ? ? ?pc
>>> > + ?nop
>>> > + ?.arm
>>>
>>> Also here, the .align has the potential to introduce a zero halfword in
>>> the instruction stream before the bx. ?What about:
>>>
>>> ? ? ?adr ? ? r3, 1f
>>> ? ? ?bx ? ? ?r3
>>> ? ? ?.align
>>> ? ? ?.arm
>>> 1: ? ...
>>
>> .align inserts a 16-bit nop when misaligned in Thumb in a text section,
>> and a word-aligned bx pc is a specific architecturally allowed way
>> to do an inline switch to ARM. ?The linker uses this trick for PLT
>> veneers etc.
>>
>> A nicer fix for doing this sort of call from low-level code which
>> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
>>
>> Generally, we can do this for all arches >= v5, without any
>> incompatibility. ?However, since the need for it will be rare and it
>> will generate patch noise for not much real benefit,
>> I haven't proposed this.
>>
>> Updated patch below.
>
> I tested the updated patch on top of your "dirty" branch I tested with
> last week, and now see off-mode working just fine.

Thanks for testing-- that's great news.

I will have a think about whether the patch can be tidied up to revert
most of the code back to Thumb, though that isn't essential.  If I've
understood what's going on correctly, I think only the restore entry
points, SMC call sites and the entry to omap3_sram_configure_core_dpll
could need to be ARM; the rest shouldn't really make any difference...

Cheers
---Dave

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
  2011-02-15 10:45             ` Dave Martin
@ 2011-02-15 16:15               ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-02-15 16:15 UTC (permalink / raw)
  To: Dave Martin
  Cc: Nicolas Pitre, linux-arm-kernel, Tony Lindgren,
	Santosh Shilimkar, Jean Pihet, linux-omap

Dave Martin <dave.martin@linaro.org> writes:

> On Mon, Feb 14, 2011 at 11:15 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Hi Dave,
>>
>> Dave Martin <dave.martin@linaro.org> writes:
>>
>>> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
>>>> On Mon, 14 Feb 2011, Dave Martin wrote:
>>>>
>>>> > @@ -289,8 +297,20 @@ clean_l2:
>>>> >     *  - should be faster and will change with kernel
>>>> >     *  - 'might' have to copy address, load and jump to it
>>>> >     */
>>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>>> > +  /* kernel is non-interworking : must do this from Thumb */
>>>> > +  adr     r1, . + 1
>>>> > +  bx      r1
>>>> > +  .thumb
>>>> > +#endif
>>>> >    ldr     r1, kernel_flush
>>>>
>>>> Didn't you mean this instead:
>>>>
>>>>      /* kernel is non-interworking : must do this from Thumb */
>>>>      adr     r1, 1f + 1
>>>>      bx      r1
>>>>      .thumb
>>>> 1:   ldr     r1, kernel_flush
>>>>      ...
>>>
>>> Note that this is intended as an experimental hack, not a real patch
>>> (apologies if I didn't make that clear...)
>>>
>>> Well, actually I meant "add r1, pc, #1" ... which means I was too
>>> busy trying to be clever... oops!
>>>
>>> That is of course exactly equivalent to your code...
>>>
>>>>
>>>> ?
>>>>
>>>> >    blx     r1
>>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>>> > +  .align
>>>> > +  bx      pc
>>>> > +  nop
>>>> > +  .arm
>>>>
>>>> Also here, the .align has the potential to introduce a zero halfword in
>>>> the instruction stream before the bx.  What about:
>>>>
>>>>      adr     r3, 1f
>>>>      bx      r3
>>>>      .align
>>>>      .arm
>>>> 1:   ...
>>>
>>> .align inserts a 16-bit nop when misaligned in Thumb in a text section,
>>> and a word-aligned bx pc is a specific architecturally allowed way
>>> to do an inline switch to ARM.  The linker uses this trick for PLT
>>> veneers etc.
>>>
>>> A nicer fix for doing this sort of call from low-level code which
>>> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
>>>
>>> Generally, we can do this for all arches >= v5, without any
>>> incompatibility.  However, since the need for it will be rare and it
>>> will generate patch noise for not much real benefit,
>>> I haven't proposed this.
>>>
>>> Updated patch below.
>>
>> I tested the updated patch on top of your "dirty" branch I tested with
>> last week, and now see off-mode working just fine.
>
> Thanks for testing-- that's great news.
>
> I will have a think about whether the patch can be tidied up to revert
> most of the code back to Thumb, though that isn't essential.  If I've
> understood what's going on correctly, I think only the restore entry
> points, SMC call sites and the entry to omap3_sram_configure_core_dpll
> could need to be ARM; the rest shouldn't really make any difference...

Yes, that sounds right.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes
@ 2011-02-15 16:15               ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-02-15 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

Dave Martin <dave.martin@linaro.org> writes:

> On Mon, Feb 14, 2011 at 11:15 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Hi Dave,
>>
>> Dave Martin <dave.martin@linaro.org> writes:
>>
>>> On Mon, Feb 14, 2011 at 10:00:23AM -0500, Nicolas Pitre wrote:
>>>> On Mon, 14 Feb 2011, Dave Martin wrote:
>>>>
>>>> > @@ -289,8 +297,20 @@ clean_l2:
>>>> > ? ? * ?- should be faster and will change with kernel
>>>> > ? ? * ?- 'might' have to copy address, load and jump to it
>>>> > ? ? */
>>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>>> > + ?/* kernel is non-interworking : must do this from Thumb */
>>>> > + ?adr ? ? r1, . + 1
>>>> > + ?bx ? ? ?r1
>>>> > + ?.thumb
>>>> > +#endif
>>>> > ? ?ldr ? ? r1, kernel_flush
>>>>
>>>> Didn't you mean this instead:
>>>>
>>>> ? ? ?/* kernel is non-interworking : must do this from Thumb */
>>>> ? ? ?adr ? ? r1, 1f + 1
>>>> ? ? ?bx ? ? ?r1
>>>> ? ? ?.thumb
>>>> 1: ? ldr ? ? r1, kernel_flush
>>>> ? ? ?...
>>>
>>> Note that this is intended as an experimental hack, not a real patch
>>> (apologies if I didn't make that clear...)
>>>
>>> Well, actually I meant "add r1, pc, #1" ... which means I was too
>>> busy trying to be clever... oops!
>>>
>>> That is of course exactly equivalent to your code...
>>>
>>>>
>>>> ?
>>>>
>>>> > ? ?blx ? ? r1
>>>> > +#ifdef CONFIG_THUMB2_KERNEL
>>>> > + ?.align
>>>> > + ?bx ? ? ?pc
>>>> > + ?nop
>>>> > + ?.arm
>>>>
>>>> Also here, the .align has the potential to introduce a zero halfword in
>>>> the instruction stream before the bx. ?What about:
>>>>
>>>> ? ? ?adr ? ? r3, 1f
>>>> ? ? ?bx ? ? ?r3
>>>> ? ? ?.align
>>>> ? ? ?.arm
>>>> 1: ? ...
>>>
>>> .align inserts a 16-bit nop when misaligned in Thumb in a text section,
>>> and a word-aligned bx pc is a specific architecturally allowed way
>>> to do an inline switch to ARM. ?The linker uses this trick for PLT
>>> veneers etc.
>>>
>>> A nicer fix for doing this sort of call from low-level code which
>>> might be ARM is to convert arch/arm/mm/*-v7.S to use "bx lr" to return.
>>>
>>> Generally, we can do this for all arches >= v5, without any
>>> incompatibility. ?However, since the need for it will be rare and it
>>> will generate patch noise for not much real benefit,
>>> I haven't proposed this.
>>>
>>> Updated patch below.
>>
>> I tested the updated patch on top of your "dirty" branch I tested with
>> last week, and now see off-mode working just fine.
>
> Thanks for testing-- that's great news.
>
> I will have a think about whether the patch can be tidied up to revert
> most of the code back to Thumb, though that isn't essential.  If I've
> understood what's going on correctly, I think only the restore entry
> points, SMC call sites and the entry to omap3_sram_configure_core_dpll
> could need to be ARM; the rest shouldn't really make any difference...

Yes, that sounds right.

Kevin

^ permalink raw reply	[flat|nested] 30+ messages in thread
end of thread, other threads:[~2011-02-15 16:15 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-09 15:01 [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes Dave Martin
2011-02-09 15:01 ` Dave Martin
2011-02-09 15:01 ` [PATCH v4 1/5] ARM: omap4: Provide do_wfi() for Thumb-2 Dave Martin
2011-02-09 15:01   ` Dave Martin
2011-02-09 16:19   ` Nicolas Pitre
2011-02-09 16:19     ` Nicolas Pitre
2011-02-09 15:01 ` [PATCH v4 2/5] ARM: omap4: Convert END() to ENDPROC() for correct linkage with CONFIG_THUMB2_KERNEL Dave Martin
2011-02-09 15:01   ` Dave Martin
2011-02-09 15:01 ` [PATCH v4 3/5] ARM: omap3: Remove hand-encoded SMC instructions Dave Martin
2011-02-09 15:01   ` Dave Martin
2011-02-09 15:01 ` [PATCH v4 4/5] ARM: omap3: Thumb-2 compatibility for sram34xx.S Dave Martin
2011-02-09 15:01   ` Dave Martin
2011-02-09 15:01 ` [PATCH v4 5/5] ARM: omap3: Thumb-2 compatibility for sleep34xx.S Dave Martin
2011-02-09 15:01   ` Dave Martin
2011-02-11 23:31 ` [PATCH v4 0/5] ARM: omap[34]: Thumb-2 compatibility fixes Kevin Hilman
2011-02-11 23:31   ` Kevin Hilman
2011-02-14 13:17   ` Dave Martin
2011-02-14 13:17     ` Dave Martin
2011-02-14 15:00     ` Nicolas Pitre
2011-02-14 15:00       ` Nicolas Pitre
2011-02-14 15:37       ` Dave Martin
2011-02-14 15:37         ` Dave Martin
2011-02-14 20:10         ` Nicolas Pitre
2011-02-14 20:10           ` Nicolas Pitre
2011-02-14 23:15         ` Kevin Hilman
2011-02-14 23:15           ` Kevin Hilman
2011-02-15 10:45           ` Dave Martin
2011-02-15 10:45             ` Dave Martin
2011-02-15 16:15             ` Kevin Hilman
2011-02-15 16:15               ` Kevin Hilman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.