Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18457 )
Change subject: soc/intel/common: Add bootblock common stage file ......................................................................
Patch Set 38:
(2 comments)
https://review.coreboot.org/c/coreboot/+/18457/38/src/soc/intel/common/basec... File src/soc/intel/common/basecode/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/18457/38/src/soc/intel/common/basec... PS38, Line 33: #if CONFIG(PLATFORM_USES_FSP1_1) : #include <fsp/bootblock.h> : #else : static inline void bootblock_fsp_temp_ram_init(void) {} : #endif remove this
https://review.coreboot.org/c/coreboot/+/18457/38/src/soc/intel/common/basec... PS38, Line 124: i guess we have some fundamental mistake in this patch
Flow should be
assembly code calling "bootblock_c_entry()"->call should come here in common bootblock->passes to bootblock_main_with_basetime() -> again from lib/bootblock.c it comes into common stage file here with dedicated code like this bootblock_soc_early_init/bootblock_soc_init/bootblock_pch_early_init/bootblock_pch_init/bootblock_cpu_early_init/bootblock_cpu_init -> Idea is to make all common operations across core/atom here and also provide a placeholder to perform any specific SoC related job like page table at line 93 (that code shouldn't be here)
So whoever debug can trace the flow easily and new SoC port job also reduced, if nothing specific to SoC code then bootblock.c won't even exist inside SoC directory.
-
Subrata Banik (Code Review)