[U-Boot] [PATCH v4 0/4] add command env erase

[U-Boot] [PATCH v4 0/4] add command env erase

[Frank Wunderlich]

sometimes it is needed to erase the non-volatile environment
e.g. for boot-up with builtin-environment or after resizing env

this series add basic functionality for erasing environment from storage
as a first storage-driver mmc is introduced, other needs to be added later

changes since v3:
 - fixes
 - Kconfig-option as suggested by Simon Goldschmidt
 - including CONFIG_ENV_OFFSET_REDUND (4/4 is RFC)

Frank Wunderlich (4):
  env: register erase command
  env: mmc: add erase-function
  env: add option to use redundant offset
  [RFC] env: call env_erase twice if CONFIG_ENV_OFFSET_REDUND is set

 cmd/Kconfig           |  8 ++++++++
 cmd/nvedit.c          | 26 ++++++++++++++++++++++++++
 env/env.c             | 30 ++++++++++++++++++++++++++++++
 env/mmc.c             | 36 ++++++++++++++++++++++++++++++++++++
 include/environment.h | 17 +++++++++++++++++
 5 files changed, 117 insertions(+)

--
2.17.1

[U-Boot] [PATCH v4 1/4] env: register erase command

[Frank Wunderlich]

this patch adds basic changes for adding a erase-subcommand to env

with this command the environment stored on non-volatile storage written
by saveenv can be cleared.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

squashed fixes
 - start message with "Erasing"
 - mark erase-function as optional
 - env: separate eraseenv from saveenv

Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
 cmd/Kconfig           |  8 ++++++++
 cmd/nvedit.c          | 19 +++++++++++++++++++
 env/env.c             | 30 ++++++++++++++++++++++++++++++
 include/environment.h | 17 +++++++++++++++++
 4 files changed, 74 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 0b07b3b9d7..e8a99cb5a3 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -397,6 +397,14 @@ config CMD_SAVEENV
 	  Save all environment variables into the compiled-in persistent
 	  storage.

+config CMD_ERASEENV
+	bool "eraseenv"
+	default n
+	depends on CMD_SAVEENV
+	help
+	  Erase environment variables from the compiled-in persistent
+	  storage.
+
 config CMD_ENV_EXISTS
 	bool "env exists"
 	default y
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 24a6cf7824..0cbd8e8984 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -761,6 +761,19 @@ U_BOOT_CMD(
 	"save environment variables to persistent storage",
 	""
 );
+
+#if defined(CONFIG_CMD_ERASEENV)
+static int do_env_erase(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	return env_erase() ? 1 : 0;
+}
+U_BOOT_CMD(
+	eraseenv, 1, 0,	do_env_erase,
+	"erase environment variables from persistent storage",
+	""
+);
+#endif
 #endif
 #endif /* CONFIG_SPL_BUILD */

@@ -1207,6 +1220,9 @@ static cmd_tbl_t cmd_env_sub[] = {
 #endif
 #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
 	U_BOOT_CMD_MKENT(save, 1, 0, do_env_save, "", ""),
+#if defined(CONFIG_CMD_ERASEENV)
+	U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""),
+#endif
 #endif
 	U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""),
 #if defined(CONFIG_CMD_ENV_EXISTS)
@@ -1282,6 +1298,9 @@ static char env_help_text[] =
 #endif
 #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
 	"env save - save environment\n"
+#if defined(CONFIG_CMD_ERASEENV)
+	"env erase - erase environment\n"
+#endif
 #endif
 #if defined(CONFIG_CMD_NVEDIT_EFI)
 	"env set -e name [arg ...] - set UEFI variable; unset if 'arg' not specified\n"
diff --git a/env/env.c b/env/env.c
index 4b417b90a2..d3cbe2f915 100644
--- a/env/env.c
+++ b/env/env.c
@@ -24,6 +24,8 @@ void env_fix_drivers(void)
 			entry->load += gd->reloc_off;
 		if (entry->save)
 			entry->save += gd->reloc_off;
+		if (entry->erase)
+			entry->erase += gd->reloc_off;
 		if (entry->init)
 			entry->init += gd->reloc_off;
 	}
@@ -254,6 +256,34 @@ int env_save(void)
 	return -ENODEV;
 }

+int env_erase(void)
+{
+	struct env_driver *drv;
+
+	drv = env_driver_lookup(ENVOP_ERASE, gd->env_load_prio);
+	if (drv) {
+		int ret;
+
+		if (!drv->erase)
+			return -ENODEV;
+
+		if (!env_has_inited(drv->location))
+			return -ENODEV;
+
+		printf("Erasing Environment on %s... ", drv->name);
+		ret = drv->erase();
+		if (ret)
+			printf("Failed (%d)\n", ret);
+		else
+			printf("OK\n");
+
+		if (!ret)
+			return 0;
+	}
+
+	return -ENODEV;
+}
+
 int env_init(void)
 {
 	struct env_driver *drv;
diff --git a/include/environment.h b/include/environment.h
index cd96676141..de67cf4f0e 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -200,6 +200,7 @@ enum env_operation {
 	ENVOP_INIT,	/* we want to call the init function */
 	ENVOP_LOAD,	/* we want to call the load function */
 	ENVOP_SAVE,	/* we want to call the save function */
+	ENVOP_ERASE,	/* we want to call the erase function */
 };

 struct env_driver {
@@ -225,6 +226,15 @@ struct env_driver {
 	 */
 	int (*save)(void);

+	/**
+	 * erase() - Erase the environment on storage
+	 *
+	 * This method is optional and required for 'eraseenv' to work.
+	 *
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*erase)(void);
+
 	/**
 	 * init() - Set up the initial pre-relocation environment
 	 *
@@ -303,6 +313,13 @@ int env_load(void);
  */
 int env_save(void);

+/**
+ * env_erase() - Erase the environment on storage
+ *
+ * @return 0 if OK, -ve on error
+ */
+int env_erase(void);
+
 /**
  * env_fix_drivers() - Updates envdriver as per relocation
  */
--
2.17.1

[U-Boot] [PATCH v4 2/4] env: mmc: add erase-function

[Frank Wunderlich]

this adds erase environment for mmc storage

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

squashed fixes:
 - fix bogus indent
 - add CONFIG_CMD_ERASEENV

Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
 env/mmc.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/env/mmc.c b/env/mmc.c
index c3cf35d01b..9ae9b1a66a 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -242,6 +242,34 @@ fini:
 	fini_mmc_for_env(mmc);
 	return ret;
 }
+
+#if defined(CONFIG_CMD_ERASEENV)
+static int env_mmc_erase(void)
+{
+	int dev = mmc_get_env_dev();
+	struct mmc *mmc = find_mmc_device(dev);
+	int n, blk, cnt;
+
+	if (!mmc)
+		return CMD_RET_FAILURE;
+
+	blk = CONFIG_ENV_OFFSET / mmc->read_bl_len;
+	cnt = CONFIG_ENV_SIZE / mmc->read_bl_len;
+
+	printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d (0x%x)\n",
+	       dev, blk, blk * mmc->read_bl_len,
+	       cnt, cnt * mmc->read_bl_len);
+
+	if (mmc_getwp(mmc) == 1) {
+		printf("Error: card is write protected!\n");
+		return CMD_RET_FAILURE;
+	}
+	n = blk_derase(mmc_get_blk_desc(mmc), blk, cnt);
+	printf("%d blocks erased: %s\n", n, (n == cnt) ? "OK" : "ERROR");
+
+	return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
+}
+#endif /* CONFIG_CMD_ERASEENV */
 #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */

 static inline int read_env(struct mmc *mmc, unsigned long size,
@@ -351,5 +379,8 @@ U_BOOT_ENV_LOCATION(mmc) = {
 	.load		= env_mmc_load,
 #ifndef CONFIG_SPL_BUILD
 	.save		= env_save_ptr(env_mmc_save),
+#if defined(CONFIG_CMD_ERASEENV)
+	.erase		= env_mmc_erase,
+#endif
 #endif
 };
--
2.17.1

[U-Boot] [PATCH v4 3/4] env: add option to use redundant offset

[Frank Wunderlich]

allow env erase on secondary offset using CONFIG_ENV_OFFSET_REDUND

Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 cmd/nvedit.c          | 2 +-
 env/env.c             | 4 ++--
 env/mmc.c             | 9 +++++++--
 include/environment.h | 4 ++--
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 0cbd8e8984..2071bcf443 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -766,7 +766,7 @@ U_BOOT_CMD(
 static int do_env_erase(cmd_tbl_t *cmdtp, int flag, int argc,
 			char * const argv[])
 {
-	return env_erase() ? 1 : 0;
+	return env_erase(false) ? 1 : 0;
 }
 U_BOOT_CMD(
 	eraseenv, 1, 0,	do_env_erase,
diff --git a/env/env.c b/env/env.c
index d3cbe2f915..bf7f3b9684 100644
--- a/env/env.c
+++ b/env/env.c
@@ -256,7 +256,7 @@ int env_save(void)
 	return -ENODEV;
 }

-int env_erase(void)
+int env_erase(bool use_redund)
 {
 	struct env_driver *drv;

@@ -271,7 +271,7 @@ int env_erase(void)
 			return -ENODEV;

 		printf("Erasing Environment on %s... ", drv->name);
-		ret = drv->erase();
+		ret = drv->erase(use_redund);
 		if (ret)
 			printf("Failed (%d)\n", ret);
 		else
diff --git a/env/mmc.c b/env/mmc.c
index 9ae9b1a66a..647bc693fa 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -244,7 +244,7 @@ fini:
 }

 #if defined(CONFIG_CMD_ERASEENV)
-static int env_mmc_erase(void)
+static int env_mmc_erase(bool use_redund)
 {
 	int dev = mmc_get_env_dev();
 	struct mmc *mmc = find_mmc_device(dev);
@@ -253,7 +253,12 @@ static int env_mmc_erase(void)
 	if (!mmc)
 		return CMD_RET_FAILURE;

-	blk = CONFIG_ENV_OFFSET / mmc->read_bl_len;
+#ifdef CONFIG_ENV_OFFSET_REDUND
+	if (use_redund)
+		blk = CONFIG_ENV_OFFSET_REDUND / mmc->read_bl_len;
+	else
+#endif
+		blk = CONFIG_ENV_OFFSET / mmc->read_bl_len;
 	cnt = CONFIG_ENV_SIZE / mmc->read_bl_len;

 	printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d (0x%x)\n",
diff --git a/include/environment.h b/include/environment.h
index de67cf4f0e..a823948da2 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -233,7 +233,7 @@ struct env_driver {
 	 *
 	 * @return 0 if OK, -ve on error
 	 */
-	int (*erase)(void);
+	int (*erase)(bool use_redund);

 	/**
 	 * init() - Set up the initial pre-relocation environment
@@ -318,7 +318,7 @@ int env_save(void);
  *
  * @return 0 if OK, -ve on error
  */
-int env_erase(void);
+int env_erase(bool use_redund);

 /**
  * env_fix_drivers() - Updates envdriver as per relocation
--
2.17.1

[U-Boot] [PATCH v4 4/4] [RFC] env: call env_erase twice if CONFIG_ENV_OFFSET_REDUND is set

[Frank Wunderlich]

erase also the redundant environment location if offset is defined

this is a possible implementation without adding additional parameter
to env erase command

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 cmd/nvedit.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 2071bcf443..f11972e8f1 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -766,8 +766,15 @@ U_BOOT_CMD(
 static int do_env_erase(cmd_tbl_t *cmdtp, int flag, int argc,
 			char * const argv[])
 {
-	return env_erase(false) ? 1 : 0;
+	int ret;
+
+	ret = env_erase(false) ? 1 : 0;
+	#ifdef CONFIG_ENV_OFFSET_REDUND
+		ret = ret || (env_erase(true) ? 1 : 0);
+	#endif
+	return ret;
 }
+
 U_BOOT_CMD(
 	eraseenv, 1, 0,	do_env_erase,
 	"erase environment variables from persistent storage",
--
2.17.1

[U-Boot] [PATCH v4 0/4] add command env erase

[Frank Wunderlich]

Hi,

Just a friendly reminder.

Any comments on my patches?

Regards Frank

Am 28. April 2019 10:51:24 MESZ schrieb Frank Wunderlich <frank-w@public-files.de>:
>sometimes it is needed to erase the non-volatile environment
>e.g. for boot-up with builtin-environment or after resizing env
>
>this series add basic functionality for erasing environment from
>storage
>as a first storage-driver mmc is introduced, other needs to be added
>later
>
>changes since v3:
> - fixes
> - Kconfig-option as suggested by Simon Goldschmidt
> - including CONFIG_ENV_OFFSET_REDUND (4/4 is RFC)
>
>Frank Wunderlich (4):
>  env: register erase command
>  env: mmc: add erase-function
>  env: add option to use redundant offset
>  [RFC] env: call env_erase twice if CONFIG_ENV_OFFSET_REDUND is set
>
> cmd/Kconfig           |  8 ++++++++
> cmd/nvedit.c          | 26 ++++++++++++++++++++++++++
> env/env.c             | 30 ++++++++++++++++++++++++++++++
> env/mmc.c             | 36 ++++++++++++++++++++++++++++++++++++
> include/environment.h | 17 +++++++++++++++++
> 5 files changed, 117 insertions(+)
>
>--
>2.17.1

[U-Boot] [PATCH v4 0/4] add command env erase

[Frank Wunderlich]

no opinion about the last version?

regards Frank

> Am 28. April 2019 10:51:24 MESZ schrieb Frank Wunderlich <frank-w@public-files.de>:
> >sometimes it is needed to erase the non-volatile environment
> >e.g. for boot-up with builtin-environment or after resizing env
> >
> >this series add basic functionality for erasing environment from
> >storage
> >as a first storage-driver mmc is introduced, other needs to be added
> >later
> >
> >changes since v3:
> > - fixes
> > - Kconfig-option as suggested by Simon Goldschmidt
> > - including CONFIG_ENV_OFFSET_REDUND (4/4 is RFC)
> >
> >Frank Wunderlich (4):
> >  env: register erase command
> >  env: mmc: add erase-function
> >  env: add option to use redundant offset
> >  [RFC] env: call env_erase twice if CONFIG_ENV_OFFSET_REDUND is set
> >
> > cmd/Kconfig           |  8 ++++++++
> > cmd/nvedit.c          | 26 ++++++++++++++++++++++++++
> > env/env.c             | 30 ++++++++++++++++++++++++++++++
> > env/mmc.c             | 36 ++++++++++++++++++++++++++++++++++++
> > include/environment.h | 17 +++++++++++++++++
> > 5 files changed, 117 insertions(+)
> >
> >--
> >2.17.1
>

[U-Boot] [PATCH v4 0/4] add command env erase

[Tom Rini]

On Mon, Jun 24, 2019 at 12:30:01PM +0200, Frank Wunderlich wrote:
> no opinion about the last version?
> 
> regards Frank
> 
> > Am 28. April 2019 10:51:24 MESZ schrieb Frank Wunderlich <frank-w@public-files.de>:
> > >sometimes it is needed to erase the non-volatile environment
> > >e.g. for boot-up with builtin-environment or after resizing env
> > >
> > >this series add basic functionality for erasing environment from
> > >storage
> > >as a first storage-driver mmc is introduced, other needs to be added
> > >later
> > >
> > >changes since v3:
> > > - fixes
> > > - Kconfig-option as suggested by Simon Goldschmidt
> > > - including CONFIG_ENV_OFFSET_REDUND (4/4 is RFC)
> > >
> > >Frank Wunderlich (4):
> > >  env: register erase command
> > >  env: mmc: add erase-function
> > >  env: add option to use redundant offset
> > >  [RFC] env: call env_erase twice if CONFIG_ENV_OFFSET_REDUND is set
> > >
> > > cmd/Kconfig           |  8 ++++++++
> > > cmd/nvedit.c          | 26 ++++++++++++++++++++++++++
> > > env/env.c             | 30 ++++++++++++++++++++++++++++++
> > > env/mmc.c             | 36 ++++++++++++++++++++++++++++++++++++
> > > include/environment.h | 17 +++++++++++++++++
> > > 5 files changed, 117 insertions(+)

Seems like a reasonable concept and I believe I looked it over and
everything new is under a CONFIG guard, so, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190624/64eb8181/attachment.sig>

[U-Boot] [PATCH v4 0/4] add command env erase

[Simon Goldschmidt]

Hi Frank,

Am 24.06.2019 um 12:30 schrieb Frank Wunderlich:
> no opinion about the last version?

Sorry, I tried multiple times to review this but failed to find the 
time. Seems like I found the time now ;-)

I'll reply to the original patch mails.

Regards,
Simon

> 
> regards Frank
> 
>> Am 28. April 2019 10:51:24 MESZ schrieb Frank Wunderlich <frank-w@public-files.de>:
>>> sometimes it is needed to erase the non-volatile environment
>>> e.g. for boot-up with builtin-environment or after resizing env
>>>
>>> this series add basic functionality for erasing environment from
>>> storage
>>> as a first storage-driver mmc is introduced, other needs to be added
>>> later
>>>
>>> changes since v3:
>>> - fixes
>>> - Kconfig-option as suggested by Simon Goldschmidt
>>> - including CONFIG_ENV_OFFSET_REDUND (4/4 is RFC)
>>>
>>> Frank Wunderlich (4):
>>>   env: register erase command
>>>   env: mmc: add erase-function
>>>   env: add option to use redundant offset
>>>   [RFC] env: call env_erase twice if CONFIG_ENV_OFFSET_REDUND is set
>>>
>>> cmd/Kconfig           |  8 ++++++++
>>> cmd/nvedit.c          | 26 ++++++++++++++++++++++++++
>>> env/env.c             | 30 ++++++++++++++++++++++++++++++
>>> env/mmc.c             | 36 ++++++++++++++++++++++++++++++++++++
>>> include/environment.h | 17 +++++++++++++++++
>>> 5 files changed, 117 insertions(+)
>>>
>>> --
>>> 2.17.1
>>

[U-Boot] [PATCH v4 1/4] env: register erase command

[Simon Goldschmidt]

Am 28.04.2019 um 10:51 schrieb Frank Wunderlich:
> this patch adds basic changes for adding a erase-subcommand to env
> 
> with this command the environment stored on non-volatile storage written
> by saveenv can be cleared.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

However, due to changes, this doesn't apply any more, so a v5 is needed.

Regards,
Simon

> 
> squashed fixes
>   - start message with "Erasing"
>   - mark erase-function as optional
>   - env: separate eraseenv from saveenv
> 
> Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> =2D--
>   cmd/Kconfig           |  8 ++++++++
>   cmd/nvedit.c          | 19 +++++++++++++++++++
>   env/env.c             | 30 ++++++++++++++++++++++++++++++
>   include/environment.h | 17 +++++++++++++++++
>   4 files changed, 74 insertions(+)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 0b07b3b9d7..e8a99cb5a3 100644
> =2D-- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -397,6 +397,14 @@ config CMD_SAVEENV
>   	  Save all environment variables into the compiled-in persistent
>   	  storage.
> 
> +config CMD_ERASEENV
> +	bool "eraseenv"
> +	default n
> +	depends on CMD_SAVEENV
> +	help
> +	  Erase environment variables from the compiled-in persistent
> +	  storage.
> +
>   config CMD_ENV_EXISTS
>   	bool "env exists"
>   	default y
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 24a6cf7824..0cbd8e8984 100644
> =2D-- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -761,6 +761,19 @@ U_BOOT_CMD(
>   	"save environment variables to persistent storage",
>   	""
>   );
> +
> +#if defined(CONFIG_CMD_ERASEENV)
> +static int do_env_erase(cmd_tbl_t *cmdtp, int flag, int argc,
> +			char * const argv[])
> +{
> +	return env_erase() ? 1 : 0;
> +}
> +U_BOOT_CMD(
> +	eraseenv, 1, 0,	do_env_erase,
> +	"erase environment variables from persistent storage",
> +	""
> +);
> +#endif
>   #endif
>   #endif /* CONFIG_SPL_BUILD */
> 
> @@ -1207,6 +1220,9 @@ static cmd_tbl_t cmd_env_sub[] =3D {
>   #endif
>   #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
>   	U_BOOT_CMD_MKENT(save, 1, 0, do_env_save, "", ""),
> +#if defined(CONFIG_CMD_ERASEENV)
> +	U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""),
> +#endif
>   #endif
>   	U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""),
>   #if defined(CONFIG_CMD_ENV_EXISTS)
> @@ -1282,6 +1298,9 @@ static char env_help_text[] =3D
>   #endif
>   #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
>   	"env save - save environment\n"
> +#if defined(CONFIG_CMD_ERASEENV)
> +	"env erase - erase environment\n"
> +#endif
>   #endif
>   #if defined(CONFIG_CMD_NVEDIT_EFI)
>   	"env set -e name [arg ...] - set UEFI variable; unset if 'arg' not speci=
> fied\n"
> diff --git a/env/env.c b/env/env.c
> index 4b417b90a2..d3cbe2f915 100644
> =2D-- a/env/env.c
> +++ b/env/env.c
> @@ -24,6 +24,8 @@ void env_fix_drivers(void)
>   			entry->load +=3D gd->reloc_off;
>   		if (entry->save)
>   			entry->save +=3D gd->reloc_off;
> +		if (entry->erase)
> +			entry->erase +=3D gd->reloc_off;
>   		if (entry->init)
>   			entry->init +=3D gd->reloc_off;
>   	}
> @@ -254,6 +256,34 @@ int env_save(void)
>   	return -ENODEV;
>   }
> 
> +int env_erase(void)
> +{
> +	struct env_driver *drv;
> +
> +	drv =3D env_driver_lookup(ENVOP_ERASE, gd->env_load_prio);
> +	if (drv) {
> +		int ret;
> +
> +		if (!drv->erase)
> +			return -ENODEV;
> +
> +		if (!env_has_inited(drv->location))
> +			return -ENODEV;
> +
> +		printf("Erasing Environment on %s... ", drv->name);
> +		ret =3D drv->erase();
> +		if (ret)
> +			printf("Failed (%d)\n", ret);
> +		else
> +			printf("OK\n");
> +
> +		if (!ret)
> +			return 0;
> +	}
> +
> +	return -ENODEV;
> +}
> +
>   int env_init(void)
>   {
>   	struct env_driver *drv;
> diff --git a/include/environment.h b/include/environment.h
> index cd96676141..de67cf4f0e 100644
> =2D-- a/include/environment.h
> +++ b/include/environment.h
> @@ -200,6 +200,7 @@ enum env_operation {
>   	ENVOP_INIT,	/* we want to call the init function */
>   	ENVOP_LOAD,	/* we want to call the load function */
>   	ENVOP_SAVE,	/* we want to call the save function */
> +	ENVOP_ERASE,	/* we want to call the erase function */
>   };
> 
>   struct env_driver {
> @@ -225,6 +226,15 @@ struct env_driver {
>   	 */
>   	int (*save)(void);
> 
> +	/**
> +	 * erase() - Erase the environment on storage
> +	 *
> +	 * This method is optional and required for 'eraseenv' to work.
> +	 *
> +	 * @return 0 if OK, -ve on error
> +	 */
> +	int (*erase)(void);
> +
>   	/**
>   	 * init() - Set up the initial pre-relocation environment
>   	 *
> @@ -303,6 +313,13 @@ int env_load(void);
>    */
>   int env_save(void);
> 
> +/**
> + * env_erase() - Erase the environment on storage
> + *
> + * @return 0 if OK, -ve on error
> + */
> +int env_erase(void);
> +
>   /**
>    * env_fix_drivers() - Updates envdriver as per relocation
>    */
> =2D-
> 2.17.1
> 

[U-Boot] [PATCH v4 2/4] env: mmc: add erase-function

[Simon Goldschmidt]

Am 28.04.2019 um 10:51 schrieb Frank Wunderlich:
> this adds erase environment for mmc storage
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

I think this is still too complex.

I'd drop patches 3/4 and 4/4 and just erase both redundant storages here 
in env/mmc.c by calling the actual erase function twice.

To do that, it would help to do as I suggested in response to v3: copy 
the 2-function style used by the save env code, then you can just call 
that 2nd function twice with a different offset (all in env/mmc.c).

Regards,
Simon

> 
> squashed fixes:
>   - fix bogus indent
>   - add CONFIG_CMD_ERASEENV
> 
> Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> =2D--
>   env/mmc.c | 31 +++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/env/mmc.c b/env/mmc.c
> index c3cf35d01b..9ae9b1a66a 100644
> =2D-- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -242,6 +242,34 @@ fini:
>   	fini_mmc_for_env(mmc);
>   	return ret;
>   }
> +
> +#if defined(CONFIG_CMD_ERASEENV)
> +static int env_mmc_erase(void)
> +{
> +	int dev =3D mmc_get_env_dev();
> +	struct mmc *mmc =3D find_mmc_device(dev);
> +	int n, blk, cnt;
> +
> +	if (!mmc)
> +		return CMD_RET_FAILURE;
> +
> +	blk =3D CONFIG_ENV_OFFSET / mmc->read_bl_len;
> +	cnt =3D CONFIG_ENV_SIZE / mmc->read_bl_len;
> +
> +	printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d (0x%x)\n"=
> ,
> +	       dev, blk, blk * mmc->read_bl_len,
> +	       cnt, cnt * mmc->read_bl_len);
> +
> +	if (mmc_getwp(mmc) =3D=3D 1) {
> +		printf("Error: card is write protected!\n");
> +		return CMD_RET_FAILURE;
> +	}
> +	n =3D blk_derase(mmc_get_blk_desc(mmc), blk, cnt);
> +	printf("%d blocks erased: %s\n", n, (n =3D=3D cnt) ? "OK" : "ERROR");
> +
> +	return (n =3D=3D cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> +}
> +#endif /* CONFIG_CMD_ERASEENV */
>   #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */
> 
>   static inline int read_env(struct mmc *mmc, unsigned long size,
> @@ -351,5 +379,8 @@ U_BOOT_ENV_LOCATION(mmc) =3D {
>   	.load		=3D env_mmc_load,
>   #ifndef CONFIG_SPL_BUILD
>   	.save		=3D env_save_ptr(env_mmc_save),
> +#if defined(CONFIG_CMD_ERASEENV)
> +	.erase		=3D env_mmc_erase,
> +#endif
>   #endif
>   };
> =2D-
> 2.17.1
> 

[U-Boot] [PATCH v4 2/4] env: mmc: add erase-function

[Frank Wunderlich]

You mean passing the offset (normal/redundant) instead of a bool?

Am 24. Juni 2019 21:40:36 MESZ schrieb Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>:
>Am 28.04.2019 um 10:51 schrieb Frank Wunderlich:
>> this adds erase environment for mmc storage
>> 
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>
>I think this is still too complex.
>
>I'd drop patches 3/4 and 4/4 and just erase both redundant storages
>here 
>in env/mmc.c by calling the actual erase function twice.
>
>To do that, it would help to do as I suggested in response to v3: copy 
>the 2-function style used by the save env code, then you can just call 
>that 2nd function twice with a different offset (all in env/mmc.c).
>
>Regards,
>Simon
>
>> 
>> squashed fixes:
>>   - fix bogus indent
>>   - add CONFIG_CMD_ERASEENV
>> 
>> Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> =2D--
>>   env/mmc.c | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>> 
>> diff --git a/env/mmc.c b/env/mmc.c
>> index c3cf35d01b..9ae9b1a66a 100644
>> =2D-- a/env/mmc.c
>> +++ b/env/mmc.c
>> @@ -242,6 +242,34 @@ fini:
>>   	fini_mmc_for_env(mmc);
>>   	return ret;
>>   }
>> +
>> +#if defined(CONFIG_CMD_ERASEENV)
>> +static int env_mmc_erase(void)
>> +{
>> +	int dev =3D mmc_get_env_dev();
>> +	struct mmc *mmc =3D find_mmc_device(dev);
>> +	int n, blk, cnt;
>> +
>> +	if (!mmc)
>> +		return CMD_RET_FAILURE;
>> +
>> +	blk =3D CONFIG_ENV_OFFSET / mmc->read_bl_len;
>> +	cnt =3D CONFIG_ENV_SIZE / mmc->read_bl_len;
>> +
>> +	printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d
>(0x%x)\n"=
>> ,
>> +	       dev, blk, blk * mmc->read_bl_len,
>> +	       cnt, cnt * mmc->read_bl_len);
>> +
>> +	if (mmc_getwp(mmc) =3D=3D 1) {
>> +		printf("Error: card is write protected!\n");
>> +		return CMD_RET_FAILURE;
>> +	}
>> +	n =3D blk_derase(mmc_get_blk_desc(mmc), blk, cnt);
>> +	printf("%d blocks erased: %s\n", n, (n =3D=3D cnt) ? "OK" :
>"ERROR");
>> +
>> +	return (n =3D=3D cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
>> +}
>> +#endif /* CONFIG_CMD_ERASEENV */
>>   #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */
>> 
>>   static inline int read_env(struct mmc *mmc, unsigned long size,
>> @@ -351,5 +379,8 @@ U_BOOT_ENV_LOCATION(mmc) =3D {
>>   	.load		=3D env_mmc_load,
>>   #ifndef CONFIG_SPL_BUILD
>>   	.save		=3D env_save_ptr(env_mmc_save),
>> +#if defined(CONFIG_CMD_ERASEENV)
>> +	.erase		=3D env_mmc_erase,
>> +#endif
>>   #endif
>>   };
>> =2D-
>> 2.17.1
>> 

[U-Boot] [PATCH v4 2/4] env: mmc: add erase-function

[Simon Goldschmidt]

Frank Wunderlich <frank-w@public-files.de> schrieb am Mo., 24. Juni 2019,
22:09:

> You mean passing the offset (normal/redundant) instead of a bool?
>

Would you mind keeping the mail style of this list and stop top-responses
please?

I don't care where you make the difference between bool and actual offset,
but you should keep all this as a changeset to env/mmc.c only to keep it
consistent to existing redundant env code. Don't expose the bool to the env
driver interface or the command file.

Regards,
Simon


> Am 24. Juni 2019 21:40:36 MESZ schrieb Simon Goldschmidt <
> simon.k.r.goldschmidt at gmail.com>:
> >Am 28.04.2019 um 10:51 schrieb Frank Wunderlich:
> >> this adds erase environment for mmc storage
> >>
> >> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> >
> >I think this is still too complex.
> >
> >I'd drop patches 3/4 and 4/4 and just erase both redundant storages
> >here
> >in env/mmc.c by calling the actual erase function twice.
> >
> >To do that, it would help to do as I suggested in response to v3: copy
> >the 2-function style used by the save env code, then you can just call
> >that 2nd function twice with a different offset (all in env/mmc.c).
> >
> >Regards,
> >Simon
> >
> >>
> >> squashed fixes:
> >>   - fix bogus indent
> >>   - add CONFIG_CMD_ERASEENV
> >>
> >> Suggested-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >> =2D--
> >>   env/mmc.c | 31 +++++++++++++++++++++++++++++++
> >>   1 file changed, 31 insertions(+)
> >>
> >> diff --git a/env/mmc.c b/env/mmc.c
> >> index c3cf35d01b..9ae9b1a66a 100644
> >> =2D-- a/env/mmc.c
> >> +++ b/env/mmc.c
> >> @@ -242,6 +242,34 @@ fini:
> >>      fini_mmc_for_env(mmc);
> >>      return ret;
> >>   }
> >> +
> >> +#if defined(CONFIG_CMD_ERASEENV)
> >> +static int env_mmc_erase(void)
> >> +{
> >> +    int dev =3D mmc_get_env_dev();
> >> +    struct mmc *mmc =3D find_mmc_device(dev);
> >> +    int n, blk, cnt;
> >> +
> >> +    if (!mmc)
> >> +            return CMD_RET_FAILURE;
> >> +
> >> +    blk =3D CONFIG_ENV_OFFSET / mmc->read_bl_len;
> >> +    cnt =3D CONFIG_ENV_SIZE / mmc->read_bl_len;
> >> +
> >> +    printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d
> >(0x%x)\n"=
> >> ,
> >> +           dev, blk, blk * mmc->read_bl_len,
> >> +           cnt, cnt * mmc->read_bl_len);
> >> +
> >> +    if (mmc_getwp(mmc) =3D=3D 1) {
> >> +            printf("Error: card is write protected!\n");
> >> +            return CMD_RET_FAILURE;
> >> +    }
> >> +    n =3D blk_derase(mmc_get_blk_desc(mmc), blk, cnt);
> >> +    printf("%d blocks erased: %s\n", n, (n =3D=3D cnt) ? "OK" :
> >"ERROR");
> >> +
> >> +    return (n =3D=3D cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> >> +}
> >> +#endif /* CONFIG_CMD_ERASEENV */
> >>   #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */
> >>
> >>   static inline int read_env(struct mmc *mmc, unsigned long size,
> >> @@ -351,5 +379,8 @@ U_BOOT_ENV_LOCATION(mmc) =3D {
> >>      .load           =3D env_mmc_load,
> >>   #ifndef CONFIG_SPL_BUILD
> >>      .save           =3D env_save_ptr(env_mmc_save),
> >> +#if defined(CONFIG_CMD_ERASEENV)
> >> +    .erase          =3D env_mmc_erase,
> >> +#endif
> >>   #endif
> >>   };
> >> =2D-
> >> 2.17.1
> >>
>