[U-Boot] [PATCH 1/7] common/board_f: add setup of initial stack frame for MIPS

[Daniel Schwierzeck]

On 19.11.2014 23:22, Simon Glass wrote:
> Hi Daniel,
> 
> On 19 November 2014 16:59, Daniel Schwierzeck
> <daniel.schwierzeck at 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.schwierzeck at 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.schwierzeck at 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(sjg at 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