> > > Scott Wood <scottw...@freescale.com> wrote on 2012/07/20 23:11:33: > > > > 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.tjernl...@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@h > > > - ori r1,r1,CONFIG_SYS_INIT_SP_OFFSET@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. > > I guess that would be a tiny improvement, but it is nothing compared > with the r1 abuse in this file :) > > > > > > + 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. > > Right, it should be +4 where the NULL LR should be. As you said, it doesn't > matter > because it is not valid for this stack frame and could be left out. > > I do not want to create a frame as the current code does, it > might create an memory access to 0xfffffffc which I don't want. > How about this then: > > lis r3,(CONFIG_SYS_INIT_RAM_ADDR)@h > ori r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */ > li r0,0 > stw r0,0(r3) /* Terminate Back Chain */ > stw r0,+4(r3) /* NULL LR, to be nice. */ > mr r1,r3 /* Transfer to SP(r1) */ > > or if that reset address is wanted: > lis r3,(CONFIG_SYS_INIT_RAM_ADDR)@h > ori r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */ > li r0,0 > stw r0,0(r3) /* Terminate Back Chain */ > stw r0,+4(r3) /* NULL LR, to be nice. */ > stwu r3,-16(r3) /* save back chain and move SP */ > lis r0,RESET_VECTOR@h > ori r0,RESET_VECTOR@l > stw r0,+20(r3) /* save return address */ > mr r1,r3 /* Transfer to SP(r1) */ > > I would have to test with the emulator if both are OK or if there will > be an access to RESET_VECTOR(I don't have that connected ATM).
So I got the BDI connected and did som experiments. Having RESET_VECTOR in the initial stack chain makes gdb print this: (gdb) bt #0 cpu_init_early_f () at cpu_init_early.c:116 #1 0xeff800a4 in _start_cont () at start.S:877 Backtrace stopped: frame did not save the PC (gdb) So I don't think there is any benefit trying to squeeze RESET_VECTOR into the stack chain. Jocke _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot