[U-Boot] [PATCH 32/39] env: Rename the redundancy flags

Tom Rini trini at konsulko.com
Thu Aug 1 23:04:10 UTC 2019


On Thu, Aug 01, 2019 at 12:55:18AM +0000, Joe Hershberger wrote:
> Hi Tom,
> 
> On Wed, Jul 31, 2019 at 7:53 PM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Wed, Jul 31, 2019 at 09:20:58PM +0000, Joe Hershberger wrote:
> > > On Wed, Jul 31, 2019 at 4:00 PM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Joe,
> > > >
> > > > On Tue, 30 Jul 2019 at 15:42, Joe Hershberger <joe.hershberger at ni.com> wrote:
> > > > >
> > > > > On Sun, Jul 28, 2019 at 9:27 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > >
> > > > > > Add an ENV prefix to these two flags so that it is clear what they relate
> > > > > > to. Also move them to env.h since they are part of the public API. Use an
> > > > > > enum rather than a #define to tie them together.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > ---
> > > > > >
> > > > > >  cmd/nvedit.c          |  2 +-
> > > > > >  env/eeprom.c          | 10 ++++++----
> > > > > >  env/flash.c           | 18 ++++++++++--------
> > > > > >  env/sf.c              |  6 ++----
> > > > > >  include/env.h         |  6 ++++++
> > > > > >  include/environment.h |  5 +----
> > > > > >  tools/env/fw_env.c    | 23 +++++++++++++----------
> > > > > >  7 files changed, 39 insertions(+), 31 deletions(-)
> > > > > >
> > > > > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> > > > > > index 7908d6cf0c..d6a86abb03 100644
> > > > > > --- a/cmd/nvedit.c
> > > > > > +++ b/cmd/nvedit.c
> > > > > > @@ -1014,7 +1014,7 @@ NXTARG:           ;
> > > > > >                 envp->crc = crc32(0, envp->data,
> > > > > >                                 size ? size - offsetof(env_t, data) : ENV_SIZE);
> > > > > >  #ifdef CONFIG_ENV_ADDR_REDUND
> > > > > > -               envp->flags = ACTIVE_FLAG;
> > > > > > +               envp->flags = ENVF_REDUND_ACTIVE;
> > > > > >  #endif
> > > > > >         }
> > > > > >         env_set_hex("filesize", len + offsetof(env_t, data));
> > > > > > diff --git a/env/eeprom.c b/env/eeprom.c
> > > > > > index 8d82cf892c..0c30ca459c 100644
> > > > > > --- a/env/eeprom.c
> > > > > > +++ b/env/eeprom.c
> > > > > > @@ -132,9 +132,11 @@ static int env_eeprom_load(void)
> > > > > >                 gd->env_valid = ENV_REDUND;
> > > > > >         } else {
> > > > > >                 /* both ok - check serial */
> > > > > > -               if (flags[0] == ACTIVE_FLAG && flags[1] == OBSOLETE_FLAG)
> > > > > > +               if (flags[0] == ENVF_REDUND_ACTIVE &&
> > > > > > +                   flags[1] == ENVF_REDUND_OBSOLETE)
> > > > > >                         gd->env_valid = ENV_VALID;
> > > > > > -               else if (flags[0] == OBSOLETE_FLAG && flags[1] == ACTIVE_FLAG)
> > > > > > +               else if (flags[0] == ENVF_REDUND_OBSOLETE &&
> > > > > > +                        flags[1] == ENVF_REDUND_ACTIVE)
> > > > > >                         gd->env_valid = ENV_REDUND;
> > > > > >                 else if (flags[0] == 0xFF && flags[1] == 0)
> > > > > >                         gd->env_valid = ENV_REDUND;
> > > > > > @@ -194,7 +196,7 @@ static int env_eeprom_save(void)
> > > > > >         unsigned int off        = CONFIG_ENV_OFFSET;
> > > > > >  #ifdef CONFIG_ENV_OFFSET_REDUND
> > > > > >         unsigned int off_red    = CONFIG_ENV_OFFSET_REDUND;
> > > > > > -       char flag_obsolete      = OBSOLETE_FLAG;
> > > > > > +       char flag_obsolete      = ENVF_REDUND_OBSOLETE;
> > > > > >  #endif
> > > > > >
> > > > > >         rc = env_export(&env_new);
> > > > > > @@ -207,7 +209,7 @@ static int env_eeprom_save(void)
> > > > > >                 off_red = CONFIG_ENV_OFFSET;
> > > > > >         }
> > > > > >
> > > > > > -       env_new.flags = ACTIVE_FLAG;
> > > > > > +       env_new.flags = ENVF_REDUND_ACTIVE;
> > > > > >  #endif
> > > > > >
> > > > > >         rc = eeprom_bus_write(CONFIG_SYS_DEF_EEPROM_ADDR,
> > > > > > diff --git a/env/flash.c b/env/flash.c
> > > > > > index 7a73466cf2..9566dd7f05 100644
> > > > > > --- a/env/flash.c
> > > > > > +++ b/env/flash.c
> > > > > > @@ -95,10 +95,12 @@ static int env_flash_init(void)
> > > > > >         } else if (!crc1_ok && !crc2_ok) {
> > > > > >                 gd->env_addr    = addr_default;
> > > > > >                 gd->env_valid   = ENV_INVALID;
> > > > > > -       } else if (flag1 == ACTIVE_FLAG && flag2 == OBSOLETE_FLAG) {
> > > > > > +       } else if (flag1 == ENVF_REDUND_ACTIVE &&
> > > > > > +                  flag2 == ENVF_REDUND_OBSOLETE) {
> > > > > >                 gd->env_addr    = addr1;
> > > > > >                 gd->env_valid   = ENV_VALID;
> > > > > > -       } else if (flag1 == OBSOLETE_FLAG && flag2 == ACTIVE_FLAG) {
> > > > > > +       } else if (flag1 == ENVF_REDUND_OBSOLETE &&
> > > > > > +                  flag2 == ENVF_REDUND_ACTIVE) {
> > > > > >                 gd->env_addr    = addr2;
> > > > > >                 gd->env_valid   = ENV_VALID;
> > > > > >         } else if (flag1 == flag2) {
> > > > > > @@ -121,7 +123,7 @@ static int env_flash_save(void)
> > > > > >  {
> > > > > >         env_t   env_new;
> > > > > >         char    *saved_data = NULL;
> > > > > > -       char    flag = OBSOLETE_FLAG, new_flag = ACTIVE_FLAG;
> > > > > > +       char    flag = ENVF_REDUND_OBSOLETE, new_flag = ENVF_REDUND_ACTIVE;
> > > > > >         int     rc = 1;
> > > > > >  #if CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE
> > > > > >         ulong   up_data = 0;
> > > > > > @@ -322,9 +324,9 @@ static int env_flash_load(void)
> > > > > >                 end_addr_new = ltmp;
> > > > > >         }
> > > > > >
> > > > > > -       if (flash_addr_new->flags != OBSOLETE_FLAG &&
> > > > > > +       if (flash_addr_new->flags != ENVF_REDUND_OBSOLETE &&
> > > > > >             crc32(0, flash_addr_new->data, ENV_SIZE) == flash_addr_new->crc) {
> > > > > > -               char flag = OBSOLETE_FLAG;
> > > > > > +               char flag = ENVF_REDUND_OBSOLETE;
> > > > > >
> > > > > >                 gd->env_valid = ENV_REDUND;
> > > > > >                 flash_sect_protect(0, (ulong)flash_addr_new, end_addr_new);
> > > > > > @@ -334,9 +336,9 @@ static int env_flash_load(void)
> > > > > >                 flash_sect_protect(1, (ulong)flash_addr_new, end_addr_new);
> > > > > >         }
> > > > > >
> > > > > > -       if (flash_addr->flags != ACTIVE_FLAG &&
> > > > > > -           (flash_addr->flags & ACTIVE_FLAG) == ACTIVE_FLAG) {
> > > > > > -               char flag = ACTIVE_FLAG;
> > > > > > +       if (flash_addr->flags != ENVF_REDUND_ACTIVE &&
> > > > > > +           (flash_addr->flags & ENVF_REDUND_ACTIVE) == ENVF_REDUND_ACTIVE) {
> > > > > > +               char flag = ENVF_REDUND_ACTIVE;
> > > > > >
> > > > > >                 gd->env_valid = ENV_REDUND;
> > > > > >                 flash_sect_protect(0, (ulong)flash_addr, end_addr);
> > > > > > diff --git a/env/sf.c b/env/sf.c
> > > > > > index 5531293e05..ae4136926b 100644
> > > > > > --- a/env/sf.c
> > > > > > +++ b/env/sf.c
> > > > > > @@ -30,8 +30,6 @@ static ulong env_offset               = CONFIG_ENV_OFFSET;
> > > > > >  static ulong env_new_offset    = CONFIG_ENV_OFFSET_REDUND;
> > > > > >  #endif
> > > > > >
> > > > > > -#define ACTIVE_FLAG    1
> > > > > > -#define OBSOLETE_FLAG  0
> > > > > >  #endif /* CONFIG_ENV_OFFSET_REDUND */
> > > > > >
> > > > > >  DECLARE_GLOBAL_DATA_PTR;
> > > > > > @@ -74,7 +72,7 @@ static int setup_flash_device(void)
> > > > > >  static int env_sf_save(void)
> > > > > >  {
> > > > > >         env_t   env_new;
> > > > > > -       char    *saved_buffer = NULL, flag = OBSOLETE_FLAG;
> > > > > > +       char    *saved_buffer = NULL, flag = ENVF_REDUND_OBSOLETE;
> > > > > >         u32     saved_size, saved_offset, sector;
> > > > > >         int     ret;
> > > > > >
> > > > > > @@ -85,7 +83,7 @@ static int env_sf_save(void)
> > > > > >         ret = env_export(&env_new);
> > > > > >         if (ret)
> > > > > >                 return -EIO;
> > > > > > -       env_new.flags   = ACTIVE_FLAG;
> > > > > > +       env_new.flags   = ENVF_REDUND_ACTIVE;
> > > > > >
> > > > > >         if (gd->env_valid == ENV_VALID) {
> > > > > >                 env_new_offset = CONFIG_ENV_OFFSET_REDUND;
> > > > > > diff --git a/include/env.h b/include/env.h
> > > > > > index 3dbdf276cd..884b036d02 100644
> > > > > > --- a/include/env.h
> > > > > > +++ b/include/env.h
> > > > > > @@ -55,6 +55,12 @@ struct env_clbk_tbl {
> > > > > >         {#name, callback}
> > > > > >  #endif
> > > > > >
> > > > > > +/** enum env_redund_flags - Flags for the redundand_environment */
> > > > >
> > > > > redundand_environment  -> redundant_environment
> > > > >
> > > > > Argh... that's actually the name of the function!?
> > > >
> > > > Yes
> > > >
> > > > >
> > > > > Now I see that there was a patch for this [1] , but is assigned to Tom
> > > > > and is deferred. What happened!?
> > > > >
> > > > > [1] - https://patchwork.ozlabs.org/patch/275224/
> > > >
> > > > Not a lot :-)
> > >
> > > Was there a problem with it or just forgotten about. Tom?
> > >
> > > It would be nice to recreate it in the modern codebase.
> >
> > Wasn't entirely sure I wanted to do the correction at the time and as
> > part of deferring patches that've been out for more than a year
> > (honestly, if something hasn't been picked up after a year, it probably
> > won't be) I marked it as deferred.
> 
> If it was recreated now are you open to taking it?

I suppose so, yeah.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190801/a50eb22f/attachment.sig>


More information about the U-Boot mailing list