env: Multiple env support and env transition for sunxi

The nvedit command is the only user of env_driver_lookup_default outside of the environment code itself, and it uses it only to print the environment it's about to save to during env save.

As we're about to rework the environment to be able to handle multiple environment sources, we might not have an idea of what environment backend is going to be used before trying (and possibly failing for some).

Therefore, it makes sense to remove that message and move it to the env_save function itself. As a side effect, we also can get rid of the call to env_driver_lookup default that is also about to get refactored.

Signed-off-by Maxime Ripard <[email protected]>

---
 cmd/nvedit.c | 4 ----
 env/env.c    | 2 ++
 2 files changed, 2 insertions(+), 4 deletions(-)

cmd/nvedit.c +0 -4

index 4e79d03856fe..a690d743cd46 100644
@@ -708,10 +708,6 @@ ulong env_get_ulong(const char *name, int base,
 ulong default_val) static int do_env_save(cmd_tbl_t *cmdtp, int flag,
 int argc, char * const argv[])
 {
-	struct env_driver *env = env_driver_lookup_default();
-
-	printf("Saving Environment to %s...\n", env->name);
-
 	return env_save() ? 1 : 0;
 }
 

env/env.c +2 -0

index 76a5608628fc..84c12e27bc3f 100644
@@ -115,6 +115,8 @@ int env_save(void)
 		return -ENODEV;
 	if (!drv->save)
 		return -ENOSYS;
+
+	printf("Saving Environment to %s...\n", drv->name);
 	ret = drv->save();
 	if (ret) {
 		debug("%s: Environment failed to save (err=%d)\n",

__func__,

No that there's no users of env_driver_lookup_default outside of env/env.c, we can mark that function static and remove it from the environment header.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c             | 2 +-
 include/environment.h | 7 -------
 2 files changed, 1 insertion(+), 8 deletions(-)

env/env.c +1 -1

index 84c12e27bc3f..094538ff5b62 100644
@@ -52,7 +52,7 @@ static enum env_location
 env_get_default_location(void) return ENVL_UNKNOWN;
 }
 
-struct env_driver *env_driver_lookup_default(void)
+static struct env_driver *env_driver_lookup_default(void)
 {
 	enum env_location loc = env_get_default_location();
 	struct env_driver *drv;

include/environment.h +0 -2

index 7b9821638960..226e3ef2d23a 100644
@@ -302,13 +302,6 @@ int env_export(env_t *env_out);
 int env_import_redund(const char *buf1, const char *buf2);
 #endif
 
-/**
- * env_driver_lookup_default() - Look up the default environment

driver
- *
- * @return pointer to driver, or NULL if none (which should not happen)
- */

No that there's no users of env_driver_lookup_default outside of env/env.c, we can mark that function static and remove it from the environment header.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c             | 2 +-
 include/environment.h | 7 -------
 2 files changed, 1 insertion(+), 8 deletions(-)

env/env.c +1 -1

index 84c12e27bc3f..094538ff5b62 100644
@@ -52,7 +52,7 @@ static enum env_location env_get_default_location(void)
 		return ENVL_UNKNOWN;
 }
 
-struct env_driver *env_driver_lookup_default(void)
+static struct env_driver *env_driver_lookup_default(void)
 {
 	enum env_location loc = env_get_default_location();
 	struct env_driver *drv;

include/environment.h +0 -7

index 7b9821638960..226e3ef2d23a 100644
@@ -302,13 +302,6 @@ int env_export(env_t *env_out);
 int env_import_redund(const char *buf1, const char *buf2);
 #endif
 
-/**
- * env_driver_lookup_default() - Look up the default environment driver
- *
- * @return pointer to driver, or NULL if none (which should not happen)
- */
-struct env_driver *env_driver_lookup_default(void);
-
 /**
  * env_get_char() - Get a character from the early environment
  *

The env_driver_lookup_default and env_get_default_location functions are about to get refactored to support loading from multiple environment.

The name is therefore not really well suited anymore. Drop the default part to be a bit more relevant.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

env/env.c +1 -1

index 094538ff5b62..97ada5b5a6fd 100644
@@ -10,7 +10,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static struct env_driver *env_driver_lookup(enum env_location loc)
+static struct env_driver *_env_driver_lookup(enum env_location loc)
 {
 	struct env_driver *drv;
 	const int n_ents = ll_entry_count(struct env_driver,

env_driver); @@ -26,7 +26,7 @@ static struct env_driver *env_driver_lookup(enum env_location loc) return NULL;
}

env/env.c +7 -7

-static enum env_location env_get_default_location(void)
+static enum env_location env_get_location(void)
 {
 	if IS_ENABLED(CONFIG_ENV_IS_IN_EEPROM)
 		return ENVL_EEPROM;

@@ -52,12 +52,12 @@ static enum env_location
 env_get_default_location(void) return ENVL_UNKNOWN;
 }
 
-static struct env_driver *env_driver_lookup_default(void)
+static struct env_driver *env_driver_lookup(void)
 {
-	enum env_location loc = env_get_default_location();
+	enum env_location loc = env_get_location();
 	struct env_driver *drv;
 
-	drv = env_driver_lookup(loc);
+	drv = _env_driver_lookup(loc);
 	if (!drv) {
 		debug("%s: No environment driver for location %d\n",
 __func__, loc);

@@ -69,7 +69,7 @@ static struct env_driver
 *env_driver_lookup_default(void) 
 int env_get_char(int index)
 {
-	struct env_driver *drv = env_driver_lookup_default();
+	struct env_driver *drv = env_driver_lookup();
 	int ret;
 
 	if (gd->env_valid == ENV_INVALID)

@@ -89,7 +89,7 @@ int env_get_char(int index)
 
 int env_load(void)
 {
-	struct env_driver *drv = env_driver_lookup_default();
+	struct env_driver *drv = env_driver_lookup();
 	int ret = 0;
 
 	if (!drv)

@@ -108,7 +108,7 @@ int env_load(void)
 
 int env_save(void)
 {
-	struct env_driver *drv = env_driver_lookup_default();
+	struct env_driver *drv = env_driver_lookup();
 	int ret;
 
 	if (!drv)

In preparation for the multiple environment support, let's introduce two new parameters to the environment driver lookup function: the priority and operation.

The operation parameter is meant to identify, obviously, the operation you might want to perform on the environment.

The priority is a number passed to identify the environment priority you want to retrieve. The lowest priority parameter (0) will be the primary source.

Combining the two parameters allow you to support multiple environments through different priorities, and to change those priorities between read and writes operations.

This is especially useful to implement migration mechanisms where you want to always use the same environment first, be it to read or write, while the common case is more likely to use the same environment it has read from to write it to.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c             | 122

++++++++++++++++++++++++++++++--------------------
include/environment.h | 7 +++ 2 files changed, 80 insertions(+), 49
deletions(-)

env/env.c +0 -0

index 97ada5b5a6fd..673bfa6ba41b 100644

env/env.c +6 -2

+
 	if IS_ENABLED(CONFIG_ENV_IS_IN_EEPROM)
 		return ENVL_EEPROM;
 	else if IS_ENABLED(CONFIG_ENV_IS_IN_FAT)

@@ -52,11 +55,14 @@ static enum env_location env_get_location(void)
 		return ENVL_UNKNOWN;
 }
 
-static struct env_driver *env_driver_lookup(void)
+static struct env_driver *env_driver_lookup(enum env_operation op,
 int prio) {
-	enum env_location loc = env_get_location();
+	enum env_location loc = env_get_location(op, prio);
 	struct env_driver *drv;
 
+	if (loc == ENVL_UNKNOWN)
+		return NULL;
+
 	drv = _env_driver_lookup(loc);
 	if (!drv) {
 		debug("%s: No environment driver for location %d\n",

__func__, @@ -69,83 +75,101 @@ static struct env_driver *env_driver_lookup(void)
int env_get_char(int index)
{

env/env.c +4 -10

-	struct env_driver *drv = env_driver_lookup();
-	int ret;
+	struct env_driver *drv;
+	int prio;
 
 	if (gd->env_valid == ENV_INVALID)
 		return default_environment[index];
-	if (!drv)
-		return -ENODEV;
-	if (!drv->get_char)
-		return *(uchar *)(gd->env_addr + index);
-	ret = drv->get_char(index);
-	if (ret < 0) {
-		debug("%s: Environment failed to load (err=%d)\n",
-		      __func__, ret);
+
+	for (prio = 0; (drv = env_driver_lookup(ENVO_GET_CHAR,

prio)); prio++) {

env/env.c +10 -0

+		int ret;
+
+		if (!drv->get_char)
+			continue;
+
+		ret = drv->get_char(index);
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to load

(err=%d)\n", __func__,

env/env.c +4 -10

+		      drv->name, ret);
 	}
 
-	return ret;
+	return -ENODEV;
 }
 
 int env_load(void)
 {
-	struct env_driver *drv = env_driver_lookup();
-	int ret = 0;
+	struct env_driver *drv;
+	int prio;
 
-	if (!drv)
-		return -ENODEV;
-	if (!drv->load)
-		return 0;
-	ret = drv->load();
-	if (ret) {
-		debug("%s: Environment failed to load (err=%d)\n",

__func__,

env/env.c +1 -2

-		      ret);
-		return ret;
+	for (prio = 0; (drv = env_driver_lookup(ENVO_LOAD, prio));

prio++) {

env/env.c +10 -0

+		int ret;
+
+		if (!drv->load)
+			continue;
+
+		ret = drv->load();
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to load

(err=%d)\n", __func__,

env/env.c +4 -12

+		      drv->name, ret);
 	}
 
-	return 0;
+	return -ENODEV;
 }
 
 int env_save(void)
 {
-	struct env_driver *drv = env_driver_lookup();
-	int ret;
+	struct env_driver *drv;
+	int prio;
 
-	if (!drv)
-		return -ENODEV;
-	if (!drv->save)
-		return -ENOSYS;
-
-	printf("Saving Environment to %s...\n", drv->name);
-	ret = drv->save();
-	if (ret) {
-		debug("%s: Environment failed to save (err=%d)\n",

__func__,

env/env.c +1 -2

-		      ret);
-		return ret;
+	for (prio = 0; (drv = env_driver_lookup(ENVO_SAVE, prio));

prio++) {

env/env.c +11 -0

+		int ret;
+
+		if (!drv->save)
+			continue;
+
+		printf("Saving Environment to %s...\n", drv->name);
+		ret = drv->save();
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to save

(err=%d)\n", __func__,

env/env.c +6 -2

+		      drv->name, ret);
 	}
 
-	return 0;
+	return -ENODEV;
 }
 
 int env_init(void)
 {
-	struct env_driver *drv = env_driver_lookup();
+	struct env_driver *drv;
 	int ret = -ENOENT;
+	int prio;
+
+	for (prio = 0; (drv = env_driver_lookup(ENVO_INIT, prio));

prio++) {

env/env.c +6 -3

+		if (!drv->init)
+			continue;
 
-	if (!drv)
-		return -ENODEV;
-	if (drv->init)
 		ret = drv->init();
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to init

(err=%d)\n", __func__,

env/env.c +6 -2

+		      drv->name, ret);
+	}
+
+	if (!prio)
+		return -ENODEV;
+
 	if (ret == -ENOENT) {
 		gd->env_addr = (ulong)&default_environment[0];
 		gd->env_valid = ENV_VALID;
 
 		return 0;
-	} else if (ret) {
-		debug("%s: Environment failed to init (err=%d)\n",

__func__,

env/env.c +1 -3

-		      ret);
-		return ret;
 	}
 
-	return 0;
+	return ret;
 }

include/environment.h +0 -0

index 226e3ef2d23a..7fa8b98cc0db 100644
@@ -215,6 +215,13 @@ enum env_location {
 	ENVL_UNKNOWN,
 };
 

In preparation for the multiple environment support, let's introduce two new parameters to the environment driver lookup function: the priority and operation.

The operation parameter is meant to identify, obviously, the operation you might want to perform on the environment.

The priority is a number passed to identify the environment priority you want to retrieve. The lowest priority parameter (0) will be the primary source.

Combining the two parameters allow you to support multiple environments through different priorities, and to change those priorities between read and writes operations.

This is especially useful to implement migration mechanisms where you want to always use the same environment first, be it to read or write, while the common case is more likely to use the same environment it has read from to write it to.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c             | 122 ++++++++++++++++++++++++++++++--------------------
 include/environment.h |   7 +++
 2 files changed, 80 insertions(+), 49 deletions(-)

env/env.c +60 -41

index 97ada5b5a6fd..673bfa6ba41b 100644
@@ -26,8 +26,11 @@ static struct env_driver *_env_driver_lookup(enum env_location loc)
 	return NULL;
 }
 
-static enum env_location env_get_location(void)
+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)

@@ -52,11 +55,14 @@ static enum env_location env_get_location(void)
 		return ENVL_UNKNOWN;
 }
 
-static struct env_driver *env_driver_lookup(void)
+static struct env_driver *env_driver_lookup(enum env_operation op, int prio)
 {
-	enum env_location loc = env_get_location();
+	enum env_location loc = env_get_location(op, prio);
 	struct env_driver *drv;
 
+	if (loc == ENVL_UNKNOWN)
+		return NULL;
+
 	drv = _env_driver_lookup(loc);
 	if (!drv) {
 		debug("%s: No environment driver for location %d\n", __func__,

@@ -69,83 +75,101 @@ static struct env_driver *env_driver_lookup(void)
 
 int env_get_char(int index)
 {
-	struct env_driver *drv = env_driver_lookup();
-	int ret;
+	struct env_driver *drv;
+	int prio;
 
 	if (gd->env_valid == ENV_INVALID)
 		return default_environment[index];
-	if (!drv)
-		return -ENODEV;
-	if (!drv->get_char)
-		return *(uchar *)(gd->env_addr + index);
-	ret = drv->get_char(index);
-	if (ret < 0) {
-		debug("%s: Environment failed to load (err=%d)\n",
-		      __func__, ret);
+
+	for (prio = 0; (drv = env_driver_lookup(ENVO_GET_CHAR, prio)); prio++) {
+		int ret;
+
+		if (!drv->get_char)
+			continue;
+
+		ret = drv->get_char(index);
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to load (err=%d)\n", __func__,
+		      drv->name, ret);
 	}
 
-	return ret;
+	return -ENODEV;
 }
 
 int env_load(void)
 {
-	struct env_driver *drv = env_driver_lookup();
-	int ret = 0;
+	struct env_driver *drv;
+	int prio;
 
-	if (!drv)
-		return -ENODEV;
-	if (!drv->load)
-		return 0;
-	ret = drv->load();
-	if (ret) {
-		debug("%s: Environment failed to load (err=%d)\n", __func__,
-		      ret);
-		return ret;
+	for (prio = 0; (drv = env_driver_lookup(ENVO_LOAD, prio)); prio++) {
+		int ret;
+
+		if (!drv->load)
+			continue;
+
+		ret = drv->load();
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to load (err=%d)\n", __func__,
+		      drv->name, ret);
 	}
 
-	return 0;
+	return -ENODEV;
 }
 
 int env_save(void)
 {
-	struct env_driver *drv = env_driver_lookup();
-	int ret;
+	struct env_driver *drv;
+	int prio;
 
-	if (!drv)
-		return -ENODEV;
-	if (!drv->save)
-		return -ENOSYS;
-
-	printf("Saving Environment to %s...\n", drv->name);
-	ret = drv->save();
-	if (ret) {
-		debug("%s: Environment failed to save (err=%d)\n", __func__,
-		      ret);
-		return ret;
+	for (prio = 0; (drv = env_driver_lookup(ENVO_SAVE, prio)); prio++) {
+		int ret;
+
+		if (!drv->save)
+			continue;
+
+		printf("Saving Environment to %s...\n", drv->name);
+		ret = drv->save();
+		if (!ret)
+			return 0;
+
+		debug("%s: Environment %s failed to save (err=%d)\n", __func__,
+		      drv->name, ret);
 	}
 
-	return 0;
+	return -ENODEV;
 }
 
 int env_init(void)
 {
-	struct env_driver *drv = env_driver_lookup();
+	struct env_driver *drv;
 	int ret = -ENOENT;
+	int prio;
+
+	for (prio = 0; (drv = env_driver_lookup(ENVO_INIT, prio)); prio++) {

Since we'll soon have support for multiple environments, the environment saving message might end up being printed multiple times if the higher priority environment cannot be used.

That might confuse the user, so let's make it explicit if the operation failed or not.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

env/env.c +0 -0

index 673bfa6ba41b..1d13220aa79b 100644

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 <[email protected]>

---
 env/env.c | 75

++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 25 deletions(-)

env/env.c +50 -25

index 1d13220aa79b..6af9f897b0ae 100644
@@ -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)

Hi Maxime,

Quoted 103 lines

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 <[email protected]>

---
 env/env.c | 75

++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 25 deletions(-)

env/env.c +50 -25

index 1d13220aa79b..6af9f897b0ae 100644
@@ -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;
 }
 

Quoted 2 lines

env/env.c +0 -0

 static struct env_driver *env_driver_lookup(enum env_operation op,

int prio)

I'm just wondering...

Do you think that it may be helpful to have a way to force particular environment to be used (despite of the priority)?

Quoted 111 lines · Lukasz Majewski

Hi Maxime,

Quoted 105 lines

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 <[email protected]>

---
 env/env.c | 75

++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 25 deletions(-)

env/env.c +50 -25

index 1d13220aa79b..6af9f897b0ae 100644
@@ -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

Quoted 1 line

op, int prio)

I'm just wondering...

Quoted 2 lines

Do you think that it may be helpful to have a way to force particular environment to be used (despite of the priority)?

That's one of the hard parts of this, yes. I think I had suggested before that we handle that in a follow up series?

Quoted 115 lines · On Fri, 17 Nov 2017 08:41:05 -0500 Tom Rini <[email protected]> wrote:

Quoted 113 lines · Lukasz Majewski

Hi Maxime,

Quoted 105 lines

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 <[email protected]>

---
 env/env.c | 75

++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 25 deletions(-)

env/env.c +50 -25

index 1d13220aa79b..6af9f897b0ae 100644
@@ -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

Quoted 1 line

op, int prio)

I'm just wondering...

Do you think that it may be helpful to have a way to force particular environment to be used (despite of the priority)?

Quoted 3 lines

That's one of the hard parts of this, yes. I think I had suggested before that we handle that in a follow up series?

I don't want to say that this must be done now. I just can imagine that we may need such feature in some point.

Hi Lukasz, Tom,

Quoted 118 lines · Lukasz Majewski

Quoted 115 lines · On Fri, 17 Nov 2017 08:41:05 -0500 Tom Rini <[email protected]> wrote:

Quoted 113 lines · Lukasz Majewski

Hi Maxime,

Quoted 105 lines

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
<[email protected]> ---

 env/env.c | 75
++++++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 50 insertions(+), 25 deletions(-)

env/env.c +44 -25

index 1d13220aa79b..6af9f897b0ae 100644
@@ -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]; +

env/env.c +5 -0

+	case ENVO_SAVE:
+		return env_load_location;
+	}
+
+	return ENVL_UNKNOWN;
 }
 
 static struct env_driver *env_driver_lookup(enum

Quoted 1 line

env_operation op, int prio)

I'm just wondering...

Do you think that it may be helpful to have a way to force particular environment to be used (despite of the priority)?

Quoted 6 lines

Quoted 3 lines

That's one of the hard parts of this, yes. I think I had suggested before that we handle that in a follow up series?

I don't want to say that this must be done now. I just can imagine that we may need such feature in some point.

So I guess this question is also part of the comment Lukasz had on the other patch.

The current implementation is effectively hardcoding the environment by doing two things:
- Rejecting any priority other than 0, which is the highest. It basically says to the code that there is no further alternative and that its loop should stop there.
- Returning the first environment in the array.

If one wanted to hardcode the environment to something else than the default behaviour, you can always implement env_get_location in your board by doing something like:

enum env_location env_get_location (enum env_operation op, int prio)

{
	if (prio)
		return ENVL_UNKNOWN;

	return ENVL_MMC; /* Or whatever you want */
}

Like I was saying, the default implementation already kind of does that. Or maybe we want the default one to not hardcode it and just deal with alternatives?

Thanks!
Maxime

Hi Maxime,

Quoted 142 lines

Hi Lukasz, Tom,

Quoted 118 lines · Lukasz Majewski

Quoted 115 lines · On Fri, 17 Nov 2017 08:41:05 -0500 Tom Rini <[email protected]> wrote:

Quoted 113 lines · Lukasz Majewski

Hi Maxime,

Quoted 105 lines

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
<[email protected]> ---

 env/env.c | 75
++++++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 50 insertions(+), 25 deletions(-)

env/env.c +44 -25

index 1d13220aa79b..6af9f897b0ae 100644
@@ -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]; +

env/env.c +5 -0

+	case ENVO_SAVE:
+		return env_load_location;
+	}
+
+	return ENVL_UNKNOWN;
 }
 
 static struct env_driver *env_driver_lookup(enum

Quoted 1 line

env_operation op, int prio)

I'm just wondering...

Do you think that it may be helpful to have a way to force particular environment to be used (despite of the priority)?

Quoted 6 lines

Quoted 3 lines

That's one of the hard parts of this, yes. I think I had suggested before that we handle that in a follow up series?

I don't want to say that this must be done now. I just can imagine that we may need such feature in some point.

So I guess this question is also part of the comment Lukasz had on the other patch.

The current implementation is effectively hardcoding the environment by doing two things:
- Rejecting any priority other than 0, which is the highest. It basically says to the code that there is no further alternative and that its loop should stop there.
- Returning the first environment in the array.

If one wanted to hardcode the environment to something else than the default behaviour, you can always implement env_get_location in your board by doing something like:

Quoted 7 lines

enum env_location env_get_location (enum env_operation op, int prio)

{
	if (prio)
		return ENVL_UNKNOWN;

	return ENVL_MMC; /* Or whatever you want */
}

The env_get_location defined per board would work for me.

Quoted 1 line

Quoted 3 lines

Like I was saying, the default implementation already kind of does that. Or maybe we want the default one to not hardcode it and just deal with alternatives?

I think that having the code hardcoded (as it is now) is acceptable for making the feature way to main line. I suppose that
you would add the priorities handling in further patches?

Now that we have everything in place in the code, let's allow to build multiple environments backend through Kconfig.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/Kconfig | 65

++++++++++++++++++++++++++++++------------------------------- 1 file changed, 32 insertions(+), 33 deletions(-)

env/Kconfig +0 -10

index 8c9d800f485f..bf6eab6b4ace 100644
@@ -1,38 +1,18 @@
 menu "Environment"
 
-choice
-	prompt "Select the location of the environment"
-	default ENV_IS_IN_MMC if ARCH_SUNXI
-	default ENV_IS_IN_MMC if ARCH_EXYNOS4
-	default ENV_IS_IN_MMC if MX6SX || MX7D
-	default ENV_IS_IN_MMC if TEGRA30 || TEGRA124
-	default ENV_IS_IN_MMC if TEGRA_ARMV8_COMMON
-	default ENV_IS_IN_FLASH if ARCH_CINTEGRATOR
-	default ENV_IS_IN_FLASH if ARCH_INTEGRATOR_CP
-	default ENV_IS_IN_FLASH if M548x || M547x || M5282 ||

MCF547x_8x

env/Kconfig +0 -3

-	default ENV_IS_IN_FLASH if MCF532x || MCF52x2
-	default ENV_IS_IN_FLASH if MPC86xx || MPC83xx
-	default ENV_IS_IN_FLASH if ARCH_MPC8572 || ARCH_MPC8548 ||

ARCH_MPC8641

env/Kconfig +0 -13

-	default ENV_IS_IN_FLASH if SH && !CPU_SH4
-	default ENV_IS_IN_SPI_FLASH if ARMADA_XP
-	default ENV_IS_IN_SPI_FLASH if INTEL_BAYTRAIL
-	default ENV_IS_IN_SPI_FLASH if INTEL_BRASWELL
-	default ENV_IS_IN_SPI_FLASH if INTEL_BROADWELL
-	default ENV_IS_IN_SPI_FLASH if NORTHBRIDGE_INTEL_IVYBRIDGE
-	default ENV_IS_IN_SPI_FLASH if INTEL_QUARK
-	default ENV_IS_IN_SPI_FLASH if INTEL_QUEENSBAY
-	default ENV_IS_IN_FAT if ARCH_BCM283X
-	default ENV_IS_IN_FAT if MMC_OMAP_HS && TI_COMMON_CMD_OPTIONS
-	default ENV_IS_NOWHERE
-	help
-	  At present the environment can be stored in only one

place. Use this

env/Kconfig +0 -1

-	  option to select the location. This is either a device

(where the

env/Kconfig +0 -1

-	  environemnt information is simply written to a fixed

location or

env/Kconfig +0 -1

-	  partition on the device) or a filesystem (where the

environment

env/Kconfig +11 -2

-	  information is written to a file).
-
 config ENV_IS_NOWHERE
 	bool "Environment is not stored"
+	depends on !ENV_IS_IN_EEPROM
+	depends on !ENV_IS_IN_FAT
+	depends on !ENV_IS_IN_FLASH
+	depends on !ENV_IS_IN_MMC
+	depends on !ENV_IS_IN_NAND
+	depends on !ENV_IS_IN_NVRAM
+	depends on !ENV_IS_IN_ONENAND
+	depends on !ENV_IS_IN_REMOTE
+	depends on !ENV_IS_IN_SPI_FLASH
+	depends on !ENV_IS_IN_UBI
+	default y
 	help
 	  Define this if you don't want to or can't have an

environment stored on a storage medium. In this case the environemnt will still exist @@ -74,6 +54,8 @@ config ENV_IS_IN_EEPROM
config ENV_IS_IN_FAT

env/Kconfig +2 -0

 	bool "Environment is in a FAT filesystem"
 	depends on !CHAIN_OF_TRUST
+	default y if ARCH_BCM283X
+	default y if MMC_OMAP_HS && TI_COMMON_CMD_OPTIONS
 	select FAT_WRITE
 	help
 	  Define this if you want to use the FAT file system for the

environment. @@ -84,6 +66,13 @@ config ENV_IS_IN_FAT
config ENV_IS_IN_FLASH

env/Kconfig +7 -0

 	bool "Environment in flash memory"
 	depends on !CHAIN_OF_TRUST
+	default y if ARCH_CINTEGRATOR
+	default y if ARCH_INTEGRATOR_CP
+	default y if M548x || M547x || M5282 || MCF547x_8x
+	default y if MCF532x || MCF52x2
+	default y if MPC86xx || MPC83xx
+	default y if ARCH_MPC8572 || ARCH_MPC8548 || ARCH_MPC8641
+	default y if SH && !CPU_SH4
 	help
 	  Define this if you have a flash device which you want to

use for the environment.

env/Kconfig +5 -0

@@ -156,6 +145,11 @@ config ENV_IS_IN_FLASH
 config ENV_IS_IN_MMC
 	bool "Environment in an MMC device"
 	depends on !CHAIN_OF_TRUST
+	default y if ARCH_SUNXI
+	default y if ARCH_EXYNOS4
+	default y if MX6SX || MX7D
+	default y if TEGRA30 || TEGRA124
+	default y if TEGRA_ARMV8_COMMON
 	help
 	  Define this if you have an MMC device which you want to

use for the environment.

env/Kconfig +7 -0

@@ -293,6 +287,13 @@ config ENV_IS_IN_REMOTE
 config ENV_IS_IN_SPI_FLASH
 	bool "Environment is in SPI flash"
 	depends on !CHAIN_OF_TRUST
+	default y if ARMADA_XP
+	default y if INTEL_BAYTRAIL
+	default y if INTEL_BRASWELL
+	default y if INTEL_BROADWELL
+	default y if NORTHBRIDGE_INTEL_IVYBRIDGE
+	default y if INTEL_QUARK
+	default y if INTEL_QUEENSBAY
 	help
 	  Define this if you have a SPI Flash memory device which you
 	  want to use for the environment.

@@ -358,8 +359,6 @@ config ENV_IS_IN_UBI
 	  You will probably want to define these to avoid a really

noisy system when storing the env in UBI.

Allow boards and architectures to override the default environment lookup code by overriding env_get_location.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/env.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

env/env.c +0 -1

index 6af9f897b0ae..aceb77d4073c 100644
@@ -62,7 +62,7 @@ static enum env_location env_locations[] = {
 
 static enum env_location env_load_location;
 
-static enum env_location env_get_location(enum env_operation op, int

prio) +__weak enum env_location env_get_location(enum env_operation op, int prio) {

The current environment has been hardcoded to an offset that starts to be an issue given the current size of our main U-Boot binary.

By implementing a custom environment location routine, we can always favor the FAT-based environment, and fallback to the MMC if we don't find something in the FAT partition. We also implement the same order when saving the environment, so that hopefully we can slowly migrate the users over to FAT-based environment and away from the raw MMC one.

Eventually, and hopefully before we reach that limit again, we will have most of our users using that setup, and we'll be able to retire the raw environment, and gain more room for the U-Boot binary.

Signed-off-by Maxime Ripard <[email protected]>

---
 board/sunxi/board.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

board/sunxi/board.c +9 -0

index dcacdf3e626d..8891961dcc6b 100644
@@ -173,6 +173,22 @@ void i2c_init_board(void)
 #endif
 }
 
+#if defined(CONFIG_ENV_IS_IN_MMC) && defined(CONFIG_ENV_IS_IN_FAT)
+enum env_location env_get_location(enum env_operation op, int prio)
+{
+	switch (prio) {
+	case 0:
+		return ENVL_FAT;
+
+	case 1:
+		return ENVL_MMC;

Now that we have everything in place to implement the transition scheme, let's enable it by default.

Signed-off-by Maxime Ripard <[email protected]>

---
 env/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

env/Kconfig +2 -0

index bf6eab6b4ace..19524638e6e1 100644
@@ -55,6 +55,7 @@ config ENV_IS_IN_FAT
 	bool "Environment is in a FAT filesystem"
 	depends on !CHAIN_OF_TRUST
 	default y if ARCH_BCM283X
+	default y if ARCH_SUNXI
 	default y if MMC_OMAP_HS && TI_COMMON_CMD_OPTIONS
 	select FAT_WRITE
 	help

@@ -370,6 +371,7 @@ config ENV_AES
 config ENV_FAT_INTERFACE
 	string "Name of the block device for the environment"
 	depends on ENV_IS_IN_FAT
+	default "mmc" if ARCH_SUNXI
 	default "mmc" if TI_COMMON_CMD_OPTIONS || ARCH_ZYNQMP ||

ARCH_AT91 help
Define this to a string that is the name of the block
device. @@ -379,6 +381,8 @@ config ENV_FAT_DEVICE_AND_PART

env/Kconfig +2 -0

 	depends on ENV_IS_IN_FAT
 	default "0:1" if TI_COMMON_CMD_OPTIONS
 	default "0:auto" if ARCH_ZYNQMP
+	default "0:auto" if ARCH_SUNXI && MMC_SUNXI_SLOT_EXTRA = -1
+	default "1:auto" if ARCH_SUNXI && MMC_SUNXI_SLOT_EXTRA != -1
 	default "0" if ARCH_AT91
 	help
 	  Define this to a string to specify the partition of the

device. It can