[RFC] Fix omap3 booting with thumb2 compiled kernel

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Tony Lindgren]

The power management related assembly needs to interact with
ARM mode bootrom code, so we need to keep most of the related
assembly in ARM mode.

Currently we are entering into and ARM mode assembly function
from thumb2 mode, so we need to make sure we switch to ARM
mode. And we need to do that again after the cache flush.

---

Kevin told me about this earlier today.. Anybody got better ideas
for a fix here?

--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -151,6 +151,17 @@ ENTRY(save_secure_ram_context_sz)
  */
 	.align	3
 ENTRY(omap34xx_cpu_suspend)
+
+	/*
+	 * This ARM assembly can also be called from thumb2 kernel code.
+	 * Make sure we switch to ARM mode first.
+	 */
+ THUMB(	.thumb		)
+ THUMB(	.align		)
+ THUMB(	bx	pc	)
+ THUMB(	nop		)
+	.arm
+
 	stmfd	sp!, {r4 - r11, lr}	@ save registers on stack
 
 	/*
@@ -187,6 +198,18 @@ save_context_wfi:
 	bx	r1
 
 	/*
+	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
+	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
+	 * This sequence switches back to ARM.  Note that .align may insert a
+	 * nop: bx pc needs to be word-aligned in order to work.
+	 */
+ THUMB(	.thumb		)
+ THUMB(	.align		)
+ THUMB(	bx	pc	)
+ THUMB(	nop		)
+	.arm
+
+	/*
 	 * Clear the SCTLR.C bit to prevent further data cache
 	 * allocation. Clearing SCTLR.C would make all the data accesses
 	 * strongly ordered and would not hit the cache.
@@ -203,12 +226,8 @@ save_context_wfi:
 	 */
 	ldr	r1, kernel_flush
 	blx	r1
-	/*
-	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
-	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
-	 * This sequence switches back to ARM.  Note that .align may insert a
-	 * nop: bx pc needs to be word-aligned in order to work.
-	 */
+
+	/* See the comments above about v7_flush_dcache_all */
  THUMB(	.thumb		)
  THUMB(	.align		)
  THUMB(	bx	pc	)

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Kevin Hilman]

[ fix email for Dave Martin, +Tyler ]

Tony Lindgren <tony@atomide.com> writes:

> The power management related assembly needs to interact with
> ARM mode bootrom code, so we need to keep most of the related
> assembly in ARM mode.
>
> Currently we are entering into and ARM mode assembly function
> from thumb2 mode, so we need to make sure we switch to ARM
> mode. And we need to do that again after the cache flush.
>
> ---
>
> Kevin told me about this earlier today.. 


And for full boot log/panics, see the kernelci.org thumb2 kernel boots
that fail: http://kernelci.org/boot/?THUMB2_KERNEL&fail

> Anybody got better ideas for a fix here?

FWIW, a quick test of this patch makes my omap3-beagle-xm pass a simple
boot test, but it fails with off idle.

Kevin

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [150527 15:20]:
> [ fix email for Dave Martin, +Tyler ]
> 
> Tony Lindgren <tony@atomide.com> writes:
> 
> > The power management related assembly needs to interact with
> > ARM mode bootrom code, so we need to keep most of the related
> > assembly in ARM mode.
> >
> > Currently we are entering into and ARM mode assembly function
> > from thumb2 mode, so we need to make sure we switch to ARM
> > mode. And we need to do that again after the cache flush.
> >
> > ---
> >
> > Kevin told me about this earlier today.. 
> 
> 
> And for full boot log/panics, see the kernelci.org thumb2 kernel boots
> that fail: http://kernelci.org/boot/?THUMB2_KERNEL&fail
> 
> > Anybody got better ideas for a fix here?
> 
> FWIW, a quick test of this patch makes my omap3-beagle-xm pass a simple
> boot test, but it fails with off idle.

Thanks to Stephen Boyd's suggestion of checking the missing ENDPROC,
here's a better fix :) Now off idle works too.

Regards,

Tony

8 <----------------------
From: Tony Lindgren <tony@atomide.com>
Date: Wed, 27 May 2015 15:33:57 -0700
Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel

We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:

Internal error: Oops: 80000005 [#1] SMP THUMB2
...
[<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
(omap3_enter_idle_bm+0xc5/0x178)
[<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
(cpuidle_enter_state+0x77/0x27c)
[<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
(cpu_startup_entry+0x155/0x23c)
[<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
(start_kernel+0x32f/0x338)
[<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)

The power management related assembly on moaps needs to interact with
ARM mode bootrom code, so we need to keep most of the related assembly
in ARM mode.

Turns out this error is because of missing ENDPROC for assembly code
as suggested by Stephen Boyd <sboyd@codeaurora.org>. Let's add the
missing ENDPROC in two places to sleep34xx.S, and also remove the
earlier mystery code that was probably also caused by missing ENDPROC
for earlier kernels.

Reported-by: Kevin Hilman <khilman@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -203,23 +203,8 @@ save_context_wfi:
 	 */
 	ldr	r1, kernel_flush
 	blx	r1
-	/*
-	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
-	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
-	 * This sequence switches back to ARM.  Note that .align may insert a
-	 * nop: bx pc needs to be word-aligned in order to work.
-	 */
- THUMB(	.thumb		)
- THUMB(	.align		)
- THUMB(	bx	pc	)
- THUMB(	nop		)
-	.arm
-
 	b	omap3_do_wfi
-
-/*
- * Local variables
- */
+ENDPROC(omap34xx_cpu_suspend)
 omap3_do_wfi_sram_addr:
 	.word omap3_do_wfi_sram
 kernel_flush:
@@ -364,10 +349,7 @@ exit_nonoff_modes:
  * ===================================
  */
 	ldmfd	sp!, {r4 - r11, pc}	@ restore regs and return
-
-/*
- * Local variables
- */
+ENDPROC(omap3_do_wfi)
 sdrc_power:
 	.word	SDRC_POWER_V
 cm_idlest1_core:

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@kernel.org> [150527 15:20]:
>> [ fix email for Dave Martin, +Tyler ]
>> 
>> Tony Lindgren <tony@atomide.com> writes:
>> 
>> > The power management related assembly needs to interact with
>> > ARM mode bootrom code, so we need to keep most of the related
>> > assembly in ARM mode.
>> >
>> > Currently we are entering into and ARM mode assembly function
>> > from thumb2 mode, so we need to make sure we switch to ARM
>> > mode. And we need to do that again after the cache flush.
>> >
>> > ---
>> >
>> > Kevin told me about this earlier today.. 
>> 
>> 
>> And for full boot log/panics, see the kernelci.org thumb2 kernel boots
>> that fail: http://kernelci.org/boot/?THUMB2_KERNEL&fail
>> 
>> > Anybody got better ideas for a fix here?
>> 
>> FWIW, a quick test of this patch makes my omap3-beagle-xm pass a simple
>> boot test, but it fails with off idle.
>
> Thanks to Stephen Boyd's suggestion of checking the missing ENDPROC,
> here's a better fix :) Now off idle works too.
>
> Regards,
>
> Tony
>
> 8 <----------------------
> From: Tony Lindgren <tony@atomide.com>
> Date: Wed, 27 May 2015 15:33:57 -0700
> Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel
>
> We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:
>
> Internal error: Oops: 80000005 [#1] SMP THUMB2
> ...
> [<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
> (omap3_enter_idle_bm+0xc5/0x178)
> [<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
> (cpuidle_enter_state+0x77/0x27c)
> [<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
> (cpu_startup_entry+0x155/0x23c)
> [<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
> (start_kernel+0x32f/0x338)
> [<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)
>
> The power management related assembly on moaps needs to interact with
> ARM mode bootrom code, so we need to keep most of the related assembly
> in ARM mode.
>
> Turns out this error is because of missing ENDPROC for assembly code
> as suggested by Stephen Boyd <sboyd@codeaurora.org>. Let's add the
> missing ENDPROC in two places to sleep34xx.S, and also remove the
> earlier mystery code that was probably also caused by missing ENDPROC
> for earlier kernels.
>
> Reported-by: Kevin Hilman <khilman@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Yup, boot test is now passing for me on omap3-beagle, omap3-beagle-xm,
omap3-n900, omap3-overo-tobi, omap3-overo-storm-tobi.

Tested-by: Kevin Hilman <khilman@linaro.org>

Thanks for the quick fix.

Next step is to look at the exynos4412-odroidu3 failure which probably
has a similar cause.

Kevin

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Grazvydas Ignotas]

On Thu, May 28, 2015 at 1:55 AM, Tony Lindgren <tony@atomide.com> wrote:
> 8 <----------------------
> From: Tony Lindgren <tony@atomide.com>
> Date: Wed, 27 May 2015 15:33:57 -0700
> Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel
>
> We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:
>
> Internal error: Oops: 80000005 [#1] SMP THUMB2
> ...
> [<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
> (omap3_enter_idle_bm+0xc5/0x178)
> [<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
> (cpuidle_enter_state+0x77/0x27c)
> [<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
> (cpu_startup_entry+0x155/0x23c)
> [<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
> (start_kernel+0x32f/0x338)
> [<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)
>
> The power management related assembly on moaps needs to interact with

moaps -> omaps


Gra?vydas

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Tony Lindgren]

* Grazvydas Ignotas <notasas@gmail.com> [150528 04:45]:
> On Thu, May 28, 2015 at 1:55 AM, Tony Lindgren <tony@atomide.com> wrote:
> > 8 <----------------------
> > From: Tony Lindgren <tony@atomide.com>
> > Date: Wed, 27 May 2015 15:33:57 -0700
> > Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel
> >
> > We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:
> >
> > Internal error: Oops: 80000005 [#1] SMP THUMB2
> > ...
> > [<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
> > (omap3_enter_idle_bm+0xc5/0x178)
> > [<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
> > (cpuidle_enter_state+0x77/0x27c)
> > [<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
> > (cpu_startup_entry+0x155/0x23c)
> > [<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
> > (start_kernel+0x32f/0x338)
> > [<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)
> >
> > The power management related assembly on moaps needs to interact with
> 
> moaps -> omaps

Thanks, fixed now. I also added a link for why ENDPROC is needed to the
description. I'll apply the patch below into fixes today.

Regards,

Tony

8< ----------------------
From: Tony Lindgren <tony@atomide.com>
Date: Wed, 27 May 2015 15:33:57 -0700
Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel

We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:

Internal error: Oops: 80000005 [#1] SMP THUMB2
...
[<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
(omap3_enter_idle_bm+0xc5/0x178)
[<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
(cpuidle_enter_state+0x77/0x27c)
[<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
(cpu_startup_entry+0x155/0x23c)
[<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
(start_kernel+0x32f/0x338)
[<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)

The power management related assembly on omaps needs to interact with
ARM mode bootrom code, so we need to keep most of the related assembly
in ARM mode.

Turns out this error is because of missing ENDPROC for assembly code
as suggested by Stephen Boyd <sboyd@codeaurora.org>. Let's add the
missing ENDPROC in two places to sleep34xx.S, and also remove the
earlier mystery code that was probably also caused by missing ENDPROC
for earlier kernels.

The reason why ENDPROC makes a difference is it sets .type and then
the compiler knows what to do with the thumb bit as explained at:

https://wiki.ubuntu.com/ARM/Thumb2PortingHowto

Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <khilman@linaro.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -203,23 +203,8 @@ save_context_wfi:
 	 */
 	ldr	r1, kernel_flush
 	blx	r1
-	/*
-	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
-	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
-	 * This sequence switches back to ARM.  Note that .align may insert a
-	 * nop: bx pc needs to be word-aligned in order to work.
-	 */
- THUMB(	.thumb		)
- THUMB(	.align		)
- THUMB(	bx	pc	)
- THUMB(	nop		)
-	.arm
-
 	b	omap3_do_wfi
-
-/*
- * Local variables
- */
+ENDPROC(omap34xx_cpu_suspend)
 omap3_do_wfi_sram_addr:
 	.word omap3_do_wfi_sram
 kernel_flush:
@@ -364,10 +349,7 @@ exit_nonoff_modes:
  * ===================================
  */
 	ldmfd	sp!, {r4 - r11, pc}	@ restore regs and return
-
-/*
- * Local variables
- */
+ENDPROC(omap3_do_wfi)
 sdrc_power:
 	.word	SDRC_POWER_V
 cm_idlest1_core:

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Dave Martin]

On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> * Grazvydas Ignotas <notasas@gmail.com> [150528 04:45]:
> > On Thu, May 28, 2015 at 1:55 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > 8 <----------------------
> > > From: Tony Lindgren <tony@atomide.com>
> > > Date: Wed, 27 May 2015 15:33:57 -0700
> > > Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel
> > >
> > > We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:
> > >
> > > Internal error: Oops: 80000005 [#1] SMP THUMB2
> > > ...
> > > [<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
> > > (omap3_enter_idle_bm+0xc5/0x178)
> > > [<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
> > > (cpuidle_enter_state+0x77/0x27c)
> > > [<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
> > > (cpu_startup_entry+0x155/0x23c)
> > > [<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
> > > (start_kernel+0x32f/0x338)
> > > [<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)
> > >
> > > The power management related assembly on moaps needs to interact with
> > 
> > moaps -> omaps
> 
> Thanks, fixed now. I also added a link for why ENDPROC is needed to the
> description. I'll apply the patch below into fixes today.
> 
> Regards,
> 
> Tony
> 
> 8< ----------------------
> From: Tony Lindgren <tony@atomide.com>
> Date: Wed, 27 May 2015 15:33:57 -0700
> Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel
> 
> We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:
> 
> Internal error: Oops: 80000005 [#1] SMP THUMB2
> ...
> [<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
> (omap3_enter_idle_bm+0xc5/0x178)
> [<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
> (cpuidle_enter_state+0x77/0x27c)
> [<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
> (cpu_startup_entry+0x155/0x23c)
> [<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
> (start_kernel+0x32f/0x338)
> [<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)
> 
> The power management related assembly on omaps needs to interact with
> ARM mode bootrom code, so we need to keep most of the related assembly
> in ARM mode.
> 
> Turns out this error is because of missing ENDPROC for assembly code
> as suggested by Stephen Boyd <sboyd@codeaurora.org>. Let's add the
> missing ENDPROC in two places to sleep34xx.S, and also remove the
> earlier mystery code that was probably also caused by missing ENDPROC
> for earlier kernels.
> 
> The reason why ENDPROC makes a difference is it sets .type and then
> the compiler knows what to do with the thumb bit as explained at:
> 
> https://wiki.ubuntu.com/ARM/Thumb2PortingHowto
> 
> Reported-by: Kevin Hilman <khilman@kernel.org>
> Tested-by: Kevin Hilman <khilman@linaro.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -203,23 +203,8 @@ save_context_wfi:
>  	 */
>  	ldr	r1, kernel_flush
>  	blx	r1
> -	/*
> -	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> -	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> -	 * This sequence switches back to ARM.  Note that .align may insert a
> -	 * nop: bx pc needs to be word-aligned in order to work.
> -	 */
> - THUMB(	.thumb		)
> - THUMB(	.align		)
> - THUMB(	bx	pc	)
> - THUMB(	nop		)
> -	.arm
> -

^ This looks unrelated to the issue?  This code is simply unnecessary
after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
made v7_flush_dcache_all do a proper interworking return.

It should probably be in a separate commit, or at least mentioned
explicitly in the commit log.


>  	b	omap3_do_wfi
> -
> -/*
> - * Local variables
> - */

^ Likewise this is just tidyup, not bug fixing.


> +ENDPROC(omap34xx_cpu_suspend)
>  omap3_do_wfi_sram_addr:
>  	.word omap3_do_wfi_sram
>  kernel_flush:
> @@ -364,10 +349,7 @@ exit_nonoff_modes:
>   * ===================================
>   */
>  	ldmfd	sp!, {r4 - r11, pc}	@ restore regs and return
> -
> -/*
> - * Local variables
> - */
> +ENDPROC(omap3_do_wfi)

The ENDPROC()s look appropriate here.

[...]

Cheers
---Dave

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Tony Lindgren]

* Dave Martin <Dave.Martin@arm.com> [150528 09:19]:
> On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> > --- a/arch/arm/mach-omap2/sleep34xx.S
> > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > @@ -203,23 +203,8 @@ save_context_wfi:
> >  	 */
> >  	ldr	r1, kernel_flush
> >  	blx	r1
> > -	/*
> > -	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> > -	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> > -	 * This sequence switches back to ARM.  Note that .align may insert a
> > -	 * nop: bx pc needs to be word-aligned in order to work.
> > -	 */
> > - THUMB(	.thumb		)
> > - THUMB(	.align		)
> > - THUMB(	bx	pc	)
> > - THUMB(	nop		)
> > -	.arm
> > -
> 
> ^ This looks unrelated to the issue?  This code is simply unnecessary
> after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
> made v7_flush_dcache_all do a proper interworking return.
> 
> It should probably be in a separate commit, or at least mentioned
> explicitly in the commit log.

Thanks, I've updated the description with the commit info above.

I'll keep the removal of the duplicate code in this patch, as it's
all related to the mode switching and we don't want to do it twice.
 
> >  	b	omap3_do_wfi
> > -
> > -/*
> > - * Local variables
> > - */
> 
> ^ Likewise this is just tidyup, not bug fixing.

Will keep that too, the local variables comments are now just too
confusing while reading the code after adding the ENDPROC. And that
too is just removal.
 
> > +ENDPROC(omap34xx_cpu_suspend)
> >  omap3_do_wfi_sram_addr:
> >  	.word omap3_do_wfi_sram
> >  kernel_flush:
> > @@ -364,10 +349,7 @@ exit_nonoff_modes:
> >   * ===================================
> >   */
> >  	ldmfd	sp!, {r4 - r11, pc}	@ restore regs and return
> > -
> > -/*
> > - * Local variables
> > - */
> > +ENDPROC(omap3_do_wfi)
> 
> The ENDPROC()s look appropriate here.

OK thanks for looking, I was hoping you'd check it as you did the
original thumb2 changes for this file :) Updated patch below.

Regards,

Tony

8< ------------------------
From: Tony Lindgren <tony@atomide.com>
Date: Thu, 28 May 2015 07:22:08 -0700
Subject: [PATCH] ARM: OMAP3: Fix booting with thumb2 kernel

We get a NULL pointer dereference on omap3 for thumb2 compiled kernels:

Internal error: Oops: 80000005 [#1] SMP THUMB2
...
[<c046497b>] (_raw_spin_unlock_irqrestore) from [<c0024375>]
(omap3_enter_idle_bm+0xc5/0x178)
[<c0024375>] (omap3_enter_idle_bm) from [<c0374e63>]
(cpuidle_enter_state+0x77/0x27c)
[<c0374e63>] (cpuidle_enter_state) from [<c00627f1>]
(cpu_startup_entry+0x155/0x23c)
[<c00627f1>] (cpu_startup_entry) from [<c06b9a47>]
(start_kernel+0x32f/0x338)
[<c06b9a47>] (start_kernel) from [<8000807f>] (0x8000807f)

The power management related assembly on omaps needs to interact with
ARM mode bootrom code, so we need to keep most of the related assembly
in ARM mode.

Turns out this error is because of missing ENDPROC for assembly code
as suggested by Stephen Boyd <sboyd@codeaurora.org>. Let's fix the
problem by adding ENDPROC in two places to sleep34xx.S.

Let's also remove the now duplicate custom code for mode switching.
This has been unnecessary since commit 6ebbf2ce437b ("ARM: convert
all "mov.* pc, reg" to "bx reg" for ARMv6+").

And let's also remove the comments about local variables, they are
now just confusing after the ENDPROC.

The reason why ENDPROC makes a difference is it sets .type and then
the compiler knows what to do with the thumb bit as explained at:

https://wiki.ubuntu.com/ARM/Thumb2PortingHowto

Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <khilman@linaro.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -203,23 +203,8 @@ save_context_wfi:
 	 */
 	ldr	r1, kernel_flush
 	blx	r1
-	/*
-	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
-	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
-	 * This sequence switches back to ARM.  Note that .align may insert a
-	 * nop: bx pc needs to be word-aligned in order to work.
-	 */
- THUMB(	.thumb		)
- THUMB(	.align		)
- THUMB(	bx	pc	)
- THUMB(	nop		)
-	.arm
-
 	b	omap3_do_wfi
-
-/*
- * Local variables
- */
+ENDPROC(omap34xx_cpu_suspend)
 omap3_do_wfi_sram_addr:
 	.word omap3_do_wfi_sram
 kernel_flush:
@@ -364,10 +349,7 @@ exit_nonoff_modes:
  * ===================================
  */
 	ldmfd	sp!, {r4 - r11, pc}	@ restore regs and return
-
-/*
- * Local variables
- */
+ENDPROC(omap3_do_wfi)
 sdrc_power:
 	.word	SDRC_POWER_V
 cm_idlest1_core:

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Dave Martin]

On Thu, May 28, 2015 at 09:36:05AM -0700, Tony Lindgren wrote:
> * Dave Martin <Dave.Martin@arm.com> [150528 09:19]:
> > On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> > > --- a/arch/arm/mach-omap2/sleep34xx.S
> > > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > > @@ -203,23 +203,8 @@ save_context_wfi:
> > >  	 */
> > >  	ldr	r1, kernel_flush
> > >  	blx	r1
> > > -	/*
> > > -	 * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> > > -	 * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> > > -	 * This sequence switches back to ARM.  Note that .align may insert a
> > > -	 * nop: bx pc needs to be word-aligned in order to work.
> > > -	 */
> > > - THUMB(	.thumb		)
> > > - THUMB(	.align		)
> > > - THUMB(	bx	pc	)
> > > - THUMB(	nop		)
> > > -	.arm
> > > -
> > 
> > ^ This looks unrelated to the issue?  This code is simply unnecessary
> > after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
> > made v7_flush_dcache_all do a proper interworking return.
> > 
> > It should probably be in a separate commit, or at least mentioned
> > explicitly in the commit log.
> 
> Thanks, I've updated the description with the commit info above.
> 
> I'll keep the removal of the duplicate code in this patch, as it's
> all related to the mode switching and we don't want to do it twice.
>  
> > >  	b	omap3_do_wfi
> > > -
> > > -/*
> > > - * Local variables
> > > - */
> > 
> > ^ Likewise this is just tidyup, not bug fixing.
> 
> Will keep that too, the local variables comments are now just too
> confusing while reading the code after adding the ENDPROC. And that
> too is just removal.

I have no objection to tidyups ;)

So long as the commit message is clear about which parts are bugfix and
which parts are tidyup, I'm fine with that.

Cheers
---Dave

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Arnd Bergmann]

On Thursday 28 May 2015 09:36:05 Tony Lindgren wrote:
> * Dave Martin <Dave.Martin@arm.com> [150528 09:19]:
> > On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> > > --- a/arch/arm/mach-omap2/sleep34xx.S
> > > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > > @@ -203,23 +203,8 @@ save_context_wfi:
> > >      */
> > >     ldr     r1, kernel_flush
> > >     blx     r1
> > > -   /*
> > > -    * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> > > -    * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> > > -    * This sequence switches back to ARM.  Note that .align may insert a
> > > -    * nop: bx pc needs to be word-aligned in order to work.
> > > -    */
> > > - THUMB(    .thumb          )
> > > - THUMB(    .align          )
> > > - THUMB(    bx      pc      )
> > > - THUMB(    nop             )
> > > -   .arm
> > > -
> > 
> > ^ This looks unrelated to the issue?  This code is simply unnecessary
> > after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
> > made v7_flush_dcache_all do a proper interworking return.
> > 
> > It should probably be in a separate commit, or at least mentioned
> > explicitly in the commit log.
> 
> Thanks, I've updated the description with the commit info above.
> 
> I'll keep the removal of the duplicate code in this patch, as it's
> all related to the mode switching and we don't want to do it twice.

I would have thought that the change is actually necessary after
6ebbf2ce43, because it now returns in ARM mode, which will cause the
"bx pc; nop" thumb instruction sequence be misinterpreted as an 
ARM instruction.

	Arnd

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Tony Lindgren]

* Arnd Bergmann <arnd@arndb.de> [150528 11:57]:
> On Thursday 28 May 2015 09:36:05 Tony Lindgren wrote:
> > * Dave Martin <Dave.Martin@arm.com> [150528 09:19]:
> > > On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> > > > --- a/arch/arm/mach-omap2/sleep34xx.S
> > > > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > > > @@ -203,23 +203,8 @@ save_context_wfi:
> > > >      */
> > > >     ldr     r1, kernel_flush
> > > >     blx     r1
> > > > -   /*
> > > > -    * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> > > > -    * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> > > > -    * This sequence switches back to ARM.  Note that .align may insert a
> > > > -    * nop: bx pc needs to be word-aligned in order to work.
> > > > -    */
> > > > - THUMB(    .thumb          )
> > > > - THUMB(    .align          )
> > > > - THUMB(    bx      pc      )
> > > > - THUMB(    nop             )
> > > > -   .arm
> > > > -
> > > 
> > > ^ This looks unrelated to the issue?  This code is simply unnecessary
> > > after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
> > > made v7_flush_dcache_all do a proper interworking return.
> > > 
> > > It should probably be in a separate commit, or at least mentioned
> > > explicitly in the commit log.
> > 
> > Thanks, I've updated the description with the commit info above.
> > 
> > I'll keep the removal of the duplicate code in this patch, as it's
> > all related to the mode switching and we don't want to do it twice.
> 
> I would have thought that the change is actually necessary after
> 6ebbf2ce43, because it now returns in ARM mode, which will cause the
> "bx pc; nop" thumb instruction sequence be misinterpreted as an 
> ARM instruction.

Could be it's necessary.. But alone that change is not enough.

Regards,

Tony

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Dave P Martin]

On Thu, May 28, 2015 at 07:55:16PM +0100, Arnd Bergmann wrote:
> On Thursday 28 May 2015 09:36:05 Tony Lindgren wrote:
> > * Dave Martin <Dave.Martin@arm.com> [150528 09:19]:
> > > On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> > > > --- a/arch/arm/mach-omap2/sleep34xx.S
> > > > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > > > @@ -203,23 +203,8 @@ save_context_wfi:
> > > >      */
> > > >     ldr     r1, kernel_flush
> > > >     blx     r1
> > > > -   /*
> > > > -    * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> > > > -    * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> > > > -    * This sequence switches back to ARM.  Note that .align may insert a
> > > > -    * nop: bx pc needs to be word-aligned in order to work.
> > > > -    */
> > > > - THUMB(    .thumb          )
> > > > - THUMB(    .align          )
> > > > - THUMB(    bx      pc      )
> > > > - THUMB(    nop             )
> > > > -   .arm
> > > > -
> > > 
> > > ^ This looks unrelated to the issue?  This code is simply unnecessary
> > > after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
> > > made v7_flush_dcache_all do a proper interworking return.
> > > 
> > > It should probably be in a separate commit, or at least mentioned
> > > explicitly in the commit log.
> > 
> > Thanks, I've updated the description with the commit info above.
> > 
> > I'll keep the removal of the duplicate code in this patch, as it's
> > all related to the mode switching and we don't want to do it twice.
> 
> I would have thought that the change is actually necessary after
> 6ebbf2ce43, because it now returns in ARM mode, which will cause the
> "bx pc; nop" thumb instruction sequence be misinterpreted as an 
> ARM instruction.
> 
> 	Arnd

You're right.  The THUMB2_KERNEL indeed won't work without deleting this
code.

Cheers
---Dave

[RFC] Fix omap3 booting with thumb2 compiled kernel

[Tony Lindgren]

* Dave P Martin <Dave.Martin@arm.com> [150529 04:08]:
> On Thu, May 28, 2015 at 07:55:16PM +0100, Arnd Bergmann wrote:
> > On Thursday 28 May 2015 09:36:05 Tony Lindgren wrote:
> > > * Dave Martin <Dave.Martin@arm.com> [150528 09:19]:
> > > > On Thu, May 28, 2015 at 07:21:25AM -0700, Tony Lindgren wrote:
> > > > > --- a/arch/arm/mach-omap2/sleep34xx.S
> > > > > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > > > > @@ -203,23 +203,8 @@ save_context_wfi:
> > > > >      */
> > > > >     ldr     r1, kernel_flush
> > > > >     blx     r1
> > > > > -   /*
> > > > > -    * The kernel doesn't interwork: v7_flush_dcache_all in particluar will
> > > > > -    * always return in Thumb state when CONFIG_THUMB2_KERNEL is enabled.
> > > > > -    * This sequence switches back to ARM.  Note that .align may insert a
> > > > > -    * nop: bx pc needs to be word-aligned in order to work.
> > > > > -    */
> > > > > - THUMB(    .thumb          )
> > > > > - THUMB(    .align          )
> > > > > - THUMB(    bx      pc      )
> > > > > - THUMB(    nop             )
> > > > > -   .arm
> > > > > -
> > > > 
> > > > ^ This looks unrelated to the issue?  This code is simply unnecessary
> > > > after Russell introduced the "ret" macro in commit 6ebbf2ce43.  That
> > > > made v7_flush_dcache_all do a proper interworking return.
> > > > 
> > > > It should probably be in a separate commit, or at least mentioned
> > > > explicitly in the commit log.
> > > 
> > > Thanks, I've updated the description with the commit info above.
> > > 
> > > I'll keep the removal of the duplicate code in this patch, as it's
> > > all related to the mode switching and we don't want to do it twice.
> > 
> > I would have thought that the change is actually necessary after
> > 6ebbf2ce43, because it now returns in ARM mode, which will cause the
> > "bx pc; nop" thumb instruction sequence be misinterpreted as an 
> > ARM instruction.
> > 
> > 	Arnd
> 
> You're right.  The THUMB2_KERNEL indeed won't work without deleting this
> code.

OK thanks for checking.

Regards,

Tony