[PATCH v2 0/7] cmd: env: add option for quiet output on env info

[PATCH v2 0/7] cmd: env: add option for quiet output on env info

[Patrick Delaunay]

Hi,

It is a V2 after Wolfgang and Simon remarks for
"cmd: env: add option for quiet output on env info"
http://patchwork.ozlabs.org/project/uboot/list/?series=155122

I also add pytest for command env info.

Test for ENV_IS_IN_DEVICE will be include in future serie
(I will activate ENV_IS_IN_EXT4 support in sandbox)

To avoid compilation warning, I add prototype for
env_get_location for the patch 3/7
"cmd: env: check real location for env info command"
as it is done in
http://patchwork.ozlabs.org/patch/1230200/

Regards

Patrick


Changes in v2:
- update prototype in env_internal.h  as done in
  "env: add prototypes for weak function"
- remove comment change in env.c (implementation information)
- move env_location declaration
- activate CMD_NVEDIT_INFO in sandbox (new patch)
- activate env info command in sandbox (new)
- add pytest test_env_info and test_env_info_test (new)

Patrick Delaunay (7):
  cmd: env: add option for quiet output on env info
  cmd: env: use ENV_IS_IN_DEVICE in env info
  cmd: env: check real location for env info command
  stm32mp1: use the command env info in env_check
  stm32mp1: configs: activate CMD_ERASEENV
  configs: sandbox: Enable sub command 'env info'
  test: env: add test for env info sub-command

 arch/arm/mach-stm32mp/Kconfig       |  1 +
 cmd/Kconfig                         |  1 +
 cmd/nvedit.c                        | 39 +++++++++++++++++++------
 configs/sandbox64_defconfig         |  1 +
 configs/sandbox_defconfig           |  1 +
 configs/sandbox_flattree_defconfig  |  1 +
 configs/sandbox_spl_defconfig       |  1 +
 configs/stm32mp15_basic_defconfig   |  1 +
 configs/stm32mp15_optee_defconfig   |  1 +
 configs/stm32mp15_trusted_defconfig |  1 +
 include/configs/stm32mp1.h          |  4 +--
 include/env_internal.h              | 11 ++++++++
 test/py/tests/test_env.py           | 44 +++++++++++++++++++++++++++++
 13 files changed, 95 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] cmd: env: add option for quiet output on env info

[Patrick Delaunay]

The "env info" can be use for test with -d and -p parameter,
in scripting case the output of the command is not needed.

This patch allows to deactivate this output with a new option "-q".

For example, we can save the environment if default
environment is used and persistent storage is managed with:
  if env info -p -d -q; then env save; fi

Without the quiet option, I have the unnecessary traces
First boot:
      Default environment is used
      Environment can be persisted
      Saving Environment to EXT4... File System is consistent

Next boot:
      Environment was loaded from persistent storage
      Environment can be persisted

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 cmd/Kconfig  |  1 +
 cmd/nvedit.c | 26 +++++++++++++++++++-------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index e6ba57035e..5c859199b6 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -591,6 +591,7 @@ config CMD_NVEDIT_INFO
 	  This command can be optionally used for evaluation in scripts:
 	  [-d] : evaluate whether default environment is used
 	  [-p] : evaluate whether environment can be persisted
+	  [-q] : quiet output
 	  The result of multiple evaluations will be combined with AND.
 
 endmenu
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 81d94cd193..aaa032cd96 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1219,12 +1219,15 @@ static int print_env_info(void)
  * env info - display environment information
  * env info [-d] - evaluate whether default environment is used
  * env info [-p] - evaluate whether environment can be persisted
+ *      Add [-q] - quiet mode, use only for command result, for test by example:
+ *                 test env info -p -d -q
  */
 static int do_env_info(cmd_tbl_t *cmdtp, int flag,
 		       int argc, char * const argv[])
 {
 	int eval_flags = 0;
 	int eval_results = 0;
+	bool quiet = false;
 
 	/* display environment information */
 	if (argc <= 1)
@@ -1242,6 +1245,9 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag,
 			case 'p':
 				eval_flags |= ENV_INFO_IS_PERSISTED;
 				break;
+			case 'q':
+				quiet = true;
+				break;
 			default:
 				return CMD_RET_USAGE;
 			}
@@ -1251,20 +1257,24 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag,
 	/* evaluate whether default environment is used */
 	if (eval_flags & ENV_INFO_IS_DEFAULT) {
 		if (gd->flags & GD_FLG_ENV_DEFAULT) {
-			printf("Default environment is used\n");
+			if (!quiet)
+				printf("Default environment is used\n");
 			eval_results |= ENV_INFO_IS_DEFAULT;
 		} else {
-			printf("Environment was loaded from persistent storage\n");
+			if (!quiet)
+				printf("Environment was loaded from persistent storage\n");
 		}
 	}
 
 	/* evaluate whether environment can be persisted */
 	if (eval_flags & ENV_INFO_IS_PERSISTED) {
 #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
-		printf("Environment can be persisted\n");
+		if (!quiet)
+			printf("Environment can be persisted\n");
 		eval_results |= ENV_INFO_IS_PERSISTED;
 #else
-		printf("Environment cannot be persisted\n");
+		if (!quiet)
+			printf("Environment cannot be persisted\n");
 #endif
 	}
 
@@ -1321,7 +1331,7 @@ static cmd_tbl_t cmd_env_sub[] = {
 	U_BOOT_CMD_MKENT(import, 5, 0, do_env_import, "", ""),
 #endif
 #if defined(CONFIG_CMD_NVEDIT_INFO)
-	U_BOOT_CMD_MKENT(info, 2, 0, do_env_info, "", ""),
+	U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""),
 #endif
 	U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_env_print, "", ""),
 #if defined(CONFIG_CMD_RUN)
@@ -1400,8 +1410,10 @@ static char env_help_text[] =
 #endif
 #if defined(CONFIG_CMD_NVEDIT_INFO)
 	"env info - display environment information\n"
-	"env info [-d] - whether default environment is used\n"
-	"env info [-p] - whether environment can be persisted\n"
+	"env info [-d] [-p] [-q] - evaluate environment information\n"
+	"      \"-d\": default environment is used\n"
+	"      \"-p\": environment can be persisted\n"
+	"      \"-q\": quiet output\n"
 #endif
 	"env print [-a | name ...] - print environment\n"
 #if defined(CONFIG_CMD_NVEDIT_EFI)
-- 
2.17.1

[PATCH v2 2/7] cmd: env: use ENV_IS_IN_DEVICE in env info

[Patrick Delaunay]

Use the define ENV_IS_IN_DEVICE to test if one the
CONFIG_ENV_IS_IN_...  is defined and correct the detection of
persistent storage support in the command "env info"
if CONFIG_ENV_IS_NOWHERE is activated.

Since commit 60d5ed2593c9 ("env: allow ENV_IS_NOWHERE with
other storage target") test CONFIG_ENV_IS_NOWHERE is not
enough; see also commit 953db29a1e9c6 ("env: enable saveenv
command when one CONFIG_ENV_IS_IN is activated").

This patch avoids issue for this command in stm32mp1 platform.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

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

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index aaa032cd96..3d1054e763 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1268,7 +1268,7 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag,
 
 	/* evaluate whether environment can be persisted */
 	if (eval_flags & ENV_INFO_IS_PERSISTED) {
-#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
+#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE)
 		if (!quiet)
 			printf("Environment can be persisted\n");
 		eval_results |= ENV_INFO_IS_PERSISTED;
-- 
2.17.1

[PATCH v2 3/7] cmd: env: check real location for env info command

[Patrick Delaunay]

Check the current ENV location, dynamically provided by the weak
function env_get_location to be sure that the environment can be
persistent.

The compilation flag ENV_IS_IN_DEVICE is not enough when the board
dynamically select the available storage location (according boot
device for example).

This patch solves issue for stm32mp1 platform, when the boot device
is USB.

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

Changes in v2:
- update prototype in env_internal.h  as done in
  "env: add prototypes for weak function"
- remove comment change in env.c (implementation information)
- move env_location declaration

 cmd/nvedit.c           | 15 ++++++++++++---
 include/env_internal.h | 11 +++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 3d1054e763..218fdfbc55 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1228,6 +1228,9 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag,
 	int eval_flags = 0;
 	int eval_results = 0;
 	bool quiet = false;
+#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE)
+	enum env_location loc;
+#endif
 
 	/* display environment information */
 	if (argc <= 1)
@@ -1269,9 +1272,15 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag,
 	/* evaluate whether environment can be persisted */
 	if (eval_flags & ENV_INFO_IS_PERSISTED) {
 #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE)
-		if (!quiet)
-			printf("Environment can be persisted\n");
-		eval_results |= ENV_INFO_IS_PERSISTED;
+		loc = env_get_location(ENVOP_SAVE, gd->env_load_prio);
+		if (ENVL_NOWHERE != loc && ENVL_UNKNOWN != loc) {
+			if (!quiet)
+				printf("Environment can be persisted\n");
+			eval_results |= ENV_INFO_IS_PERSISTED;
+		} else {
+			if (!quiet)
+				printf("Environment cannot be persisted\n");
+		}
 #else
 		if (!quiet)
 			printf("Environment cannot be persisted\n");
diff --git a/include/env_internal.h b/include/env_internal.h
index 90a4df8a72..cfb60738d0 100644
--- a/include/env_internal.h
+++ b/include/env_internal.h
@@ -209,6 +209,17 @@ struct env_driver {
 
 extern struct hsearch_data env_htab;
 
+/**
+ * env_get_location()- Provide the best location for the U-Boot environment
+ *
+ * It is a weak function allowing board to overidde the environment location
+ *
+ * @op: operations performed on the environment
+ * @prio: priority between the multiple environments, 0 being the
+ *        highest priority
+ * @return  an enum env_location value on success, or -ve error code.
+ */
+enum env_location env_get_location(enum env_operation op, int prio);
 #endif /* DO_DEPS_ONLY */
 
 #endif /* _ENV_INTERNAL_H_ */
-- 
2.17.1

[PATCH v2 4/7] stm32mp1: use the command env info in env_check

[Patrick Delaunay]

Activate CMD_NVEDIT_INFO and use the new command "env info -d -p -q"
to automatically save the environment on first boot.

This patch allows to remove the env_default variable.

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

Changes in v2:
- activate CMD_NVEDIT_INFO in sandbox (new patch)

 arch/arm/mach-stm32mp/Kconfig | 1 +
 include/configs/stm32mp1.h    | 4 +---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
index 137178aa45..63dc94f894 100644
--- a/arch/arm/mach-stm32mp/Kconfig
+++ b/arch/arm/mach-stm32mp/Kconfig
@@ -45,6 +45,7 @@ config STM32MP15x
 	select STM32_RESET
 	select STM32_SERIAL
 	select SYS_ARCH_TIMER
+	imply CMD_NVEDIT_INFO
 	imply SYSRESET_PSCI if STM32MP1_TRUSTED
 	imply SYSRESET_SYSCON if !STM32MP1_TRUSTED
 	help
diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index a66534e027..02d32f2040 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -218,9 +218,7 @@
 	"fdt_high=0xffffffff\0" \
 	"initrd_high=0xffffffff\0" \
 	"altbootcmd=run bootcmd\0" \
-	"env_default=1\0" \
-	"env_check=if test $env_default -eq 1;"\
-		" then env set env_default 0;env save;fi\0" \
+	"env_check=if env info -p -d -q; then env save; fi\0" \
 	STM32MP_BOOTCMD \
 	STM32MP_MTDPARTS \
 	STM32MP_DFU_ALT_RAM \
-- 
2.17.1

[PATCH v2 5/7] stm32mp1: configs: activate CMD_ERASEENV

[Patrick Delaunay]

Activate the command env erase to reset the environment with the command:
> env erase

it is simpler than:
> env default -a
> env save

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

Changes in v2: None

 configs/stm32mp15_basic_defconfig   | 1 +
 configs/stm32mp15_optee_defconfig   | 1 +
 configs/stm32mp15_trusted_defconfig | 1 +
 3 files changed, 3 insertions(+)

diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index f691306800..1fffffd23a 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -26,6 +26,7 @@ CONFIG_SYS_PROMPT="STM32MP> "
 # CONFIG_CMD_XIMG is not set
 # CONFIG_CMD_EXPORTENV is not set
 # CONFIG_CMD_IMPORTENV is not set
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_MEMINFO=y
 CONFIG_CMD_MEMTEST=y
 CONFIG_CMD_ADC=y
diff --git a/configs/stm32mp15_optee_defconfig b/configs/stm32mp15_optee_defconfig
index 521b24e2cb..c2006291f1 100644
--- a/configs/stm32mp15_optee_defconfig
+++ b/configs/stm32mp15_optee_defconfig
@@ -15,6 +15,7 @@ CONFIG_SYS_PROMPT="STM32MP> "
 # CONFIG_CMD_XIMG is not set
 # CONFIG_CMD_EXPORTENV is not set
 # CONFIG_CMD_IMPORTENV is not set
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_MEMINFO=y
 CONFIG_CMD_MEMTEST=y
 CONFIG_CMD_ADC=y
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index c8b328d01a..fc6e6a555e 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -14,6 +14,7 @@ CONFIG_SYS_PROMPT="STM32MP> "
 # CONFIG_CMD_XIMG is not set
 # CONFIG_CMD_EXPORTENV is not set
 # CONFIG_CMD_IMPORTENV is not set
+CONFIG_CMD_ERASEENV=y
 CONFIG_CMD_MEMINFO=y
 CONFIG_CMD_MEMTEST=y
 CONFIG_CMD_ADC=y
-- 
2.17.1

[PATCH v2 6/7] configs: sandbox: Enable sub command 'env info'

[Patrick Delaunay]

Enable support for sub command 'env info' in sandbox
with CONFIG_CMD_NVEDIT_INFO. This is aimed primarily
at adding unit test.

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

Changes in v2:
- activate env info command in sandbox (new)

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

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 941b1fd2c7..cdcb0acbdc 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -27,6 +27,7 @@ CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
+CONFIG_CMD_NVEDIT_INFO=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEMINFO=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 7b02b8de7c..33a103edab 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -31,6 +31,7 @@ CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
+CONFIG_CMD_NVEDIT_INFO=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 0049da3d48..2bfbb66453 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -22,6 +22,7 @@ CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
+CONFIG_CMD_NVEDIT_INFO=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 f55692c2b5..3bf27c974a 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -33,6 +33,7 @@ CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
 CONFIG_CMD_ENV_CALLBACK=y
 CONFIG_CMD_ENV_FLAGS=y
+CONFIG_CMD_NVEDIT_INFO=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEMINFO=y
-- 
2.17.1

[PATCH v2 7/7] test: env: add test for env info sub-command

[Patrick Delaunay]

Add a pytest for testing the env info sub-command:

test_env_info: test command with several option

test_env_info_test: test the result of the sub-commandi with quiet option,
'-q' as used for support in shell test; for example:
  if env info -p -d -q; then env save; fi

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

Changes in v2:
- add pytest test_env_info and test_env_info_test (new)

 test/py/tests/test_env.py | 44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index 6ff38f1020..7b78ec4e40 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -336,3 +336,47 @@ def test_env_import_whitelist_delete(state_test_env):
     unset_var(state_test_env, 'foo2')
     unset_var(state_test_env, 'foo3')
     unset_var(state_test_env, 'foo4')
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_nvedit_info')
+def test_env_info(state_test_env):
+
+    """Test 'env info' command with several options.
+    """
+    c = state_test_env.u_boot_console
+
+    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
+
+    response = c.run_command('env info -p -d -q')
+    assert response == ""
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('cmd_nvedit_info')
+ at pytest.mark.buildconfigspec('cmd_echo')
+def test_env_info_test(state_test_env):
+
+    """Test 'env info' quiet command result with several options for test.
+    """
+    c = state_test_env.u_boot_console
+
+    response = c.run_command('env info -d -q')
+    assert response == ""
+    response = c.run_command('echo $?')
+    assert response == "0"
+
+    response = c.run_command('env info -p -q')
+    assert response == ""
+    response = c.run_command('echo $?')
+    assert response == "1"
+
+    response = c.run_command('env info -d -p -q')
+    assert response == ""
+    response = c.run_command('echo $?')
+    assert response == "1"
-- 
2.17.1

[PATCH v2 7/7] test: env: add test for env info sub-command

[Stephen Warren]

On 2/10/20 10:01 AM, Patrick Delaunay wrote:
> Add a pytest for testing the env info sub-command:
> 
> test_env_info: test command with several option
> 
> test_env_info_test: test the result of the sub-commandi with quiet option,

Nit: Remove "i" from the end of "sub-commandi".

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

> + at pytest.mark.boardspec('sandbox')

I assume that's just so things like "environment can't be persisted" can 
be guaranteed, since other boards will be different to sandbox here?

> + at pytest.mark.buildconfigspec('cmd_nvedit_info')
> + at pytest.mark.buildconfigspec('cmd_echo')
> +def test_env_info_test(state_test_env):
> +
> +    """Test 'env info' quiet command result with several options for test.
> +    """

Nit: It took me a while to realized what the "for test" and "_test" 
function name suffix meant. Perhaps _retcode might be a better function 
name suffix?

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

[PATCH v2 6/7] configs: sandbox: Enable sub command 'env info'

[Simon Glass]

On Mon, 10 Feb 2020 at 10:01, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Enable support for sub command 'env info' in sandbox
> with CONFIG_CMD_NVEDIT_INFO. This is aimed primarily
> at adding unit test.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - activate env info command in sandbox (new)
>
>  configs/sandbox64_defconfig        | 1 +
>  configs/sandbox_defconfig          | 1 +
>  configs/sandbox_flattree_defconfig | 1 +
>  configs/sandbox_spl_defconfig      | 1 +
>  4 files changed, 4 insertions(+)

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

[PATCH v2 3/7] cmd: env: check real location for env info command

[Simon Glass]

On Mon, 10 Feb 2020 at 10:01, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Check the current ENV location, dynamically provided by the weak
> function env_get_location to be sure that the environment can be
> persistent.
>
> The compilation flag ENV_IS_IN_DEVICE is not enough when the board
> dynamically select the available storage location (according boot
> device for example).
>
> This patch solves issue for stm32mp1 platform, when the boot device
> is USB.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - update prototype in env_internal.h  as done in
>   "env: add prototypes for weak function"
> - remove comment change in env.c (implementation information)
> - move env_location declaration
>
>  cmd/nvedit.c           | 15 ++++++++++++---
>  include/env_internal.h | 11 +++++++++++
>  2 files changed, 23 insertions(+), 3 deletions(-)

Definitely we need some more tests in the area of the environment.

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

[PATCH v2 7/7] test: env: add test for env info sub-command

[Patrick DELAUNAY]

Hi Stephen

> From: Stephen Warren <swarren@wwwdotorg.org>
> Sent: lundi 10 février 2020 21:25
> 
> On 2/10/20 10:01 AM, Patrick Delaunay wrote:
> > Add a pytest for testing the env info sub-command:
> >
> > test_env_info: test command with several option
> >
> > test_env_info_test: test the result of the sub-commandi with quiet
> > option,
> 
> Nit: Remove "i" from the end of "sub-commandi".

yes
 
> > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> 
> > + at pytest.mark.boardspec('sandbox')
> 
> I assume that's just so things like "environment can't be persisted" can be
> guaranteed, since other boards will be different to sandbox here?

Yes.

And also  the test is based on command "env_loc" (select the env backend used) which is sandbox specific.
So this test will failed on real platform.

 
> > + at pytest.mark.buildconfigspec('cmd_nvedit_info')
> > + at pytest.mark.buildconfigspec('cmd_echo')
> > +def test_env_info_test(state_test_env):
> > +
> > +    """Test 'env info' quiet command result with several options for test.
> > +    """
> 
> Nit: It took me a while to realized what the "for test" and "_test"
> function name suffix meant. Perhaps _retcode might be a better function name
> suffix?

ok

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