[U-Boot-Users] [PATCH]: Fix for bug: U-boot environment corruptby reading uninitialized flash memory instead of RAM.

[Ulf Samuelsson]

> In message <3efb10970805060705l112c623at96bf0521eed8a211 at 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.
> 

When you have a dataflash, this is not the normal behaviour.

A typical behaviour of an AT91 would be:

1) BootROM copies initial bootloader (at91bootstrap/dataflashboot) to internal SRAM
2) BootROM jumps to start of initial bootloader
3) Initial bootloader does low-level init (including configuring the SDRAM controller)
4) Initial bootloader copies U-Boot from dataflash to SDRAM
5) Initial bootloader jumps to U-Boot
6) U-Boot needs to skip low-level init


> 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
> 


Best Regards
Ulf Samuelsson