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

[Maxime Ripard]

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 at nxp.com>
> CC: Maxime Ripard <maxime.ripard at 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

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180208/8f54ce1f/attachment.sig>