Hi Tom, On Wed, May 15, 2019 at 11:17:22AM -0400, Tom Rini wrote: > On Tue, May 07, 2019 at 12:25:33PM -0500, Andreas Dannenberg wrote: > > > Introduce a framework that allows loading the System Firmware (SYSFW) > > binary as well as the associated configuration data from an image tree > > blob named "sysfw.itb" from an FS-based MMC boot media or from an MMC > > RAW mode partition or sector. > > > > To simplify the handling of and loading from the different boot media > > we tap into the existing U-Boot SPL framework usually used for loading > > U-Boot by building on an earlier commit that exposes some of that > > functionality. > > > > Note that this initial implementation only supports FS and RAW-based > > eMMC/SD card boot. > [snip] > > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig > > index e677a2e01b..f1731dda58 100644 > > --- a/arch/arm/mach-k3/Kconfig > > +++ b/arch/arm/mach-k3/Kconfig > > @@ -58,6 +58,46 @@ config SYS_K3_BOOT_CORE_ID > > int > > default 16 > > > > +config K3_LOAD_SYSFW > > + bool > > + depends on SPL > > + default n > > 'n' is already default, you can drop this.
Ok sure. > > [snip] > > +config K3_SYSFW_IMAGE_SIZE_MAX > > + int "Amount of memory dynamically allocated for loading SYSFW blob" > > + depends on K3_LOAD_SYSFW > > + default 269000 > > + help > > + Amount of memory reserved through dynamic allocation at runtime for > > + loading the combined System Firmware and configuration image tree > > + blob. Keep it as tight as possible, as this directly affects the > > + overall SPL memory footprint. > > This is missing a unit, and is 'int' really the best choice here (and > really, I guess, 262.6KiB as a default) ? Will add a unit. As for the specific value, in our R5 SPL memory is very very tight. The value used here is basically used for a malloc(), and any extra byte allocated will be "wasted" and not available for stack etc. Also our SYSFW image that is loaded is fixed size (except some +/-100 odd bytes if the configuration data is changed that's part of the same SYSFW.ITB blob), so a rather tailored size makes sense here. > [snip] > > diff --git a/arch/arm/mach-k3/Makefile b/arch/arm/mach-k3/Makefile > > index 0c3a4f7db1..6c895400c2 100644 > > --- a/arch/arm/mach-k3/Makefile > > +++ b/arch/arm/mach-k3/Makefile > > @@ -7,4 +7,5 @@ obj-$(CONFIG_SOC_K3_AM6) += am6_init.o > > obj-$(CONFIG_ARM64) += arm64-mmu.o > > obj-$(CONFIG_CPU_V7R) += r5_mpu.o lowlevel_init.o > > obj-$(CONFIG_TI_SECURE_DEVICE) += security.o > > +obj-$(CONFIG_K3_LOAD_SYSFW) += sysfw-loader.o > > obj-y += common.o > [snip] > > diff --git a/arch/arm/mach-k3/sysfw-loader.c > > b/arch/arm/mach-k3/sysfw-loader.c > > new file mode 100644 > > index 0000000000..a222266c27 > > --- /dev/null > > +++ b/arch/arm/mach-k3/sysfw-loader.c > > @@ -0,0 +1,263 @@ > [snip] > > +#ifdef CONFIG_SPL_BUILD > [snip of the whole body of the file] > > +#endif > > We should be using something else in the Makefile, typically: > obj-$(CONFIG_SPL_BUILD) += sysfw-loader.o > should work. Well, it won't work like this. We need to make the building of the SYSFW loader code depending on a CONFIG. This is because for K3 family devices we build the U-Boot tree into three distinct images [1]: 1) SPL image for running on an R5 (our first stage) 2) SPL image for running on an A53 (our second stage) 3) U-Boot (proper) image for running on an A53 (third stage) With the change you proposed sysfw-loader.o will get build for A53 SPL where it is neither needed nor wanted, causing build failures. What we would need is something like (CONFIG_K3_LOAD_SYSFW && CONFIG_SPL_BUILD) to conditionally build that object if we want to get rid of the #ifdef/#endif in sysfw-loader.c I suppose. Thanks, Andreas [1] https://github.com/u-boot/u-boot/blob/master/board/ti/am65x/README _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot