On Tue, May 04, 2021 at 07:24:25PM -0400, Sean Anderson wrote:
Hi Tom,
On 5/4/21 5:40 PM, Tom Rini wrote:
On Mon, May 03, 2021 at 05:10:47PM -0600, Simon Glass wrote:
Much of the image-handling code predates the introduction of Kconfig and has quite a few #ifdefs in it. It also uses its own IMAGE_... defines to help reduce the #ifdefs, which is unnecessary now that we can use IS_ENABLED() et al.
The image code is also where quite a bit of code is shared with the host tools. At present this uses a lot of checks of USE_HOSTCC.
This series introduces 'host' Kconfig options and a way to use CONFIG_IS_ENABLED() to check them. This works in a similar way to SPL, so
CONFIG_IS_ENABLED(FIT)will evaluate to true on the host build (USE_HOSTCC) if CONFIG_HOST_FIT is enabled. This allows quite a bit of clean-up of the image.h header file and many of the image C files.
The 'host' Kconfig options should help to solve a more general problem in that we mostly want the host tools to build with all features enabled, no matter which features the 'target' build actually uses. This is a pain to arrange at present, but with 'host' Kconfigs, we can just define them all to y.
There are cases where the host tools do not have features which are present on the target, for example environment and physical addressing. To help with this, some of the core image code is split out into image-board.c and image-host.c files.
Even with these changes, some #ifdefs remain (101 down to 42 in common/image*). But the code is somewhat easier to follow and there are fewer build paths.
In service of the above, this series includes a patch to add an API function for zstd, so the code can be dropped from bootm.c
It also introduces a function to handle manual relocation.
I like this idea overall. The good news is this reduces the size in a few places. The bad news, but I can live with if we can't restructure the changes more, is a few functions grow a bit. This shows the good and the bad (something like sama5d2_ptc_ek_mmc shows only growth, to be clear):
What tool do you use to generate this output? Thanks.
buildman will give that for you. This was from my world build before/after wrapper, but for a single machine I have: #!/bin/bash
# Initial and constant buildman args ARGS="-devl" ALL=0 KEEP=0
# Find our arguments while test $# -ne 0; do if [ "$1" == "--all" ]; then ALL=1 shift 1 elif [ "$1" == "--branch" ]; then BRANCH=$2 shift 2 elif [ "$1" == "--keep" ]; then KEEP=1 ARGS="$ARGS -k" shift 1 else MACHINE=$1 shift fi done
if [ -z $MACHINE ]; then echo Usage: $0 MACHINE [--all] [--keep] [--branch BRANCH] exit 1 fi
# If not all, then only first/last if [ $ALL -ne 1 ]; then ARGS="$ARGS --step 0" fi
if [ ! -z $BRANCH ]; then ARGS="$ARGS -b $BRANCH" else ARGS="$ARGS -b `git rev-parse --abbrev-ref HEAD`" fi
mkdir -p /tmp/$MACHINE
export SOURCE_DATE_EPOCH=`date +%s` ./tools/buildman/buildman -o /tmp/$MACHINE $ARGS -SBC $MACHINE ./tools/buildman/buildman -o /tmp/$MACHINE $ARGS -SsB $MACHINE
[ $KEEP -eq 0 ] && rm -rf /tmp/$MACHINE
In a script called "uboot-size-test.sh" to dig in to individual cases more.