[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 at gmail.com> [150528 04:45]:
> > On Thu, May 28, 2015 at 1:55 AM, Tony Lindgren <tony at atomide.com> wrote:
> > > 8 <----------------------
> > > From: Tony Lindgren <tony at 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 at 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 at 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 at kernel.org>
> Tested-by: Kevin Hilman <khilman at linaro.org>
> Signed-off-by: Tony Lindgren <tony at 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