Simon Goldschmidt <simon.k.r.goldschm...@gmail.com> schrieb am Di., 30. Okt. 2018, 11:34:
> > > Stefan Roese <s...@denx.de> schrieb am Di., 30. Okt. 2018, 10:36: > >> Hi Simon, >> >> On 30.10.18 10:07, Simon Goldschmidt wrote: >> > >> > >> > Stefan Roese <s...@denx.de <mailto:s...@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. >> > > Well, you configure the bootcounter location in defconfig as an address, > so you cannot really use this for a calculation other than your approach. > > >> And you probably need to add new defines to Kconfig as well >> (e.g. SOCFPGA_INIT_RAM_END_RESERVE). >> > > An even cleaner solution would be to be able to override > CONFIG_SYS_INIT_RAM_SIZE, but since this is a globally used define, I don't > think I can provide a patch for this soon. > > Maybe we'll start with your patch to fix the bootcounter in the mainline > boards and I'll provide a patch for my additional requirements once I find > the time. > Drop the maybe. I'll test your patch and drop mine, but see the comment below. > Simon > > >> Thanks, >> Stefan >> >> > Simon >> > >> > >> > Signed-off-by: Stefan Roese <s...@denx.de <mailto:s...@denx.de>> >> > Cc: Marek Vasut <ma...@denx.de <mailto:ma...@denx.de>> >> > Cc: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com <mailto: >> simon.k.r.goldschm...@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 >> > Can we have this check on CONFIG_INIT_RAM_SIZE instead of the initial stack pointer? That would ensure the SPL size checks stay intact. Simon > >> > #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: s...@denx.de >> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
