[U-Boot] [PATCH 04/13] arm: K3: Introduce System Firmware loader framework

[Andreas Dannenberg]

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