| Message ID | 20240510113834.826783-1-jasper@fancydomain.eu |
|---|---|
| State | Needs Review / ACK |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | env: Invert gd->env_valid for env_mmc_save | expand |
On Fri, May 10, 2024 at 01:38:34PM +0200, jasper@fancydomain.eu wrote: > From: Jasper Orschulko <jasper@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@iris-sensing.com> > Signed-off-by: Jasper Orschulko <jasper@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@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@heimpold.de> as well. Thanks!
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@konsulko.com> wrote: >On Fri, May 10, 2024 at 01:38:34PM +0200, jasper@fancydomain.eu wrote: > >> From: Jasper Orschulko <jasper@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@iris-sensing.com> >> Signed-off-by: Jasper Orschulko <jasper@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@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@heimpold.de> > >as well. Thanks! > >-- >Tom
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);