Re: [PATCH v2 2/2] spl: mmc: support dynamic selection in raw partition mode

[Tom Rini <trini@konsulko.com>]

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

On Fri, Sep 11, 2026 at 02:05:59PM +0200, Carlo Caione wrote:

> Boards using boot metadata to select U-Boot should not need to configure
> an unused fixed partition number. Add an explicit runtime-selection
> option within the existing partition mode, retaining its loader and
> platform dependencies.
> 
> A failed dynamic selection must not boot from an unrelated filesystem
> partition. Reject an invalid selection for this MMC attempt while keeping
> the existing fallback behavior when dynamic selection is disabled.
> 
> STM32's existing hook implements fixed partition selection and references
> the static partition settings. Restrict it and its MMC2 setting to fixed
> mode so dynamic selection can provide its own hook.
> 
> The board hook owns hardware-area selection, including access to metadata
> and leaving the image area selected.
> 
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
> ---
>  arch/arm/mach-stm32mp/Kconfig        |  1 +
>  arch/arm/mach-stm32mp/stm32mp1/spl.c |  2 +-
>  common/spl/Kconfig                   |  9 +++++++++
>  common/spl/spl_mmc.c                 | 19 +++++++++++++------
>  include/spl.h                        | 14 +++++++++-----
>  5 files changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
> index f45010ddbd0..bea4a1b6aaf 100644
> --- a/arch/arm/mach-stm32mp/Kconfig
> +++ b/arch/arm/mach-stm32mp/Kconfig
> @@ -175,6 +175,7 @@ config DDR_CACHEABLE_SIZE
>  config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_MMC2
>  	hex "Partition on MMC2 to use to load U-Boot from"
>  	depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> +	depends on !SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_DYNAMIC
>  	default 1
>  	help
>  	  Partition on the second MMC to load U-Boot from when the MMC is being
> diff --git a/arch/arm/mach-stm32mp/stm32mp1/spl.c b/arch/arm/mach-stm32mp/stm32mp1/spl.c
> index 2229ebf5eeb..c1be468cb1d 100644
> --- a/arch/arm/mach-stm32mp/stm32mp1/spl.c
> +++ b/arch/arm/mach-stm32mp/stm32mp1/spl.c
> @@ -61,7 +61,7 @@ u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
>  	return MMCSD_MODE_RAW;
>  }
>  
> -#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> +#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
>  int spl_mmc_boot_partition(struct mmc *mmc, const u32 boot_device)
>  {
>  	switch (boot_device) {
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 0618f42c941..9e1c23ea0f4 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -576,6 +576,14 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
>  
>  endchoice
>  
> +config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_DYNAMIC
> +	bool "Select the raw partition at runtime"
> +	depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> +	select SPL_PARTITIONS
> +	help
> +	  Select the partition at runtime instead of using a fixed number.
> +	  The board must implement spl_mmc_boot_partition().
> +
>  config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
>  	hex "Address on the MMC to load U-Boot from"
>  	depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> @@ -609,6 +617,7 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET
>  config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
>  	hex "Partition to use to load U-Boot from"
>  	depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
> +	depends on !SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_DYNAMIC
>  	default 1
>  	help
>  	  Partition on the MMC to load U-Boot from when the MMC is being
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 3e637f5adbd..d538fbb9c41 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -303,7 +303,11 @@ u32 __weak spl_mmc_boot_mode(struct mmc *mmc, const u32 boot_device)
>  #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
>  int __weak spl_mmc_boot_partition(struct mmc *mmc, const u32 boot_device)
>  {
> +#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
>  	return CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION;
> +#else
> +	return -ENOSYS;
> +#endif
>  }
>  #endif
>  
> @@ -432,8 +436,15 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>  #elif defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION) || \
>        defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE)
>  		if (IS_ENABLED(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION) &&
> -		    raw_part == -1)
> +		    raw_part == -1) {
>  			raw_part = spl_mmc_boot_partition(mmc, bootdev->boot_device);
> +			if (IS_ENABLED(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_DYNAMIC) &&
> +			    raw_part < 1) {
> +				ret = raw_part < 0 ? raw_part : -EINVAL;
> +				printf("spl: partition selection failed: %d\n", ret);
> +				return ret;
> +			}
> +		}
>  		ret = mmc_load_image_raw_partition(spl_image, bootdev,
>  						   mmc, raw_part,
>  						   raw_sect);

This is better than v1. Did you look in to my suggestion about turning
this in to a choice between hard-coded number, hard-coded type and this
new dynamic option? I do wonder if that would let us get rid of a lot of
the if/else nesting we have in these functions. Thanks!

-- 
Tom

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