[U-Boot] [PATCH] arm: socfpga: check total size of SPL

[Simon Goldschmidt]

On Tue, Oct 30, 2018 at 11:02 PM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> On 10/30/2018 10:30 PM, Simon Goldschmidt wrote:
> > On 30.10.2018 22:26, Marek Vasut wrote:
> >> On 10/30/2018 10:23 PM, Simon Goldschmidt wrote:
> >>> Correctly define CONFIG_SPL_MAX_FOOTPRINT to make the default arm
> >>> linker script for SPL check the total SRAM size available for SPL
> >>> (code, data, bss, heap, global data).
> >>>
> >>> The previously existing define CONFIG_SPL_MAX_SIZE seems to only
> >>> check the binary size (which is without bss, heap and gd).
> >>>
> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >>> ---
> >>>
> >>>   include/configs/socfpga_common.h | 7 +++++++
> >>>   1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/include/configs/socfpga_common.h
> >>> b/include/configs/socfpga_common.h
> >>> index 2330143cf1..9103d0a966 100644
> >>> --- a/include/configs/socfpga_common.h
> >>> +++ b/include/configs/socfpga_common.h
> >>> @@ -242,6 +242,13 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
> >>>   #define CONFIG_SPL_TEXT_BASE        CONFIG_SYS_INIT_RAM_ADDR
> >>>   #define CONFIG_SPL_MAX_SIZE        CONFIG_SYS_INIT_RAM_SIZE
> >>>   +/* Check total size of SPL including BSS, malloc area and gd */
> >>> +#include <generated/generic-asm-offsets.h>
> >>> +#define CONFIG_SPL_MAX_FOOTPRINT    (CONFIG_SYS_INIT_SP_ADDR - \
> >>> +                     CONFIG_SYS_INIT_RAM_ADDR - \
> >>> +                     CONFIG_SYS_MALLOC_F_LEN - \
> >>> +                     GENERATED_GBL_DATA_SIZE)
> >> Are you sure this calculation is correct ? INIT_SP_ADDR is I think the
> >> SRAM offset in the address space. Shouldn't that contain INIT_SP_SIZE or
> >> something ?
> >
> > Yes, I'm pretty sure. INIT_SP_ADDR is defined as INIT_RAM_ADDR +
> > INIT_RAM_SIZE.
> > So by subtracting INIT_RAM_ADDR again, I effectively get "INIT_RAM_ADDR
> > - MALLOC_F_LEN - GBL_DATA_SIZE".
> >
> > But I did it this way to keep it working after Stefan's fix for
> > reserving the boot counter location is applied.
>
> Now that's confusing :-) Add a comment explaining this into a V2 please,
> otherwise the confusion will continue ...

You're probably right. I'll send a V2 making this more clear.


Simon