[PATCH v3 00/14] env: ext4: corrections and add test for env in ext4

[PATCH v3 00/14] env: ext4: corrections and add test for env in ext4

[Patrick Delaunay]

Hi,

V3 of the serie [1].

In this serie, I add sandbox test with CONFIG_ENV_IS_NOWHERE
activated with EXT4 location: load, save and erase.

To test this feature, I add 2 new commands to change the
ENV location:
- env select [target]
- env load

This serie depends on previous env test introduced in [2]
"cmd: env: add option for quiet output on env info"

To be able to test invalid file (bad CRC), I also add the support of
the command "env erase" for EXT4 env location.

[1] http://patchwork.ozlabs.org/project/uboot/list/?series=183620
[2] http://patchwork.ozlabs.org/project/uboot/list/?series=184539

Regards

Patrick


Changes in v3:
- new
- new
- new: add ?load ops in nowhere
- new: load operation becomes mandatory
- new: add 'env load' command
- new: add 'env select' command
- change env_get_location to avoid gd->env_load_prio modification
- replace specific sandbox command by generic command
  'env select' and 'env load'
- change title "sandbox: support the change of env location"
- replace specific sandbox command by generic command
  'env select' and 'env load'
- update after Stephen Warren comments
- replace sandbox command by generic command 'env load' in test_env

Changes in v2:
- change cmd_tbl_t to struct cmd_tbl
- use CONFIG_IS_ENABLED to set .erase (same as .save)

Patrick Delaunay (14):
  env: add absolute path at CONFIG_ENV_EXT4_FILE
  env: ext4: set gd->env_valid
  env: sf: avoid space in backend name
  env: correctly handle env_load_prio
  env: nowhere: add .load ops
  env: the ops driver load becomes mandatory in struct env_driver
  cmd: env: add env load command
  cmd: env: add env select command
  configs: sandbox: activate env in ext4 support
  configs: sandbox: activate command env select and env load
  test: environment in ext4
  env: ext4: introduce new function env_ext4_save_buffer
  env: ext4: add support of command env erase
  test: sandbox: add test for erase command

 board/sandbox/sandbox.c            |  15 ++++
 cmd/Kconfig                        |  11 +++
 cmd/nvedit.c                       |  29 ++++++++
 configs/sandbox64_defconfig        |   7 ++
 configs/sandbox_defconfig          |   7 ++
 configs/sandbox_flattree_defconfig |   7 ++
 configs/sandbox_spl_defconfig      |   7 ++
 env/Kconfig                        |   2 +-
 env/env.c                          |  80 ++++++++++++++++++--
 env/ext4.c                         |  54 ++++++++++++--
 env/nowhere.c                      |   9 +++
 env/sf.c                           |   2 +-
 include/env.h                      |  15 +++-
 include/env_internal.h             |   3 +-
 test/py/tests/test_env.py          | 113 ++++++++++++++++++++++++++++-
 15 files changed, 341 insertions(+), 20 deletions(-)

-- 
2.17.1

[PATCH v3 01/14] env: add absolute path at CONFIG_ENV_EXT4_FILE

[Patrick Delaunay]

Add the absolute path to the default value of
CONFIG_ENV_EXT4_FILE = "/uboot.env".

This patch avoid the error :
  Saving Environment to EXT4... File System is consistent
  Please supply Absolute path

Reviewed-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

For information, it is the value used today by all the boards:

dragonboard820c_defconfig:29:CONFIG_ENV_EXT4_FILE="/uboot.env"
hikey960_defconfig:25:CONFIG_ENV_EXT4_FILE="/uboot.env"
stm32mp15_basic_defconfig:64:CONFIG_ENV_EXT4_FILE="/uboot.env"
stm32mp15_trusted_defconfig:50:CONFIG_ENV_EXT4_FILE="/uboot.env"


(no changes since v1)

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

diff --git a/env/Kconfig b/env/Kconfig
index ca7fef682b..9dad1cf8c1 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -469,7 +469,7 @@ config ENV_EXT4_DEVICE_AND_PART
 config ENV_EXT4_FILE
 	string "Name of the EXT4 file to use for the environment"
 	depends on ENV_IS_IN_EXT4
-	default "uboot.env"
+	default "/uboot.env"
 	help
 	  It's a string of the EXT4 file name. This file use to store the
 	  environment (explicit path to the file)
-- 
2.17.1

[PATCH v3 02/14] env: ext4: set gd->env_valid

[Patrick Delaunay]

Add a missing initialization of gd->env_valid in env_ext4_load
as it is already done in some other env device.

Set gd->env_valid = ENV_VALID in env_ext4_save() and env_ext4_load().

This patch allows to have a correct information in 'env info' command.

Reviewed-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

(no changes since v1)

 env/ext4.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/env/ext4.c b/env/ext4.c
index 8e90bb71b7..ac9f126bec 100644
--- a/env/ext4.c
+++ b/env/ext4.c
@@ -32,6 +32,8 @@
 #include <ext4fs.h>
 #include <mmc.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 __weak const char *env_ext4_get_intf(void)
 {
 	return (const char *)CONFIG_ENV_EXT4_INTERFACE;
@@ -79,6 +81,7 @@ static int env_ext4_save(void)
 			CONFIG_ENV_EXT4_FILE, ifname, dev, part);
 		return 1;
 	}
+	gd->env_valid = ENV_VALID;
 
 	puts("done\n");
 	return 0;
@@ -124,7 +127,11 @@ static int env_ext4_load(void)
 		goto err_env_relocate;
 	}
 
-	return env_import(buf, 1);
+	err = env_import(buf, 1);
+	if (!err)
+		gd->env_valid = ENV_VALID;
+
+	return err;
 
 err_env_relocate:
 	env_set_default(NULL, 0);
-- 
2.17.1

[PATCH v3 03/14] env: sf: avoid space in backend name

[Patrick Delaunay]

Remove space in ENV backend name for SPI Flash (SF)
to avoid issue with env select command.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- new

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

diff --git a/env/sf.c b/env/sf.c
index 02ed846fc7..515ffd4f1d 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -305,7 +305,7 @@ static int env_sf_init(void)
 
 U_BOOT_ENV_LOCATION(sf) = {
 	.location	= ENVL_SPI_FLASH,
-	ENV_NAME("SPI Flash")
+	ENV_NAME("SPIFlash")
 	.load		= env_sf_load,
 	.save		= CONFIG_IS_ENABLED(SAVEENV) ? ENV_SAVE_PTR(env_sf_save) : NULL,
 #if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)
-- 
2.17.1

[PATCH v3 04/14] env: correctly handle env_load_prio

[Patrick Delaunay]

Only update gd->env_load_prio in generic function env_load()
and no more in the weak function env_get_location() which is
called in many place (for example in env_driver_lookup, even
for ENVOP_SAVE operation).

This patch is a preliminary step to use env_driver_lookup()/
env_get_location() in new function env_select() without
updating gd->env_load_prio.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- new

 env/env.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/env/env.c b/env/env.c
index dcc25c030b..181aaa222c 100644
--- a/env/env.c
+++ b/env/env.c
@@ -131,8 +131,6 @@ __weak enum env_location env_get_location(enum env_operation op, int prio)
 	if (prio >= ARRAY_SIZE(env_locations))
 		return ENVL_UNKNOWN;
 
-	gd->env_load_prio = prio;
-
 	return env_locations[prio];
 }
 
@@ -204,6 +202,8 @@ int env_load(void)
 		ret = drv->load();
 		if (!ret) {
 			printf("OK\n");
+			gd->env_load_prio = prio;
+
 			return 0;
 		} else if (ret == -ENOMSG) {
 			/* Handle "bad CRC" case */
@@ -227,7 +227,8 @@ int env_load(void)
 		debug("Selecting environment with bad CRC\n");
 	else
 		best_prio = 0;
-	env_get_location(ENVOP_LOAD, best_prio);
+
+	gd->env_load_prio = best_prio;
 
 	return -ENODEV;
 }
-- 
2.17.1

[PATCH v3 05/14] env: nowhere: add .load ops

[Patrick Delaunay]

Add the ops .load for nowhere ENV backend to load the
default environment.

This ops is needed for the command 'env load'



Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- new: add ?load ops in nowhere

 env/nowhere.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/env/nowhere.c b/env/nowhere.c
index f5b0a17652..6949810a1f 100644
--- a/env/nowhere.c
+++ b/env/nowhere.c
@@ -27,8 +27,17 @@ static int env_nowhere_init(void)
 	return 0;
 }
 
+static int env_nowhere_load(void)
+{
+	env_set_default(NULL, 0);
+	gd->env_valid	= ENV_INVALID;
+
+	return 0;
+}
+
 U_BOOT_ENV_LOCATION(nowhere) = {
 	.location	= ENVL_NOWHERE,
 	.init		= env_nowhere_init,
+	.load		= env_nowhere_load,
 	ENV_NAME("nowhere")
 };
-- 
2.17.1

[PATCH v3 06/14] env: the ops driver load becomes mandatory in struct env_driver

[Patrick Delaunay]

The ops driver load becomes mandatory in struct env_drive,
change the comment for this ops and remove unnecessary test.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- new: load operation becomes mandatory

 env/env.c              | 3 ---
 include/env_internal.h | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/env/env.c b/env/env.c
index 181aaa222c..a1696cd77e 100644
--- a/env/env.c
+++ b/env/env.c
@@ -187,9 +187,6 @@ int env_load(void)
 	for (prio = 0; (drv = env_driver_lookup(ENVOP_LOAD, prio)); prio++) {
 		int ret;
 
-		if (!drv->load)
-			continue;
-
 		if (!env_has_inited(drv->location))
 			continue;
 
diff --git a/include/env_internal.h b/include/env_internal.h
index 66550434c3..795941daee 100644
--- a/include/env_internal.h
+++ b/include/env_internal.h
@@ -154,8 +154,7 @@ struct env_driver {
 	/**
 	 * load() - Load the environment from storage
 	 *
-	 * This method is optional. If not provided, no environment will be
-	 * loaded.
+	 * This method is required for loading environment
 	 *
 	 * @return 0 if OK, -ve on error
 	 */
-- 
2.17.1

[PATCH v3 07/14] cmd: env: add env load command

[Patrick Delaunay]

Add the new command env load to load the environment from
the current location gd->env_load_prio.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- new: add 'env load' command

 cmd/Kconfig   |  6 ++++++
 cmd/nvedit.c  | 14 ++++++++++++++
 env/env.c     | 28 ++++++++++++++++++++++++++++
 include/env.h |  7 +++++++
 4 files changed, 55 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 1de57988ae..679b1c32b4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -610,6 +610,12 @@ config CMD_NVEDIT_INFO
 	  [-q] : quiet output
 	  The result of multiple evaluations will be combined with AND.
 
+config CMD_NVEDIT_LOAD
+	bool "env load"
+	help
+	  Load all environment variables from the compiled-in persistent
+	  storage.
+
 endmenu
 
 menu "Memory commands"
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 0f9cea96f3..7a39d9cd66 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -794,6 +794,14 @@ U_BOOT_CMD(
 );
 #endif
 #endif
+
+#if defined(CONFIG_CMD_NVEDIT_LOAD)
+static int do_env_load(struct cmd_tbl *cmdtp, int flag, int argc,
+		       char *const argv[])
+{
+	return env_reload() ? 1 : 0;
+}
+#endif
 #endif /* CONFIG_SPL_BUILD */
 
 int env_match(uchar *s1, int i2)
@@ -1346,6 +1354,9 @@ static struct cmd_tbl cmd_env_sub[] = {
 #endif
 #if defined(CONFIG_CMD_NVEDIT_INFO)
 	U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""),
+#endif
+#if defined(CONFIG_CMD_NVEDIT_LOAD)
+	U_BOOT_CMD_MKENT(load, 1, 0, do_env_load, "", ""),
 #endif
 	U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_env_print, "", ""),
 #if defined(CONFIG_CMD_RUN)
@@ -1442,6 +1453,9 @@ static char env_help_text[] =
 	"env erase - erase environment\n"
 #endif
 #endif
+#if defined(CONFIG_CMD_NVEDIT_LOAD)
+	"env load - load environment\n"
+#endif
 #if defined(CONFIG_CMD_NVEDIT_EFI)
 	"env set -e [-nv][-bs][-rt][-at][-a][-i addr,size][-v] name [arg ...]\n"
 	"    - set UEFI variable; unset if '-i' or 'arg' not specified\n"
diff --git a/env/env.c b/env/env.c
index a1696cd77e..7ad9623ef2 100644
--- a/env/env.c
+++ b/env/env.c
@@ -230,6 +230,34 @@ int env_load(void)
 	return -ENODEV;
 }
 
+int env_reload(void)
+{
+	struct env_driver *drv;
+
+	drv = env_driver_lookup(ENVOP_LOAD, gd->env_load_prio);
+	if (drv) {
+		int ret;
+
+		printf("Loading Environment from %s... ", drv->name);
+
+		if (!env_has_inited(drv->location)) {
+			printf("not initialized\n");
+			return -ENODEV;
+		}
+
+		ret = drv->load();
+		if (ret)
+			printf("Failed (%d)\n", ret);
+		else
+			printf("OK\n");
+
+		if (!ret)
+			return 0;
+	}
+
+	return -ENODEV;
+}
+
 int env_save(void)
 {
 	struct env_driver *drv;
diff --git a/include/env.h b/include/env.h
index d6c2d751d6..68e0f4fa56 100644
--- a/include/env.h
+++ b/include/env.h
@@ -265,6 +265,13 @@ int env_set_default_vars(int nvars, char *const vars[], int flags);
  */
 int env_load(void);
 
+/**
+ * env_reload() - Re-Load the environment from current storage
+ *
+ * @return 0 if OK, -ve on error
+ */
+int env_reload(void);
+
 /**
  * env_save() - Save the environment to storage
  *
-- 
2.17.1

[PATCH v3 08/14] cmd: env: add env select command

[Patrick Delaunay]

Add the new command 'env select' to force the persistent storage
of environment, saved in gd->env_load_prio.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- new: add 'env select' command

 cmd/Kconfig   |  5 +++++
 cmd/nvedit.c  | 15 +++++++++++++++
 env/env.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/env.h |  8 +++++++-
 4 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 679b1c32b4..5673b56343 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -616,6 +616,11 @@ config CMD_NVEDIT_LOAD
 	  Load all environment variables from the compiled-in persistent
 	  storage.
 
+config CMD_NVEDIT_SELECT
+	bool "env select"
+	help
+	  Select the compiled-in persistent storage of environment variables.
+
 endmenu
 
 menu "Memory commands"
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 7a39d9cd66..9fc33d7db1 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -802,6 +802,15 @@ static int do_env_load(struct cmd_tbl *cmdtp, int flag, int argc,
 	return env_reload() ? 1 : 0;
 }
 #endif
+
+#if defined(CONFIG_CMD_NVEDIT_SELECT)
+static int do_env_select(struct cmd_tbl *cmdtp, int flag, int argc,
+			 char *const argv[])
+{
+	return env_select(argv[1]) ? 1 : 0;
+}
+#endif
+
 #endif /* CONFIG_SPL_BUILD */
 
 int env_match(uchar *s1, int i2)
@@ -1367,6 +1376,9 @@ static struct cmd_tbl cmd_env_sub[] = {
 #if defined(CONFIG_CMD_ERASEENV)
 	U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""),
 #endif
+#endif
+#if defined(CONFIG_CMD_NVEDIT_SELECT)
+	U_BOOT_CMD_MKENT(select, 2, 0, do_env_select, "", ""),
 #endif
 	U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""),
 #if defined(CONFIG_CMD_ENV_EXISTS)
@@ -1456,6 +1468,9 @@ static char env_help_text[] =
 #if defined(CONFIG_CMD_NVEDIT_LOAD)
 	"env load - load environment\n"
 #endif
+#if defined(CONFIG_CMD_NVEDIT_SELECT)
+	"env select [target] - select environment target\n"
+#endif
 #if defined(CONFIG_CMD_NVEDIT_EFI)
 	"env set -e [-nv][-bs][-rt][-at][-a][-i addr,size][-v] name [arg ...]\n"
 	"    - set UEFI variable; unset if '-i' or 'arg' not specified\n"
diff --git a/env/env.c b/env/env.c
index 7ad9623ef2..91c76133b0 100644
--- a/env/env.c
+++ b/env/env.c
@@ -340,3 +340,45 @@ int env_init(void)
 
 	return ret;
 }
+
+int env_select(const char *name)
+{
+	struct env_driver *drv;
+	const int n_ents = ll_entry_count(struct env_driver, env_driver);
+	struct env_driver *entry;
+	int prio;
+	bool found = false;
+
+	printf("Select Environment on %s: ", name);
+
+	/* search ENV driver by name */
+	drv = ll_entry_start(struct env_driver, env_driver);
+	for (entry = drv; entry != drv + n_ents; entry++) {
+		if (!strcmp(entry->name, name)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		printf("driver not found\n");
+		return -ENODEV;
+	}
+
+	/* search priority by driver */
+	for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) {
+		if (entry->location == env_get_location(ENVOP_LOAD, prio)) {
+			/* when priority change, reset the ENV flags */
+			if (gd->env_load_prio != prio) {
+				gd->env_load_prio = prio;
+				gd->env_valid = ENV_INVALID;
+				gd->flags &= ~GD_FLG_ENV_DEFAULT;
+			}
+			printf("OK\n");
+			return 0;
+		}
+	}
+	printf("priority not found\n");
+
+	return -ENODEV;
+}
diff --git a/include/env.h b/include/env.h
index 68e0f4fa56..665857f032 100644
--- a/include/env.h
+++ b/include/env.h
@@ -286,6 +286,13 @@ int env_save(void);
  */
 int env_erase(void);
 
+/**
+ * env_select() - Select the environment storage
+ *
+ * @return 0 if OK, -ve on error
+ */
+int env_select(const char *name);
+
 /**
  * env_import() - Import from a binary representation into hash table
  *
@@ -349,5 +356,4 @@ int env_get_char(int index);
  * This is used for those unfortunate archs with crappy toolchains
  */
 void env_reloc(void);
-
 #endif
-- 
2.17.1

[PATCH v3 09/14] configs: sandbox: activate env in ext4 support

[Patrick Delaunay]

Activate ENV in EXT4 support in sandbox.

The sandbox behavior don't change; the default environment with
the nowhere backend (CONFIG_ENV_IS_NOWHERE)is still used:
the weak function env_get_location() return ENVL_NOWHERE for priority 0.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- change env_get_location to avoid gd->env_load_prio modification

 board/sandbox/sandbox.c            | 15 +++++++++++++++
 configs/sandbox64_defconfig        |  4 ++++
 configs/sandbox_defconfig          |  4 ++++
 configs/sandbox_flattree_defconfig |  4 ++++
 configs/sandbox_spl_defconfig      |  4 ++++
 5 files changed, 31 insertions(+)

diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 1372003018..ef886f6378 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -7,6 +7,7 @@
 #include <cpu_func.h>
 #include <cros_ec.h>
 #include <dm.h>
+#include <env_internal.h>
 #include <init.h>
 #include <led.h>
 #include <os.h>
@@ -44,6 +45,20 @@ unsigned long timer_read_counter(void)
 }
 #endif
 
+/* specific order for sandbox: nowhere is the first value, used by default */
+static enum env_location env_locations[] = {
+	ENVL_NOWHERE,
+	ENVL_EXT4,
+};
+
+enum env_location env_get_location(enum env_operation op, int prio)
+{
+	if (prio >= ARRAY_SIZE(env_locations))
+		return ENVL_UNKNOWN;
+
+	return env_locations[prio];
+}
+
 int dram_init(void)
 {
 	gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index a2410b3368..b70272c0b0 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -81,6 +81,10 @@ CONFIG_OF_CONTROL=y
 CONFIG_OF_LIVE=y
 CONFIG_OF_HOSTFILE=y
 CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
+CONFIG_ENV_IS_NOWHERE=y
+CONFIG_ENV_IS_IN_EXT4=y
+CONFIG_ENV_EXT4_INTERFACE="host"
+CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_REGMAP=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 0c9e0b9f1f..715f5dc39d 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -90,6 +90,10 @@ CONFIG_OF_CONTROL=y
 CONFIG_OF_LIVE=y
 CONFIG_OF_HOSTFILE=y
 CONFIG_DEFAULT_DEVICE_TREE="sandbox"
+CONFIG_ENV_IS_NOWHERE=y
+CONFIG_ENV_IS_IN_EXT4=y
+CONFIG_ENV_EXT4_INTERFACE="host"
+CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_REGMAP=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index bc4819f998..ce806270bd 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -64,6 +64,10 @@ CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
 CONFIG_OF_HOSTFILE=y
 CONFIG_DEFAULT_DEVICE_TREE="sandbox"
+CONFIG_ENV_IS_NOWHERE=y
+CONFIG_ENV_IS_IN_EXT4=y
+CONFIG_ENV_EXT4_INTERFACE="host"
+CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_REGMAP=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 64f2ed5102..ea11c9bded 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -80,6 +80,10 @@ CONFIG_SPL_OF_CONTROL=y
 CONFIG_OF_HOSTFILE=y
 CONFIG_DEFAULT_DEVICE_TREE="sandbox"
 CONFIG_SPL_OF_PLATDATA=y
+CONFIG_ENV_IS_NOWHERE=y
+CONFIG_ENV_IS_IN_EXT4=y
+CONFIG_ENV_EXT4_INTERFACE="host"
+CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
 CONFIG_SPL_DM=y
-- 
2.17.1

[PATCH v3 10/14] configs: sandbox: activate command env select and env load

[Patrick Delaunay]

Add support of environment location with the new env command:
'env select' and 'env load'

The ENV backend is selected by priority order
- 0 = "nowhere" (default at boot)
- 1 = "EXT4"

To test EXT4 env support, this backend is selected by name:
> env select EXT4

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- replace specific sandbox command by generic command
  'env select' and 'env load'
- change title "sandbox: support the change of env location"

Changes in v2:
- change cmd_tbl_t to struct cmd_tbl

 configs/sandbox64_defconfig        | 2 ++
 configs/sandbox_defconfig          | 2 ++
 configs/sandbox_flattree_defconfig | 2 ++
 configs/sandbox_spl_defconfig      | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index b70272c0b0..6b9b3a7b75 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -31,6 +31,8 @@ CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
 CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_NVEDIT_INFO=y
+CONFIG_CMD_NVEDIT_LOAD=y
+CONFIG_CMD_NVEDIT_SELECT=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEMINFO=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 715f5dc39d..5b26fff7c9 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -35,6 +35,8 @@ CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
 CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_NVEDIT_INFO=y
+CONFIG_CMD_NVEDIT_LOAD=y
+CONFIG_CMD_NVEDIT_SELECT=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEMINFO=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index ce806270bd..ccbc6374e7 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -25,6 +25,8 @@ CONFIG_CMD_BOOTEFI_HELLO=y
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
 CONFIG_CMD_NVEDIT_INFO=y
+CONFIG_CMD_NVEDIT_LOAD=y
+CONFIG_CMD_NVEDIT_SELECT=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEMINFO=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index ea11c9bded..534f2d3239 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -35,6 +35,8 @@ CONFIG_CMD_GREPENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
 CONFIG_CMD_NVEDIT_INFO=y
+CONFIG_CMD_NVEDIT_LOAD=y
+CONFIG_CMD_NVEDIT_SELECT=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEMINFO=y
-- 
2.17.1

[PATCH v3 11/14] test: environment in ext4

[Patrick Delaunay]

Add basic test to persistent environment in ext4:
save and load in host ext4 file 'uboot.env'.

On first execution an empty EXT4 file system is created in
persistent data dir: env.ext4.img.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- replace specific sandbox command by generic command
  'env select' and 'env load'
- update after Stephen Warren comments

 test/py/tests/test_env.py | 97 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index a64aaa9bc5..70913c8d9a 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -4,6 +4,10 @@
 
 # Test operation of shell commands relating to environment variables.
 
+import os
+import os.path
+from subprocess import call, check_call, CalledProcessError
+
 import pytest
 import u_boot_utils
 
@@ -374,7 +378,6 @@ def test_env_info(state_test_env):
 @pytest.mark.buildconfigspec('cmd_nvedit_info')
 @pytest.mark.buildconfigspec('cmd_echo')
 def test_env_info_sandbox(state_test_env):
-
     """Test 'env info' command result with several options on sandbox
        with a known ENV configuration: ready & default & persistent
     """
@@ -399,3 +402,95 @@ def test_env_info_sandbox(state_test_env):
     response = c.run_command('env info -d -p -q')
     response = c.run_command('echo $?')
     assert response == "1"
+
+def mk_env_ext4(state_test_env):
+
+    """Create a empty ext4 file system volume."""
+    c = state_test_env.u_boot_console
+    filename = 'env.ext4.img'
+    persistent = c.config.persistent_data_dir + '/' + filename
+    fs_img = c.config.result_dir  + '/' + filename
+
+    if os.path.exists(persistent):
+        c.log.action('Disk image file ' + persistent + ' already exists')
+    else:
+        try:
+            u_boot_utils.run_and_log(c, 'dd if=/dev/zero of=%s bs=1M count=16' % persistent)
+            u_boot_utils.run_and_log(c, 'mkfs.ext4 -O ^metadata_csum %s' % persistent)
+        except CalledProcessError:
+            call('rm -f %s' % persistent, shell=True)
+            raise
+
+    u_boot_utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
+    return fs_img
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_echo')
+ at pytest.mark.buildconfigspec('cmd_nvedit_info')
+ at pytest.mark.buildconfigspec('cmd_nvedit_load')
+ at pytest.mark.buildconfigspec('cmd_nvedit_select')
+ at pytest.mark.buildconfigspec('env_is_in_ext4')
+def test_env_ext4(state_test_env):
+
+    """Test ENV in EXT4 on sandbox."""
+    c = state_test_env.u_boot_console
+    fs_img = ''
+    try:
+        fs_img = mk_env_ext4(state_test_env)
+
+        c.run_command('host bind 0  %s' % fs_img)
+
+        response = c.run_command('ext4ls host 0:0')
+        assert 'uboot.env' not in response
+
+        # force env location: EXT4 (prio 1 in sandbox)
+        response = c.run_command('env select EXT4')
+        assert 'Select Environment on EXT4: OK' in response
+
+        response = c.run_command('env save')
+        assert 'Saving Environment to EXT4' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from EXT4... OK' in response
+
+        response = c.run_command('ext4ls host 0:0')
+        assert '8192 uboot.env' in response
+
+        response = c.run_command('env info')
+        assert 'env_valid = valid' in response
+        assert 'env_ready = true' in response
+        assert 'env_use_default = false' in response
+
+        response = c.run_command('env info -p -d')
+        assert 'Environment was loaded from persistent storage' in response
+        assert 'Environment can be persisted' in response
+
+        response = c.run_command('env info -d -q')
+        assert response == ""
+        response = c.run_command('echo $?')
+        assert response == "1"
+
+        response = c.run_command('env info -p -q')
+        assert response == ""
+        response = c.run_command('echo $?')
+        assert response == "0"
+
+        # restore env location: NOWHERE (prio 0 in sandbox)
+        response = c.run_command('env select nowhere')
+        assert 'Select Environment on nowhere: OK' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from nowhere... OK' in response
+
+        response = c.run_command('env info')
+        assert 'env_valid = invalid' in response
+        assert 'env_ready = true' in response
+        assert 'env_use_default = true' in response
+
+        response = c.run_command('env info -p -d')
+        assert 'Default environment is used' in response
+        assert 'Environment cannot be persisted' in response
+
+    finally:
+        if fs_img:
+            call('rm -f %s' % fs_img, shell=True)
-- 
2.17.1

[PATCH v3 12/14] env: ext4: introduce new function env_ext4_save_buffer

[Patrick Delaunay]

Split the function env_ext4_save to prepare the erase support.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

(no changes since v1)

 env/ext4.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/env/ext4.c b/env/ext4.c
index ac9f126bec..0a10a5e050 100644
--- a/env/ext4.c
+++ b/env/ext4.c
@@ -44,9 +44,8 @@ __weak const char *env_ext4_get_dev_part(void)
 	return (const char *)CONFIG_ENV_EXT4_DEVICE_AND_PART;
 }
 
-static int env_ext4_save(void)
+static int env_ext4_save_buffer(env_t *env_new)
 {
-	env_t	env_new;
 	struct blk_desc *dev_desc = NULL;
 	struct disk_partition info;
 	int dev, part;
@@ -54,10 +53,6 @@ static int env_ext4_save(void)
 	const char *ifname = env_ext4_get_intf();
 	const char *dev_and_part = env_ext4_get_dev_part();
 
-	err = env_export(&env_new);
-	if (err)
-		return err;
-
 	part = blk_get_device_part_str(ifname, dev_and_part,
 				       &dev_desc, &info, 1);
 	if (part < 0)
@@ -72,7 +67,7 @@ static int env_ext4_save(void)
 		return 1;
 	}
 
-	err = ext4fs_write(CONFIG_ENV_EXT4_FILE, (void *)&env_new,
+	err = ext4fs_write(CONFIG_ENV_EXT4_FILE, (void *)env_new,
 			   sizeof(env_t), FILETYPE_REG);
 	ext4fs_close();
 
@@ -81,9 +76,26 @@ static int env_ext4_save(void)
 			CONFIG_ENV_EXT4_FILE, ifname, dev, part);
 		return 1;
 	}
-	gd->env_valid = ENV_VALID;
 
+	return 0;
+}
+
+static int env_ext4_save(void)
+{
+	env_t env_new;
+	int err;
+
+	err = env_export(&env_new);
+	if (err)
+		return err;
+
+	err = env_ext4_save_buffer(&env_new);
+	if (err)
+		return err;
+
+	gd->env_valid = ENV_VALID;
 	puts("done\n");
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH v3 13/14] env: ext4: add support of command env erase

[Patrick Delaunay]

Add support of opts erase for env in ext4,
this opts is used by command 'env erase'.

This command only fill the env file (CONFIG_ENV_EXT4_FILE)
with 0, the CRC and the saved environment becomes invalid.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

(no changes since v2)

Changes in v2:
- use CONFIG_IS_ENABLED to set .erase (same as .save)

 env/ext4.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/env/ext4.c b/env/ext4.c
index 0a10a5e050..cc36504154 100644
--- a/env/ext4.c
+++ b/env/ext4.c
@@ -99,6 +99,23 @@ static int env_ext4_save(void)
 	return 0;
 }
 
+static int env_ext4_erase(void)
+{
+	env_t env_new;
+	int err;
+
+	memset(&env_new, 0, sizeof(env_t));
+
+	err = env_ext4_save_buffer(&env_new);
+	if (err)
+		return err;
+
+	gd->env_valid = ENV_INVALID;
+	puts("done\n");
+
+	return 0;
+}
+
 static int env_ext4_load(void)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
@@ -156,4 +173,6 @@ U_BOOT_ENV_LOCATION(ext4) = {
 	ENV_NAME("EXT4")
 	.load		= env_ext4_load,
 	.save		= ENV_SAVE_PTR(env_ext4_save),
+	.erase		= CONFIG_IS_ENABLED(CMD_ERASEENV) ? env_ext4_erase :
+							    NULL,
 };
-- 
2.17.1

[PATCH v3 14/14] test: sandbox: add test for erase command

[Patrick Delaunay]

Add test for the erase command tested on ENV in EXT4.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- replace sandbox command by generic command 'env load' in test_env

 configs/sandbox64_defconfig        |  1 +
 configs/sandbox_defconfig          |  1 +
 configs/sandbox_flattree_defconfig |  1 +
 configs/sandbox_spl_defconfig      |  1 +
 test/py/tests/test_env.py          | 16 ++++++++++++++++
 5 files changed, 20 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 6b9b3a7b75..906b92de8c 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -27,6 +27,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
 CONFIG_CMD_NVEDIT_EFI=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 5b26fff7c9..97953fda08 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -31,6 +31,7 @@ CONFIG_CMD_ABOOTIMG=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
 CONFIG_CMD_NVEDIT_EFI=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index ccbc6374e7..ba9f652689 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -24,6 +24,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_NVEDIT_INFO=y
 CONFIG_CMD_NVEDIT_LOAD=y
 CONFIG_CMD_NVEDIT_SELECT=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 534f2d3239..fb54c86bf6 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -32,6 +32,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
 CONFIG_CMD_NVEDIT_INFO=y
diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index 70913c8d9a..86ec1b36d3 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -475,6 +475,22 @@ def test_env_ext4(state_test_env):
         response = c.run_command('echo $?')
         assert response == "0"
 
+        response = c.run_command('env erase')
+        assert 'OK' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from EXT4... ' in response
+        assert 'bad CRC, using default environment' in response
+
+        response = c.run_command('env info')
+        assert 'env_valid = invalid' in response
+        assert 'env_ready = true' in response
+        assert 'env_use_default = true' in response
+
+        response = c.run_command('env info -p -d')
+        assert 'Default environment is used' in response
+        assert 'Environment can be persisted' in response
+
         # restore env location: NOWHERE (prio 0 in sandbox)
         response = c.run_command('env select nowhere')
         assert 'Select Environment on nowhere: OK' in response
-- 
2.17.1

[PATCH v3 08/14] cmd: env: add env select command

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:52AM +0200, Patrick Delaunay wrote:

> Add the new command 'env select' to force the persistent storage
> of environment, saved in gd->env_load_prio.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
[snip]
> +	/* search priority by driver */
> +	for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) {
> +		if (entry->location == env_get_location(ENVOP_LOAD, prio)) {
> +			/* when priority change, reset the ENV flags */
> +			if (gd->env_load_prio != prio) {
> +				gd->env_load_prio = prio;
> +				gd->env_valid = ENV_INVALID;
> +				gd->flags &= ~GD_FLG_ENV_DEFAULT;
> +			}
> +			printf("OK\n");
> +			return 0;
> +		}
> +	}

So, after we do this, is some follow up env command required to
initialize the environment to now exist somewhere else?  Or will we have
initialized all configured locations during boot, and don't have to?
But what will happen if we select say "nand" but it's not present so
didn't init.  Will things fail gracefully (not panic) ?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200626/1e282377/attachment.sig>

[PATCH v3 03/14] env: sf: avoid space in backend name

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:47AM +0200, Patrick Delaunay wrote:

> Remove space in ENV backend name for SPI Flash (SF)
> to avoid issue with env select command.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200626/256e26bd/attachment.sig>

[PATCH v3 04/14] env: correctly handle env_load_prio

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:48AM +0200, Patrick Delaunay wrote:

> Only update gd->env_load_prio in generic function env_load()
> and no more in the weak function env_get_location() which is
> called in many place (for example in env_driver_lookup, even
> for ENVOP_SAVE operation).
> 
> This patch is a preliminary step to use env_driver_lookup()/
> env_get_location() in new function env_select() without
> updating gd->env_load_prio.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200626/d250222c/attachment.sig>

[PATCH v3 05/14] env: nowhere: add .load ops

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:49AM +0200, Patrick Delaunay wrote:

> Add the ops .load for nowhere ENV backend to load the
> default environment.
> 
> This ops is needed for the command 'env load'
> 
> 
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200626/9813b753/attachment.sig>

[PATCH v3 06/14] env: the ops driver load becomes mandatory in struct env_driver

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:50AM +0200, Patrick Delaunay wrote:

> The ops driver load becomes mandatory in struct env_drive,
> change the comment for this ops and remove unnecessary test.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200626/b45bbc8a/attachment.sig>

[PATCH v3 07/14] cmd: env: add env load command

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:51AM +0200, Patrick Delaunay wrote:

> Add the new command env load to load the environment from
> the current location gd->env_load_prio.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200626/21ff9b90/attachment.sig>

[PATCH v3 08/14] cmd: env: add env select command

[Patrick DELAUNAY]

Hi Tom

> From: Tom Rini <trini@konsulko.com>
> Sent: vendredi 26 juin 2020 22:55
> 
> On Thu, Jun 25, 2020 at 09:59:52AM +0200, Patrick Delaunay wrote:
> 
> > Add the new command 'env select' to force the persistent storage of
> > environment, saved in gd->env_load_prio.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> [snip]
> > +	/* search priority by driver */
> > +	for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) {
> > +		if (entry->location == env_get_location(ENVOP_LOAD, prio)) {
> > +			/* when priority change, reset the ENV flags */
> > +			if (gd->env_load_prio != prio) {
> > +				gd->env_load_prio = prio;
> > +				gd->env_valid = ENV_INVALID;
> > +				gd->flags &= ~GD_FLG_ENV_DEFAULT;
> > +			}
> > +			printf("OK\n");
> > +			return 0;
> > +		}
> > +	}
> 
> So, after we do this, is some follow up env command required to initialize the
> environment to now exist somewhere else?  Or will we have initialized all
> configured locations during boot, and don't have to?
> But what will happen if we select say "nand" but it's not present so didn't init.  Will
> things fail gracefully (not panic) ?  Thanks!

I was not sure if a automatic load was needed in this command, as I add a sparate load command
in this patch I don't add an  automatic load => and default env is used

My expected sequence for the env commands was:

	env  select  <target> 
	env load
	...
	env save

when user try to select a not compiled backend the command return "driver not found"
(tested on sandbox, I will add unitary test for this point)

or when backend is not accessible by any priority, the select command return pritority not found
	manual test on sandbox, with env_locations[] = { ENVL_NOWHERE };	

	=> env select EXT4
	Select Environment on EXT4: priority not found

I don't think that abort can occur for other commands because
- the env backend for all priotity are initialized at boot (env_init use the same loop than env select)

- in all env function (env_load / env_reload / env_save it is tested again
   (to check if the backend was correctly initilializaed, if .init() retur 0= 
		if (!env_has_inited(drv->location))
			return -ENODEV;

- all direct acces (in env_get_char for example) are intercepted because gd->env_valid == ENV_INVALID
     => by default (without explicite reload) the default environent is used, exactly when the load failed.
     => "gd->env_addr" is never used


but user can also execute the sequence 

	env  select  <target>
	env save

and here the command behavior  is strange....
because the default is forced on each select

	env select EXT4
	env load

	env select MMC    (reset on default  environment)
	env save

=> I expect at the end of the sequence the EXT4 env was copied in MMC....
     But in finally only the default environment is saved in MMC

I don't sure of the expected behavior by user for these commands....

what it is better solution ?
add a option to select command ? 
force the load after the select ?

Patrick

[PATCH v3 11/14] test: environment in ext4

[Stephen Warren]

On 6/25/20 1:59 AM, Patrick Delaunay wrote:
> Add basic test to persistent environment in ext4:
> save and load in host ext4 file 'uboot.env'.
> 
> On first execution an empty EXT4 file system is created in
> persistent data dir: env.ext4.img.

Acked-by: Stephen Warren <swarren@nvidia.com>

A couple nits below; feel free not to skip them, but if you end up
sending another revision for other reasons, may as well fix them.

> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py

> +from subprocess import call, check_call, CalledProcessError

I believe only CalledProcessError is used now.

> +def test_env_ext4(state_test_env):
> +
> +    """Test ENV in EXT4 on sandbox."""
> +    c = state_test_env.u_boot_console
> +    fs_img = ''

"None" would be a better unset value.

[PATCH v3 14/14] test: sandbox: add test for erase command

[Stephen Warren]

On 6/25/20 1:59 AM, Patrick Delaunay wrote:
> Add test for the erase command tested on ENV in EXT4.

Acked-by: Stephen Warren <swarren@nvidia.com>

[PATCH v3 05/14] env: nowhere: add .load ops

[Tom Rini]

On Thu, Jun 25, 2020 at 09:59:49AM +0200, Patrick Delaunay wrote:

> Add the ops .load for nowhere ENV backend to load the
> default environment.
> 
> This ops is needed for the command 'env load'
> 
> 
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> ---
> 
> Changes in v3:
> - new: add ?load ops in nowhere
> 
>  env/nowhere.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/env/nowhere.c b/env/nowhere.c
> index f5b0a17652..6949810a1f 100644
> --- a/env/nowhere.c
> +++ b/env/nowhere.c
> @@ -27,8 +27,17 @@ static int env_nowhere_init(void)
>  	return 0;
>  }
>  
> +static int env_nowhere_load(void)
> +{
> +	env_set_default(NULL, 0);
> +	gd->env_valid	= ENV_INVALID;
> +
> +	return 0;
> +}
> +
>  U_BOOT_ENV_LOCATION(nowhere) = {
>  	.location	= ENVL_NOWHERE,
>  	.init		= env_nowhere_init,
> +	.load		= env_nowhere_load,
>  	ENV_NAME("nowhere")
>  };

Build testing this, we get 8KiB size increase in SPL in targets which
have ENV_NOWHERE in SPL.  Can we guard this somehow, with a logical
tie-in to being needed for 'env load' ? Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200726/b8b3e88a/attachment.sig>