On Thu, May 7, 2015 at 3:42 PM, Patrick Georgi via coreboot coreboot@coreboot.org wrote:
2015-05-07 22:00 GMT+02:00 Stefan Reinauer stefan.reinauer@coreboot.org:
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