Dear Guennadi Liakhovetski, In message <pine.lnx.4.64.0902030842390.4...@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: w...@denx.de "Maintain an awareness for contribution -- to your schedule, your project, our company." - A Group of Employees _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot