Hi Tom,
On Thu, May 16, 2019 at 10:47:30AM -0500, Andreas Dannenberg wrote:
Hi Tom,
On Wed, May 15, 2019 at 05:50:31PM -0400, Tom Rini wrote:
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 forloading the combined System Firmware and configuration image treeblob. Keep it as tight as possible, as this directly affects theoverall 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) ?
I think 'int' is a more natural fit as this is to reserve memory for the 'sysfw.itb' blob we load from media. This blob is build outside U-Boot using the "System Firmware Image Generator" project [1], and if after build somebody does an 'll' in the build directory the file size would show up as decimal, and that's what needs to fit inside K3_SYSFW_IMAGE_SIZE_MAX.
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.
The sysfw.itb blob consumed by the SYSFW loader comprises a fixed component (the SYSFW itself), plus a few smaller chunks of variable data containing user configuration data. During initial development the combined size of this ITB was about 264,900 bytes, and 4KB was added as slack to accommodate configuration changes, resulting in what looks like the arbitrary number under discussion. The sysfw.itb blob has actually since grown in size but there is still plenty of space for it to further grow if needed. Since we use this value already in production since last year with no issues I'd rather leave it alone (due to using stack based malloc in R5 SPL increasing that value would directly decrease available stack space with our R5 SPL memory layout where the stack is growing down towards the R5 SPL image itself).
Small clarification, the size of the early malloc pool is of course controlled by CONFIG_SYS_MALLOC_F_LEN with K3_SYSFW_IMAGE_SIZE_MAX using the majority of it. However CONFIG_SYS_MALLOC_F_LEN is dimensioned such to "tightly wrap" both K3_SYSFW_IMAGE_SIZE_MAX and other users of the early malloc pool (fat fs is a major one), so the principle is the same. I'd rather leave K3_SYSFW_IMAGE_SIZE_MAX as it stands.
Some more details on the memory usage to follow...
-- Andreas Dannenberg Texas Instruments Inc