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

[Simon Glass]

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?

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.

Regards,
Simon