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. -- - Daniel _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
