On Fri, Nov 21, 2014 at 09:46:09PM +0100, Daniel Schwierzeck wrote: > Hi Simon, > > On 20.11.2014 18:22, Simon Glass wrote: > > Hi Daniel, > > > > On 20 November 2014 16:54, Daniel Schwierzeck > > <daniel.schwierz...@gmail.com> wrote: > >> > >> > >> On 19.11.2014 23:22, Simon Glass wrote: > >>> Hi Daniel, > >>> > >>> On 19 November 2014 16:59, Daniel Schwierzeck > >>> <daniel.schwierz...@gmail.com> wrote: > >>>> Hi Simon, > >>>> > >>>> On 17.11.2014 07:24, Simon Glass wrote: > >>>>> Hi Daniel, > >>>>> > >>>>> On 15 November 2014 22:46, Daniel Schwierzeck > >>>>> <daniel.schwierz...@gmail.com> wrote: > >>>>>> The MIPS specific setup of the initial stack frame was not > >>>>>> ported to generic board_f. > >>>>>> > >>>>>> Signed-off-by: Daniel Schwierzeck <daniel.schwierz...@gmail.com> > >>>>>> > >>>>>> --- > >>>>>> > >>>>>> common/board_f.c | 8 +++++++- > >>>>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/common/board_f.c b/common/board_f.c > >>>>>> index b5bebc9..57e8a67 100644 > >>>>>> --- a/common/board_f.c > >>>>>> +++ b/common/board_f.c > >>>>>> @@ -579,7 +579,7 @@ static int reserve_stacks(void) > >>>>>> gd->irq_sp = gd->start_addr_sp; > >>>>>> # endif > >>>>>> #else > >>>>>> -# ifdef CONFIG_PPC > >>>>>> +# if defined(CONFIG_PPC) || defined(CONFIG_MIPS) > >>>>>> ulong *s; > >>>>>> # endif > >>>>>> > >>>>>> @@ -609,6 +609,12 @@ static int reserve_stacks(void) > >>>>>> s = (ulong *) gd->start_addr_sp; > >>>>>> *s = 0; /* Terminate back chain */ > >>>>>> *++s = 0; /* NULL return address */ > >>>>>> +# elif defined(CONFIG_MIPS) > >>>>>> + /* Clear initial stack frame */ > >>>>>> + s = (ulong *) gd->start_addr_sp; > >>>>>> + *s-- = 0; > >>>>>> + *s-- = 0; > >>>>>> + gd->start_addr_sp = (ulong) s; > >>>>>> # endif /* Architecture specific code */ > >>>>> > >>>>> Great to see this happening. > >>>>> > >>>>> There is a comment in the code here: > >>>>> > >>>>> /* > >>>>> * Handle architecture-specific things here > >>>>> * TODO(s...@chromium.org): Perhaps create arch_reserve_stack() > >>>>> * to handle this and put in arch/xxx/lib/stack.c > >>>>> */ > >>>>> > >>>>> Perhaps we should do this. You could create a weak function which is > >>>>> called for all archs, and implement it just for MIPS at present. I'm > >>>>> not sure about a good prototype. Perhaps pass it gd and comment that > >>>>> it is allowed to change memory to set up the stack, and adjust > >>>>> gd->start_addr_sp and other stack-related values. > >>>>> > >>>>> Also while I see that PPC writes above the stack pointer, I'm not sure > >>>>> why it is valid. Should you in fact use: > >>>>> > >>>>> *--s = 0; > >>>>> *--s = 0; > >>>> > >>>> I'd like to have those patches merged for 2015.01. So I want to keep the > >>>> current code to not break anything. Maybe this is not necessary at all. > >>>> The MIPS Malta board already uses generic board and does not seem to > >>>> have any problems. > >>> > >>> I don't see a problem with merging this for 2015.01. Are you saying > >>> you don't think it is needed but can't be sure? So you want to merge > >>> it and see what people report? > >> > >> that code is taken unmodified from arch/mips/lib/board.c to not change > >> or break anything. But that code is old and maybe copied from PowerPC in > >> the early phases of U-Boot. I'm only saying that I need to investigate > >> if that code could be dropped or not. But that is a task for the next > >> merge window. > >> > >>> > >>> In that case I think you should add a comment to that effect, but also > >>> do the function as I mentioned above. We are trying to remove the > >>> arch-specific code in this file and certainly don't want to add more. > >>> > >> > >> ok I'll send an updated patch. > > > > Thanks - and as I mentioned it seems wrong to write to a word above > > the top of the stack. > > > > I discard this patch. The only requirement for the stack pointer on MIPS > is alignment on a 8 Byte boundary. reserve_stacks already aligns it to > 16 Byte (gd->start_addr_sp &= ~0xf;). > > To make stack walking and backtraces working flawlessy in gdb, I found > another solution [1]. > > [1] http://patchwork.ozlabs.org/patch/413182/
And I of course missed this email while sorting out other problems with stuff I was applying. I'll just back this out and take the better fix. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
