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

[Tom Rini]

On Wed, May 15, 2019 at 04:39:22PM -0500, Andreas Dannenberg wrote:
> 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.

Right, that makes sense.  But how did you get to that
not-exactly-"round" number rather than say 0x41A00 or some other
slightly smaller value in hex?  If 0x41AC8 is right, then, OK, it's
right.  It just seems strange at first.

>  
> > [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]:

Then you do:
ifeq ($(CONFIG_SPL_BUILD),y)
obj-$(CONFIG_K3_LOAD_SYSFW) += sysfs-loader.o
endif

As seen in other places, as we do not whole-body ifdef code.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190515/95760cd8/attachment.sig>