[U-Boot] [PATCH] env_sf - Do not free flash environment on successful read

[U-Boot] [PATCH] env_sf - Do not free flash environment on successful read

[Oliver Dillinger]

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;

[U-Boot] [PATCH] env_sf - Do not free flash environment on successful read

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I think they're going to take all this money that we  spend  now  on
war and death --"                   "And make them spend it on life."
	-- Edith Keeler and Kirk, "The City on the Edge of Forever",
	   stardate unknown.

[U-Boot] [PATCH] env_sf - Do not free flash environment on successful read

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] env_sf - Do not free flash environment on successful read

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anyone who knows history, particularly the history of Europe, will, I
think, recognize that the domination of education or of government by
any one particular religious faith is never a happy  arrangement  for
the people.                                       - Eleanor Roosevelt