[U-Boot] [PATCH] ARM: socfpga: Put stack at the end of SRAM

[Marek Vasut]

On 05/14/2018 11:01 AM, Simon Goldschmidt wrote:
> 
> 
> On 14.05.2018 10:17, Marek Vasut wrote:
>> On 05/14/2018 10:03 AM, Simon Goldschmidt wrote:
>>> On 12.05.2018 22:27, Marek Vasut wrote:
>>>> The global data are in the .data section, so there's no point in
>>>> reserving any space for it above stack. Put stack at the end of
>>>> SRAM.
>>>>
>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>> Cc: Chin Liang See <chin.liang.see at intel.com>
>>>> Cc: Dinh Nguyen <dinguyen at kernel.org>
>>>> ---
>>>>    include/configs/socfpga_common.h | 4 +---
>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/configs/socfpga_common.h
>>>> b/include/configs/socfpga_common.h
>>>> index 4de2aa7929..cb67d539b1 100644
>>>> --- a/include/configs/socfpga_common.h
>>>> +++ b/include/configs/socfpga_common.h
>>>> @@ -35,10 +35,8 @@
>>>>    #define CONFIG_SYS_INIT_RAM_ADDR    0xFFE00000
>>>>    #define CONFIG_SYS_INIT_RAM_SIZE    0x40000 /* 256KB */
>>>>    #endif
>>>> -#define CONFIG_SYS_INIT_SP_OFFSET        \
>>>> -    (CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE)
>>>>    #define CONFIG_SYS_INIT_SP_ADDR            \
>>>> -    (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
>>>> +    (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE)
>>>
>>> I think this interferes with the bootcount being stored at the end of
>>> SRAM. See the defconfigs of 'is1' and 'sr1500' boards - or my local
>>> defconfig here :-)
>>>
>>> I'd really like to keep some room here for bootcounter and some related
>>> things I store here.
>>
>> So if I understand it correctly, before this patch, the boards overwrote
>> the global data ?
> 
> Ehrm, reading up on the init code path in the S and C files, it seems
> like that, yes. I just used the two boards as an example. Are they
> actively maintained? get_maintainer.pl shows the boards are maintained
> by denx, so maybe you could clarify this?

+CC Stefan.

> Chances are high that these bytes just fit into the 16-byte alignment
> allocated by GENERATED_GBL_DATA_SIZE (and on my board it *is* like that).
> 
>>
>> Anyway, if you need some extra space, look at
>> board_init_f_alloc_reserve() .
>>
> 
> OK, but that function does not seem to be overridable? How can I reserve
> my own memory here?

Add a __weak there I guess ?

> And reading this function, I guess your commit message is a bit
> misleading. It seems that the purpose of CONFIG_SYS_INIT_SP_OFFSET like
> it was was to reserve memory for the initial gd (before relocation),
> which is now done in board_init_f_alloc_reserve().

The code is confusing as hell, I give you that.

-- 
Best regards,
Marek Vasut