[PATCH 1/3] ARM: tegra: fix build error when THUMB2_KERNEL enabled

[PATCH 1/3] ARM: tegra: fix build error when THUMB2_KERNEL enabled

[Stephen Warren]

From: Joseph Lo <josephl@nvidia.com>

This patch fix the build failure when CONFIG_THUBM2_KERNEL enabled. You
clould see the error message below:

arch/arm/mach-tegra/sleep-tegra30.S:69: Error: shift must be constant --
`orr r12,r12,r4,lsl r3'

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Arnd, Olof, here are 3 fixes from Joseph for some Tegra build issues,
mostly exposed by multi-platform enablement and/or related randconfig
testing.

Can you please apply these to arm-soc? For reference, I tested applying
them to Tegra's for-3.10/fixes branch and they apply there fine, and
merge with the other Tegra for-3.10 branches at least, without any issue.

Note: patch 3 triggers checkpatch, but I can't work out why; the code
seems to be formatted the same as patch 1 and 2, and is consistent with
other usage of ARM() and THUMB() macros in arch/arm/.

 arch/arm/mach-tegra/sleep-tegra30.S |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 63a15bd..d29dfcc 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -66,7 +66,9 @@ ENTRY(tegra30_cpu_shutdown)
 		FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG | \
 		FLOW_CTRL_CSR_ENABLE
 	mov	r4, #(1 << 4)
-	orr	r12, r12, r4, lsl r3
+ ARM(	orr	r12, r12, r4, lsl r3	)
+ THUMB(	lsl	r4, r4, r3		)
+ THUMB(	orr	r12, r12, r4		)
 	str	r12, [r1]
 
 	/* Halt this CPU. */
-- 
1.7.10.4

[PATCH 2/3] ARM: tegra: fix relocation truncated error when THUMB2_KERNEL enabled

[Stephen Warren]

From: Joseph Lo <josephl@nvidia.com>

The conditional branch instruction in Thumb2 only available to short range.
The linker will fail when the conditional branch over the range. Then
resulting in link error when generating kernel image. e.g.:

arch/arm/mach-tegra/reset-handler.S:47:(.text+0xf8e):
relocation truncated to fit: R_ARM_THM_JUMP19 against symbol
`cpu_resume' defined in .data section in arch/arm/kernel/built-in.o

This patch using a Thumb2 instruction IT (if-then) to have a longer branch
range.

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/mach-tegra/reset-handler.S |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index 1676aba..e6de88a 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -44,6 +44,7 @@ ENTRY(tegra_resume)
 
 	cpu_id	r0
 	cmp	r0, #0				@ CPU0?
+ THUMB(	it	ne )
 	bne	cpu_resume			@ no
 
 #ifdef CONFIG_ARCH_TEGRA_3x_SOC
-- 
1.7.10.4

[PATCH 2/3] ARM: tegra: fix relocation truncated error when THUMB2_KERNEL enabled

[Dave Martin]

On Mon, Apr 15, 2013 at 04:50:54PM -0600, Stephen Warren wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> The conditional branch instruction in Thumb2 only available to short range.
> The linker will fail when the conditional branch over the range. Then
> resulting in link error when generating kernel image. e.g.:
> 
> arch/arm/mach-tegra/reset-handler.S:47:(.text+0xf8e):
> relocation truncated to fit: R_ARM_THM_JUMP19 against symbol
> `cpu_resume' defined in .data section in arch/arm/kernel/built-in.o
> 
> This patch using a Thumb2 instruction IT (if-then) to have a longer branch
> range.
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

FWIW, Reviewed-by: Dave Martin <dave.martin@linaro.org>

> ---
>  arch/arm/mach-tegra/reset-handler.S |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> index 1676aba..e6de88a 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -44,6 +44,7 @@ ENTRY(tegra_resume)
>  
>  	cpu_id	r0
>  	cmp	r0, #0				@ CPU0?
> + THUMB(	it	ne )
>  	bne	cpu_resume			@ no
>  
>  #ifdef CONFIG_ARCH_TEGRA_3x_SOC
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] ARM: tegra: fix relocation truncated error when THUMB2_KERNEL enabled

[Olof Johansson]

On Mon, Apr 15, 2013 at 04:50:54PM -0600, Stephen Warren wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> The conditional branch instruction in Thumb2 only available to short range.
> The linker will fail when the conditional branch over the range. Then
> resulting in link error when generating kernel image. e.g.:
> 
> arch/arm/mach-tegra/reset-handler.S:47:(.text+0xf8e):
> relocation truncated to fit: R_ARM_THM_JUMP19 against symbol
> `cpu_resume' defined in .data section in arch/arm/kernel/built-in.o
> 
> This patch using a Thumb2 instruction IT (if-then) to have a longer branch
> range.
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>


Applied to fixes-non-critical.


-Olof

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Stephen Warren]

From: Joseph Lo <josephl@nvidia.com>

When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
assembly may not on the 4 byte alignment. Then causing a data abort when
accessing the pointer. This patch add a ".align" flag in the head of the
pointer. And always using 32-bit ADR Thumb instruction to make sure it
won't build failure.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/mach-tegra/reset-handler.S |    1 +
 arch/arm/mach-tegra/sleep.h         |    3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index e6de88a..519a8c5 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
 
 #ifdef CONFIG_CACHE_L2X0
 	.globl	l2x0_saved_regs_addr
+	.align
 l2x0_saved_regs_addr:
 	.long	0
 #endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 4ffae54..8e9b6af 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -92,7 +92,8 @@
 
 #ifdef CONFIG_CACHE_L2X0
 .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
-	adr	\tmp1, \phys_l2x0_saved_regs
+ ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
+ THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
 	ldr	\tmp1, [\tmp1]
 	ldr	\tmp2, [\tmp1, #L2X0_R_PHY_BASE]
 	ldr	\tmp3, [\tmp2, #L2X0_CTRL]
-- 
1.7.10.4

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Dave Martin]

On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
> assembly may not on the 4 byte alignment. Then causing a data abort when
> accessing the pointer. This patch add a ".align" flag in the head of the
> pointer. And always using 32-bit ADR Thumb instruction to make sure it
> won't build failure.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  arch/arm/mach-tegra/reset-handler.S |    1 +
>  arch/arm/mach-tegra/sleep.h         |    3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> index e6de88a..519a8c5 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
>  
>  #ifdef CONFIG_CACHE_L2X0
>  	.globl	l2x0_saved_regs_addr
> +	.align
>  l2x0_saved_regs_addr:
>  	.long	0
>  #endif
> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
> index 4ffae54..8e9b6af 100644
> --- a/arch/arm/mach-tegra/sleep.h
> +++ b/arch/arm/mach-tegra/sleep.h
> @@ -92,7 +92,8 @@
>  
>  #ifdef CONFIG_CACHE_L2X0
>  .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
> -	adr	\tmp1, \phys_l2x0_saved_regs
> + ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
> + THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )

Can you give an example of the assembler error you get without this?

The target symbol is local and the assembler can see where it is,
so it should choose the correct variant for the adr instruction
with no need for the ".w" suffix.

If not, it could mean that there is a bug in the version of the
assembler you're using.


If it's definitely needed, you can append a Thumb-only .w suffix with

	W(adr)	\tmp1, \phys_l2x0_saved_regs

...which is slightly neater.

Cheers
---Dave

>  	ldr	\tmp1, [\tmp1]
>  	ldr	\tmp2, [\tmp1, #L2X0_R_PHY_BASE]
>  	ldr	\tmp3, [\tmp2, #L2X0_CTRL]
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Stephen Warren]

On 04/16/2013 08:13 AM, Dave Martin wrote:
> On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
>> From: Joseph Lo <josephl@nvidia.com>
>>
>> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
>> assembly may not on the 4 byte alignment. Then causing a data abort when
>> accessing the pointer. This patch add a ".align" flag in the head of the
>> pointer. And always using 32-bit ADR Thumb instruction to make sure it
>> won't build failure.
>>
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  arch/arm/mach-tegra/reset-handler.S |    1 +
>>  arch/arm/mach-tegra/sleep.h         |    3 ++-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
>> index e6de88a..519a8c5 100644
>> --- a/arch/arm/mach-tegra/reset-handler.S
>> +++ b/arch/arm/mach-tegra/reset-handler.S
>> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
>>  
>>  #ifdef CONFIG_CACHE_L2X0
>>  	.globl	l2x0_saved_regs_addr
>> +	.align
>>  l2x0_saved_regs_addr:
>>  	.long	0
>>  #endif
>> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
>> index 4ffae54..8e9b6af 100644
>> --- a/arch/arm/mach-tegra/sleep.h
>> +++ b/arch/arm/mach-tegra/sleep.h
>> @@ -92,7 +92,8 @@
>>  
>>  #ifdef CONFIG_CACHE_L2X0
>>  .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
>> -	adr	\tmp1, \phys_l2x0_saved_regs
>> + ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
>> + THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
> 
> Can you give an example of the assembler error you get without this?

arch/arm/mach-tegra/reset-handler.S: Assembler messages:
arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for
address calculation (value = 0x00000004)

This is with gcc-4.5.3, as:
GNU assembler (crosstool-NG hg_unknown at 20110628.165246) 2.20.1.20100303

> The target symbol is local and the assembler can see where it is,
> so it should choose the correct variant for the adr instruction
> with no need for the ".w" suffix.
> 
> If not, it could mean that there is a bug in the version of the
> assembler you're using.

True. Switching to the Ubuntu-packaged ARM cross-compiler that ships
with Ubuntu 12.10, I do not see this problem.

Still, many people probably still use older compilers.

> If it's definitely needed, you can append a Thumb-only .w suffix with
> 
> 	W(adr)	\tmp1, \phys_l2x0_saved_regs
> 
> ...which is slightly neater.

Yes, that certainly works too, even without the .align change in
reset-handler.S.

Arnd, Olof, I guess let's drop this once patch from this series, but
apply the other two, and I'll send an updated version of this one patch.
Thanks.

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Dave Martin]

On Tue, Apr 16, 2013 at 10:20:11AM -0600, Stephen Warren wrote:
> On 04/16/2013 08:13 AM, Dave Martin wrote:
> > On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
> >> From: Joseph Lo <josephl@nvidia.com>
> >>
> >> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
> >> assembly may not on the 4 byte alignment. Then causing a data abort when
> >> accessing the pointer. This patch add a ".align" flag in the head of the
> >> pointer. And always using 32-bit ADR Thumb instruction to make sure it
> >> won't build failure.
> >>
> >> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >> ---
> >>  arch/arm/mach-tegra/reset-handler.S |    1 +
> >>  arch/arm/mach-tegra/sleep.h         |    3 ++-
> >>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> >> index e6de88a..519a8c5 100644
> >> --- a/arch/arm/mach-tegra/reset-handler.S
> >> +++ b/arch/arm/mach-tegra/reset-handler.S
> >> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
> >>  
> >>  #ifdef CONFIG_CACHE_L2X0
> >>  	.globl	l2x0_saved_regs_addr
> >> +	.align
> >>  l2x0_saved_regs_addr:
> >>  	.long	0
> >>  #endif
> >> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
> >> index 4ffae54..8e9b6af 100644
> >> --- a/arch/arm/mach-tegra/sleep.h
> >> +++ b/arch/arm/mach-tegra/sleep.h
> >> @@ -92,7 +92,8 @@
> >>  
> >>  #ifdef CONFIG_CACHE_L2X0
> >>  .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
> >> -	adr	\tmp1, \phys_l2x0_saved_regs
> >> + ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
> >> + THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
> > 
> > Can you give an example of the assembler error you get without this?
> 
> arch/arm/mach-tegra/reset-handler.S: Assembler messages:
> arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for
> address calculation (value = 0x00000004)

The immediate in the 16-bit form of ADR is alignment-sensitive, and
can't address anything that's not on a word boundary.  What if you just
have the .align, without adr.w?

> This is with gcc-4.5.3, as:
> GNU assembler (crosstool-NG hg_unknown at 20110628.165246) 2.20.1.20100303
> 
> > The target symbol is local and the assembler can see where it is,
> > so it should choose the correct variant for the adr instruction
> > with no need for the ".w" suffix.
> > 
> > If not, it could mean that there is a bug in the version of the
> > assembler you're using.
> 
> True. Switching to the Ubuntu-packaged ARM cross-compiler that ships
> with Ubuntu 12.10, I do not see this problem.
> 
> Still, many people probably still use older compilers.
> 
> > If it's definitely needed, you can append a Thumb-only .w suffix with
> > 
> > 	W(adr)	\tmp1, \phys_l2x0_saved_regs
> > 
> > ...which is slightly neater.
> 
> Yes, that certainly works too, even without the .align change in
> reset-handler.S.

Sure, although the unaligned access is not such a good idea anyway.

Keeping the adr fix is harmless, I just wanted to understand why it's
needed.

Cheers
---Dave

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Stephen Warren]

On 04/17/2013 04:30 AM, Dave Martin wrote:
> On Tue, Apr 16, 2013 at 10:20:11AM -0600, Stephen Warren wrote:
>> On 04/16/2013 08:13 AM, Dave Martin wrote:
>>> On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
>>>> From: Joseph Lo <josephl@nvidia.com>
>>>>
>>>> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
>>>> assembly may not on the 4 byte alignment. Then causing a data abort when
>>>> accessing the pointer. This patch add a ".align" flag in the head of the
>>>> pointer. And always using 32-bit ADR Thumb instruction to make sure it
>>>> won't build failure.
>>>>
>>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>> ---
>>>>  arch/arm/mach-tegra/reset-handler.S |    1 +
>>>>  arch/arm/mach-tegra/sleep.h         |    3 ++-
>>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
>>>> index e6de88a..519a8c5 100644
>>>> --- a/arch/arm/mach-tegra/reset-handler.S
>>>> +++ b/arch/arm/mach-tegra/reset-handler.S
>>>> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
>>>>  
>>>>  #ifdef CONFIG_CACHE_L2X0
>>>>  	.globl	l2x0_saved_regs_addr
>>>> +	.align
>>>>  l2x0_saved_regs_addr:
>>>>  	.long	0
>>>>  #endif
>>>> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
>>>> index 4ffae54..8e9b6af 100644
>>>> --- a/arch/arm/mach-tegra/sleep.h
>>>> +++ b/arch/arm/mach-tegra/sleep.h
>>>> @@ -92,7 +92,8 @@
>>>>  
>>>>  #ifdef CONFIG_CACHE_L2X0
>>>>  .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
>>>> -	adr	\tmp1, \phys_l2x0_saved_regs
>>>> + ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
>>>> + THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
>>>
>>> Can you give an example of the assembler error you get without this?
>>
>> arch/arm/mach-tegra/reset-handler.S: Assembler messages:
>> arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for
>> address calculation (value = 0x00000004)
> 
> The immediate in the 16-bit form of ADR is alignment-sensitive, and
> can't address anything that's not on a word boundary.  What if you just
> have the .align, without adr.w?
> 
>> This is with gcc-4.5.3, as:
>> GNU assembler (crosstool-NG hg_unknown at 20110628.165246) 2.20.1.20100303
>>
>>> The target symbol is local and the assembler can see where it is,
>>> so it should choose the correct variant for the adr instruction
>>> with no need for the ".w" suffix.
>>>
>>> If not, it could mean that there is a bug in the version of the
>>> assembler you're using.
>>
>> True. Switching to the Ubuntu-packaged ARM cross-compiler that ships
>> with Ubuntu 12.10, I do not see this problem.
>>
>> Still, many people probably still use older compilers.
>>
>>> If it's definitely needed, you can append a Thumb-only .w suffix with
>>>
>>> 	W(adr)	\tmp1, \phys_l2x0_saved_regs
>>>
>>> ...which is slightly neater.
>>
>> Yes, that certainly works too, even without the .align change in
>> reset-handler.S.
> 
> Sure, although the unaligned access is not such a good idea anyway.
> 
> Keeping the adr fix is harmless, I just wanted to understand why it's
> needed.

With just the following, and no adr.w:

> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> index e6de88a..519a8c5 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
>  
>  #ifdef CONFIG_CACHE_L2X0
>         .globl  l2x0_saved_regs_addr
> +       .align
>  l2x0_saved_regs_addr:
>         .long   0
>  #endif

I still get:

> arch/arm/mach-tegra/reset-handler.S: Assembler messages:
> arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for address calculation (value = 0x00000004)

That's with GNU assembler (crosstool-NG hg_unknown at 20110628.165246)
2.20.1.20100303.

BTW, I'm on vacation for the next 2 weeks starting this afternoon.
Hopefully Joseph can keep this thread alive while I'm gone if needed.

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Dave Martin]

On Wed, Apr 17, 2013 at 10:22:02AM -0600, Stephen Warren wrote:
> On 04/17/2013 04:30 AM, Dave Martin wrote:
> > On Tue, Apr 16, 2013 at 10:20:11AM -0600, Stephen Warren wrote:
> >> On 04/16/2013 08:13 AM, Dave Martin wrote:
> >>> On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
> >>>> From: Joseph Lo <josephl@nvidia.com>
> >>>>
> >>>> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
> >>>> assembly may not on the 4 byte alignment. Then causing a data abort when
> >>>> accessing the pointer. This patch add a ".align" flag in the head of the
> >>>> pointer. And always using 32-bit ADR Thumb instruction to make sure it
> >>>> won't build failure.
> >>>>
> >>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> >>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >>>> ---
> >>>>  arch/arm/mach-tegra/reset-handler.S |    1 +
> >>>>  arch/arm/mach-tegra/sleep.h         |    3 ++-
> >>>>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> >>>> index e6de88a..519a8c5 100644
> >>>> --- a/arch/arm/mach-tegra/reset-handler.S
> >>>> +++ b/arch/arm/mach-tegra/reset-handler.S
> >>>> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
> >>>>  
> >>>>  #ifdef CONFIG_CACHE_L2X0
> >>>>  	.globl	l2x0_saved_regs_addr
> >>>> +	.align
> >>>>  l2x0_saved_regs_addr:
> >>>>  	.long	0
> >>>>  #endif
> >>>> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
> >>>> index 4ffae54..8e9b6af 100644
> >>>> --- a/arch/arm/mach-tegra/sleep.h
> >>>> +++ b/arch/arm/mach-tegra/sleep.h
> >>>> @@ -92,7 +92,8 @@
> >>>>  
> >>>>  #ifdef CONFIG_CACHE_L2X0
> >>>>  .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
> >>>> -	adr	\tmp1, \phys_l2x0_saved_regs
> >>>> + ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
> >>>> + THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
> >>>
> >>> Can you give an example of the assembler error you get without this?
> >>
> >> arch/arm/mach-tegra/reset-handler.S: Assembler messages:
> >> arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for
> >> address calculation (value = 0x00000004)
> > 
> > The immediate in the 16-bit form of ADR is alignment-sensitive, and
> > can't address anything that's not on a word boundary.  What if you just
> > have the .align, without adr.w?
> > 
> >> This is with gcc-4.5.3, as:
> >> GNU assembler (crosstool-NG hg_unknown at 20110628.165246) 2.20.1.20100303
> >>
> >>> The target symbol is local and the assembler can see where it is,
> >>> so it should choose the correct variant for the adr instruction
> >>> with no need for the ".w" suffix.
> >>>
> >>> If not, it could mean that there is a bug in the version of the
> >>> assembler you're using.
> >>
> >> True. Switching to the Ubuntu-packaged ARM cross-compiler that ships
> >> with Ubuntu 12.10, I do not see this problem.
> >>
> >> Still, many people probably still use older compilers.
> >>
> >>> If it's definitely needed, you can append a Thumb-only .w suffix with
> >>>
> >>> 	W(adr)	\tmp1, \phys_l2x0_saved_regs
> >>>
> >>> ...which is slightly neater.
> >>
> >> Yes, that certainly works too, even without the .align change in
> >> reset-handler.S.
> > 
> > Sure, although the unaligned access is not such a good idea anyway.
> > 
> > Keeping the adr fix is harmless, I just wanted to understand why it's
> > needed.
> 
> With just the following, and no adr.w:
> 
> > diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> > index e6de88a..519a8c5 100644
> > --- a/arch/arm/mach-tegra/reset-handler.S
> > +++ b/arch/arm/mach-tegra/reset-handler.S
> > @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
> >  
> >  #ifdef CONFIG_CACHE_L2X0
> >         .globl  l2x0_saved_regs_addr
> > +       .align
> >  l2x0_saved_regs_addr:
> >         .long   0
> >  #endif
> 
> I still get:
> 
> > arch/arm/mach-tegra/reset-handler.S: Assembler messages:
> > arch/arm/mach-tegra/reset-handler.S:78: Error: invalid immediate for address calculation (value = 0x00000004)
> 
> That's with GNU assembler (crosstool-NG hg_unknown at 20110628.165246)
> 2.20.1.20100303.
> 
> BTW, I'm on vacation for the next 2 weeks starting this afternoon.
> Hopefully Joseph can keep this thread alive while I'm gone if needed.

OK.  I'm happy to go with the W() so long definitely fixes something :)

Cheers
---Dave

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Olof Johansson]

On Tue, Apr 16, 2013 at 10:20:11AM -0600, Stephen Warren wrote:
> Arnd, Olof, I guess let's drop this once patch from this series, but
> apply the other two, and I'll send an updated version of this one patch.
> Thanks.

Ok.

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Dave Martin]

On Mon, Apr 15, 2013 at 04:50:55PM -0600, Stephen Warren wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
> assembly may not on the 4 byte alignment. Then causing a data abort when
> accessing the pointer. This patch add a ".align" flag in the head of the
> pointer. And always using 32-bit ADR Thumb instruction to make sure it
> won't build failure.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

With the change to W(adr), Reviewed-by: Dave Martin <dave.martin@linaro.org>

> ---
>  arch/arm/mach-tegra/reset-handler.S |    1 +
>  arch/arm/mach-tegra/sleep.h         |    3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> index e6de88a..519a8c5 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
>  
>  #ifdef CONFIG_CACHE_L2X0
>  	.globl	l2x0_saved_regs_addr
> +	.align
>  l2x0_saved_regs_addr:
>  	.long	0
>  #endif
> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
> index 4ffae54..8e9b6af 100644
> --- a/arch/arm/mach-tegra/sleep.h
> +++ b/arch/arm/mach-tegra/sleep.h
> @@ -92,7 +92,8 @@
>  
>  #ifdef CONFIG_CACHE_L2X0
>  .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
> -	adr	\tmp1, \phys_l2x0_saved_regs
> + ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
> + THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
>  	ldr	\tmp1, [\tmp1]
>  	ldr	\tmp2, [\tmp1, #L2X0_R_PHY_BASE]
>  	ldr	\tmp3, [\tmp2, #L2X0_CTRL]
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] ARM: tegra: fix build error when THUMB2_KERNEL enabled

[Dave Martin]

On Mon, Apr 15, 2013 at 04:50:53PM -0600, Stephen Warren wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> This patch fix the build failure when CONFIG_THUBM2_KERNEL enabled. You
> clould see the error message below:
> 
> arch/arm/mach-tegra/sleep-tegra30.S:69: Error: shift must be constant --
> `orr r12,r12,r4,lsl r3'
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

FWIW, Reviewed-by: Dave Martin <dave.martin@linaro.org>

> ---
> Arnd, Olof, here are 3 fixes from Joseph for some Tegra build issues,
> mostly exposed by multi-platform enablement and/or related randconfig
> testing.
> 
> Can you please apply these to arm-soc? For reference, I tested applying
> them to Tegra's for-3.10/fixes branch and they apply there fine, and
> merge with the other Tegra for-3.10 branches at least, without any issue.
> 
> Note: patch 3 triggers checkpatch, but I can't work out why; the code
> seems to be formatted the same as patch 1 and 2, and is consistent with
> other usage of ARM() and THUMB() macros in arch/arm/.
> 
>  arch/arm/mach-tegra/sleep-tegra30.S |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> index 63a15bd..d29dfcc 100644
> --- a/arch/arm/mach-tegra/sleep-tegra30.S
> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -66,7 +66,9 @@ ENTRY(tegra30_cpu_shutdown)
>  		FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG | \
>  		FLOW_CTRL_CSR_ENABLE
>  	mov	r4, #(1 << 4)
> -	orr	r12, r12, r4, lsl r3
> + ARM(	orr	r12, r12, r4, lsl r3	)
> + THUMB(	lsl	r4, r4, r3		)
> + THUMB(	orr	r12, r12, r4		)
>  	str	r12, [r1]
>  
>  	/* Halt this CPU. */
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] ARM: tegra: fix build error when THUMB2_KERNEL enabled

[Olof Johansson]

On Mon, Apr 15, 2013 at 04:50:53PM -0600, Stephen Warren wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> This patch fix the build failure when CONFIG_THUBM2_KERNEL enabled. You
> clould see the error message below:
> 
> arch/arm/mach-tegra/sleep-tegra30.S:69: Error: shift must be constant --
> `orr r12,r12,r4,lsl r3'
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> Arnd, Olof, here are 3 fixes from Joseph for some Tegra build issues,
> mostly exposed by multi-platform enablement and/or related randconfig
> testing.
> 
> Can you please apply these to arm-soc? For reference, I tested applying
> them to Tegra's for-3.10/fixes branch and they apply there fine, and
> merge with the other Tegra for-3.10 branches at least, without any issue.
> 
> Note: patch 3 triggers checkpatch, but I can't work out why; the code
> seems to be formatted the same as patch 1 and 2, and is consistent with
> other usage of ARM() and THUMB() macros in arch/arm/.


Applied to fixes-non-critical.


-Olof

[PATCH 1/3] ARM: tegra: fix build error when THUMB2_KERNEL enabled

[Joseph Lo]

This patch fix the build failure when CONFIG_THUBM2_KERNEL enabled. You
clould see the error message below:

arch/arm/mach-tegra/sleep-tegra30.S:69: Error: shift must be constant --
`orr r12,r12,r4,lsl r3'

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/sleep-tegra30.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index 63a15bd..d29dfcc 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -66,7 +66,9 @@ ENTRY(tegra30_cpu_shutdown)
 		FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG | \
 		FLOW_CTRL_CSR_ENABLE
 	mov	r4, #(1 << 4)
-	orr	r12, r12, r4, lsl r3
+ ARM(	orr	r12, r12, r4, lsl r3	)
+ THUMB(	lsl	r4, r4, r3		)
+ THUMB(	orr	r12, r12, r4		)
 	str	r12, [r1]
 
 	/* Halt this CPU. */
-- 
1.8.2

[PATCH 3/3] ARM: tegra: make sure the pointer on 4 byte align when THUMB2_KERNEL enabled

[Joseph Lo]

When building kernel with CONFIG_THUMB2_KERNEL, the data pointer in the
assembly may not on the 4 byte alignment. Then causing a data abort when
accessing the pointer. This patch add a ".align" flag in the head of the
pointer. And always using 32-bit ADR Thumb instruction to make sure it
won't build failure.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/reset-handler.S | 1 +
 arch/arm/mach-tegra/sleep.h         | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index e6de88a..519a8c5 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -83,6 +83,7 @@ ENDPROC(tegra_resume)
 
 #ifdef CONFIG_CACHE_L2X0
 	.globl	l2x0_saved_regs_addr
+	.align
 l2x0_saved_regs_addr:
 	.long	0
 #endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 970ebd5..73a49b7 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -92,7 +92,8 @@
 
 #ifdef CONFIG_CACHE_L2X0
 .macro l2_cache_resume, tmp1, tmp2, tmp3, phys_l2x0_saved_regs
-	adr	\tmp1, \phys_l2x0_saved_regs
+ ARM(	adr	\tmp1, \phys_l2x0_saved_regs )
+ THUMB(	adr.w	\tmp1, \phys_l2x0_saved_regs )
 	ldr	\tmp1, [\tmp1]
 	ldr	\tmp2, [\tmp1, #L2X0_R_PHY_BASE]
 	ldr	\tmp3, [\tmp2, #L2X0_CTRL]
-- 
1.8.2