On Thu, May 7, 2015 at 3:42 PM, Patrick Georgi via coreboot <[email protected]> wrote: > 2015-05-07 22:00 GMT+02:00 Stefan Reinauer <[email protected]>: >> With our current bootblock concept, it is never going away on x86 (for >> bootblock usage) > Which isn't that much of a problem once we provide separate headers > for x86 bootblock code. There's really very tiny overlap. > That could then be reused to deal with raminit on romcc-boards, too: > from coreboot's point of view, raminit is just an overly large > piece of cache-as-ram code, followed by a raminit noop. > This is simplified by the lack of the need for development tools (eg > printk) to develop new non-car x86 raminits. > >
I think bootblock code overlap is tiny. However, device_t is more complicated than just bootblock and !bootblock. The io routines differ by stage: romstage and bootblock use the simple u32 device_t ramstage uses the sturct device device_t And lest we not forget SMM on x86 which only has simple u32 device_t. In romstage *both* struct device and device_t are present but are completely different types. It's the romstage, ramstage, and smm overlap that is large and extremely annoying to deal with. A few examples on boiler plate one needs to deal with for code sharing: src/soc/intel/baytrail/pmutil.c -- all the get_pcu_dev() src/soc/intel/baytrail/spi.c So basically while the function names are the same for the IO routines the parameter types change. So every caller of a compilation unit of these IO routines that wants to be shared needs to deal w/ the current type changes. I personally feel that changing device_t type based on stage makes the code non-obvious and hard to follow. I'd rather we *always* provide simple u32 device_t functions in all stages while allowing struct device IO functions for use in ramstage. The simple ones in ramstage can just be a struct device lookup then call appropriate IO function. -Aaron -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
