[U-Boot] [PATCH] arm: socfpga: Fix bootcounter located at the end of internal SRAM

[Stefan Roese <sr@denx.de>]

Hi Simon,

On 30.10.18 10:07, Simon Goldschmidt wrote:
> 
> 
> Stefan Roese <sr at denx.de <mailto:sr@denx.de>> schrieb am Di., 30. Okt. 2018, 10:00:
> 
>     Commit 768f23dc8ae3 ("ARM: socfpga: Put stack at the end of SRAM") broke
>     those socfpga boards that keep the bootcounter at the end of the
>     internal SRAM as the bootcounter needs 8 bytes by default and thus the
>     very first SPL call to board_init_f_alloc_reserve overwrites the
>     bootcounter.
> 
>     This patch allows to move the initial stack pointer down a bit by
>     checking if CONFIG_SYS_BOOTCOUNT_ADDR is located in the internal SRAM
>     area and then using this address as location for the start of the
>     stack pointer.
> 
>     No new macros / defines are added by this approach.
> 
> 
> Ok, so no new macros are defined, but this is limited to the boot
> counter. However, I need to store additional data that survives a
> reboot somewhere, so I think an explicit range reservation would
> be nicer.

Ah, I was not aware that you have a different reasoning to allocate
some memory in the internal SRAM.
  
> I could still do what I want with your patch by allocating my data
> above the bootcounter, but this works in a more or less implicit/
> hidden way, not explicitly configured...

Some implicit (hidden) means is definitely not good. You can go ahead
with your approach. But please don't introduce new defines for things
that are already configured - meaning 0x8 bytes reserved and 0xfffffff8
as bootcounter location are redundant. This is a bit messy and might
get out of hand, once those defines are not in sync any more.

And you probably need to add new defines to Kconfig as well
(e.g. SOCFPGA_INIT_RAM_END_RESERVE).

Thanks,
Stefan
  
> Simon
> 
> 
>     Signed-off-by: Stefan Roese <sr at denx.de <mailto:sr@denx.de>>
>     Cc: Marek Vasut <marex at denx.de <mailto:marex@denx.de>>
>     Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>
>     ---
>       include/configs/socfpga_common.h | 13 +++++++++++++
>       1 file changed, 13 insertions(+)
> 
>     diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
>     index 2330143cf1..bd8f5c8c41 100644
>     --- a/include/configs/socfpga_common.h
>     +++ b/include/configs/socfpga_common.h
>     @@ -31,8 +31,21 @@
>       #define CONFIG_SYS_INIT_RAM_ADDR       0xFFE00000
>       #define CONFIG_SYS_INIT_RAM_SIZE       0x40000 /* 256KB */
>       #endif
>     +
>     +/*
>     + * Some boards (e.g. socfpga_sr1500) use 8 bytes at the end of the internal
>     + * SRAM as bootcounter storage. Make sure to not put the stack directly
>     + * at this address to not overwrite the bootcounter by checking, if the
>     + * bootcounter address is located in the internal SRAM.
>     + */
>     +#if ((CONFIG_SYS_BOOTCOUNT_ADDR > CONFIG_SYS_INIT_RAM_ADDR) && \
>     +     (CONFIG_SYS_BOOTCOUNT_ADDR < (CONFIG_SYS_INIT_RAM_ADDR +  \
>     +                                  CONFIG_SYS_INIT_RAM_SIZE)))
>     +#define CONFIG_SYS_INIT_SP_ADDR                CONFIG_SYS_BOOTCOUNT_ADDR
>     +#else
>       #define CONFIG_SYS_INIT_SP_ADDR                        \
>              (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE)
>     +#endif
> 
>       #define CONFIG_SYS_SDRAM_BASE          PHYS_SDRAM_1
> 
>     -- 
>     2.19.1
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de