[PATCH v3 2/2] env: mmc: Clean up env_mmc_load() ifdeffery

[Marek Vasut]

On 2/24/25 10:48 AM, Quentin Schulz wrote:
> Hi Marek,

Hi,

> On 2/21/25 7:47 PM, Marek Vasut wrote:
>> Rename the variants of env_mmc_load() for redundant and non-redundant
>> environment to env_mmc_load_redundant() and env_mmc_load_singular()
>> respectively and convert the env_mmc_load() implementation to use of
>> if (IS_ENABLED(...)). As a result, drop __maybe_unused from
>> mmc_env_is_redundant_in_both_boot_hwparts().
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> ---
>> Cc: Dragan Simic <dsimic at manjaro.org>
>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>> Cc: Mattijs Korpershoek <mkorpershoek at baylibre.com>
>> Cc: Quentin Schulz <quentin.schulz at cherry.de>
>> Cc: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Tom Rini <trini at konsulko.com>
>> Cc: u-boot at lists.denx.de
>> ---
>> V3: New patch
>> ---
>>   env/mmc.c | 25 ++++++++++++++-----------
>>   1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/env/mmc.c b/env/mmc.c
>> index 353a7ce72fb..2ef15fb72e7 100644
>> --- a/env/mmc.c
>> +++ b/env/mmc.c
>> @@ -205,7 +205,7 @@ static inline s64 mmc_offset(struct mmc *mmc, int 
>> copy)
>>   }
>>   #endif
>> -static bool __maybe_unused 
>> mmc_env_is_redundant_in_both_boot_hwparts(struct mmc *mmc)
>> +static bool mmc_env_is_redundant_in_both_boot_hwparts(struct mmc *mmc)
>>   {
>>       /*
>>        * In case the environment is redundant, stored in eMMC hardware 
>> boot
>> @@ -448,13 +448,7 @@ static inline int read_env(struct mmc *mmc, 
>> unsigned long size,
>>       return (n == blk_cnt) ? 0 : -1;
>>   }
>> -#if defined(ENV_IS_EMBEDDED)
>> -static int env_mmc_load(void)
>> -{
>> -    return 0;
>> -}
>> -#elif defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT)
>> -static int env_mmc_load(void)
>> +static int env_mmc_load_redundant(void)
>>   {
>>       struct mmc *mmc;
>>       u32 offset1, offset2;
>> @@ -510,8 +504,8 @@ err:
>>       return ret;
>>   }
>> -#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
>> -static int env_mmc_load(void)
>> +
>> +static int env_mmc_load_singular(void)
>>   {
>>       ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
>>       struct mmc *mmc;
>> @@ -556,7 +550,16 @@ err:
>>       return ret;
>>   }
>> -#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
>> +
>> +static int env_mmc_load(void)
>> +{
>> +    if (IS_ENABLED(CONFIG_ENV_IS_EMBEDDED))
>> +        return 0;
>> +    else if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
>> +        return env_mmc_load_redundant();
>> +    else
>> +        return env_mmc_load_singular();
> 
> The else (not its content!) could be removed since all conditions above 
> return.
> 
> Isn't this going to be increasing the size of the binary with code that 
> won't be run or is the compiler smart enough to remove that based on the 
> IS_ENABLED() that are constant false at build time?
If that was to be the case, then IS_ENABLED() would be entirely 
pointless in the first place, right ?