[PATCH] env: mmc: Fix env loading with CONFIG_SYS_MMC_ENV_PART

[Tom Rini]

On Fri, Jul 19, 2024 at 05:38:54PM +0200, Mattijs Korpershoek wrote:

> When CONFIG_SYS_MMC_ENV_PART=2, the environment is loaded from
> the USER partition (hwpart=0) instead of from the
> BOOT1 partition (hwpart=2).
> 
> IS_ENABLED() cannot be used for non-boolean KConfig options.
> Its documentation states:
> 
> > * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y',
> > * 0 otherwise.
> 
> So in our case, IS_ENABLED(CONFIG_SYS_MMC_ENV_PART) evaluates to 0.
> 
> Because of this, the hwpart variable is never assigned and mmc_offset()
> ends up switching back to the USER hwpart (0) instead of BOOT1 (2).
> 
> Fix it by using #define instead.
> 
> Tested with:
> 
>   # have CONFIG_SYS_MMC_ENV_PART=2 in defconfig
>   # 1. Erase mmc0boot1
>   => mmc dev 0 2
>   => mmc erase 0 400
>   => mmc read ${loadaddr} 0 400
>   => md ${loadaddr} 4
>   82000000: 00000000 00000000 00000000 00000000  ................
> 
>   # 2. Restore default environment and save to MMC
>   => env default -a -f
>   => saveenv
> 
>   # 3. Read back mmc0boot1 and confirm the env is present
>   => mmc read ${loadaddr} 0 400
>   => md ${loadaddr} 4
>   82000000: 13e0632e 72646461 7469665f 3978303d  .c..addr_fit=0x9
> 
> Fixes: 5b4acb0ff79d ("env: mmc: Apply GPT only on eMMC user HW partition")
> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
>  env/mmc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/env/mmc.c b/env/mmc.c
> index 776df0786be5..0338aa6c56ad 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -110,8 +110,9 @@ static inline s64 mmc_offset(struct mmc *mmc, int copy)
>  	int hwpart = 0;
>  	int err;
>  
> -	if (IS_ENABLED(CONFIG_SYS_MMC_ENV_PART))

> -		hwpart = mmc_get_env_part(mmc);
> +#if defined(CONFIG_SYS_MMC_ENV_PART)
> +	hwpart = mmc_get_env_part(mmc);
> +#endif
>  
>  #if defined(CONFIG_ENV_MMC_PARTITION)
>  	str = CONFIG_ENV_MMC_PARTITION;
> 

Nice catch. I did a little poking around with:
rg -g Kconfig -U '^config ([A-Z0-9_]+)\n[[:blank:]]+(int|hex)' -o -r \
  '$1' -NI | sort -u > syms

And then
for SYM in `cat syms`;do rg "IS_ENABLED\(CONFIG_${SYM}\)";done

The only other place we use IS_ENABLED like this is a debug print where
a value of zero on the symbol would be invalid usage.

Reviewed-by: Tom Rini <trini at konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240719/6cd9eca0/attachment.sig>