[01/14] cmd: nvedit: Get rid of the env lookup

Message ID 7e3a78f964aaf9c1c4cabd0b2723b429cab73cdc.1511864667.git-series.maxime.ripard@free-electrons.com
State Superseded
Headers show
Series
  • env: Multiple env support and env transition for sunxi
Related show

Commit Message

Maxime Ripard Nov. 28, 2017, 10:24 a.m.
The nvedit command is the only user of env_driver_lookup_default outside of
the environment code itself, and it uses it only to print the environment
it's about to save to during env save.

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

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

Reviewed-by: Lukasz Majewski <lukma@denx.de>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 cmd/nvedit.c          | 4 ----
 env/env.c             | 4 +++-
 include/environment.h | 7 -------
 3 files changed, 3 insertions(+), 12 deletions(-)

Comments

André Przywara Dec. 5, 2017, 10:08 a.m. | #1
On 28/11/17 10:24, Maxime Ripard wrote:
> The nvedit command is the only user of env_driver_lookup_default outside of
> the environment code itself, and it uses it only to print the environment
> it's about to save to during env save.
> 
> As we're about to rework the environment to be able to handle multiple
> environment sources, we might not have an idea of what environment backend
> is going to be used before trying (and possibly failing for some).
> 
> Therefore, it makes sense to remove that message and move it to the
> env_save function itself. As a side effect, we also can get rid of the call
> to env_driver_lookup_default that is also about to get refactored.
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  cmd/nvedit.c          | 4 ----
>  env/env.c             | 4 +++-
>  include/environment.h | 7 -------
>  3 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 4e79d03856fe..a690d743cd46 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -708,10 +708,6 @@ ulong env_get_ulong(const char *name, int base, ulong default_val)
>  static int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc,
>  		       char * const argv[])
>  {
> -	struct env_driver *env = env_driver_lookup_default();
> -
> -	printf("Saving Environment to %s...\n", env->name);
> -
>  	return env_save() ? 1 : 0;
>  }
>  
> diff --git a/env/env.c b/env/env.c
> index 76a5608628fc..094538ff5b62 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -52,7 +52,7 @@ static enum env_location env_get_default_location(void)
>  		return ENVL_UNKNOWN;
>  }
>  
> -struct env_driver *env_driver_lookup_default(void)
> +static struct env_driver *env_driver_lookup_default(void)
>  {
>  	enum env_location loc = env_get_default_location();
>  	struct env_driver *drv;
> @@ -115,6 +115,8 @@ int env_save(void)
>  		return -ENODEV;
>  	if (!drv->save)
>  		return -ENOSYS;
> +
> +	printf("Saving Environment to %s...\n", drv->name);
>  	ret = drv->save();
>  	if (ret) {
>  		debug("%s: Environment failed to save (err=%d)\n", __func__,
> diff --git a/include/environment.h b/include/environment.h
> index 7b9821638960..226e3ef2d23a 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -303,13 +303,6 @@ int env_import_redund(const char *buf1, const char *buf2);
>  #endif
>  
>  /**
> - * env_driver_lookup_default() - Look up the default environment driver
> - *
> - * @return pointer to driver, or NULL if none (which should not happen)
> - */
> -struct env_driver *env_driver_lookup_default(void);
> -
> -/**
>   * env_get_char() - Get a character from the early environment
>   *
>   * This reads from the pre-relocation environemnt
>
Simon Glass Dec. 29, 2017, 3:13 a.m. | #2
On 28 November 2017 at 03:24, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
>
> The nvedit command is the only user of env_driver_lookup_default outside of
> the environment code itself, and it uses it only to print the environment
> it's about to save to during env save.
>
> As we're about to rework the environment to be able to handle multiple
> environment sources, we might not have an idea of what environment backend
> is going to be used before trying (and possibly failing for some).
>
> Therefore, it makes sense to remove that message and move it to the
> env_save function itself. As a side effect, we also can get rid of the call
> to env_driver_lookup_default that is also about to get refactored.
>
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  cmd/nvedit.c          | 4 ----
>  env/env.c             | 4 +++-
>  include/environment.h | 7 -------
>  3 files changed, 3 insertions(+), 12 deletions(-)

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

Patch

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 4e79d03856fe..a690d743cd46 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -708,10 +708,6 @@  ulong env_get_ulong(const char *name, int base, ulong default_val)
 static int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
-	struct env_driver *env = env_driver_lookup_default();
-
-	printf("Saving Environment to %s...\n", env->name);
-
 	return env_save() ? 1 : 0;
 }
 
diff --git a/env/env.c b/env/env.c
index 76a5608628fc..094538ff5b62 100644
--- a/env/env.c
+++ b/env/env.c
@@ -52,7 +52,7 @@  static enum env_location env_get_default_location(void)
 		return ENVL_UNKNOWN;
 }
 
-struct env_driver *env_driver_lookup_default(void)
+static struct env_driver *env_driver_lookup_default(void)
 {
 	enum env_location loc = env_get_default_location();
 	struct env_driver *drv;
@@ -115,6 +115,8 @@  int env_save(void)
 		return -ENODEV;
 	if (!drv->save)
 		return -ENOSYS;
+
+	printf("Saving Environment to %s...\n", drv->name);
 	ret = drv->save();
 	if (ret) {
 		debug("%s: Environment failed to save (err=%d)\n", __func__,
diff --git a/include/environment.h b/include/environment.h
index 7b9821638960..226e3ef2d23a 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -303,13 +303,6 @@  int env_import_redund(const char *buf1, const char *buf2);
 #endif
 
 /**
- * env_driver_lookup_default() - Look up the default environment driver
- *
- * @return pointer to driver, or NULL if none (which should not happen)
- */
-struct env_driver *env_driver_lookup_default(void);
-
-/**
  * env_get_char() - Get a character from the early environment
  *
  * This reads from the pre-relocation environemnt