[U-Boot] [PATCH 2/2] mpc85xx: Initial SP alignment is wrong.

[Scott Wood]

On 07/20/2012 04:20 AM, Joakim Tjernlund wrote:
> PowerPC mandates SP to be 16 bytes aligned.
> Furthermore, a stack frame is added, pointing to the reset vector
> which is just get in the way when gdb is walking the stack because
> the reset vector may not accessible depending on emulator settings.
> Also use a temp register so gdb doesn't pick up intermediate values.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
> ---
>  arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
>  1 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git arch/powerpc/cpu/mpc85xx/start.S arch/powerpc/cpu/mpc85xx/start.S
> index 8d66cf1..8c75af9 100644
> --- arch/powerpc/cpu/mpc85xx/start.S
> +++ arch/powerpc/cpu/mpc85xx/start.S
> @@ -848,18 +848,12 @@ version_string:
>  	.globl	_start_cont
>  _start_cont:
>  	/* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/
> -	lis	r1,CONFIG_SYS_INIT_RAM_ADDR at h
> -	ori	r1,r1,CONFIG_SYS_INIT_SP_OFFSET at l
> -
> +	lis	r2,(CONFIG_SYS_INIT_RAM_ADDR)@h
> +	ori	r2,r2,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */

Please don't use r2 as a general purpose register -- even if it's early
enough that we haven't started using it for its fixed purpose yet.

> +	stw	r0,-4(r2)	/* Store a NULL LR */
> +	stw	r0,0(r2)	/* Store a NULL Back Chain */
> +	mr	r1,r2		/* Transfer to SP(r1) */

Why are you storing anything below the stack pointer?  LR goes above the
backchain, not below -- and it's not valid anyway for the current stack
frame.  If you want to have a terminating stack frame with a NULL LR,
you need to create a second stack frame, like the code currently does.

-Scott