On 02/12/2013 04:41:15 PM, Simon Glass wrote:
Hi Scott,
On Tue, Feb 12, 2013 at 2:32 PM, Scott Wood scottwood@freescale.com wrote:
On 02/08/2013 09:12:12 AM, Simon Glass wrote:
#ifndef CONFIG_SPL_BUILD static int reserve_stacks(void) { +#ifdef CONFIG_PPC
ulong *s;+#endif
/* setup stack pointer for exceptions */ gd->dest_addr_sp -= 16; gd->dest_addr_sp &= ~0xf;@@ -398,6 +532,14 @@ static int reserve_stacks(void) /* leave 3 words for abort-stack, plus 1 for alignment */ gd->dest_addr_sp -= 16;
+#ifdef CONFIG_PPC
/* Clear initial stack frame */s = (ulong *) gd->dest_addr_sp;*s = 0; /* Terminate back chain */*++s = 0; /* NULL return address */gd->dest_addr_sp = (ulong) s;+#endif
PPC ABI requires 16-byte stack alignment, which would be broken by
the
CONFIG_USE_IRQ section (which even still has an "ARM ABI" comment).
I think this entire function should be kept in arch code. Stack
layout is
inherently architecture/ABI specific. Some architectures even have
a stack
that grows upward (not sure if any such are supported by U-Boot).
Thanks for reviewing all this.
While I am working to avoid it, one option is to create a weak function which archs can override. The reason I am keen to avoid it, at least for a first implementation, is that it obscures the similarities.
That's fine for most of the file, but I think there's not much that's truly generic when it comes to setting up the stack. It's not as if this is a huge function (at least before it grows a bunch of ifdefs).
In this case we could just just force 16-byte alignment, and make the PPC code unconditional. It shouldn't hurt anything.
The CONFIG_USE_IRQ section also has references to FIQs... if it's meant to be an ARM-specific CONFIG, perhaps it should be renamed (and definitely it should be documented).
-Scott