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

[Stefan Roese]

On 14.05.2018 11:06, Marek Vasut wrote:
> 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.

Thanks. At the time I added the bootcounter support on this platform,
it used to work. I might have missed something though. Right now,
I don't have access to such a platform, so its hard for me to test
something. If you have some patches to fix any potential overlapping
memory usages, I'll gladly review them.

>> 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.

Thanks,
Stefan