On Tuesday 16 June 2009 14:02:16 Wolfgang Denk wrote: > Mike Frysinger wrote: > > Rather than rely on dirty hacks to compile the environment on the host > > and extract the CRC from that, have envcrc extract the environment > > straight from the ELF object that will be linked into u-boot itself. > > This makes the envcrc code a bit more complicated, but it simplifies the > > build process and host requirements because we don't have to try and > > recreate the environment that the target will be seeing on the host. > > This avoids some issues that crop up from time to time where the > > preprocessor defines on the host don't expand in the same way as for the > > target -- in case the target uses those defines to customize the > > environment, or the host defines conflicts with some of the target > > values. > > Can you please be a bit more specific about which sort of problems you > are talking here?
sure ... i'll be verbose so hopefully you can see what i see > We've been using this code for many years now, but I > cannot remember any problems with it. that's because the code paths have slowly acquired platform-specific hacks so that it kept working :) at any rate, here's a longer problem description and surrounding issues ... if the default environment utilizes defines that come from the toolchain in any level of indirection, the environment that is built into the target u-boot will not match the environment that is compiled on the host for crc generation. a real world example is the unified ADI config header. it generates a default environment based on functionality actually available. the BF537 family has networking hardware in some parts, but not all (BF534 does not). so we key off of the variant to determine whether to enable networking by default. or we determine that some on-chip rom functions are available for certain variants and enable only those functions. so the CPP dependency chain looks something like: - Blackfin toolchain sets up variant defines - ADI board headers set up capabilities based on variant defines - common ADI board header enables some commands based on variant defines - common ADI board header sets up default environment based on capabilities and commands available since the host toolchain does not set up the same CPP dependencies as the Blackfin toolchain, this tree falls apart and the resulting environment string that envcrc operates on differs, so the CRCs differ. i can understand your position of "setting up board configs this way is wrong", but i've structured it this way because it greatly reduces my maintenance burden while increasing regression capabilities. for example, i have a Blackfin-specific patch to the MAKEALL script that allows me to regression test not only the board-specific configuration, but also uncommon or otherwise untested cpu and boot mode configurations. for example, the bf537-stamp defaults to the BF537 cpu and BYPASS boot mode. but now my regression builds can automatically verify BF534/BF536 cpu code paths as well as PARALLEL/SPI/NAND/UART code paths. previously, some changes would accidentally break these uncommon edge cases and wouldnt be noticed until someone else happened to build. to say that these code paths were horribly broken most of the time is an understatement :). i have also seen a case or two where the host toolchain inadvertently expanded things that ended up in the environment because they were not declared as strings. for example, many defines are not: #define CONFIG_FOO "foo" but rather they are: #define CONFIG_FOO foo just grep common/env_*.c for MKSTR() to see just how many things are affected in this way. having to worry about values here which may be arbitrarily expanded across host toolchains is a bad design practice imo. if the environment is only ever compiled by one toolchain, the target one, then it significantly reduces the chance for unexpected and unwelcomed surprises. after all, the only way to notice something has gone wrong is to build the source with a specific host toolchain, burn the image, reset, and see the dreaded "CRC mismatch, using default environment" error from u-boot. unless you're familiar with this problem and/or have seen/root caused this before, attempting to track down the source of this problem can be very time consuming (as it was the first time i hit this problem). > > common/Makefile | 4 +- > > common/env_embedded.c | 30 +--------- > > tools/Makefile | 3 +- > > tools/envcrc.c | 156 > > ++++++++++++++++++++++++++++++++++++++++++------- > > The diffstat indicates that the new solution is more complicated than > the old one, so I'd like to understand why this is needed (or if). if LoC are the metric, then yes, but conceptually this change is simpler. the current behavior requires mixing target u-boot environment headers with the host toolchain, and a whole bunch of hacks to make sure things actually compile, produce the same end environment with two vastly different build environments, and end up with the same crc value. my proposed change means we only compile 1 environment object -- the one that is going into the target u- boot binary. we then extract the environment directly from the object which is actually going into u-boot (so we are absolutely assured that the environment we are working with on the host matches the target). hope this clears things up -mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
