[U-Boot] [PATCH] env_sf: Fix recovery default

[U-Boot] [PATCH] env_sf: Fix recovery default

[Mario Schuknecht]

The u-boot environment is redundantly stored in a NOR flash on our boards.
Redundant means that there are two places to store the environment. But only
one of the two is active. I discovered that on one board the u-boot (env_sf)
uses the environment from the second place and the Kernel (fw_printenv) uses
the environment from the first place.
To decide which is the active environment there is a byte inside the
environment. 1 means active and 0 means obsolete. But on that board both
environments had have a 1. This can happen if a power loss or reset occurs
during writing the environment. In this situation the u-boot (env_sf)
implementation uses the second environment as default. But the Kernel
(fw_printenv) implementation uses the first environment as default.

This commit corrects the default in the u-boot env_sf implementation when a
problem was detected. Now the recovery default is the same like in all other
environment implementations. E.g. fw_printenv and env_flash. This ensures that
u-boot and Kernel use the same environment.

Signed-off-by: Mario Schuknecht <mario.schuknecht@dresearch-fe.de>
---
 common/env_sf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/env_sf.c b/common/env_sf.c
index 5e3729c..e928f57 100644
--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -188,15 +188,17 @@ void env_relocate_spec(void)
 		   tmp_env2->flags == ACTIVE_FLAG) {
 		gd->env_valid = 2;
 	} else if (tmp_env1->flags == tmp_env2->flags) {
-		gd->env_valid = 2;
+		gd->env_valid = 1;
 	} else if (tmp_env1->flags == 0xFF) {
+		gd->env_valid = 1;
+	} else if (tmp_env2->flags == 0xFF) {
 		gd->env_valid = 2;
 	} else {
 		/*
 		 * this differs from code in env_flash.c, but I think a sane
 		 * default path is desirable.
 		 */
-		gd->env_valid = 2;
+		gd->env_valid = 1;
 	}
 
 	if (gd->env_valid == 1)
-- 
2.1.4

[U-Boot] [PATCH] env_sf: Fix recovery default

[Mario Schuknecht]

2015-03-24 8:59 GMT+01:00 Mario Schuknecht <mario.schuknecht@dresearch-fe.de>:
> The u-boot environment is redundantly stored in a NOR flash on our boards.
> Redundant means that there are two places to store the environment. But only
> one of the two is active. I discovered that on one board the u-boot (env_sf)
> uses the environment from the second place and the Kernel (fw_printenv) uses
> the environment from the first place.
> To decide which is the active environment there is a byte inside the
> environment. 1 means active and 0 means obsolete. But on that board both
> environments had have a 1. This can happen if a power loss or reset occurs
> during writing the environment. In this situation the u-boot (env_sf)
> implementation uses the second environment as default. But the Kernel
> (fw_printenv) implementation uses the first environment as default.
>
> This commit corrects the default in the u-boot env_sf implementation when a
> problem was detected. Now the recovery default is the same like in all other
> environment implementations. E.g. fw_printenv and env_flash. This ensures that
> u-boot and Kernel use the same environment.
>
> Signed-off-by: Mario Schuknecht <mario.schuknecht@dresearch-fe.de>
> ---
>  common/env_sf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/common/env_sf.c b/common/env_sf.c
> index 5e3729c..e928f57 100644
> --- a/common/env_sf.c
> +++ b/common/env_sf.c
> @@ -188,15 +188,17 @@ void env_relocate_spec(void)
>                    tmp_env2->flags == ACTIVE_FLAG) {
>                 gd->env_valid = 2;
>         } else if (tmp_env1->flags == tmp_env2->flags) {
> -               gd->env_valid = 2;
> +               gd->env_valid = 1;
>         } else if (tmp_env1->flags == 0xFF) {
> +               gd->env_valid = 1;
> +       } else if (tmp_env2->flags == 0xFF) {
>                 gd->env_valid = 2;
>         } else {
>                 /*
>                  * this differs from code in env_flash.c, but I think a sane
>                  * default path is desirable.
>                  */
> -               gd->env_valid = 2;
> +               gd->env_valid = 1;
>         }
>
>         if (gd->env_valid == 1)
> --
> 2.1.4
>

Any comment, ACK or NAK?

The problem can be easily enforced by calling "fw_setenv test abc" and
immediatly pressing ctrl-c.

Regards

Mario

[U-Boot] env_sf: Fix recovery default

[Tom Rini]

On Tue, Mar 24, 2015 at 08:59:00AM +0100, Mario Schuknecht wrote:

> The u-boot environment is redundantly stored in a NOR flash on our boards.
> Redundant means that there are two places to store the environment. But only
> one of the two is active. I discovered that on one board the u-boot (env_sf)
> uses the environment from the second place and the Kernel (fw_printenv) uses
> the environment from the first place.
> To decide which is the active environment there is a byte inside the
> environment. 1 means active and 0 means obsolete. But on that board both
> environments had have a 1. This can happen if a power loss or reset occurs
> during writing the environment. In this situation the u-boot (env_sf)
> implementation uses the second environment as default. But the Kernel
> (fw_printenv) implementation uses the first environment as default.
> 
> This commit corrects the default in the u-boot env_sf implementation when a
> problem was detected. Now the recovery default is the same like in all other
> environment implementations. E.g. fw_printenv and env_flash. This ensures that
> u-boot and Kernel use the same environment.
> 
> Signed-off-by: Mario Schuknecht <mario.schuknecht@dresearch-fe.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150407/89c3a96f/attachment.sig>