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

Joe Hershberger joe.hershberger at ni.com
Tue Jul 30 21:49:45 UTC 2019


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!?

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/

> +enum env_redund_flags {
> +       ENVF_REDUND_OBSOLETE = 0,
> +       ENVF_REDUND_ACTIVE = 1,

Again, I think just ENV here would be nicer.


> +};
> +
>  /**
>   * env_get_id() - Gets a sequence number for the environment
>   *
> diff --git a/include/environment.h b/include/environment.h
> index c3e8d7840a..70ee0fdb19 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -135,9 +135,6 @@ extern unsigned long nand_env_oob_offset;
>
>  #ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
>  # define ENV_HEADER_SIZE       (sizeof(uint32_t) + 1)
> -
> -# define ACTIVE_FLAG   1
> -# define OBSOLETE_FLAG 0
>  #else
>  # define ENV_HEADER_SIZE       (sizeof(uint32_t))
>  #endif
> @@ -147,7 +144,7 @@ extern unsigned long nand_env_oob_offset;
>  typedef struct environment_s {
>         uint32_t        crc;            /* CRC32 over data bytes        */
>  #ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
> -       unsigned char   flags;          /* active/obsolete flags        */
> +       unsigned char   flags;          /* active/obsolete flags ENVF_REDUND_ */
>  #endif
>         unsigned char   data[ENV_SIZE]; /* Environment data             */
>  } env_t;
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index f06252d916..204f9d593e 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -119,9 +119,12 @@ static struct environment environment = {
>
>  static int have_redund_env;
>
> -static unsigned char active_flag = 1;
> -/* obsolete_flag must be 0 to efficiently set it on NOR flash without erasing */
> -static unsigned char obsolete_flag = 0;
> +static unsigned char ENVF_REDUND_ACTIVE = 1;
> +/*
> + * ENVF_REDUND_OBSOLETE must be 0 to efficiently set it on NOR flash without
> + * erasing
> + */
> +static unsigned char ENVF_REDUND_OBSOLETE;
>
>  #define DEFAULT_ENV_INSTANCE_STATIC
>  #include <env_default.h>
> @@ -1142,7 +1145,7 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset)
>
>         erase.start = DEVOFFSET(dev);
>         erase.length = DEVESIZE(dev);
> -       /* This relies on the fact, that obsolete_flag == 0 */
> +       /* This relies on the fact, that ENVF_REDUND_OBSOLETE == 0 */
>         rc = lseek(fd, offset, SEEK_SET);
>         if (rc < 0) {
>                 fprintf(stderr, "Cannot seek to set the flag on %s\n",
> @@ -1150,7 +1153,7 @@ static int flash_flag_obsolete(int dev, int fd, off_t offset)
>                 return rc;
>         }
>         ioctl(fd, MEMUNLOCK, &erase);
> -       rc = write(fd, &obsolete_flag, sizeof(obsolete_flag));
> +       rc = write(fd, &ENVF_REDUND_OBSOLETE, sizeof(ENVF_REDUND_OBSOLETE));
>         ioctl(fd, MEMLOCK, &erase);
>         if (rc < 0)
>                 perror("Could not set obsolete flag");
> @@ -1169,7 +1172,7 @@ static int flash_write(int fd_current, int fd_target, int dev_target)
>                 (*environment.flags)++;
>                 break;
>         case FLAG_BOOLEAN:
> -               *environment.flags = active_flag;
> +               *environment.flags = ENVF_REDUND_ACTIVE;
>                 break;
>         default:
>                 fprintf(stderr, "Unimplemented flash scheme %u\n",
> @@ -1508,11 +1511,11 @@ int fw_env_open(struct env_opts *opts)
>                 } else {
>                         switch (environment.flag_scheme) {
>                         case FLAG_BOOLEAN:
> -                               if (flag0 == active_flag &&
> -                                   flag1 == obsolete_flag) {
> +                               if (flag0 == ENVF_REDUND_ACTIVE &&
> +                                   flag1 == ENVF_REDUND_OBSOLETE) {
>                                         dev_current = 0;
> -                               } else if (flag0 == obsolete_flag &&
> -                                          flag1 == active_flag) {
> +                               } else if (flag0 == ENVF_REDUND_OBSOLETE &&
> +                                          flag1 == ENVF_REDUND_ACTIVE) {
>                                         dev_current = 1;
>                                 } else if (flag0 == flag1) {
>                                         dev_current = 0;
> --
> 2.22.0.709.g102302147b-goog
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot


More information about the U-Boot mailing list