On Tue, 2 Sep 2008, Wolfgang Denk wrote: > In message <[EMAIL PROTECTED]> you wrote: > > > > 1. do not use the union > > > > well, I would still prefer to use it and I hope I will be allowed to do so > > in a separate NAND-tool. I agree, it would be better to use the definition > > from the environment.h directly. But: > > There is no such thing as a separate NAND tool - this mkes zero sense. > There shall be one tool that supports both NOR and NAND (and soon > probably DataFlash and OneNAND and ... ). > > > 2. do not use single.crc in redundant case > > > > This is done only at two places, yes, I realise, this is not very clean, > > Indeed. But the problem goes away automatically whenyou get rid of the > union.
I'll try to explain again _why_ i introduced the union and why I still don't see a good replacement for it. As you know, in the tool we have to decide at run-time whether we are dealing with a single environment copy or with current / redundant configuration. With NAND support when _writing_ environment to NAND you have to write page at a time, which means, at this point we _must_ have the image in a contiguous buffer in RAM. And the image can have one of the two possible formats - with and without the "flags" byte. In principle I see only three possibilities to implement this: (a) work with arbitrary non-contiguous data in RAM as before, and copy it into an additional buffer just before writing to NAND advantage: can keep the current struct disadvantage: extra malloc (b) use a plain data buffer, and, if needed, use the first byte in it for flags advantage: no extra malloc, no (explicit) union disadvantage: confusing, have to work with byte-offsets instead of struct / union members, and, in fact, this is the same as using a union, just implicit, calculating byte-offsets manually, instead of letting the compiler do it (c) use a union advantage: clean access to all environment fields without the use of byte-offsets disadvantage: slightly more complex code Please, just tell me which of these three you would prefer, or maybe there is a fourth possibility I am still overseeing. You also asked about the extra "char *data" pointer in the struct environment, whether there is no danger that a different compiler version will break it. This pointer uses no magic - it is just a plain simple pointer, I use it to point to data inside the union to avoid having to check every time whether we have the redundant environment or not. So, I check it only once at initialisation time, set this pointer, and then just use it to access the data buffer inside the image (union). No magic here. > > 4. fix MTD_OLD > > > > Would we still need this with NAND-only tool? > > Yes of course we need it. I will not accept such thing as a NAND-only tool. Ok. But NAND-support is not needed with MTD_OLD? So, if it cannot be compiled with older kernels, we may just disable it per ifdef? > > 5. clarify back-up mode > > > > This is actually a comment improvement, can do. > > Actually the whole implementation needs to be explained. Ok. > > Shall I keep support for NOR in the separate NAND version or completely > > remove it? The "type == MTD_NORFLASH" code is quite small, so, removing it > > I don't understand why you come up with such an idea. There shall be > just the one tool we have now, just with extended functionality. I > just wanted to get rid of the futile attempts to make the one huge > change looking like a series af several big but incremental changes. How would you like to make such a replacement then? If I produce a patch just from the current state to the final state, I think, it will look worse than the broken-down patch-series. Otherwise we could remove the current file and add a new one in two patches? This wouldn't be very good either - you'd have to change Makefiles etc. to keep the tree compilable. 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: [EMAIL PROTECTED] _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
