[U-Boot] [ARM] Environment variables not available during console initialisation?

[Guennadi Liakhovetski]

On Mon, 2 Feb 2009, Wolfgang Denk wrote:

> Dear Guennadi Liakhovetski,
> 
> In message <Pine.LNX.4.64.0902022141190.8602 at axis700.grange> you wrote:
> > 
> > > > -#ifdef ENV_IS_EMBEDDED
> > > > +#if defined(ENV_IS_EMBEDDED)
> > > >  extern uchar environment[];
> > > >  env_t *env_ptr = (env_t *)(&environment[0]);
> > > > +#elif defined(CFG_ENV_IS_APPENDED)
> > > 
> > > What makes you think an "appended" environment is so special that it
> > > is worth a separate #ifdef here? What about a prepended environment?
> > > Or one two sectors apart?
> > 
> > Ok, agree, the name is badly chosen. The real restriction of this specific 
> > implementation is that the environment must be on the same NAND chip as 
> > the one we are booting from. Any location on that chip can be handled, I 
> > think.
> 
> Such an explanation belongs into the commit message.

Sure, _if_ we decide, that this is the correct direction to fix this 
problem.

> And I'm not really sure why this file is affected, then.

This file provides the env_init() function, which now has to become aware 
of the new possibility - proper (not default) environment already in RAM 
(after being loaded by nand_spl).

> > > > +#if defined(ENV_IS_EMBEDDED) || defined(CFG_ENV_IS_APPENDED)
> > > >  	int crc1_ok = 0, crc2_ok = 0;
> > > > -	env_t *tmp_env1, *tmp_env2;
> > > > +	env_t *tmp_env1;
> > > >  
> > > 
> > > Note that there is a fundamental difference between embedded and
> > > non-embedded (in whatever which way) environments, but I can see no
> > > significant difference between an "appended" or a "prepended" or
> > > otherwise separate environment.
> > 
> > The reason they share the same #if case here is that both get initialised 
> > early, not as was only possible until now on NAND - after all initcalls.
> 
> But why does this matter here, and why do we have to change this file
> for that?

I haven't found a better way to do this. Maybe there is one. I understood, 
that the proper place to provide an early copy of the dynamic (not 
default / static) environment is env_init(), a copy of which is present in 
each of env_{flash,nand,nvram,onenand,dataflash,sf}.c.

> > 1. embedded, in this case the real environment is initialised at 
> > env_init() time, otherwise first default environment is used
> 
> First default environment? Do we have more than one default
> environment?

s/first/at first/ (in the beginning, before env_relocate()).

> > 2. other, env_init() initialises the default environment, the real (stored 
> > in NAND) environment is first activated at env_relocate() time.
> > 
> > With this patch we add one more case
> > 
> > 3. environment, located on the same NAND chip as the boot NAND. in this 
> > case we can also let the nand_spl code load it for is in RAM and then we 
> > can use it in env_init().
> 
> I don't see why we need a special case here.
> 
> We have two situations only:
> 
> 1) environment is on the same NAND chip as the U-Boot image; this is
>    the case we can handle cleanly. But it does not play any role of
>    the environment is embedded, or directly adjacent (either before
>    or after) to the U-Boot image, or if it is on a completely
>    different location on this NAND chip.

Well, there is a difference. If the environment is embedded, it is loaded 
by nand_spl automatically with the image with a single nand_load() (that's 
how it worked until now, I think). If the environment is not embedded but 
on the same chip, we still can handle it, but we need an additional 
nand_load(). And the copy of the environment from NAND as loaded into RAM 
will be at a different location. Maybe env_nand.c doesn't have to know 
about this difference, but I haven't gone that far to unify these two 
cases.

> 2) environment is on a different NAND chip than the U-Boot image; in
>    this case we cannot access the environment as early as needed,
>    i.e. proper operation of U-Boot is not possible.
> 
> To me this means that 1) is the case we implement, without additional
> #ifdef'fery, and 2) is a configuration error for which we should  add
> appropriate #error handling.

Hm, as I briefly looked through other env_*.c, it looks like a few of them 
resort to the default environment until env_relocate(). So, we can either 
accept that and say "on those media you can store environment, but it will 
be only available late in boot process," or we can start fixing them all 
by shifting hardware initialisation before env_init()... Or are you 
suggesting to declare them all as broken?

> > > >  #define CFG_ENV_OFFSET		0x0040000
> > > > +/* Leave enough space for bss, currently __bss_end == 0x57e74800 */
> > > > +#define CFG_NAND_ENV_DST	(CFG_UBOOT_BASE + 0x80000)
> > > 
> > > ???? What's that? What has BSS to do with the environment storage ???
> > 
> > This is smdk6400 header, on this board I chose to place the environment 
> > read in by nand_spl above bss, because, AFAIU, the area above __bss_end is 
> 
> What has BSS to do with the environment storage ???
> 
> BSS does not take any space in the U-Boot image, so why leave a
> (eventually huge) gap unused?

I need a location in RAM for nand_spl to put a copy of the environment 
into. I know that bss doesn't take space in the u-boot image. But you 
cannot put environment there, because it will be nullified by u-boot 
proper.

> > unused by u-boot. But I didn't find an easy way to pass a calculated value 
> > from u-boot proper to nand_spl or vice versa. As you know, these two 
> > objects use saparate linker scripts and are linked absolutely 
> > independently, and just cat'ed together in the end. So, I had to use a 
> > macro to specify the location of the environment copy in RAM instead of 
> > calculating it precisely.
> 
> What has the location in RAM to do with the image structure on NAND?
> We don't need to allocate any space for BSS on the NAND device, right?

1. nand_spl copies u-boot proper at a location in RAM

2. nand_spl copies environment at another location in RAM

3. nand_spl jumps to u-boot proper

4. u-boot nullifies bss

5. u-boot looks for environment - either embedded, or default, or copied 
by nand_spl in 2.

ideally you would tell nand_spl to put environment directly after bss, but 
I haven't find a way (without adding some ugly scripts) to do this.

Notice, 1) I am not an expert in U-Boot environment operation, so, there 
well might be issues I'm overseeing, 2) redundant environment is not 
handled yet, 3) would be nice to hear comments from nand(_spl) 
maintainers.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

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