Hi Gabe, On 03/12/11 08:16, Gabe Black wrote: > > > On Fri, Dec 2, 2011 at 1:10 PM, Graeme Russ <[email protected] > <mailto:[email protected]>> wrote: > > Hi Gabe, > > On 30/11/11 17:07, Gabe Black wrote: > > Otherwise it ends up in the .bss section. U-boot assumes that it doesn't > > need to copy it over during relocation, and instead fills that whole > > section with zeroes. If we really were booting from ROM that would be > > appropriate, but we need some information from the coreboot tables > (memory > > size) before then and have to fill that structure before relocation. We > > skirt u-boot's assumption by putting this in .data where it assumes > there > > is still read only but non-zero data. > > > > Signed-off-by: Gabe Black <[email protected] > <mailto:[email protected]>> > > --- > > arch/x86/cpu/coreboot/sysinfo.c | 8 +++++++- > > 1 files changed, 7 insertions(+), 1 deletions(-) > > > > diff --git a/arch/x86/cpu/coreboot/sysinfo.c > b/arch/x86/cpu/coreboot/sysinfo.c > > index 464f8a1..e74fe0a 100644 > > --- a/arch/x86/cpu/coreboot/sysinfo.c > > +++ b/arch/x86/cpu/coreboot/sysinfo.c > > @@ -30,4 +30,10 @@ > > > > #include <asm/ic/coreboot/sysinfo.h> > > > > -struct sysinfo_t lib_sysinfo; > > +/* > > + * This needs to be in the .data section so that it's copied over > during > > + * relocation. By default it's put in the .bss section which is > simply filled > > + * with zeroes when transitioning from "ROM", which is really RAM, > to other > > + * RAM. > > + */ > > +struct sysinfo_t lib_sysinfo __attribute__((section(".data"))); > > I think this can be logically folded into the first patch > > Regards, > > Graeme > > > I would rather not do that since the first patch is, modulo checkpatch and > build fixes, just importing that code from coreboot's libpayload. This > change is making an important, non-obvious change which makes it work > properly within u-boot. These are two different things, and one way or the > other this one is important enough to be singled out as its own change and > not lost in the midst of the other.
I agree that is a logical position to take, however I think that the fact that you introduce a commit which is known to be 'broken' is the overriding factor - Include to fact that the modification is made in the commit message and the comment like: /* * Note: U-Boot needs this to be in the .data section so.... * ... */ Also, can you supply any specific commit id(s) for the originating source - We typically try to include as much canonical information to identify the source to a particular commit Regards, Graeme _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
