Re: [U-Boot] [PATCH 1/2] env: Fix env_load_location

Hi,

Thanks for your patch

On Wed, Feb 07, 2018 at 02:17:11PM -0800, York Sun wrote:

Commit 7d714a24d725 ("env: Support multiple environments") added static variable env_load_location. When saving environmental variables, this variable is presumed to have the value set before. In case the value was set before relocation and U-Boot runs from a NOR flash, this variable wasn't writable. This causes failure when saving the environment. To save this location, global data must be used instead.

Signed-off-by: York Sun york.sun@nxp.com CC: Maxime Ripard maxime.ripard@free-electrons.com

---

Limited test on LS1043ARDB.

env/env.c | 8 +++----- include/asm-generic/global_data.h | 1 + include/environment.h | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/env/env.c b/env/env.c index 9a89832..edfb575 100644 --- a/env/env.c +++ b/env/env.c @@ -62,8 +62,6 @@ static enum env_location env_locations[] = { #endif };

-static enum env_location env_load_location = ENVL_UNKNOWN;

static bool env_has_inited(enum env_location location) { return gd->env_has_init & BIT(location); @@ -108,11 +106,11 @@ __weak enum env_location env_get_location(enum env_operation op, int prio) if (prio >= ARRAY_SIZE(env_locations)) return ENVL_UNKNOWN;

• env_load_location = env_locations[prio];
  

• return env_load_location;
  

• gd->env_load_location = env_locations[prio];
  

• return gd->env_load_location;
  
  

  case ENVOP_SAVE:

• return env_load_location;
  

• return gd->env_load_location;
  

  }

  return ENVL_UNKNOWN;

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index fd8cd45..10f1441 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -51,6 +51,7 @@ typedef struct global_data { unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Environment valid? enum env_valid */ unsigned long env_has_init; /* Bitmask of boolean of struct env_location offsets */

• int env_load_location;

  unsigned long ram_top; /* Top address of RAM used by U-Boot */ unsigned long relocaddr; /* Start address of U-Boot in RAM */

diff --git a/include/environment.h b/include/environment.h index a406050..0f339da 100644 --- a/include/environment.h +++ b/include/environment.h @@ -188,6 +188,7 @@ enum env_valid { };

enum env_location {

• ENVL_UNKNOWN, ENVL_EEPROM, ENVL_EXT4, ENVL_FAT,

@@ -202,7 +203,6 @@ enum env_location { ENVL_NOWHERE,

ENVL_COUNT,

• ENVL_UNKNOWN,

Why did you need to change this? This looks a bit odd.

Thanks! Maxime