Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36211 )
Change subject: cpu/intel/car: Add EC software sync to Intel romstage ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1:
(1 comment)
Patch Set 1:
Patch Set 1: Code-Review-1
(3 comments)
Romstage starts here for all Intel platforms. I don't think it makes sense to put platforms specific things here.
That's fair. I'll drop this patch and look into that.
The alternative is to add call points to all mainboard_romstage_entry() code which is also decoupled from the Kconfig itself. i.e. selecting Kconfig won't necessarily make things work generically.
Don't those typically exist already to have mainboard specific init before the raminit?
Somewhat. for our mainboards I try to push everyone to have a common flow instead of open coding the flow. Basically have less duplicated code. If you look back to the mainboards that had traditionally been around in coreboot they have a large amount of duplicated code. That is a maintenance burden. If Kconfig is set up correctly with the proper dependencies then I don't see an issue in placing calls at a strategic point that minimizes code duplication.
How about having a linker script 'section(.rodata.romstage_entry)' for such fucntion pointers similar to the cbmem initialization? Is that too much complexity?
-
Arthur Heymans (Code Review)