On Tue, Nov 07, 2023 at 05:18:01PM +0100, Thomas Richard wrote: > During the boot a copy of DM-Firmware is done in a reserved memory > area before it starts. > When resuming, R5 SPL uses this copy of DM-Firmware instead of the fit > image. > TF-A which saved itself in this same memory area, is restored in > SRAM by R5 SPL. > > Based on the work of Gregory CLEMENT <gregory.clem...@bootlin.com> > > Signed-off-by: Thomas Richard <thomas.rich...@bootlin.com> > Signed-off-by: Gregory CLEMENT <gregory.clem...@bootlin.com> > > --- > > Changes in v2: > - Check if TF-A is running in DRAM, if yes no need to restore it > - Remove BL31_START macro, and get TF-A start address from the fit image > > arch/arm/mach-k3/common.c | 48 ++++++++++++++++++++++- > arch/arm/mach-k3/include/mach/j721e_spl.h | 28 +++++++++++++ > arch/arm/mach-k3/sysfw-loader.c | 9 +++-- > 3 files changed, 81 insertions(+), 4 deletions(-)
One problem here is we aren't updating something under doc/, possibly doc/board/ti/k3.rst unless you want to start populating something entirely new under doc/develop/. And then using kernel-doc comments here and including the code in the rst. And so this is a global comment to the series as well, and I'll leave sorting out when/where to introduce that part up to your discretion. [snip] > diff --git a/arch/arm/mach-k3/include/mach/j721e_spl.h > b/arch/arm/mach-k3/include/mach/j721e_spl.h > index e8947917a6..8e0f141ed6 100644 > --- a/arch/arm/mach-k3/include/mach/j721e_spl.h > +++ b/arch/arm/mach-k3/include/mach/j721e_spl.h > @@ -42,4 +42,32 @@ > #define K3_PRIMARY_BOOTMODE 0x0 > #define K3_BACKUP_BOOTMODE 0x1 > > +/* Starting buffer address is 1MB before the stack address in DDR */ > +#define BUFFER_ADDR (CONFIG_SPL_STACK_R_ADDR - SZ_1M) Assuming this is the full level of configurability we need to BUFFER_ADDR please add some Kconfig logic to make sure we can't not set CONFIG_SPL_STACK_R_ADDR. That might take a little trial-and-error and if we just cannot enforce this via Kconfig, let me know and I'll think about what we want to do instead next. > +/* This is actually the whole size of the SRAM */ > +#define BL31_SIZE 0x20000 If this is all of SRAM then is there not already a define? > +/* This address belongs to a reserved memory region for the point of view of > + * Linux, U-boot SPL must use the same address to restore TF-A and resume > + * entry point address > + */ > +#define LPM_SAVE 0xA5000000 > +#define LPM_BL31_SAVE LPM_SAVE > +#define LPM_BL31_RESUME_SAVE LPM_BL31_SAVE + BL31_SIZE > +#define LPM_BL31_START_SAVE LPM_BL31_RESUME_SAVE + __SIZEOF_POINTER__ > +#define LPM_DM_SAVE LPM_BL31_START_SAVE + __SIZEOF_POINTER__ Is any of this somewhere in the devicetree as well? I'm not saying "and so pull it from that!" is a must, but I'd like to at least know "that would be horrible to do", and fall back to having this needed co-operation documented somewhere and ideally somewhere / somehow that getting the values wrong is clear. Debugging resume failures at run-time is much more painful than build time. > +/* Check if the copy of TF-A and DM-Firmware in DRAM does not overlap an > + * over memory section. > + * The resume address of TF-A is also saved in DRAM. > + * At build time we don't know the DM-Firmware size, so we keep 512k to > + * save it. > + */ > +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_TARGET_J7200_R5_EVM) > +#if ((LPM_DM_SAVE + SZ_512K) > BUFFER_ADDR) > +#error Not enough space to save DM-Firmware, TF-A and context for S2R > +#endif > +#endif This is really the kind of thing I'd like to see enforced via Kconfig if at all possible. Sometimes yes we just have to CPP and #error, I just want to know we can't. -- Tom
signature.asc
Description: PGP signature
