[PATCH v3] spl: remove usage of CMD_BOOT[IZ] from image parsing

[Anshul Dalal]

On Wed Mar 12, 2025 at 11:53 PM IST, Tom Rini wrote:
> On Wed, Mar 12, 2025 at 06:17:56PM +0530, Anshul Dalal wrote:
>
> > Using CMD_* configs from spl doesn't make logical sense. Therefore this
> > patch replaces the checks for CMD_BOOT[IZ] with newly added configs
> > SPL_HAS_BOOT[IZ].
> > 
> > An imply clause has been added for CMD_BOOTZ to avoid breaking configs
> > that rely on CMD_BOOTZ in falcon boot. There is no such need for
> > CMD_BOOTI since it's already enabled by default for the platforms that
> > support the image format.
> > 
> > More info:
> > https://lore.kernel.org/u-boot/20250310135544.GV2640854@bill-the-cat/
> > 
> > Signed-off-by: Anshul Dalal <anshuld at ti.com>
> > ---
> > Changes in v3:
> >  * Add imply clause for CMD_BOOTZ to enable SPL_HAS_BOOTZ
> >  * Fix broken check for bootz_setup
> > v2: https://lore.kernel.org/u-boot/20250312094241.629707-1-anshuld@ti.com/
> > Changes in v2:
> >  * Add SPL_HAS_BOOT[IZ] configs
> > v1: https://lore.kernel.org/u-boot/20250311093709.3372104-1-anshuld@ti.com/
> > ---
> >  arch/arm/lib/Makefile |  4 ++--
> >  cmd/Kconfig           |  1 +
> >  common/spl/Kconfig    | 13 +++++++++++++
> >  common/spl/spl.c      |  5 +++--
> >  4 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> > index 1c95dd6fed2..e857adddd03 100644
> > --- a/arch/arm/lib/Makefile
> > +++ b/arch/arm/lib/Makefile
> > @@ -36,8 +36,8 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o zimage.o
> >  else
> >  obj-$(CONFIG_$(PHASE_)FRAMEWORK) += spl.o
> >  ifdef CONFIG_SPL_FRAMEWORK
> > -obj-$(CONFIG_CMD_BOOTI) += image.o
> > -obj-$(CONFIG_CMD_BOOTZ) += zimage.o
> > +obj-$(CONFIG_SPL_HAS_BOOTI) += image.o
> > +obj-$(CONFIG_SPL_HAS_BOOTZ) += zimage.o
> >  endif
>
> This Makefile hunk is weird and not quite correct today, and this change
> makes it more odd. We should drop the CONFIG_SPL_FRAMEWORK check because
> we're already in an CONFIG_XPL_BUILD check, and since we're making
> needing these objects explicit now the very rare case of
> ARM+SPL+!SPL_FRAMEWORK won't try and include these when not required.
>
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index a04fcaa0e08..9f2ba4d02f8 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -352,6 +352,7 @@ config BOOTM_ELF
> >  
> >  config CMD_BOOTZ
> >  	bool "bootz"
> > +	imply SPL_HAS_BOOTZ
> >  	help
> >  	  Boot the Linux zImage
> >  
>
> We don't need this hunk at all, we already (should) be getting the right
> file via the current logic in arch/arm/lib/Makefile
>

That's correct but we also need to take into account the modified check
in common/spl/spl.c, earlier it would pass on CMD_BOOTZ and allow
booting a zImage, now it checks for SPL_HAS_BOOTZ instead.

This would fail for configs that relied on CMD_BOOTZ during falcon boot
such as display5_defconfig. This imply ensures we enable SPL_HAS_BOOTZ
in such cases.

> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > index 21a5cefee7a..e0e804ae8dc 100644
> > --- a/common/spl/Kconfig
> > +++ b/common/spl/Kconfig
> > @@ -1153,6 +1153,19 @@ config SPL_OS_BOOT
> >  	  Enable booting directly to an OS from SPL.
> >  	  for more info read doc/README.falcon
> >  
> > +config SPL_HAS_BOOTZ
> > +	bool "Allow booting to kernel zimage from SPL"
>
> How about:
> "Allow booting a zImage style Linux kernel from SPL"
>
> > +	depends on SPL_OS_BOOT
> > +	help
> > +	  Boot a linux zimage from memory in falcon boot.
> > +
> > +config SPL_HAS_BOOTI
> > +	bool "Allow booting to kernel image from SPL"
>
> How about:
> "Allow booting an Image style Linux kernel from SPL"
>

Sounds good would be addressed in v4 :)

> > +	depends on SPL_OS_BOOT
> > +	default y if ARM64 || RISCV || SANDBOX
>
> Do we need SANDBOX here?

No, having SPL_OS_BOOT on SANDBOX doesn't make sense anyway.

Anshul