Re: [PATCH 6/8] env: Rename ENV_MMC_PARTITION to ENV_SDMMC_SW_PARTITION

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Marek,

On 3/22/25 10:55 PM, Marek Vasut wrote:
> Rename the variable and add ENV_ prefix, so that all configuration
> options which are related to environment would have an CONFIG_ENV_
> prefix. No functional change.
> 
> Use ENV_SDMMC_SW_PARTITION to clarify this is the SD/MMC software
> partition table entry selector. Update the help text accordingly.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Paul Barker <paul.barker.ct@bp.renesas.com>
> Cc: Quentin Schulz <quentin.schulz@cherry.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> ---
>   doc/device-tree-bindings/config.txt |  2 +-
>   env/Kconfig                         | 21 ++++++++++++---------
>   env/mmc.c                           |  4 ++--
>   3 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt
> index 8584cc6dcf4..44443c92d5d 100644
> --- a/doc/device-tree-bindings/config.txt
> +++ b/doc/device-tree-bindings/config.txt
> @@ -76,7 +76,7 @@ u-boot,mmc-env-partition (int)
>   	precedence. In that case, only if the partition is not found,
>   	mmc-env-offset* will be tried.
>   
> -	Note that CONFIG_ENV_MMC_PARTITION overrides this device-tree setting.
> +	Note that CONFIG_ENV_SDMMC_SW_PARTITION overrides this device-tree setting.
>   
>   u-boot,no-apm-finalize (bool)
>   	For x86 devices running on coreboot, this tells U-Boot not to lock
> diff --git a/env/Kconfig b/env/Kconfig
> index fbc03777040..19bc34f8663 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -677,7 +677,7 @@ config ENV_SDMMC_DEVICE_INDEX
>   	  matches the 'mmc list' command output.
>   
>   config ENV_SDMMC_EMMC_HW_PARTITION
> -	int "mmc partition number"
> +	int "eMMC hardware partition number"

This should have been in the previous patch I believe.

>   	depends on ENV_IS_IN_MMC || ENV_IS_IN_FAT
>   	default 0
>   	help
> @@ -687,17 +687,20 @@ config ENV_SDMMC_EMMC_HW_PARTITION
>   	  partition 0 or the first boot partition, which is 1 or some other defined
>   	  partition.
>   
> -config USE_ENV_MMC_PARTITION
> -	bool "use the mmc environment partition name"
> +config ENV_SDMMC_USE_SW_PARTITION
> +	bool "Use SD/MMC environment software partition name"
>   	depends on ENV_IS_IN_MMC
>   
> -config ENV_MMC_PARTITION
> -	string "mmc environment partition name"
> -	depends on USE_ENV_MMC_PARTITION
> +config ENV_SDMMC_SW_PARTITION
> +	bool "SD/MMC environment software partition name"
> +	depends on ENV_SDMMC_USE_SW_PARTITION
>   	help
> -	  MMC partition name used to save environment variables.
> -	  If this variable is unset, u-boot will try to get the env partition name
> -	  from the device-tree's /config node.
> +	  SD/MMC software partition name used to save environment variables.
> +	  This is a software partition name, i.e. one in partition table, not
> +	  an eMMC HW partition (see ENV_SDMMC_EMMC_HW_PARTITION for eMMC HW

s/HW/hardware/

> +	  partition configuration). If this variable is unset, u-boot will

s/u-boot/U-Boot/

> +	  try to get the env partition name from the device-tree's /config
> +	  node.
>   

Should we also have the same (or very similar) help text for 
ENV_SDMMC_USE_SW_PARTITION? The issue is that one would need to use the 
search function to find ENV_SDMMC_SW_PARTITION since it won't be 
displayed until ENV_SDMMC_USE_SW_PARTITION is enabled, a symbol that we 
do not currently document.

>   config ENV_MMC_USE_DT
>   	bool "Read partition name and offset in DT"
> diff --git a/env/mmc.c b/env/mmc.c
> index 4155a5c72f1..f49af88bf5f 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -161,8 +161,8 @@ static inline s64 mmc_offset(struct mmc *mmc, int copy)
>   	hwpart = mmc_get_env_part(mmc);
>   #endif
>   
> -#if defined(CONFIG_ENV_MMC_PARTITION)
> -	str = CONFIG_ENV_MMC_PARTITION;
> +#if defined(CONFIG_ENV_SDMMC_SW_PARTITION)
> +	str = CONFIG_ENV_SDMMC_SW_PARTITION;
>   #else
>   	/* look for the partition in mmc CONFIG_ENV_SDMMC_DEVICE_INDEX */

This comment seems incorrect no? (unrelated to this patch though).

Cheers,
Quentin