On Sat, Oct 03, 2026 at 02:40:44PM +0100, Simon Glass wrote: > Hi Alexey, > > On 2026-09-18T10:52:27, Alexey Charkov <[email protected]> wrote: > > lib: Track whether a phase has a decompressor > > > > spl_decompression_enabled() names the two algorithms SPL happened to > > support, so every algorithm added to image_decomp() requires editing the > > header. It also asks about CONFIG_SPL_* from a header included in every > > phase, so a VPL build with VPL_LZMA reads SPL_LZMA and answers about the > > wrong phase. > > > > Add a hidden per-phase symbol selected by any of the compression > > algorithms and ask about that instead, which also matches how > > image_decomp() gates its own cases. > > > > SPL_BZIP2 is out of scope: lib/bzip2 has no phase-aware build rule. > > > > Signed-off-by: Alexey Charkov <[email protected]> > > Reviewed-by: Tom Rini <[email protected]> > > > > common/spl/spl_fit.c | 6 +++--- > > include/spl.h | 10 ---------- > > lib/Kconfig | 30 ++++++++++++++++++++++++++++++ > > 3 files changed, 33 insertions(+), 13 deletions(-) > > > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c > > @@ -284,15 +284,15 @@ static int load_simple_fit(struct spl_load_info > > *info, ulong fit_offset, > > - if (IS_ENABLED(CONFIG_SPL_FPGA) || > > - (IS_ENABLED(CONFIG_SPL_OS_BOOT) && spl_decompression_enabled())) { > > + if (CONFIG_IS_ENABLED(FPGA) || > > + (CONFIG_IS_ENABLED(OS_BOOT) && CONFIG_IS_ENABLED(DECOMPRESSION))) > > { > > The FPGA and OS_BOOT conversions are a separate change, not mentioned > in the commit message. The rest of this file still uses > IS_ENABLED(CONFIG_SPL_FPGA) and IS_ENABLED(CONFIG_SPL_OS_BOOT), e.g. > in spl_fit_image_is_fpga(), os_takes_devicetree() and > spl_load_simple_fit(), so this leaves the file inconsistent. There is > no TPL_FPGA, VPL_FPGA, TPL_OS_BOOT or VPL_OS_BOOT either, so in those > phases the answer now differs from the other checks. > > Please can you either leave those two as they are, or convert the > whole file in a separate patch?
No, it's fine to not worry about those others until a need comes up. > > diff --git a/lib/Kconfig b/lib/Kconfig > > @@ -974,30 +978,35 @@ config SPL_LZMA_SIZE_OPTIMIZATION > > config VPL_LZMA > > bool "Enable LZMA decompression support for VPL build" > > default y if LZMA > > + select VPL_DECOMPRESSION > > VPL_LZMA has no 'depends on VPL', so with this select every board with > LZMA enabled gets CONFIG_VPL_DECOMPRESSION=y, even with no VPL. I > tried sandbox, qemu_arm64 and evb-rk3399 and all three end up with it > in their .config file. > > Please can you add a preparatory patch to give VPL_LZMA a 'depends on > VPL'? BTW SPL_GZIP and TPL_GZIP have the same problem, although they > don't default to y so it matters less. This is also further nice-to-have cleanup work that's not relevant here. > > diff --git a/lib/Kconfig b/lib/Kconfig > > @@ -1011,10 +1020,31 @@ config TPL_ZLIB > > +config SPL_DECOMPRESSION > > + bool > > + help > > + Selected by every decompression algorithm available to SPL. Code > > + which hands a buffer to image_decomp() without caring which > > + algorithm produced it should depend on this, rather than list each > > + algorithm in turn. > > Not quite, since SPL_BZIP2 does not select it. The commit message > explains why, but please can you mention the exception here too, since > this is where people will look? > > Also, there is no symbol for U-Boot proper, so > CONFIG_IS_ENABLED(DECOMPRESSION) is silently false there. That is fine > for spl_fit.c but anyone following this advice in code shared between > phases (e.g. in boot/) will get the wrong answer. I suspect it is > worth either adding a DECOMPRESSION symbol for U-Boot proper, or > saying in the help that this is only for xPL code. What do you think? The whole set of things we're doing here is xPL, so we don't need a symbol for full U-Boot at this time. > I wish it were DECOMP instead of the rather verbose DECOMPRESSION It's also not worth redoing the series over, either. DECOMPRESSION is fine. -- Tom
signature.asc
Description: PGP signature
