Attention is currently required from: Martin Roth, Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Meera Ravindranath, Michael Niewöhner, Usha P, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50955 )
Change subject: soc/intel/common/basecode: Add bootblock common code ......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/basecode/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/50955/comment/57315822_9f8345d8 PS2, Line 27: void bootblock_soc_init(void)
I agree with the reasoning. […]
I see the problem now. Maybe there should be another layer. But I haven't thought this through:
generic coreboot code flow code chip code common chip code | : : : | call : : : |----------------->| : call : | |---------------------------->| | | : ret | | |<----------------------------| | | call : : | |----------->| call : | | |--------------->| | | | ret | | | ret |<---------------| | ret |<-----------| : |<-----------------| : :
Caveat: We can't keep things straight with 3 entities currently, this might make things worse.
It's kind of what your patch originally did, just with the code in different places. e.g. generic code calls base code calls a mix of common code and specific code.
So, how you it look like with the above ordering?
/* flow code */ void bootblock_soc_init(void) { soc_report_platform_info(); /* calls common code with soc-specific info as argument */ bootblock_pch_init(); tco_configure(); }
/* soc-specific code */ void soc_report_platform_info(void) { report_cpu_info(cpu_table, ARRAY_SIZE(cpu_table)); report_pci_info(pci_table, ARRAY_SIZE(pci_table)); }
However, I would start by naming and documenting the purpose of soc/intel/common/* subdirectories and make it clear which code is supposed to call where. Currently it's just a mesh and I'd like to avoid that. IOW, I would try to avoid to place this common bootblock_soc_init() implementation anywhere close to code that is supposed to be called from soc-specific code.