diff mbox series

[U-Boot,RFC,v4,13/16] cmd: env: show variable storage attribute in "env flags" command

Message ID 20190717082525.891-14-takahiro.akashi@linaro.org
State RFC
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: non-volatile variables support | expand

Commit Message

AKASHI Takahiro July 17, 2019, 8:25 a.m. UTC 
With this patch, "env flags" commands shows information about
storage attribute as well as other attributes.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 cmd/nvedit.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

Comments

Wolfgang Denk July 19, 2019, 9:05 a.m. UTC | #1 
Dear AKASHI Takahiro,

In message <20190717082525.891-14-takahiro.akashi@linaro.org> you wrote:
>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 70fbb5dd8f2f..77e8846b601a 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -618,10 +618,12 @@ static int print_static_flags(const char *var_name, const char *flags,
>  {
>  	enum env_flags_vartype type = env_flags_parse_vartype(flags);
>  	enum env_flags_varaccess access = env_flags_parse_varaccess(flags);
> +	enum env_flags_varstorage storage = env_flags_parse_varstorage(flags);
>  
> -	printf("\t%-20s %-20s %-20s\n", var_name,
> +	printf("\t%-20s %-20s %-20s %-20s\n", var_name,
>  		env_flags_get_vartype_name(type),
> -		env_flags_get_varaccess_name(access));
> +		env_flags_get_varaccess_name(access),
> +		env_flags_get_varstorage_name(storage));

This needs changing, see before.

>  
> -	if (entry->flags == 0)
> +#ifdef CONFIG_EFI_LOADER
> +	if (entry->context == ENVCTX_UEFI &&
> +	    entry->flags == ENV_FLAGS_VARSTORAGE_NON_VOLATILE_AUTOSAVE)
> +		return 0;
> +#endif
> +	/* ENVCTX_UBOOT */
> +	if (entry->flags == ENV_FLAGS_VARSTORAGE_NON_VOLATILE)
>  		return 0;

No; we should not have such #ifdefery here.  Please get rid of this.
We have contexts, we should not handle these through #ifdef.


Best regards,

Wolfgang Denk
diff mbox series

Patch

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 70fbb5dd8f2f..77e8846b601a 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -618,10 +618,12 @@  static int print_static_flags(const char *var_name, const char *flags,
 {
 	enum env_flags_vartype type = env_flags_parse_vartype(flags);
 	enum env_flags_varaccess access = env_flags_parse_varaccess(flags);
+	enum env_flags_varstorage storage = env_flags_parse_varstorage(flags);
 
-	printf("\t%-20s %-20s %-20s\n", var_name,
+	printf("\t%-20s %-20s %-20s %-20s\n", var_name,
 		env_flags_get_vartype_name(type),
-		env_flags_get_varaccess_name(access));
+		env_flags_get_varaccess_name(access),
+		env_flags_get_varstorage_name(storage));
 
 	return 0;
 }
@@ -630,16 +632,25 @@  static int print_active_flags(ENTRY *entry)
 {
 	enum env_flags_vartype type;
 	enum env_flags_varaccess access;
+	enum env_flags_varstorage storage;
 
-	if (entry->flags == 0)
+#ifdef CONFIG_EFI_LOADER
+	if (entry->context == ENVCTX_UEFI &&
+	    entry->flags == ENV_FLAGS_VARSTORAGE_NON_VOLATILE_AUTOSAVE)
+		return 0;
+#endif
+	/* ENVCTX_UBOOT */
+	if (entry->flags == ENV_FLAGS_VARSTORAGE_NON_VOLATILE)
 		return 0;
 
 	type = (enum env_flags_vartype)
 		(entry->flags & ENV_FLAGS_VARTYPE_BIN_MASK);
 	access = env_flags_parse_varaccess_from_binflags(entry->flags);
-	printf("\t%-20s %-20s %-20s\n", entry->key,
+	storage = env_flags_parse_varstorage_from_binflags(entry->flags);
+	printf("\t%-20s %-20s %-20s %-20s\n", entry->key,
 		env_flags_get_vartype_name(type),
-		env_flags_get_varaccess_name(access));
+		env_flags_get_varaccess_name(access),
+		env_flags_get_varstorage_name(storage));
 
 	return 0;
 }
@@ -667,19 +678,19 @@  int do_env_flags(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	/* Print the static flags that may exist */
 	puts("Static flags:\n");
-	printf("\t%-20s %-20s %-20s\n", "Variable Name", "Variable Type",
-		"Variable Access");
-	printf("\t%-20s %-20s %-20s\n", "-------------", "-------------",
-		"---------------");
+	printf("\t%-20s %-20s %-20s %-20s\n", "Variable Name", "Variable Type",
+		"Variable Access", "Variable Storage");
+	printf("\t%-20s %-20s %-20s %-20s\n", "-------------", "-------------",
+		"---------------", "-------------");
 	env_attr_walk(ENV_FLAGS_LIST_STATIC, print_static_flags, NULL);
 	puts("\n");
 
 	/* walk through each variable and print the flags if non-default */
 	puts("Active flags:\n");
-	printf("\t%-20s %-20s %-20s\n", "Variable Name", "Variable Type",
-		"Variable Access");
-	printf("\t%-20s %-20s %-20s\n", "-------------", "-------------",
-		"---------------");
+	printf("\t%-20s %-20s %-20s %-20s\n", "Variable Name", "Variable Type",
+		"Variable Access", "Variable Storage");
+	printf("\t%-20s %-20s %-20s %-20s\n", "-------------", "-------------",
+		"---------------", "-------------");
 	hwalk_r(&env_htab, print_active_flags);
 	return 0;
 }