On 17/09/08 17:05 +0200, Stefan Reinauer wrote:
Robert Millan wrote:
On Wed, Sep 17, 2008 at 11:50:07AM +0200, Peter Stuge wrote:
Robert Millan wrote:
Void-ify a few return types that assume presence of a native coreboot table (and weren't actually used for anything).
Signed-off-by: Robert Millan rmh@aybabtu.com
Sorry, but I still don't understand the motivation for this. Tell me why it's needed and I'll probably ack straight away. :)
With my Multiboot patch, one can no longer assume native coreboot tables unconditionally, since Multiboot might be used instead. coreboot_table.c, which provides get_lb_mem(), might not be linked in.
However, arch_write_tables() uses get_lb_mem() unconditionally as its return value. But I observed that this value isn't actually used for anything. The caller (write_tables()) in turn uses it as return value, and then it is discarded by stage2().
So, rather than introducing a bunch of ifdefs to conditionalize this, my patches opt to remove it. This way both build options get the size benefit.
Also, IMHO it didn't make much sense for a function that generates different types of data structures (write_tables()) to have a return value that is specific to only one of these.
Just my 2ct:
I have a bit of a hick-up with removing the coreboot table, as this renders utility like flashrom (in some cases) and nvramtool (in all cases) pretty much unusable.
Also, I think bootloader stuff is libpayload/filo/grub2 territory.
I'm not groking the resistance to these tables. The coreboot tables are good, but they have the distinct disadvantage of not being groked by anything out side of our little ecosystem of payloads. Tables are cheap, and it doesn't hurt us at all to include as many as we can.
Jordan