Re: [PATCH v2 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit

[Tom Rini <trini@konsulko.com>]

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

On Thu, Jul 06, 2023 at 07:35:02PM +0200, Pali Rohár wrote:
> To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition()
> function better understandable, rewrite it via explicit switch-case code
> pattern.
> 
> Also add a warning when eMMC EXT_CSD[179] register is configured by user to
> value which is not suitable for eMMC booting and SPL do not know how to
> interpret it.
> 
> Note that when booting from eMMC device via EXT_CSD[179] register is
> explicitly disabled then SPL still loads and boots from this eMMC device
> from User Area partition. This behavior was not changed in this commit and
> should be revisited in the future.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v2:
> * Disable showing warning on sama5d2_xplained due to size restrictions
> ---
> This patch depends on another patch:
> mmc: spl: Add comments for default_spl_mmc_emmc_boot_partition()
> https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@kernel.org/
> ---
>  common/spl/Kconfig   |  7 +++++++
>  common/spl/spl_mmc.c | 46 ++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 865571d4579c..0574d22b3b25 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -855,6 +855,13 @@ config SPL_MMC_WRITE
>  	help
>  	  Enable write access to MMC and SD Cards in SPL
>  
> +config SPL_MMC_WARNINGS
> +	bool "Print MMC warnings"
> +	depends on SPL_MMC
> +	default y if !TARGET_SAMA5D2_XPLAINED
> +	help
> +	  Print SPL MMC warnings. You can disable this option to reduce SPL size.
> +
>  
>  config SPL_MPC8XXX_INIT_DDR
>  	bool "Support MPC8XXX DDR init"
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index f7a42a11477d..ec424ceded0e 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -408,15 +408,45 @@ int default_spl_mmc_emmc_boot_partition(struct mmc *mmc)
>  	 *
>  	 * Note: See difference between EXT_CSD_EXTRACT_PARTITION_ACCESS
>  	 * and EXT_CSD_EXTRACT_BOOT_PART, specially about User area value.
> -	 *
> -	 * FIXME: When booting from this eMMC device is explicitly
> -	 * disabled then we use User area for booting. This is incorrect.
> -	 * Probably we should skip this eMMC device and select the next
> -	 * one for booting. Or at least throw warning about this fallback.
>  	 */
> -	part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> -	if (part == 7)
> -		part = 0;
> +	if (mmc->part_config == MMCPART_NOAVAILABLE)
> +		part = 0; /* If partitions are not supported then we have only User Area partition */
> +	else {
> +		switch(EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config)) {
> +		case 0: /* Booting from this eMMC device is disabled */
> +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> +#ifdef CONFIG_SPL_MMC_WARNINGS
> +			puts("spl: WARNING: Booting from this eMMC device is disabled in EXT_CSD[179] register\n");
> +			puts("spl: WARNING: Continuing anyway and selecting User Area partition for booting\n");
> +#else
> +			puts("spl: mmc: fallback to user area\n");
> +#endif
> +#endif
> +			/* FIXME: This is incorrect and probably we should select next eMMC device for booting */
> +			part = 0;
> +			break;
> +		case 1: /* Boot partition 1 is used for booting */
> +			part = 1;
> +			break;
> +		case 2: /* Boot partition 2 is used for booting */
> +			part = 2;
> +			break;
> +		case 7: /* User area is used for booting */
> +			part = 0;
> +			break;
> +		default: /* Other values are reserved */
> +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> +#ifdef CONFIG_SPL_MMC_WARNINGS
> +			puts("spl: WARNING: EXT_CSD[179] register is configured to boot from Reserved value\n");
> +			puts("spl: WARNING: Selecting User Area partition for booting\n");
> +#else
> +			puts("spl: mmc: fallback to user area\n");
> +#endif
> +#endif
> +			part = 0;
> +			break;
> +		}
> +	}
>  #endif

Please just use debug() for these messages.

-- 
Tom

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