[PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Marek Vasut]

Add test for environment stored in SPI NOR. The test works in a very
similar way to the current test for environment stored in ext4 FS,
except it generates spi.bin file backing the SPI NOR.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jerome Forissier <jerome.forissier@linaro.org>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
V2: No change
---
 test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index 383e26c03b0..48e31f19b3c 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
     utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
     return fs_img
 
+def mk_env_spi_flash(state_test_env):
+
+    """Create an empty SPI NOR image."""
+    c = state_test_env.ubman
+    filename = 'spi.bin'
+    persistent = c.config.persistent_data_dir + '/' + filename
+    spi_flash_img = c.config.source_dir  + '/' + filename
+
+    if os.path.exists(persistent):
+        c.log.action('SPI NOR image file ' + persistent + ' already exists')
+    else:
+        try:
+            utils.run_and_log(c, 'dd if=/dev/zero of=%s bs=1M count=2' % persistent)
+        except CalledProcessError:
+            call('rm -f %s' % persistent, shell=True)
+            raise
+
+    utils.run_and_log(c, ['cp',  '-f', persistent, spi_flash_img])
+    return spi_flash_img
+
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_echo')
 @pytest.mark.buildconfigspec('cmd_nvedit_info')
@@ -544,6 +564,85 @@ def test_env_ext4(state_test_env):
         if fs_img:
             call('rm -f %s' % fs_img, shell=True)
 
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_echo')
+@pytest.mark.buildconfigspec('cmd_nvedit_info')
+@pytest.mark.buildconfigspec('cmd_nvedit_load')
+@pytest.mark.buildconfigspec('cmd_nvedit_select')
+@pytest.mark.buildconfigspec('env_is_in_spi_flash')
+def test_env_spi_flash(state_test_env):
+
+    """Test ENV in SPI NOR on sandbox."""
+    c = state_test_env.ubman
+    spi_flash_img = ''
+    try:
+        spi_flash_img = mk_env_spi_flash(state_test_env)
+
+        # force env location: SF
+        response = c.run_command('env select SPIFlash')
+        assert 'Select Environment on SPIFlash: OK' in response
+
+        response = c.run_command('env save')
+        assert 'Saving Environment to SPIFlash' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from SPIFlash... OK' in response
+
+        response = c.run_command('env info')
+        assert 'env_valid = valid' in response
+        assert 'env_ready = true' in response
+        assert 'env_use_default = false' in response
+
+        response = c.run_command('env info -p -d')
+        assert 'Environment was loaded from persistent storage' in response
+        assert 'Environment can be persisted' in response
+
+        response = c.run_command('env info -d -q')
+        assert response == ""
+        response = c.run_command('echo $?')
+        assert response == "1"
+
+        response = c.run_command('env info -p -q')
+        assert response == ""
+        response = c.run_command('echo $?')
+        assert response == "0"
+
+        response = c.run_command('env erase')
+        assert 'OK' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from SPIFlash... ' in response
+        assert 'bad CRC, using default environment' in response
+
+        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 can be persisted' in response
+
+        # restore env location: NOWHERE (prio 0 in sandbox)
+        response = c.run_command('env select nowhere')
+        assert 'Select Environment on nowhere: OK' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from nowhere... OK' in response
+
+        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
+
+    finally:
+        if spi_flash_img:
+            call('rm -f %s' % spi_flash_img, shell=True)
+
 def test_env_text(ubman):
     """Test the script that converts the environment to a text file"""
 
-- 
2.51.0

[PATCH v2 2/3] env: Add single to redundant environment upgrade path

[Marek Vasut]

Add support for converting single-copy environment to redundant environment.
In case CRC checks on both redundant environment copies fail, try one more
CRC check on the primary environment copy and treat it as single environment.
If that check does pass, rewrite the single-copy environment into redundant
environment format, indicate the environment is valid, and import that as
usual primary copy of redundant environment. Follow up 'env save' will then
store two environment copies and the system will continue to operate as
regular redundant environment system.

Add test which validates this upgrade path. The test starts with spi.bin
which is pre-populated as single-copy environment and then upgrades that
environment to dual-copy environment.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jerome Forissier <jerome.forissier@linaro.org>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
V2: - Gate the option behind ENV_REDUNDANT_UPGRADE
    - Fix up mkenvimage path in env test
---
 env/Kconfig               | 11 ++++++
 env/common.c              | 31 +++++++++++++++-
 test/py/tests/test_env.py | 74 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/env/Kconfig b/env/Kconfig
index 4430669964c..b312f9b5324 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -489,6 +489,17 @@ config ENV_REDUNDANT
 	  which is used by env import/export commands which are independent of
 	  storing variables to redundant location on a non volatile device.
 
+config ENV_REDUNDANT_UPGRADE
+	bool "Enable single-copy to redundant environment upgrade support"
+	depends on ENV_REDUNDANT
+	help
+	  Normally, redundant environment is expected to always operate on
+	  two copies of the environment. However, hardware that may have
+	  originally shipped with single-copy environment can be upgraded
+	  to redundant environment without loss of existing environment
+	  content by correctly configuring the location of the redundant
+	  environment copy and by enabling this option.
+
 config ENV_FAT_INTERFACE
 	string "Name of the block device for the environment"
 	depends on ENV_IS_IN_FAT
diff --git a/env/common.c b/env/common.c
index 05e78d63874..b2adbe93dbe 100644
--- a/env/common.c
+++ b/env/common.c
@@ -473,14 +473,24 @@ int env_import(const char *buf, int check, int flags)
 #ifdef CONFIG_ENV_REDUNDANT
 static unsigned char env_flags;
 
+#define ENV_SINGLE_HEADER_SIZE	(sizeof(uint32_t))
+#define ENV_SINGLE_SIZE		(CONFIG_ENV_SIZE - ENV_SINGLE_HEADER_SIZE)
+
+typedef struct {
+	uint32_t	crc;			/* CRC32 over data bytes */
+	unsigned char	data[ENV_SINGLE_SIZE];	/* Environment data */
+} env_single_t;
+
 int env_check_redund(const char *buf1, int buf1_read_fail,
 		     const char *buf2, int buf2_read_fail)
 {
-	int crc1_ok = 0, crc2_ok = 0;
+	int crc1_ok = 0, crc2_ok = 0, i;
 	env_t *tmp_env1, *tmp_env2;
+	env_single_t *tmp_envs;
 
 	tmp_env1 = (env_t *)buf1;
 	tmp_env2 = (env_t *)buf2;
+	tmp_envs = (env_single_t *)buf1;
 
 	if (buf1_read_fail && buf2_read_fail) {
 		puts("*** Error - No Valid Environment Area found\n");
@@ -498,6 +508,25 @@ int env_check_redund(const char *buf1, int buf1_read_fail,
 				tmp_env2->crc;
 
 	if (!crc1_ok && !crc2_ok) {
+		/*
+		 * Upgrade single-copy environment to redundant environment.
+		 * In case CRC checks on both environment copies fail, try
+		 * one more CRC check on the primary environment copy and
+		 * treat it as single-copy environment. If that check does
+		 * pass, rewrite the single-copy environment into redundant
+		 * environment format and indicate the environment is valid.
+		 * The follow up calls will import the environment as if it
+		 * was a redundant environment. Follow up 'env save' will
+		 * then store two environment copies.
+		 */
+		if (CONFIG_IS_ENABLED(ENV_REDUNDANT_UPGRADE) && !buf1_read_fail &&
+		    crc32(0, tmp_envs->data, ENV_SINGLE_SIZE) == tmp_envs->crc) {
+			for (i = ENV_SIZE - 1; i >= 0; i--)
+				tmp_env1->data[i] = tmp_envs->data[i];
+			tmp_env1->flags = 0;
+			gd->env_valid = ENV_VALID;
+			return 0;
+		}
 		gd->env_valid = ENV_INVALID;
 		return -ENOMSG; /* needed for env_load() */
 	} else if (crc1_ok && !crc2_ok) {
diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index 48e31f19b3c..f8713a59ba9 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -477,6 +477,22 @@ def mk_env_spi_flash(state_test_env):
     utils.run_and_log(c, ['cp',  '-f', persistent, spi_flash_img])
     return spi_flash_img
 
+def mk_env_spi_flash_single(state_test_env):
+
+    """Create an single-copy SPI NOR image with foo=bar entry."""
+    c = state_test_env.ubman
+    filename = 'spi.bin'
+    spi_flash_img = c.config.source_dir  + '/' + filename
+
+    try:
+        mkenvimage = os.path.join(c.config.build_dir, 'tools/mkenvimage')
+        call('( echo foo=bar | %s -s 8192 -p 0x00 - ; dd if=/dev/zero bs=2088960 count=1 2>/dev/null ) > %s' % ( mkenvimage , spi_flash_img ), shell=True)
+    except CalledProcessError:
+        call('rm -f %s' % spi_flash_img, shell=True)
+        raise
+
+    return spi_flash_img
+
 @pytest.mark.boardspec('sandbox')
 @pytest.mark.buildconfigspec('cmd_echo')
 @pytest.mark.buildconfigspec('cmd_nvedit_info')
@@ -574,6 +590,64 @@ def test_env_spi_flash(state_test_env):
 
     """Test ENV in SPI NOR on sandbox."""
     c = state_test_env.ubman
+    spi_flash_img = ''
+    try:
+        spi_flash_img = mk_env_spi_flash_single(state_test_env)
+
+        response = c.run_command('sf probe')
+        assert 'SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB' in response
+
+        # force env location: SF
+        response = c.run_command('env select SPIFlash')
+        assert 'Select Environment on SPIFlash: OK' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from SPIFlash... OK' in response
+
+        response = c.run_command('env print foo')
+        assert 'foo=bar' in response
+
+        response = c.run_command('env save')
+        assert 'Saving Environment to SPIFlash' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from SPIFlash... OK' in response
+
+        response = c.run_command('env print foo')
+        assert 'foo=bar' in response
+
+        response = c.run_command('env save')
+        assert 'Saving Environment to SPIFlash' in response
+
+        response = c.run_command('env save')
+        assert 'Saving Environment to SPIFlash' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from SPIFlash... OK' in response
+
+        response = c.run_command('env print foo')
+        assert 'foo=bar' in response
+
+        # restore env location: NOWHERE (prio 0 in sandbox)
+        response = c.run_command('env select nowhere')
+        assert 'Select Environment on nowhere: OK' in response
+
+        response = c.run_command('env load')
+        assert 'Loading Environment from nowhere... OK' in response
+
+        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
+
+    finally:
+        if spi_flash_img:
+            call('rm -f %s' % spi_flash_img, shell=True)
+
     spi_flash_img = ''
     try:
         spi_flash_img = mk_env_spi_flash(state_test_env)
-- 
2.51.0

Re: [PATCH v2 2/3] env: Add single to redundant environment upgrade path

[Heinrich Schuchardt]

On 12/23/25 15:31, Marek Vasut wrote:
> Add support for converting single-copy environment to redundant environment.
> In case CRC checks on both redundant environment copies fail, try one more
> CRC check on the primary environment copy and treat it as single environment.

Why would a CRC check suddenly succeed if it has failed before?

This needs some more explanation.

> If that check does pass, rewrite the single-copy environment into redundant
> environment format, indicate the environment is valid, and import that as
> usual primary copy of redundant environment. Follow up 'env save' will then
> store two environment copies and the system will continue to operate as
> regular redundant environment system.
> 
> Add test which validates this upgrade path. The test starts with spi.bin
> which is pre-populated as single-copy environment and then upgrades that
> environment to dual-copy environment.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jerome Forissier <jerome.forissier@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
> V2: - Gate the option behind ENV_REDUNDANT_UPGRADE
>      - Fix up mkenvimage path in env test
> ---
>   env/Kconfig               | 11 ++++++
>   env/common.c              | 31 +++++++++++++++-
>   test/py/tests/test_env.py | 74 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 115 insertions(+), 1 deletion(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index 4430669964c..b312f9b5324 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -489,6 +489,17 @@ config ENV_REDUNDANT
>   	  which is used by env import/export commands which are independent of
>   	  storing variables to redundant location on a non volatile device.
>   
> +config ENV_REDUNDANT_UPGRADE
> +	bool "Enable single-copy to redundant environment upgrade support"
> +	depends on ENV_REDUNDANT
> +	help
> +	  Normally, redundant environment is expected to always operate on
> +	  two copies of the environment. However, hardware that may have
> +	  originally shipped with single-copy environment can be upgraded

%s/with single-copy/with a single-copy/
%s/can be/that can be/

> +	  to redundant environment without loss of existing environment
> +	  content by correctly configuring the location of the redundant
> +	  environment copy and by enabling this option.

Why do we have to make this an option?
Shouldn't we always try to restore the environment?

> +
>   config ENV_FAT_INTERFACE
>   	string "Name of the block device for the environment"
>   	depends on ENV_IS_IN_FAT
> diff --git a/env/common.c b/env/common.c
> index 05e78d63874..b2adbe93dbe 100644
> --- a/env/common.c
> +++ b/env/common.c
> @@ -473,14 +473,24 @@ int env_import(const char *buf, int check, int flags)
>   #ifdef CONFIG_ENV_REDUNDANT
>   static unsigned char env_flags;
>   
> +#define ENV_SINGLE_HEADER_SIZE	(sizeof(uint32_t))
> +#define ENV_SINGLE_SIZE		(CONFIG_ENV_SIZE - ENV_SINGLE_HEADER_SIZE)
> +
> +typedef struct {
> +	uint32_t	crc;			/* CRC32 over data bytes */
> +	unsigned char	data[ENV_SINGLE_SIZE];	/* Environment data */
> +} env_single_t;
> +
>   int env_check_redund(const char *buf1, int buf1_read_fail,
>   		     const char *buf2, int buf2_read_fail)
>   {
> -	int crc1_ok = 0, crc2_ok = 0;
> +	int crc1_ok = 0, crc2_ok = 0, i;
>   	env_t *tmp_env1, *tmp_env2;
> +	env_single_t *tmp_envs;
>   
>   	tmp_env1 = (env_t *)buf1;
>   	tmp_env2 = (env_t *)buf2;
> +	tmp_envs = (env_single_t *)buf1;
>   
>   	if (buf1_read_fail && buf2_read_fail) {
>   		puts("*** Error - No Valid Environment Area found\n");
> @@ -498,6 +508,25 @@ int env_check_redund(const char *buf1, int buf1_read_fail,
>   				tmp_env2->crc;
>   
>   	if (!crc1_ok && !crc2_ok) {

Do we really have a third location to copy from when both store 1 and 
store 2 are defective? I would have expected that if a vendor provides a 
single copy then exactly one of crc1_ok or crc2_ok is true and the other 
is false.

Please, provide a documentation update explaining how this all works.

Best regards

Heinrich

> +		/*
> +		 * Upgrade single-copy environment to redundant environment.
> +		 * In case CRC checks on both environment copies fail, try
> +		 * one more CRC check on the primary environment copy and
> +		 * treat it as single-copy environment. If that check does
> +		 * pass, rewrite the single-copy environment into redundant
> +		 * environment format and indicate the environment is valid.
> +		 * The follow up calls will import the environment as if it
> +		 * was a redundant environment. Follow up 'env save' will
> +		 * then store two environment copies.
> +		 */
> +		if (CONFIG_IS_ENABLED(ENV_REDUNDANT_UPGRADE) && !buf1_read_fail &&
> +		    crc32(0, tmp_envs->data, ENV_SINGLE_SIZE) == tmp_envs->crc) {
> +			for (i = ENV_SIZE - 1; i >= 0; i--)
> +				tmp_env1->data[i] = tmp_envs->data[i];
> +			tmp_env1->flags = 0;
> +			gd->env_valid = ENV_VALID;
> +			return 0;
> +		}
>   		gd->env_valid = ENV_INVALID;
>   		return -ENOMSG; /* needed for env_load() */
>   	} else if (crc1_ok && !crc2_ok) {
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> index 48e31f19b3c..f8713a59ba9 100644
> --- a/test/py/tests/test_env.py
> +++ b/test/py/tests/test_env.py
> @@ -477,6 +477,22 @@ def mk_env_spi_flash(state_test_env):
>       utils.run_and_log(c, ['cp',  '-f', persistent, spi_flash_img])
>       return spi_flash_img
>   
> +def mk_env_spi_flash_single(state_test_env):
> +
> +    """Create an single-copy SPI NOR image with foo=bar entry."""
> +    c = state_test_env.ubman
> +    filename = 'spi.bin'
> +    spi_flash_img = c.config.source_dir  + '/' + filename
> +
> +    try:
> +        mkenvimage = os.path.join(c.config.build_dir, 'tools/mkenvimage')
> +        call('( echo foo=bar | %s -s 8192 -p 0x00 - ; dd if=/dev/zero bs=2088960 count=1 2>/dev/null ) > %s' % ( mkenvimage , spi_flash_img ), shell=True)
> +    except CalledProcessError:
> +        call('rm -f %s' % spi_flash_img, shell=True)
> +        raise
> +
> +    return spi_flash_img
> +
>   @pytest.mark.boardspec('sandbox')
>   @pytest.mark.buildconfigspec('cmd_echo')
>   @pytest.mark.buildconfigspec('cmd_nvedit_info')
> @@ -574,6 +590,64 @@ def test_env_spi_flash(state_test_env):
>   
>       """Test ENV in SPI NOR on sandbox."""
>       c = state_test_env.ubman
> +    spi_flash_img = ''
> +    try:
> +        spi_flash_img = mk_env_spi_flash_single(state_test_env)
> +
> +        response = c.run_command('sf probe')
> +        assert 'SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB' in response
> +
> +        # force env location: SF
> +        response = c.run_command('env select SPIFlash')
> +        assert 'Select Environment on SPIFlash: OK' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... OK' in response
> +
> +        response = c.run_command('env print foo')
> +        assert 'foo=bar' in response
> +
> +        response = c.run_command('env save')
> +        assert 'Saving Environment to SPIFlash' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... OK' in response
> +
> +        response = c.run_command('env print foo')
> +        assert 'foo=bar' in response
> +
> +        response = c.run_command('env save')
> +        assert 'Saving Environment to SPIFlash' in response
> +
> +        response = c.run_command('env save')
> +        assert 'Saving Environment to SPIFlash' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... OK' in response
> +
> +        response = c.run_command('env print foo')
> +        assert 'foo=bar' in response
> +
> +        # restore env location: NOWHERE (prio 0 in sandbox)
> +        response = c.run_command('env select nowhere')
> +        assert 'Select Environment on nowhere: OK' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from nowhere... OK' in response
> +
> +        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
> +
> +    finally:
> +        if spi_flash_img:
> +            call('rm -f %s' % spi_flash_img, shell=True)
> +
>       spi_flash_img = ''
>       try:
>           spi_flash_img = mk_env_spi_flash(state_test_env)

Re: [PATCH v2 2/3] env: Add single to redundant environment upgrade path

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3039 bytes --]

On Tue, Dec 23, 2025 at 06:43:00PM +0100, Heinrich Schuchardt wrote:
> On 12/23/25 15:31, Marek Vasut wrote:
> > Add support for converting single-copy environment to redundant environment.
> > In case CRC checks on both redundant environment copies fail, try one more
> > CRC check on the primary environment copy and treat it as single environment.
> 
> Why would a CRC check suddenly succeed if it has failed before?
> 
> This needs some more explanation.
> 
> > If that check does pass, rewrite the single-copy environment into redundant
> > environment format, indicate the environment is valid, and import that as
> > usual primary copy of redundant environment. Follow up 'env save' will then
> > store two environment copies and the system will continue to operate as
> > regular redundant environment system.
> > 
> > Add test which validates this upgrade path. The test starts with spi.bin
> > which is pre-populated as single-copy environment and then upgrades that
> > environment to dual-copy environment.
> > 
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > ---
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Jerome Forissier <jerome.forissier@linaro.org>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: u-boot@lists.denx.de
> > ---
> > V2: - Gate the option behind ENV_REDUNDANT_UPGRADE
> >      - Fix up mkenvimage path in env test
> > ---
> >   env/Kconfig               | 11 ++++++
> >   env/common.c              | 31 +++++++++++++++-
> >   test/py/tests/test_env.py | 74 +++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 115 insertions(+), 1 deletion(-)
> > 
> > diff --git a/env/Kconfig b/env/Kconfig
> > index 4430669964c..b312f9b5324 100644
> > --- a/env/Kconfig
> > +++ b/env/Kconfig
> > @@ -489,6 +489,17 @@ config ENV_REDUNDANT
> >   	  which is used by env import/export commands which are independent of
> >   	  storing variables to redundant location on a non volatile device.
> > +config ENV_REDUNDANT_UPGRADE
> > +	bool "Enable single-copy to redundant environment upgrade support"
> > +	depends on ENV_REDUNDANT
> > +	help
> > +	  Normally, redundant environment is expected to always operate on
> > +	  two copies of the environment. However, hardware that may have
> > +	  originally shipped with single-copy environment can be upgraded
> 
> %s/with single-copy/with a single-copy/
> %s/can be/that can be/
> 
> > +	  to redundant environment without loss of existing environment
> > +	  content by correctly configuring the location of the redundant
> > +	  environment copy and by enabling this option.
> 
> Why do we have to make this an option?
> Shouldn't we always try to restore the environment?

Because the use case here is (I believe) converting a platform which had
non-redundant environment to a functional redundant environment. Without
making this optional we get size growth on platforms which enable
redundant env today (see v1).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/3] env: Add single to redundant environment upgrade path

[Marek Vasut]

On 12/23/25 6:43 PM, Heinrich Schuchardt wrote:
> On 12/23/25 15:31, Marek Vasut wrote:
>> Add support for converting single-copy environment to redundant 
>> environment.
>> In case CRC checks on both redundant environment copies fail, try one 
>> more
>> CRC check on the primary environment copy and treat it as single 
>> environment.
> 
> Why would a CRC check suddenly succeed if it has failed before?

The previous CRC check was for redundant env configuration, the follow 
up test is for a single-copy environment. This is used for single-copy 
env to redundant env upgrade path.

> This needs some more explanation.

Please read the full commit message, I hope the rest of it clarifies 
what is going on here.

>> If that check does pass, rewrite the single-copy environment into 
>> redundant
>> environment format, indicate the environment is valid, and import that as
>> usual primary copy of redundant environment. Follow up 'env save' will 
>> then
>> store two environment copies and the system will continue to operate as
>> regular redundant environment system.
>>
>> Add test which validates this upgrade path. The test starts with spi.bin
>> which is pre-populated as single-copy environment and then upgrades that
>> environment to dual-copy environment.

[...]

>> +++ b/env/common.c
>> @@ -473,14 +473,24 @@ int env_import(const char *buf, int check, int 
>> flags)
>>   #ifdef CONFIG_ENV_REDUNDANT
>>   static unsigned char env_flags;
>> +#define ENV_SINGLE_HEADER_SIZE    (sizeof(uint32_t))
>> +#define ENV_SINGLE_SIZE        (CONFIG_ENV_SIZE - 
>> ENV_SINGLE_HEADER_SIZE)
>> +
>> +typedef struct {
>> +    uint32_t    crc;            /* CRC32 over data bytes */
>> +    unsigned char    data[ENV_SINGLE_SIZE];    /* Environment data */
>> +} env_single_t;
>> +
>>   int env_check_redund(const char *buf1, int buf1_read_fail,
>>                const char *buf2, int buf2_read_fail)
>>   {
>> -    int crc1_ok = 0, crc2_ok = 0;
>> +    int crc1_ok = 0, crc2_ok = 0, i;
>>       env_t *tmp_env1, *tmp_env2;
>> +    env_single_t *tmp_envs;
>>       tmp_env1 = (env_t *)buf1;
>>       tmp_env2 = (env_t *)buf2;
>> +    tmp_envs = (env_single_t *)buf1;
>>       if (buf1_read_fail && buf2_read_fail) {
>>           puts("*** Error - No Valid Environment Area found\n");
>> @@ -498,6 +508,25 @@ int env_check_redund(const char *buf1, int 
>> buf1_read_fail,
>>                   tmp_env2->crc;
>>       if (!crc1_ok && !crc2_ok) {
> 
> Do we really have a third location to copy from when both store 1 and 
> store 2 are defective? I would have expected that if a vendor provides a 
> single copy then exactly one of crc1_ok or crc2_ok is true and the other 
> is false.
> 
> Please, provide a documentation update explaining how this all works.
This implements upgrade path from single-copy env to redundant env, see 
also what Tom wrote. So no, there is no third copy, there is only 
primary copy which is single-copy env, and that primary copy is upgraded 
to redundant (dual-copy) env.

[PATCH v2 3/3] configs: sandbox: Enable environment in SPI NOR support

[Marek Vasut]

Make environment support in SPI NOR available in sandbox,
so the environment storage in SPI NOR can be tested in CI.
Enable redundant environment support as well to cover this
in CI tests too.

Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jerome Forissier <jerome.forissier@linaro.org>
Cc: Simon Glass <sjg@chromium.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
V2: Enable ENV_REDUNDANT_UPGRADE
---
 board/sandbox/sandbox.c     | 1 +
 configs/sandbox64_defconfig | 7 +++++++
 configs/sandbox_defconfig   | 7 +++++++
 3 files changed, 15 insertions(+)

diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index d0bb3e3bb48..13006a0ffc2 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -89,6 +89,7 @@ static enum env_location env_locations[] = {
 	ENVL_NOWHERE,
 	ENVL_EXT4,
 	ENVL_FAT,
+	ENVL_SPI_FLASH,
 };
 
 enum env_location env_get_location(enum env_operation op, int prio)
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 70c757640c0..22de4acbd88 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -2,10 +2,13 @@ CONFIG_TEXT_BASE=0
 CONFIG_SYS_MALLOC_LEN=0x6000000
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x2000
+CONFIG_ENV_OFFSET=0x0
+CONFIG_ENV_SECT_SIZE=0x1000
 CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
 CONFIG_DM_RESET=y
 CONFIG_SYS_LOAD_ADDR=0x0
 CONFIG_PRE_CON_BUF_ADDR=0x100000
+CONFIG_ENV_OFFSET_REDUND=0x10000
 CONFIG_PCI=y
 CONFIG_SANDBOX64=y
 CONFIG_DEBUG_UART=y
@@ -106,6 +109,10 @@ CONFIG_OF_LIVE=y
 CONFIG_ENV_IS_NOWHERE=y
 CONFIG_ENV_IS_IN_EXT4=y
 CONFIG_ENV_IS_IN_FAT=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_ENV_SECT_SIZE_AUTO=y
+CONFIG_ENV_REDUNDANT=y
+CONFIG_ENV_REDUNDANT_UPGRADE=y
 CONFIG_ENV_EXT4_INTERFACE="host"
 CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_ENV_IMPORT_FDT=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index dfdaaff1eff..97f5d5dc074 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -2,9 +2,12 @@ CONFIG_TEXT_BASE=0
 CONFIG_SYS_MALLOC_LEN=0x6000000
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x2000
+CONFIG_ENV_OFFSET=0x0
+CONFIG_ENV_SECT_SIZE=0x1000
 CONFIG_DM_RESET=y
 CONFIG_SYS_LOAD_ADDR=0x0
 CONFIG_PRE_CON_BUF_ADDR=0xf0000
+CONFIG_ENV_OFFSET_REDUND=0x10000
 CONFIG_PCI=y
 CONFIG_DEBUG_UART=y
 CONFIG_SYS_MEMTEST_START=0x00100000
@@ -154,6 +157,10 @@ CONFIG_OF_LIVE=y
 CONFIG_ENV_IS_NOWHERE=y
 CONFIG_ENV_IS_IN_EXT4=y
 CONFIG_ENV_IS_IN_FAT=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_ENV_SECT_SIZE_AUTO=y
+CONFIG_ENV_REDUNDANT=y
+CONFIG_ENV_REDUNDANT_UPGRADE=y
 CONFIG_ENV_EXT4_INTERFACE="host"
 CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
 CONFIG_ENV_IMPORT_FDT=y
-- 
2.51.0

Re: [PATCH v2 3/3] configs: sandbox: Enable environment in SPI NOR support

[Heinrich Schuchardt]

On 12/23/25 15:31, Marek Vasut wrote:
> Make environment support in SPI NOR available in sandbox,
> so the environment storage in SPI NOR can be tested in CI.
> Enable redundant environment support as well to cover this
> in CI tests too.

Applying: configs: sandbox: Enable environment in SPI NOR support
error: patch failed: configs/sandbox64_defconfig:106
error: configs/sandbox64_defconfig: patch does not apply
error: patch failed: configs/sandbox_defconfig:154
error: configs/sandbox_defconfig: patch does not apply
Patch failed at 0001 configs: sandbox: Enable environment in SPI NOR support


@Simon:

Can we move the sandbox documentation to doc/board/. This is where all 
other architectures are described. I has a hard time finding that 
backing file is spi.bin for this functionality.

The sandbox driver should properly detect the SPI size.

$ truncate -s 8M spi.bin
=> sf probe
SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB

$ truncate -s 1M spi.bin
=> sf probe
SF: Detected m25p16 with page size 256 Bytes, erase size 64 KiB, total 2 MiB

> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jerome Forissier <jerome.forissier@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
> V2: Enable ENV_REDUNDANT_UPGRADE
> ---
>   board/sandbox/sandbox.c     | 1 +
>   configs/sandbox64_defconfig | 7 +++++++
>   configs/sandbox_defconfig   | 7 +++++++
>   3 files changed, 15 insertions(+)
> 
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index d0bb3e3bb48..13006a0ffc2 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -89,6 +89,7 @@ static enum env_location env_locations[] = {
>   	ENVL_NOWHERE,
>   	ENVL_EXT4,
>   	ENVL_FAT,
> +	ENVL_SPI_FLASH,
>   };
>   
>   enum env_location env_get_location(enum env_operation op, int prio)
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index 70c757640c0..22de4acbd88 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -2,10 +2,13 @@ CONFIG_TEXT_BASE=0
>   CONFIG_SYS_MALLOC_LEN=0x6000000
>   CONFIG_NR_DRAM_BANKS=1
>   CONFIG_ENV_SIZE=0x2000
> +CONFIG_ENV_OFFSET=0x0
> +CONFIG_ENV_SECT_SIZE=0x1000
>   CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
>   CONFIG_DM_RESET=y
>   CONFIG_SYS_LOAD_ADDR=0x0
>   CONFIG_PRE_CON_BUF_ADDR=0x100000
> +CONFIG_ENV_OFFSET_REDUND=0x10000
>   CONFIG_PCI=y
>   CONFIG_SANDBOX64=y
>   CONFIG_DEBUG_UART=y
> @@ -106,6 +109,10 @@ CONFIG_OF_LIVE=y
>   CONFIG_ENV_IS_NOWHERE=y
>   CONFIG_ENV_IS_IN_EXT4=y
>   CONFIG_ENV_IS_IN_FAT=y

This line does not exist

> +CONFIG_ENV_IS_IN_SPI_FLASH=y
> +CONFIG_ENV_SECT_SIZE_AUTO=y
> +CONFIG_ENV_REDUNDANT=y
> +CONFIG_ENV_REDUNDANT_UPGRADE=y
>   CONFIG_ENV_EXT4_INTERFACE="host"
>   CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
>   CONFIG_ENV_IMPORT_FDT=y
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index dfdaaff1eff..97f5d5dc074 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -2,9 +2,12 @@ CONFIG_TEXT_BASE=0
>   CONFIG_SYS_MALLOC_LEN=0x6000000
>   CONFIG_NR_DRAM_BANKS=1
>   CONFIG_ENV_SIZE=0x2000
> +CONFIG_ENV_OFFSET=0x0
> +CONFIG_ENV_SECT_SIZE=0x1000
>   CONFIG_DM_RESET=y
>   CONFIG_SYS_LOAD_ADDR=0x0
>   CONFIG_PRE_CON_BUF_ADDR=0xf0000
> +CONFIG_ENV_OFFSET_REDUND=0x10000
>   CONFIG_PCI=y
>   CONFIG_DEBUG_UART=y
>   CONFIG_SYS_MEMTEST_START=0x00100000
> @@ -154,6 +157,10 @@ CONFIG_OF_LIVE=y
>   CONFIG_ENV_IS_NOWHERE=y
>   CONFIG_ENV_IS_IN_EXT4=y
>   CONFIG_ENV_IS_IN_FAT=y

This line does not exist.

Best regards

Heinrich

> +CONFIG_ENV_IS_IN_SPI_FLASH=y
> +CONFIG_ENV_SECT_SIZE_AUTO=y
> +CONFIG_ENV_REDUNDANT=y
> +CONFIG_ENV_REDUNDANT_UPGRADE=y
>   CONFIG_ENV_EXT4_INTERFACE="host"
>   CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
>   CONFIG_ENV_IMPORT_FDT=y

Re: [PATCH v2 3/3] configs: sandbox: Enable environment in SPI NOR support

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Tue, Dec 23, 2025 at 06:58:13PM +0100, Heinrich Schuchardt wrote:
> On 12/23/25 15:31, Marek Vasut wrote:
> > Make environment support in SPI NOR available in sandbox,
> > so the environment storage in SPI NOR can be tested in CI.
> > Enable redundant environment support as well to cover this
> > in CI tests too.
> 
> Applying: configs: sandbox: Enable environment in SPI NOR support
> error: patch failed: configs/sandbox64_defconfig:106
> error: configs/sandbox64_defconfig: patch does not apply
> error: patch failed: configs/sandbox_defconfig:154
> error: configs/sandbox_defconfig: patch does not apply
> Patch failed at 0001 configs: sandbox: Enable environment in SPI NOR support

This depends on the FAT fix that Marek posted around v1 timeframe.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Heinrich Schuchardt]

On 12/23/25 15:31, Marek Vasut wrote:
> Add test for environment stored in SPI NOR. The test works in a very
> similar way to the current test for environment stored in ext4 FS,
> except it generates spi.bin file backing the SPI NOR.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jerome Forissier <jerome.forissier@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
> V2: No change
> ---
>   test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 99 insertions(+)
> 
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> index 383e26c03b0..48e31f19b3c 100644
> --- a/test/py/tests/test_env.py
> +++ b/test/py/tests/test_env.py
> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>       utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>       return fs_img
>   
> +def mk_env_spi_flash(state_test_env):

Thank you for adding the test.

Unfortunately pylint doesn't like your code. Please have a look.

W0621: Redefining name 'state_test_env' from outer scope (line 95) 
(redefined-outer-name)

> +
> +    """Create an empty SPI NOR image."""
> +    c = state_test_env.ubman
> +    filename = 'spi.bin'
> +    persistent = c.config.persistent_data_dir + '/' + filename
> +    spi_flash_img = c.config.source_dir  + '/' + filename
> +
> +    if os.path.exists(persistent):
> +        c.log.action('SPI NOR image file ' + persistent + ' already exists')
> +    else:
> +        try:
> +            utils.run_and_log(c, 'dd if=/dev/zero of=%s bs=1M count=2' % persistent)

C0209: Formatting a regular string which could be an f-string 
(consider-using-f-string)

> +        except CalledProcessError:
> +            call('rm -f %s' % persistent, shell=True)

C0209: Formatting a regular string which could be an f-string 
(consider-using-f-string)

Except for the pylint issues the patch looks good to me.

Best regards

Heinrich

> +            raise
> +
> +    utils.run_and_log(c, ['cp',  '-f', persistent, spi_flash_img])
> +    return spi_flash_img
> +
>   @pytest.mark.boardspec('sandbox')
>   @pytest.mark.buildconfigspec('cmd_echo')
>   @pytest.mark.buildconfigspec('cmd_nvedit_info')
> @@ -544,6 +564,85 @@ def test_env_ext4(state_test_env):
>           if fs_img:
>               call('rm -f %s' % fs_img, shell=True)
>   
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_echo')
> +@pytest.mark.buildconfigspec('cmd_nvedit_info')
> +@pytest.mark.buildconfigspec('cmd_nvedit_load')
> +@pytest.mark.buildconfigspec('cmd_nvedit_select')
> +@pytest.mark.buildconfigspec('env_is_in_spi_flash')
> +def test_env_spi_flash(state_test_env):
> +
> +    """Test ENV in SPI NOR on sandbox."""
> +    c = state_test_env.ubman
> +    spi_flash_img = ''
> +    try:
> +        spi_flash_img = mk_env_spi_flash(state_test_env)
> +
> +        # force env location: SF
> +        response = c.run_command('env select SPIFlash')
> +        assert 'Select Environment on SPIFlash: OK' in response
> +
> +        response = c.run_command('env save')
> +        assert 'Saving Environment to SPIFlash' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... OK' in response
> +
> +        response = c.run_command('env info')
> +        assert 'env_valid = valid' in response
> +        assert 'env_ready = true' in response
> +        assert 'env_use_default = false' in response
> +
> +        response = c.run_command('env info -p -d')
> +        assert 'Environment was loaded from persistent storage' in response
> +        assert 'Environment can be persisted' in response
> +
> +        response = c.run_command('env info -d -q')
> +        assert response == ""
> +        response = c.run_command('echo $?')
> +        assert response == "1"
> +
> +        response = c.run_command('env info -p -q')
> +        assert response == ""
> +        response = c.run_command('echo $?')
> +        assert response == "0"
> +
> +        response = c.run_command('env erase')
> +        assert 'OK' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... ' in response
> +        assert 'bad CRC, using default environment' in response
> +
> +        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 can be persisted' in response
> +
> +        # restore env location: NOWHERE (prio 0 in sandbox)
> +        response = c.run_command('env select nowhere')
> +        assert 'Select Environment on nowhere: OK' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from nowhere... OK' in response
> +
> +        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
> +
> +    finally:
> +        if spi_flash_img:
> +            call('rm -f %s' % spi_flash_img, shell=True)
> +
>   def test_env_text(ubman):
>       """Test the script that converts the environment to a text file"""
>   

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Marek Vasut]

On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
> On 12/23/25 15:31, Marek Vasut wrote:
>> Add test for environment stored in SPI NOR. The test works in a very
>> similar way to the current test for environment stored in ext4 FS,
>> except it generates spi.bin file backing the SPI NOR.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> ---
>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Cc: Jerome Forissier <jerome.forissier@linaro.org>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: u-boot@lists.denx.de
>> ---
>> V2: No change
>> ---
>>   test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 99 insertions(+)
>>
>> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
>> index 383e26c03b0..48e31f19b3c 100644
>> --- a/test/py/tests/test_env.py
>> +++ b/test/py/tests/test_env.py
>> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>>       utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>>       return fs_img
>> +def mk_env_spi_flash(state_test_env):
> 
> Thank you for adding the test.
> 
> Unfortunately pylint doesn't like your code. Please have a look.

Is this something we run in CI ? If so, CI builds did pass.

If not, how do I trigger this ?

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]

On Tue, Dec 23, 2025 at 07:55:45PM +0100, Marek Vasut wrote:
> On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
> > On 12/23/25 15:31, Marek Vasut wrote:
> > > Add test for environment stored in SPI NOR. The test works in a very
> > > similar way to the current test for environment stored in ext4 FS,
> > > except it generates spi.bin file backing the SPI NOR.
> > > 
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > ---
> > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > Cc: Jerome Forissier <jerome.forissier@linaro.org>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > Cc: u-boot@lists.denx.de
> > > ---
> > > V2: No change
> > > ---
> > >   test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 99 insertions(+)
> > > 
> > > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> > > index 383e26c03b0..48e31f19b3c 100644
> > > --- a/test/py/tests/test_env.py
> > > +++ b/test/py/tests/test_env.py
> > > @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
> > >       utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
> > >       return fs_img
> > > +def mk_env_spi_flash(state_test_env):
> > 
> > Thank you for adding the test.
> > 
> > Unfortunately pylint doesn't like your code. Please have a look.
> 
> Is this something we run in CI ? If so, CI builds did pass.
> 
> If not, how do I trigger this ?

We only run pylint_err not pylint in CI. The "make pylint" target has
been unused for so long that the baseline itself is out of date. I don't
object to trying to improve our python code to match standard practices
better, but this isn't the place to start adding complaints.

All that said, a newline before a new function is just a normal good
practice and so at least the quoted here example should be fixed, just
like if it was in C :)

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Marek Vasut]

On 12/24/25 12:32 AM, Tom Rini wrote:
> On Tue, Dec 23, 2025 at 07:55:45PM +0100, Marek Vasut wrote:
>> On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
>>> On 12/23/25 15:31, Marek Vasut wrote:
>>>> Add test for environment stored in SPI NOR. The test works in a very
>>>> similar way to the current test for environment stored in ext4 FS,
>>>> except it generates spi.bin file backing the SPI NOR.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>>> ---
>>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> Cc: Jerome Forissier <jerome.forissier@linaro.org>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> Cc: Tom Rini <trini@konsulko.com>
>>>> Cc: u-boot@lists.denx.de
>>>> ---
>>>> V2: No change
>>>> ---
>>>>    test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 99 insertions(+)
>>>>
>>>> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
>>>> index 383e26c03b0..48e31f19b3c 100644
>>>> --- a/test/py/tests/test_env.py
>>>> +++ b/test/py/tests/test_env.py
>>>> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>>>>        utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>>>>        return fs_img
>>>> +def mk_env_spi_flash(state_test_env):
>>>
>>> Thank you for adding the test.
>>>
>>> Unfortunately pylint doesn't like your code. Please have a look.
>>
>> Is this something we run in CI ? If so, CI builds did pass.
>>
>> If not, how do I trigger this ?
> 
> We only run pylint_err not pylint in CI. The "make pylint" target has
> been unused for so long that the baseline itself is out of date. I don't
> object to trying to improve our python code to match standard practices
> better, but this isn't the place to start adding complaints.
> 
> All that said, a newline before a new function is just a normal good
> practice and so at least the quoted here example should be fixed, just
> like if it was in C :)
There is a newline before the function, the code is incorrectly quoted 
in this discussion, see e.g. patchwork for the actual content of this patch:

https://patchwork.ozlabs.org/project/uboot/patch/20251223143130.16266-1-marek.vasut+renesas@mailbox.org/

So, what exactly should be fixed in this patch ?

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

On Wed, Dec 31, 2025 at 05:07:31PM +0100, Marek Vasut wrote:
> On 12/24/25 12:32 AM, Tom Rini wrote:
> > On Tue, Dec 23, 2025 at 07:55:45PM +0100, Marek Vasut wrote:
> > > On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
> > > > On 12/23/25 15:31, Marek Vasut wrote:
> > > > > Add test for environment stored in SPI NOR. The test works in a very
> > > > > similar way to the current test for environment stored in ext4 FS,
> > > > > except it generates spi.bin file backing the SPI NOR.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > > ---
> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > Cc: Jerome Forissier <jerome.forissier@linaro.org>
> > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > Cc: u-boot@lists.denx.de
> > > > > ---
> > > > > V2: No change
> > > > > ---
> > > > >    test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 99 insertions(+)
> > > > > 
> > > > > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> > > > > index 383e26c03b0..48e31f19b3c 100644
> > > > > --- a/test/py/tests/test_env.py
> > > > > +++ b/test/py/tests/test_env.py
> > > > > @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
> > > > >        utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
> > > > >        return fs_img
> > > > > +def mk_env_spi_flash(state_test_env):
> > > > 
> > > > Thank you for adding the test.
> > > > 
> > > > Unfortunately pylint doesn't like your code. Please have a look.
> > > 
> > > Is this something we run in CI ? If so, CI builds did pass.
> > > 
> > > If not, how do I trigger this ?
> > 
> > We only run pylint_err not pylint in CI. The "make pylint" target has
> > been unused for so long that the baseline itself is out of date. I don't
> > object to trying to improve our python code to match standard practices
> > better, but this isn't the place to start adding complaints.
> > 
> > All that said, a newline before a new function is just a normal good
> > practice and so at least the quoted here example should be fixed, just
> > like if it was in C :)
> There is a newline before the function, the code is incorrectly quoted in
> this discussion, see e.g. patchwork for the actual content of this patch:
> 
> https://patchwork.ozlabs.org/project/uboot/patch/20251223143130.16266-1-marek.vasut+renesas@mailbox.org/
> 
> So, what exactly should be fixed in this patch ?

Thanks for explaining. The only problem I have now is you didn't give a
cover letter explaining the feature and so I need to come up with
something when merging the series as a merge commit. I can do that, it's
just easier when I don't have to.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Marek Vasut]

On 12/31/25 5:11 PM, Tom Rini wrote:
> On Wed, Dec 31, 2025 at 05:07:31PM +0100, Marek Vasut wrote:
>> On 12/24/25 12:32 AM, Tom Rini wrote:
>>> On Tue, Dec 23, 2025 at 07:55:45PM +0100, Marek Vasut wrote:
>>>> On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
>>>>> On 12/23/25 15:31, Marek Vasut wrote:
>>>>>> Add test for environment stored in SPI NOR. The test works in a very
>>>>>> similar way to the current test for environment stored in ext4 FS,
>>>>>> except it generates spi.bin file backing the SPI NOR.
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>>>>> ---
>>>>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>>> Cc: Jerome Forissier <jerome.forissier@linaro.org>
>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>>>> Cc: u-boot@lists.denx.de
>>>>>> ---
>>>>>> V2: No change
>>>>>> ---
>>>>>>     test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 99 insertions(+)
>>>>>>
>>>>>> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
>>>>>> index 383e26c03b0..48e31f19b3c 100644
>>>>>> --- a/test/py/tests/test_env.py
>>>>>> +++ b/test/py/tests/test_env.py
>>>>>> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>>>>>>         utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>>>>>>         return fs_img
>>>>>> +def mk_env_spi_flash(state_test_env):
>>>>>
>>>>> Thank you for adding the test.
>>>>>
>>>>> Unfortunately pylint doesn't like your code. Please have a look.
>>>>
>>>> Is this something we run in CI ? If so, CI builds did pass.
>>>>
>>>> If not, how do I trigger this ?
>>>
>>> We only run pylint_err not pylint in CI. The "make pylint" target has
>>> been unused for so long that the baseline itself is out of date. I don't
>>> object to trying to improve our python code to match standard practices
>>> better, but this isn't the place to start adding complaints.
>>>
>>> All that said, a newline before a new function is just a normal good
>>> practice and so at least the quoted here example should be fixed, just
>>> like if it was in C :)
>> There is a newline before the function, the code is incorrectly quoted in
>> this discussion, see e.g. patchwork for the actual content of this patch:
>>
>> https://patchwork.ozlabs.org/project/uboot/patch/20251223143130.16266-1-marek.vasut+renesas@mailbox.org/
>>
>> So, what exactly should be fixed in this patch ?
> 
> Thanks for explaining. The only problem I have now is you didn't give a
> cover letter explaining the feature and so I need to come up with
> something when merging the series as a merge commit. I can do that, it's
> just easier when I don't have to.
2/3 basically describes why this series is needed, and implements the 
actual functionality. I can send a V3 with no changes and cover letter 
if still needed.

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3557 bytes --]

On Wed, Dec 31, 2025 at 05:50:31PM +0100, Marek Vasut wrote:
> On 12/31/25 5:11 PM, Tom Rini wrote:
> > On Wed, Dec 31, 2025 at 05:07:31PM +0100, Marek Vasut wrote:
> > > On 12/24/25 12:32 AM, Tom Rini wrote:
> > > > On Tue, Dec 23, 2025 at 07:55:45PM +0100, Marek Vasut wrote:
> > > > > On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
> > > > > > On 12/23/25 15:31, Marek Vasut wrote:
> > > > > > > Add test for environment stored in SPI NOR. The test works in a very
> > > > > > > similar way to the current test for environment stored in ext4 FS,
> > > > > > > except it generates spi.bin file backing the SPI NOR.
> > > > > > > 
> > > > > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > > > > ---
> > > > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > > > Cc: Jerome Forissier <jerome.forissier@linaro.org>
> > > > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > > > Cc: u-boot@lists.denx.de
> > > > > > > ---
> > > > > > > V2: No change
> > > > > > > ---
> > > > > > >     test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
> > > > > > >     1 file changed, 99 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> > > > > > > index 383e26c03b0..48e31f19b3c 100644
> > > > > > > --- a/test/py/tests/test_env.py
> > > > > > > +++ b/test/py/tests/test_env.py
> > > > > > > @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
> > > > > > >         utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
> > > > > > >         return fs_img
> > > > > > > +def mk_env_spi_flash(state_test_env):
> > > > > > 
> > > > > > Thank you for adding the test.
> > > > > > 
> > > > > > Unfortunately pylint doesn't like your code. Please have a look.
> > > > > 
> > > > > Is this something we run in CI ? If so, CI builds did pass.
> > > > > 
> > > > > If not, how do I trigger this ?
> > > > 
> > > > We only run pylint_err not pylint in CI. The "make pylint" target has
> > > > been unused for so long that the baseline itself is out of date. I don't
> > > > object to trying to improve our python code to match standard practices
> > > > better, but this isn't the place to start adding complaints.
> > > > 
> > > > All that said, a newline before a new function is just a normal good
> > > > practice and so at least the quoted here example should be fixed, just
> > > > like if it was in C :)
> > > There is a newline before the function, the code is incorrectly quoted in
> > > this discussion, see e.g. patchwork for the actual content of this patch:
> > > 
> > > https://patchwork.ozlabs.org/project/uboot/patch/20251223143130.16266-1-marek.vasut+renesas@mailbox.org/
> > > 
> > > So, what exactly should be fixed in this patch ?
> > 
> > Thanks for explaining. The only problem I have now is you didn't give a
> > cover letter explaining the feature and so I need to come up with
> > something when merging the series as a merge commit. I can do that, it's
> > just easier when I don't have to.
> 2/3 basically describes why this series is needed, and implements the actual
> functionality. I can send a V3 with no changes and cover letter if still
> needed.

I'll just paraphrase things from 2/3 and note this extends tests. But
for future reference, please do cover letters. It both makes my life
easier (b4 shazam -SM, re-read, commit, begin tests) and helps make sure
I don't mis-characterize something.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Marek Vasut]

On 12/31/25 6:18 PM, Tom Rini wrote:
> On Wed, Dec 31, 2025 at 05:50:31PM +0100, Marek Vasut wrote:
>> On 12/31/25 5:11 PM, Tom Rini wrote:
>>> On Wed, Dec 31, 2025 at 05:07:31PM +0100, Marek Vasut wrote:
>>>> On 12/24/25 12:32 AM, Tom Rini wrote:
>>>>> On Tue, Dec 23, 2025 at 07:55:45PM +0100, Marek Vasut wrote:
>>>>>> On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
>>>>>>> On 12/23/25 15:31, Marek Vasut wrote:
>>>>>>>> Add test for environment stored in SPI NOR. The test works in a very
>>>>>>>> similar way to the current test for environment stored in ext4 FS,
>>>>>>>> except it generates spi.bin file backing the SPI NOR.
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>>>>>>> ---
>>>>>>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>>>>> Cc: Jerome Forissier <jerome.forissier@linaro.org>
>>>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>>>>>> Cc: u-boot@lists.denx.de
>>>>>>>> ---
>>>>>>>> V2: No change
>>>>>>>> ---
>>>>>>>>      test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>>>>>>>>      1 file changed, 99 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
>>>>>>>> index 383e26c03b0..48e31f19b3c 100644
>>>>>>>> --- a/test/py/tests/test_env.py
>>>>>>>> +++ b/test/py/tests/test_env.py
>>>>>>>> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>>>>>>>>          utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>>>>>>>>          return fs_img
>>>>>>>> +def mk_env_spi_flash(state_test_env):
>>>>>>>
>>>>>>> Thank you for adding the test.
>>>>>>>
>>>>>>> Unfortunately pylint doesn't like your code. Please have a look.
>>>>>>
>>>>>> Is this something we run in CI ? If so, CI builds did pass.
>>>>>>
>>>>>> If not, how do I trigger this ?
>>>>>
>>>>> We only run pylint_err not pylint in CI. The "make pylint" target has
>>>>> been unused for so long that the baseline itself is out of date. I don't
>>>>> object to trying to improve our python code to match standard practices
>>>>> better, but this isn't the place to start adding complaints.
>>>>>
>>>>> All that said, a newline before a new function is just a normal good
>>>>> practice and so at least the quoted here example should be fixed, just
>>>>> like if it was in C :)
>>>> There is a newline before the function, the code is incorrectly quoted in
>>>> this discussion, see e.g. patchwork for the actual content of this patch:
>>>>
>>>> https://patchwork.ozlabs.org/project/uboot/patch/20251223143130.16266-1-marek.vasut+renesas@mailbox.org/
>>>>
>>>> So, what exactly should be fixed in this patch ?
>>>
>>> Thanks for explaining. The only problem I have now is you didn't give a
>>> cover letter explaining the feature and so I need to come up with
>>> something when merging the series as a merge commit. I can do that, it's
>>> just easier when I don't have to.
>> 2/3 basically describes why this series is needed, and implements the actual
>> functionality. I can send a V3 with no changes and cover letter if still
>> needed.
> 
> I'll just paraphrase things from 2/3 and note this extends tests. But
> for future reference, please do cover letters. It both makes my life
> easier (b4 shazam -SM, re-read, commit, begin tests) and helps make sure
> I don't mis-characterize something.
ACK, thanks !

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Heinrich Schuchardt]

Am 23. Dezember 2025 19:55:45 MEZ schrieb Marek Vasut <marek.vasut@mailbox.org>:
>On 12/23/25 6:32 PM, Heinrich Schuchardt wrote:
>> On 12/23/25 15:31, Marek Vasut wrote:
>>> Add test for environment stored in SPI NOR. The test works in a very
>>> similar way to the current test for environment stored in ext4 FS,
>>> except it generates spi.bin file backing the SPI NOR.
>>> 
>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>> ---
>>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Cc: Jerome Forissier <jerome.forissier@linaro.org>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Tom Rini <trini@konsulko.com>
>>> Cc: u-boot@lists.denx.de
>>> ---
>>> V2: No change
>>> ---
>>>   test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 99 insertions(+)
>>> 
>>> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
>>> index 383e26c03b0..48e31f19b3c 100644
>>> --- a/test/py/tests/test_env.py
>>> +++ b/test/py/tests/test_env.py
>>> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>>>       utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>>>       return fs_img
>>> +def mk_env_spi_flash(state_test_env):
>> 
>> Thank you for adding the test.
>> 
>> Unfortunately pylint doesn't like your code. Please have a look.
>
>Is this something we run in CI ? If so, CI builds did pass.
>
>If not, how do I trigger this ?

I just ran pylint from the command line.

Best regards 

Heinrich

Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Tom Rini]

On Tue, 23 Dec 2025 15:31:10 +0100, Marek Vasut wrote:

> Add test for environment stored in SPI NOR. The test works in a very
> similar way to the current test for environment stored in ext4 FS,
> except it generates spi.bin file backing the SPI NOR.
> 
> 

Applied to u-boot/master, thanks!

[1/3] test: env: Add test for environment storage in SPI NOR
      commit: 88de22a4db03d6dfbc4779bf942383fa390eac34
[2/3] env: Add single to redundant environment upgrade path
      commit: 1f131385810fddaeea23c5cf0269497d7e637534
[3/3] configs: sandbox: Enable environment in SPI NOR support
      commit: 8dd76166e3dc47f898155a047f1594fdc7a63d65
-- 
Tom