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

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0902030842390.4902 at axis700.grange> you wrote:
> 
> > I think we should not artifically split this into two cases when it's
> > really one. There is two things that need to be done: (1) nand_spl must
> > locate a valid copy of the environment and load it into RAM, and (2)
> > the address must be passed to env_nand.c, but this is probably easy as
> > it can be statically defined, I think.
> 
> for (1) - "valid copy" - you're not suggesting to calculate crc32 in 
> nand_spl to decide upon env validity, are you? I think, we should add as 
> little as possible to nand_spl, remember, it has to fit in a very limited 
> size like 16, or 4, or even 2K. So, probably, just add one (or two for 
> redundant env) nand_load calls and let env_init() evaluate them? Or is 
> this what you meant?

I enjoy the position of taking a higher level view without caring
about implementation details :-)

Technically, you have to check which of the possibly N copies of  the
environment  in NAND are valid, and which of them is the most current
copy. With NAND based storage this always means you have to load  all
the data in RAM anyway, so it seems OK with me to do just the loading
in  the  NAND  bootstrap loader and leave the rest of the work to the
full U-Boot initialization.

But as mentioned before: that's an implementation details.


> > > 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 
> > 
> > Which ones? I think this is almost certainly broken, then.
> 
> Correct me if I'm wrong, but I think code like
> 
> int env_init(void)
> {
> 	/* SPI flash isn't usable before relocation */
> 	gd->env_addr = (ulong)&default_environment[0];
> 	gd->env_valid = 1;
> 
> 	return 0;
> }
> 
> does mean the specific environment limits itself to default until 
> env_relocate()? Then, this snippet os from env_sf.c, env_onenand.c does 
> exactly the same, env_nand.c if env is not embedded. The rest are actually 
> reading the medium.

You are right - all the examples you listed are basicly broken and
should be fixed.

> > But that wa snot what I suggested above. I wrote that we should  bail
> > out  with  an  error  message  if  somebody  attempts  to  compile  a
> > configuration where the environment is on a different NAND chip  than
> > the U-Boot image - it's a non-supported configuration.
> 
> Why? Maybe there are users out there ATM that have environment in NAND and 
> boot from another storage and are reasonably happy being only able to 
> affect late configuration dynamically (e.g., Linux boot procedure) and 
> having to recompile u-boot to change early configuration (console 
> baudrate, etc.)?

Why should we add additional complexity to the  code  for  a  unknown
(but in any case tiny) number of systems with an exotic setup? So far
no  such  need  is known, and nobody in a sane state of mind would do
this. If we clearly state that  such  a  mode  of  operation  is  not
supported,  we also have a clear position if somebody asks about such
a design.

However, if we allow for this now, and add all the complexity to  the
implementation, you can be sure that soon somebody will pop up who is
stupid  enough  to  actually  use this, so we never can get rid of it
again, not even after realizing how stupid it actually was.

Let's do it right, especially when doing it right results in clean and
simple code that works as expected.


> > Thus please start by documenting the intended memory map.
> 
> This all sounds good, but, as I see it, this memory map, and the location 
> of the env copies read out by nand_spl are platform-dependent, so, every 
> implementor will just have to take care of the specific configuration. 

No. It is architecture dependent (unforatunately, becaus ethe initial
armboot port tried to "simplify" the design we had in place for PPC),
but it is in no way board dependent.

Yes, certain areas may or may not be present  for  a  specific  board
(there  may or may not be pram or a shared log buffer or video memory
or ...), but the relative location of all these areas is exactly  the
same for all boards of the same architecture.

> besides, this location only has to survive until env_relocate(), where it 
> anyway will be copied to a malloc()ed buffer. Although, if we reuse the 
> ENV_IS_EMBEDDED case then we can keep the buffers and avoid the extra 
> malloc() / memcpy(). But documenting is always good, sure:-)

You don't want to keep all the buffers. See above - you will probably
read N sectors of redundant environment, and in the end you will  use
just a part of one of these sectors.

> > > 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.
> > 
> > Agreed. I don't see any mention of BSS in your description, though -
> > as expected.
> 
> in 4. - as a reason why you have to skip it.

Actually your sequence is wrong. U-Boot begins accessing the
environment before setting up the bss. And it seems obvious that you
must not meddle with any of the areas used by U-Boot - bss is not
special in this respect. Stack area, malloc area, pram, shared log
buffer, video memory etc. - all have to be considered. 

See above: start with designing the memory map for this usage case.

> We still have the other possibility - properly initialise NAND before 
> calling env_init() and then read out env? This would be a preferred 

You are missing a very basic point of U-Boot design: please see
http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#6_Keep_it_Debuggable

The more code we move before the intialization of the  console  port,
the  less  easy  we can debug U-Boot. That's why I will always resist
against movin code towards "earlier".

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
"Maintain an awareness for contribution --  to  your  schedule,  your
project, our company."                         - A Group of Employees