[09/14] env: Support multiple environments

Message ID 2b747a810c8103878d673660bbdc560431d19c73.1511864667.git-series.maxime.ripard@free-electrons.com
State Superseded
Headers show
Series
  • env: Multiple env support and env transition for sunxi
Related show

Commit Message

Maxime Ripard Nov. 28, 2017, 10:24 a.m.
Now that we have everything in place to support multiple environment, let's
make sure the current code can use it.

The priority used between the various environment is the same one that was
used in the code previously.

At read / init times, the highest priority environment is going to be
detected, and we'll use the same one without lookup during writes. This
should implement the same behaviour than we currently have.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 env/env.c | 75 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 50 insertions(+), 25 deletions(-)

Comments

André Przywara Dec. 5, 2017, 10:10 a.m. | #1
On 28/11/17 10:24, Maxime Ripard wrote:
> Now that we have everything in place to support multiple environment, let's
> make sure the current code can use it.
> 
> The priority used between the various environment is the same one that was
> used in the code previously.
> 
> At read / init times, the highest priority environment is going to be
> detected, and we'll use the same one without lookup during writes. This
> should implement the same behaviour than we currently have.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  env/env.c | 75 +++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/env/env.c b/env/env.c
> index 44f9908e2c2d..5176700133d3 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -26,33 +26,58 @@ static struct env_driver *_env_driver_lookup(enum env_location loc)
>  	return NULL;
>  }
>  
> +static enum env_location env_locations[] = {
> +#ifdef CONFIG_ENV_IS_IN_EEPROM
> +	ENVL_EEPROM,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_FAT
> +	ENVL_FAT,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_FLASH
> +	ENVL_FLASH,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_MMC
> +	ENVL_MMC,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_NAND
> +	ENVL_NAND,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_NVRAM
> +	ENVL_NVRAM,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_REMOTE
> +	ENVL_REMOTE,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
> +	ENVL_SPI_FLASH,
> +#endif
> +#ifdef CONFIG_ENV_IS_IN_UBI
> +	ENVL_UBI,
> +#endif
> +#ifdef CONFIG_ENV_IS_NOWHERE
> +	ENVL_NOWHERE,
> +#endif
> +	ENVL_UNKNOWN,
> +};
> +
> +static enum env_location env_load_location;
> +
>  static enum env_location env_get_location(enum env_operation op, int prio)
>  {
> -	if (prio >= 1)
> -		return ENVL_UNKNOWN;
> -
> -	if IS_ENABLED(CONFIG_ENV_IS_IN_EEPROM)
> -		return ENVL_EEPROM;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_FAT)
> -		return ENVL_FAT;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_FLASH)
> -		return ENVL_FLASH;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_MMC)
> -		return ENVL_MMC;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_NAND)
> -		return ENVL_NAND;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_NVRAM)
> -		return ENVL_NVRAM;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_REMOTE)
> -		return ENVL_REMOTE;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)
> -		return ENVL_SPI_FLASH;
> -	else if IS_ENABLED(CONFIG_ENV_IS_IN_UBI)
> -		return ENVL_UBI;
> -	else if IS_ENABLED(CONFIG_ENV_IS_NOWHERE)
> -		return ENVL_NOWHERE;
> -	else
> -		return ENVL_UNKNOWN;
> +	switch (op) {
> +	case ENVO_GET_CHAR:
> +	case ENVO_INIT:
> +	case ENVO_LOAD:
> +		if (prio >= ARRAY_SIZE(env_locations))
> +			return -ENODEV;
> +
> +		return env_load_location = env_locations[prio];

That looks a bit fishy. Can you please make this two lines?

Otherwise: Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> +
> +	case ENVO_SAVE:
> +		return env_load_location;
> +	}
> +
> +	return ENVL_UNKNOWN;
>  }
>  
>  static struct env_driver *env_driver_lookup(enum env_operation op, int prio)
>
Simon Glass Dec. 29, 2017, 3:13 a.m. | #2
On 28 November 2017 at 03:24, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Now that we have everything in place to support multiple environment, let's
> make sure the current code can use it.
>
> The priority used between the various environment is the same one that was
> used in the code previously.
>
> At read / init times, the highest priority environment is going to be
> detected, and we'll use the same one without lookup during writes. This
> should implement the same behaviour than we currently have.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  env/env.c | 75 +++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 50 insertions(+), 25 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Patch

diff --git a/env/env.c b/env/env.c
index 44f9908e2c2d..5176700133d3 100644
--- a/env/env.c
+++ b/env/env.c
@@ -26,33 +26,58 @@  static struct env_driver *_env_driver_lookup(enum env_location loc)
 	return NULL;
 }
 
+static enum env_location env_locations[] = {
+#ifdef CONFIG_ENV_IS_IN_EEPROM
+	ENVL_EEPROM,
+#endif
+#ifdef CONFIG_ENV_IS_IN_FAT
+	ENVL_FAT,
+#endif
+#ifdef CONFIG_ENV_IS_IN_FLASH
+	ENVL_FLASH,
+#endif
+#ifdef CONFIG_ENV_IS_IN_MMC
+	ENVL_MMC,
+#endif
+#ifdef CONFIG_ENV_IS_IN_NAND
+	ENVL_NAND,
+#endif
+#ifdef CONFIG_ENV_IS_IN_NVRAM
+	ENVL_NVRAM,
+#endif
+#ifdef CONFIG_ENV_IS_IN_REMOTE
+	ENVL_REMOTE,
+#endif
+#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
+	ENVL_SPI_FLASH,
+#endif
+#ifdef CONFIG_ENV_IS_IN_UBI
+	ENVL_UBI,
+#endif
+#ifdef CONFIG_ENV_IS_NOWHERE
+	ENVL_NOWHERE,
+#endif
+	ENVL_UNKNOWN,
+};
+
+static enum env_location env_load_location;
+
 static enum env_location env_get_location(enum env_operation op, int prio)
 {
-	if (prio >= 1)
-		return ENVL_UNKNOWN;
-
-	if IS_ENABLED(CONFIG_ENV_IS_IN_EEPROM)
-		return ENVL_EEPROM;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_FAT)
-		return ENVL_FAT;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_FLASH)
-		return ENVL_FLASH;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_MMC)
-		return ENVL_MMC;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_NAND)
-		return ENVL_NAND;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_NVRAM)
-		return ENVL_NVRAM;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_REMOTE)
-		return ENVL_REMOTE;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH)
-		return ENVL_SPI_FLASH;
-	else if IS_ENABLED(CONFIG_ENV_IS_IN_UBI)
-		return ENVL_UBI;
-	else if IS_ENABLED(CONFIG_ENV_IS_NOWHERE)
-		return ENVL_NOWHERE;
-	else
-		return ENVL_UNKNOWN;
+	switch (op) {
+	case ENVO_GET_CHAR:
+	case ENVO_INIT:
+	case ENVO_LOAD:
+		if (prio >= ARRAY_SIZE(env_locations))
+			return -ENODEV;
+
+		return env_load_location = env_locations[prio];
+
+	case ENVO_SAVE:
+		return env_load_location;
+	}
+
+	return ENVL_UNKNOWN;
 }
 
 static struct env_driver *env_driver_lookup(enum env_operation op, int prio)