Hi Gabe, On Tue, Dec 6, 2011 at 9:06 AM, Gabe Black <gabebl...@google.com> wrote: > > > On Sat, Dec 3, 2011 at 4:52 PM, Graeme Russ <graeme.r...@gmail.com> wrote: >> >> Hi Gabe, >> >> Last nit, I promise, and then I'll apply it all to u-boot-x86/next >> > >> > +/* Implementation defined function to install an e820 map. */ >> > +unsigned install_e820_map(unsigned max_entries, struct e820entry *); >> > + >> >> Should be u8 as boot_params.e820_entries is u8 (__u8 really, but we don't >> use the kernel data types) >> > > I don't think this is a good idea. The value here isn't consumed by the > boot_params structure, it's taken from the size of the e820 array there and > consumed by whatever function is written to copy over e820 entries. That > function doesn't really care about the boot_params structure at all. It > could be used to fill in entries for, say, some future boot protocol, > debugging where you want to see all entries beyond the first 256 (a lot, but > EFI tends to have a lot), or maybe I want to boot some other OS that doesn't > use a u8 to determine the size of the e820 table. There isn't any danger of > the size being mismatched because it's called with the size of the array > being filled, and the boot_params structure is smart enough to use a data > type that's sized appropriately for the array.
Good point - I'm happy for you to leave as is I must say, I am somewhat suprised that the memory map would ever need to be split into so many pieces... Regards, Graeme _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot