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

[PATCH 1/5] 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>
---

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

diff --git a/cmd/Kconfig b/cmd/Kconfig
index b1a1cbcab2..458dbcedac 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -567,6 +567,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 2/5] 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>
---

 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 3/5] 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>
---

 cmd/nvedit.c           | 13 ++++++++++---
 env/env.c              | 18 ------------------
 include/env_internal.h | 20 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 3d1054e763..a37b7c094a 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -1269,9 +1269,16 @@ 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;
+		enum env_location 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/env/env.c b/env/env.c
index 9237bb9c74..4e7a3c0c48 100644
--- a/env/env.c
+++ b/env/env.c
@@ -105,24 +105,6 @@ static void env_set_inited(enum env_location location)
 	gd->env_has_init |= BIT(location);
 }
 
-/**
- * env_get_location() - Returns the best env location for a board
- * @op: operations performed on the environment
- * @prio: priority between the multiple environments, 0 being the
- *        highest priority
- *
- * This will return the preferred environment for the given priority.
- * This is overridable by boards if they need to.
- *
- * All implementations are free to use the operation, the priority and
- * any other data relevant to their choice, but must take into account
- * the fact that the lowest prority (0) is the most important location
- * in the system. The following locations should be returned by order
- * of descending priorities, from the highest to the lowest priority.
- *
- * Returns:
- * an enum env_location value on success, a negative error code otherwise
- */
 __weak enum env_location env_get_location(enum env_operation op, int prio)
 {
 	if (prio >= ARRAY_SIZE(env_locations))
diff --git a/include/env_internal.h b/include/env_internal.h
index 90a4df8a72..af29ff0cd6 100644
--- a/include/env_internal.h
+++ b/include/env_internal.h
@@ -209,6 +209,26 @@ struct env_driver {
 
 extern struct hsearch_data env_htab;
 
+/**
+ * env_get_location() - Returns the best env location for a board
+ * @op: operations performed on the environment
+ * @prio: priority between the multiple environments, 0 being the
+ *        highest priority
+ *
+ * This will return the preferred environment for the given priority.
+ * This is overridable by boards if they need to.
+ *
+ * All implementations are free to use the operation, the priority and
+ * any other data relevant to their choice, but must take into account
+ * the fact that the lowest prority (0) is the most important location
+ * in the system. The following locations should be returned by order
+ * of descending priorities, from the highest to the lowest priority.
+ *
+ * Returns:
+ * an enum env_location value on success, a negative error code otherwise
+ */
+enum env_location env_get_location(enum env_operation op, int prio);
+
 #endif /* DO_DEPS_ONLY */
 
 #endif /* _ENV_INTERNAL_H_ */
-- 
2.17.1

[PATCH 4/5] 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>
---

 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 5/5] 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>
---

 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 0b646da2b1..4e743561df 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 b45462b2f0..d597a13123 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 5dc530f1ab..2804df0a12 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 3/5] cmd: env: check real location for env info command

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200124133332.3.I42c79507524e5ad68e85fd60bbd686c4c59523ae@changeid> you 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>
> ---
>
>  cmd/nvedit.c           | 13 ++++++++++---
>  env/env.c              | 18 ------------------
>  include/env_internal.h | 20 ++++++++++++++++++++
>  3 files changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 3d1054e763..a37b7c094a 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -1269,9 +1269,16 @@ 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;
> +		enum env_location loc = env_get_location(ENVOP_SAVE,

Please do not declare variables right in the middle of the code!


> +++ b/env/env.c
> @@ -105,24 +105,6 @@ static void env_set_inited(enum env_location location)
>  	gd->env_has_init |= BIT(location);
>  }
>  
> -/**
> - * env_get_location() - Returns the best env location for a board
> - * @op: operations performed on the environment
> - * @prio: priority between the multiple environments, 0 being the
> - *        highest priority
> - *
> - * This will return the preferred environment for the given priority.
> - * This is overridable by boards if they need to.
> - *
> - * All implementations are free to use the operation, the priority and
> - * any other data relevant to their choice, but must take into account
> - * the fact that the lowest prority (0) is the most important location
> - * in the system. The following locations should be returned by order
> - * of descending priorities, from the highest to the lowest priority.
> - *
> - * Returns:
> - * an enum env_location value on success, a negative error code otherwise
> - */
>  __weak enum env_location env_get_location(enum env_operation op, int prio)

I think it is a really bad idea to remove the comment from the
implementation.  Please keep it here.

> --- a/include/env_internal.h
> +++ b/include/env_internal.h
> @@ -209,6 +209,26 @@ struct env_driver {
>  
>  extern struct hsearch_data env_htab;
>  
> +/**
> + * env_get_location() - Returns the best env location for a board
> + * @op: operations performed on the environment
> + * @prio: priority between the multiple environments, 0 being the
> + *        highest priority
> + *
> + * This will return the preferred environment for the given priority.
> + * This is overridable by boards if they need to.
> + *
> + * All implementations are free to use the operation, the priority and
> + * any other data relevant to their choice, but must take into account
> + * the fact that the lowest prority (0) is the most important location
> + * in the system. The following locations should be returned by order
> + * of descending priorities, from the highest to the lowest priority.
> + *
> + * Returns:
> + * an enum env_location value on success, a negative error code otherwise
> + */
> +enum env_location env_get_location(enum env_operation op, int prio);

If absolutely necessary, copuy only what is needed for API
documentation.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"This is a test of the Emergency Broadcast System. If this had been an
actual emergency, do you really think we'd stick around to tell you?"

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

[Patrick DELAUNAY]

Hi Wolfgang,

> From: Wolfgang Denk <wd@denx.de>
> Sent: vendredi 24 janvier 2020 14:17
> 
> Dear Patrick Delaunay,
> 
> In message
> <20200124133332.3.I42c79507524e5ad68e85fd60bbd686c4c59523ae@changeid>
> you 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>
> > ---
> >
> >  cmd/nvedit.c           | 13 ++++++++++---
> >  env/env.c              | 18 ------------------
> >  include/env_internal.h | 20 ++++++++++++++++++++
> >  3 files changed, 30 insertions(+), 21 deletions(-)
> >
> > diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 3d1054e763..a37b7c094a
> > 100644
> > --- a/cmd/nvedit.c
> > +++ b/cmd/nvedit.c
> > @@ -1269,9 +1269,16 @@ 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;
> > +		enum env_location loc = env_get_location(ENVOP_SAVE,
> 
> Please do not declare variables right in the middle of the code!

Yes, I will modify it....
I am surprised that this issue pas the check patch.
 
> 
> > +++ b/env/env.c
> > @@ -105,24 +105,6 @@ static void env_set_inited(enum env_location location)
> >  	gd->env_has_init |= BIT(location);
> >  }
> >
> > -/**
> > - * env_get_location() - Returns the best env location for a board
> > - * @op: operations performed on the environment
> > - * @prio: priority between the multiple environments, 0 being the
> > - *        highest priority
> > - *
> > - * This will return the preferred environment for the given priority.
> > - * This is overridable by boards if they need to.
> > - *
> > - * All implementations are free to use the operation, the priority
> > and
> > - * any other data relevant to their choice, but must take into
> > account
> > - * the fact that the lowest prority (0) is the most important
> > location
> > - * in the system. The following locations should be returned by order
> > - * of descending priorities, from the highest to the lowest priority.
> > - *
> > - * Returns:
> > - * an enum env_location value on success, a negative error code
> > otherwise
> > - */
> >  __weak enum env_location env_get_location(enum env_operation op, int
> > prio)
> 
> I think it is a really bad idea to remove the comment from the implementation.
> Please keep it here.

Yes I  agree .
I will come back on this par.
 
> > --- a/include/env_internal.h
> > +++ b/include/env_internal.h
> > @@ -209,6 +209,26 @@ struct env_driver {
> >
> >  extern struct hsearch_data env_htab;
> >
> > +/**
> > + * env_get_location() - Returns the best env location for a board
> > + * @op: operations performed on the environment
> > + * @prio: priority between the multiple environments, 0 being the
> > + *        highest priority
> > + *
> > + * This will return the preferred environment for the given priority.
> > + * This is overridable by boards if they need to.
> > + *
> > + * All implementations are free to use the operation, the priority
> > +and
> > + * any other data relevant to their choice, but must take into
> > +account
> > + * the fact that the lowest prority (0) is the most important
> > +location
> > + * in the system. The following locations should be returned by order
> > + * of descending priorities, from the highest to the lowest priority.
> > + *
> > + * Returns:
> > + * an enum env_location value on success, a negative error code
> > +otherwise  */ enum env_location env_get_location(enum env_operation
> > +op, int prio);
> 
> If absolutely necessary, copuy only what is needed for API documentation.

Ok

> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "This
> is a test of the Emergency Broadcast System. If this had been an actual
> emergency, do you really think we'd stick around to tell you?"

Thanks

Patrick

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

[Simon Glass]

On Fri, 24 Jan 2020 at 05:34, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> 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>
> ---
>
>  cmd/Kconfig  |  1 +
>  cmd/nvedit.c | 26 +++++++++++++++++++-------
>  2 files changed, 20 insertions(+), 7 deletions(-)

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

We could have a test for this command

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

[Simon Glass]

On Fri, 24 Jan 2020 at 05:34, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> 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>
> ---
>
>  cmd/nvedit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

We should add more tests for the environment functionality.

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

[Patrick DELAUNAY]

Hi,

> From: Simon Glass <sjg@chromium.org>
> Sent: jeudi 30 janvier 2020 03:18
> 
> On Fri, 24 Jan 2020 at 05:34, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > 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>
> > ---
> >
> >  cmd/Kconfig  |  1 +
> >  cmd/nvedit.c | 26 +++++++++++++++++++-------
> >  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks for the review

 > We could have a test for this command

Yes, I will do it.

Regards

Patrick

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

[Patrick DELAUNAY]

Hi Simon,

> From: Simon Glass <sjg@chromium.org>
> Sent: jeudi 30 janvier 2020 03:18
> 
> On Fri, 24 Jan 2020 at 05:34, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > 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>
> > ---
> >
> >  cmd/nvedit.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> We should add more tests for the environment functionality.

Sorry for the delay,

I need to activate a location in sandbox to test this feature....

I am working on a other serie for it (I am trying to add test for env in EXT4).

Patrick