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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

On Wed, Apr 02, 2025 at 04:00:01PM +0530, Anshul Dalal wrote:
> On Tue Apr 1, 2025 at 4:33 AM IST, Tom Rini wrote:
> > On Fri, Mar 14, 2025 at 09:25:04AM +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].
> >> 
> >> SPL_HAS_BOOTZ is enabled by default for 32-bit ARM systems and
> >> SPL_HAS_BOOTI is enabled by default for 64-bit ARM and RISCV. This
> >> ensures configs relying on CMD_BOOT[IZ] in falcon boot still work.
> >> 
> >> Signed-off-by: Anshul Dalal <anshuld@ti.com>
> >
> > OK, so this needs to be introducing some library symbol which then both
> > CMD_BOOTx and SPL_...something select.
> 
> I was thinking of chaning the Makefile with the following diff:
> 
> -obj-$(CONFIG_CMD_BOOTI) += bootm.o image.o
> +obj-$(CONFIG_LIB_BOOTI) += image.o
>  obj-$(CONFIG_CMD_BOOTM) += bootm.o
> -obj-$(CONFIG_CMD_BOOTZ) += bootm.o zimage.o
> +obj-$(CONFIG_LIB_BOOTZ) += zimage.o
>  else
>  obj-$(CONFIG_$(PHASE_)FRAMEWORK) += spl.o
> -obj-$(CONFIG_SPL_HAS_BOOTI) += image.o
> -obj-$(CONFIG_SPL_HAS_BOOTZ) += zimage.o
>  obj-$(CONFIG_OF_LIBFDT) += bootm-fdt.o
>  endif
> 
> This would simplify the Makefile by not having duplicated configs for
> image/zimage and the removed bootm.o for non-spl builds should be fine
> since except for colibri_vf, all defconfigs with CMD_BOOTx already have
> CMD_BOOTM enabled. And for colibri_vf, we can safely enable CMD_BOOTM?
> 
> 
> > What we have here introduces
> > failure to build on some imx8 platforms such as 
> > mx8mp_evk and then size growth on others such as imx28_xea.
> 
> The imx8mp_evk build should not fail since the patch should only effect
> falcon mode. Only difference this patch makes is now image.o would not
> be compiled for the spl which is not used in non falcon boot anyways.
> 
> For the imx28_xea and 3 other (imx6qdl_icore_mipi|mmc|rqs) defconfigs
> that use falcon mode but don't make use of CMD_BOOTx. Their size growth
> is expected since SPL_HAS_BOOTx is default y in falcon boot. To keep the
> same size, the SPL_HAS_BOOTx can be explicitly disabled for those 4
> configs.
> 
> For future reference is there any CI tests I can run to detect any
> regressions before posting patches upstream?

To run CI please see
https://docs.u-boot.org/en/latest/develop/ci_testing.html

And yes, please re-work the symbols and logic so there's no size growth
until the K3 R5 platforms enable things, as the default should not
change behavior. Using
https://source.denx.de/u-boot/u-boot-extras/-/blob/master/contrib/trini/u-boot-size-test.sh?ref_type=heads
might make it easier to compare various platforms before/after.

> 
> Regards,
> Anshul

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]