From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Fri, 09 May 2008 22:44:55 +0200
Subject: [U-Boot-Users] [PATCH]: Fix for bug: U-boot environment corrupt
	by reading uninitialized flash memory instead of RAM.
In-Reply-To: Your message of "Tue, 06 May 2008 16:05:09 +0200."
	<3efb10970805060705l112c623at96bf0521eed8a211@mail.gmail.com>
Message-ID: <20080509204455.3F05024764@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

In message <3efb10970805060705l112c623at96bf0521eed8a211@mail.gmail.com> you wrote:
>
> Commit c0559be371b2a64b1a817088c3308688e2182f93 introduces a bug in
> the environment setting storage in U-boot-1.3.3-rc3.
> Settings are retrieved from dataflash when only settings in RAM are
> valid, resulting in corrupt environment settings, failing printenv
> command, and
> duplicate variables.
> 
> This patch fixes this by always using the RAM area when it is created
> and initialized. (Matches more the behavior as it was prior to this
> particular commit.)

Sorry, but this patch makes littles sense to me.

> See attached (Sorry, my mailer does not handle inline-patches properly)

Chose another one? Or rather use "git-send-email" directly?

-	/* if relocated to RAM */
-	if (gd->flags & GD_FLG_RELOC)
+	/* if relocated to RAM, OR if the environment in Malloc-ed RAM is valid */
+	if ((gd->flags & GD_FLG_RELOC) || (gd->env_valid))

Let's keep in mind that the normal logic of the U-Boot startup
sequence is like this:

* U-Boot boots and initializes the RAM
* U-Boot relocates itself into RAM, sets GD_FLG_RELOC and continues
  running from RAM
* U-Boot continues with the initialization, for xample by setting up
  the malloc arena, loading the working copy of the environment into
  RAM (after which it set's gd->env_valid), etc.

So the relocation to RAM always preceeds any use of malloc() and the
setting of gd->env_valid. Or, put the other way round, we always set
GD_FLG_RELOC long before gd->env_valid get's set.

Thus your change above is just redundant.


Now, if your board  does  not  perform  proper  relocation  for  some
reason, it should still set GD_FLG_RELOC at the appropriate place, i.
e. as soon as U-Boot is ready for and starts running out of RAM.

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
Democracy is mob rule, but with income taxes.