[PATCH] env: Invert gd->env_valid for env_mmc_save

[Jasper Orschulko]

Hi Tom,

you are right. If this patch is to be merged, we should change all the occurrences. That was an oversight on my part, sorry about that. I'll send an updated patch to the mailing list later this week.

My colleague Michael can give more technical background on this than I can, so I'll let him do explaining here ;)

Best,
Jasper

On 17 May 2024 20:51:44 CEST, Tom Rini <trini at konsulko.com> wrote:
>On Fri, May 10, 2024 at 01:38:34PM +0200, jasper at fancydomain.eu wrote:
>
>> From: Jasper Orschulko <jasper at fancydomain.eu>
>> 
>> The A/B update strategy of the env's has a gap in the first 2 calls of saveenv.
>> The env's are stored twice on the first memory area if:
>> gd->env_valid == ENV_INVALID.
>> 
>> u-boot=> saveenv
>> Saving Environment to MMC... Writing to MMC(1)... OK
>> u-boot=> saveenv
>> Saving Environment to MMC... Writing to MMC(1)... OK  <-- !!!
>> u-boot=> saveenv
>> Saving Environment to MMC... Writing to redundant MMC(1)... OK
>> u-boot=> saveenv
>> Saving Environment to MMC... Writing to MMC(1)... OK
>> 
>> Signed-off-by: Michael Glembotzki <Michael.Glembotzki at iris-sensing.com>
>> Signed-off-by: Jasper Orschulko <jasper at fancydomain.eu>
>> ---
>>  env/mmc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/env/mmc.c b/env/mmc.c
>> index 7afb733e890..2bef30c973c 100644
>> --- a/env/mmc.c
>> +++ b/env/mmc.c
>> @@ -299,7 +299,7 @@ static int env_mmc_save(void)
>>  	ret = 0;
>>  
>>  	if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
>> -		gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
>> +		gd->env_valid = gd->env_valid == ENV_VALID ? ENV_REDUND : ENV_VALID;
>>  
>>  fini:
>>  	fini_mmc_for_env(mmc);
>
>Can you please explain a little more on how you get to this problem? The
>same code / test exists in env/fat.c, env/sf.c and env/ubi.c. In the
>case of env/mmc.c it's been that way since introduction in:
>commit d196bd880347373237d73e0d115b4d51c68cf2ad
>Author: Michael Heimpold <mhei at heimpold.de>
>Date:   Wed Apr 10 10:36:19 2013 +0000
>
>    env_mmc: add support for redundant environment
>    
>    This patch add support for storing the environment redundant on
>    mmc devices. Substantially it re-uses the logic from the NAND implementation,
>    that means using an incremental counter for marking newer data.
>    
>    Signed-off-by: Michael Heimpold <mhei at heimpold.de>
>
>as well. Thanks!
>
>-- 
>Tom