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 <mkorpersh...@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 <tr...@konsulko.com> -- Tom
signature.asc
Description: PGP signature