[U-Boot] [PATCH] env_sf - Do not free flash environment on successful read
In env_relocate_spec(), env_flash is freed and set to NULL if CONFIG_ENV_OFFSET_REDUND is undefined. This leads to an "Environment SPI flash not initialized" error when performing a saveenv.
br, Oliver
diff --git a/common/env_sf.c b/common/env_sf.c index fb0c39b..fc5f9f3 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -384,7 +384,10 @@ void env_relocate_spec(void) ret = env_import(buf, 1);
if (ret) + { gd->env_valid = 1; + return; + } out: spi_flash_free(env_flash); env_flash = NULL;
Dear Oliver Dillinger,
In message loom.20101022T120849-268@post.gmane.org you wrote:
In env_relocate_spec(), env_flash is freed and set to NULL if CONFIG_ENV_OFFSET_REDUND is undefined. This leads to an "Environment SPI flash not initialized" error when performing a saveenv.
br, Oliver
Signed-off-by: line missing.
Please read http://www.denx.de/wiki/U-Boot/Patches
diff --git a/common/env_sf.c b/common/env_sf.c index fb0c39b..fc5f9f3 100644 --- a/common/env_sf.c +++ b/common/env_sf.c @@ -384,7 +384,10 @@ void env_relocate_spec(void) ret = env_import(buf, 1);
if (ret)
{
Incorrect brace style.
gd->env_valid = 1;
return;
Your patch is white-space corrupted.
Also, this patch is not correct. It is OK to call spi_flash_free() here.
The bug is in saveenv() for the non-redundant case. The function has not been dapted to the new environment code, at all; for example, it fails to actually export the internally stored environment [there is no call to hexport()].
Best regards,
Wolfgang Denk
On 10/22/2010 12:56 PM, Wolfgang Denk wrote:
Dear Oliver Dillinger,
Hi Wolfgang,
Also, this patch is not correct. It is OK to call spi_flash_free() here.
The bug is in saveenv() for the non-redundant case. The function has not been dapted to the new environment code, at all; for example, it fails to actually export the internally stored environment [there is no call to hexport()].
You mean there are several bugs here....if spi_flash_free() is correct, then spi_flash_probe must be called inside the saveenv function, in case env_flash is not set (so it is called only once). And IMHO spi_flash_free() should be called for the redundant case, too (why is it different from the non-redundant case?).
Best regards, Stefano Babic
Dear Stefano Babic,
In message 4CC17A27.7010404@denx.de you wrote:
You mean there are several bugs here....if spi_flash_free() is correct, then spi_flash_probe must be called inside the saveenv function, in case env_flash is not set (so it is called only once). And IMHO spi_flash_free() should be called for the redundant case, too (why is it different from the non-redundant case?).
Right. There are quite a number of different bugs in that code, and potential for cleanup / optimization - ther eis probably no need to have two different versions of the saveenv() function when only a few lines are different.
The submitted patch would not help, though, as only the old environment would be written back.
Best regards,
Wolfgang Denk
participants (3)
-
Oliver Dillinger -
Stefano Babic -
Wolfgang Denk