mpc85xx: Initial SP alignment is wrong.

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

powerpc/cpu/mpc85xx/start.S +5 -12

index 8d66cf1..4973682 100644
@@ -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   r3,(CONFIG_SYS_INIT_RAM_ADDR)@h
+   ori   r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */
    li   r0,0
-   stwu   r0,-4(r1)
-   stwu   r0,-4(r1)      /* Terminate call chain */
-
-   stwu   r1,-8(r1)      /* Save back chain and move SP */
-   lis   r0,RESET_VECTOR at h   /* Address of reset vector */
-   ori   r0,r0,RESET_VECTOR at l
-   stwu   r1,-8(r1)      /* Save back chain and move SP */
-   stw   r0,+12(r1)      /* Save return addr (underflow vect) */
+   stw   r0,0(r3)   /* Terminate Back Chain */
+   stw   r0,+4(r3)   /* NULL return address. */
+   mr   r1,r3      /* Transfer to SP(r1) */
 
    GET_GOT
    bl   cpu_init_early_f
--

1.7.3.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Ping?

Quoted 13 lines

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

Quoted 9 lines

Ping?

Quoted 7 lines

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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.

Quoted 6 lines

Quoted 6 lines

Signed-off-by Joakim Tjernlund <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

Andy Fleming is the 85xx custodian, but you didn't CC him.

The code first aligns the SP to 16 then subtract 8, making it 8 bytes aligned. Furthermore the initial stack frame not quite correct either.

Signed-off-by Joakim Tjernlund <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/lib/board.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

powerpc/lib/board.c +2 -4

index d5b75e5..a46942c 100644
@@ -521,9 +521,8 @@ void board_init_f(ulong bootflag)
    addr_sp -= 16;
    addr_sp &= ~0xF;
    s = (ulong *) addr_sp;
-   *s-- = 0;
-   *s-- = 0;
-   addr_sp = (ulong) s;
+   *s = 0; /* Terminate back chain */
+   *++s = 0; /* NULL return address */
    debug("Stack Pointer at: %08lx\n", addr_sp);
 
    /*
--

1.7.3.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments
arch/powerpc/cpu/mpc85xx/start.S | 16 +++++-----------

1 files changed, 5 insertions(+), 11 deletions(-)

Quoted 7 lines

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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.

Quoted 6 lines

Signed-off-by Joakim Tjernlund <[email protected]>

---

v2 - Address Scott Wood's comments
arch/powerpc/cpu/mpc85xx/start.S | 16 +++++-----------

1 files changed, 5 insertions(+), 11 deletions(-)

Acked-by Kumar Gala <[email protected]>

Kumar Gala <[email protected]> wrote on 2012/08/14 23:28:45:

Quoted 17 lines · Joakim Tjernlund

Quoted 13 lines

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments
arch/powerpc/cpu/mpc85xx/start.S | 16 +++++-----------

1 files changed, 5 insertions(+), 11 deletions(-)

Quoted 1 line

Acked-by Kumar Gala <[email protected]>

Thanks Kumar

Will you pick up?
[PATCHv2 2/2] powerpc: Stack Pointer not properly aligned

It is not 85xx specific but for all PowerPC

Quoted 18 lines · Joakim Tjernlund

Kumar Gala <[email protected]> wrote on 2012/08/14 23:28:45:

Quoted 17 lines · Joakim Tjernlund

Quoted 13 lines

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments
arch/powerpc/cpu/mpc85xx/start.S | 16 +++++-----------

1 files changed, 5 insertions(+), 11 deletions(-)

Quoted 8 lines

Quoted 1 line

Acked-by Kumar Gala <[email protected]>

Thanks Kumar

Will you pick up?
[PATCHv2 2/2] powerpc: Stack Pointer not properly aligned

It is not 85xx specific but for all PowerPC

I don't think Kumar has a U-Boot tree to pull it into. There's no general PPC custodian. Probably both patches should go via Andy.

-Scott

index 8d66cf1..4973682 100644

powerpc/cpu/mpc85xx/start.S +5 -12

@@ -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     r3,(CONFIG_SYS_INIT_RAM_ADDR)@h
+       ori     r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */
        li      r0,0
-       stwu    r0,-4(r1)
-       stwu    r0,-4(r1)               /* Terminate call chain */
-
-       stwu    r1,-8(r1)               /* Save back chain and move SP */
-       lis     r0,RESET_VECTOR at h       /* Address of reset vector */
-       ori     r0,r0,RESET_VECTOR at l
-       stwu    r1,-8(r1)               /* Save back chain and move SP */
-       stw     r0,+12(r1)              /* Save return addr (underflow vect) */
+       stw     r0,0(r3)        /* Terminate Back Chain */
+       stw     r0,+4(r3)       /* NULL return address. */
+       mr      r1,r3           /* Transfer to SP(r1) */
 
        GET_GOT
        bl      cpu_init_early_f
--

1.7.3.4

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Quoted 13 lines · Joakim Tjernlund

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

arch/powerpc/cpu/mpc85xx/start.S

Quoted 1 line

arch/powerpc/cpu/mpc85xx/start.S +0 -0

Why are your patches different from everyone else's? When I try to apply this, I get errors because it can't find "powerpc/cpu/mpc85xx...". git am leaves off the first directory, because the usual practice is to send patches with these filenames:

Andy Fleming <[email protected]> wrote on 2012/08/22 23:08:45:

Quoted 16 lines

Quoted 13 lines · Joakim Tjernlund

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

Quoted 7 lines

arch/powerpc/cpu/mpc85xx/start.S

Quoted 1 line

arch/powerpc/cpu/mpc85xx/start.S +0 -0

Why are your patches different from everyone else's? When I try to apply this, I get errors because it can't find "powerpc/cpu/mpc85xx...". git am leaves off the first directory, because the usual practice is to send patches with these filenames:

Ahh, recently I set (in ny git config):
[diff]
noprefix = true
because I got tired off stripping off that prefix each time I cut and paste file names into emacs and similar.

Seems like git really likes to see that prefix when applying patches. Don't know if git could learn not to complain about missing prefix?

Quoted 31 lines

Andy Fleming <[email protected]> wrote on 2012/08/22 23:08:45:

Quoted 16 lines

Quoted 13 lines · Joakim Tjernlund

PowerPC mandates SP to be 16 bytes aligned.
Furthermore, a stack frame is added, pointing to the reset vector which may 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 <[email protected]>

---

v2 - Address Scott Wood's comments

 arch/powerpc/cpu/mpc85xx/start.S |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

Quoted 7 lines

arch/powerpc/cpu/mpc85xx/start.S

Quoted 1 line

arch/powerpc/cpu/mpc85xx/start.S +0 -0

Why are your patches different from everyone else's? When I try to apply this, I get errors because it can't find "powerpc/cpu/mpc85xx...". git am leaves off the first directory, because the usual practice is to send patches with these filenames:

Ahh, recently I set (in ny git config):
[diff]
noprefix = true
because I got tired off stripping off that prefix each time I cut and paste file names into emacs and similar.

Quoted 2 lines

Seems like git really likes to see that prefix when applying patches. Don't know if git could learn not to complain about missing prefix?

It's not just git, but also direct use of the patch command when a patch fails to apply cleanly. A user shouldn't have to inspect a patch to determine whether to use -p0 or -p1. -p1 is standard.

How often do you copy and paste filenames out of your own patches?