Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29662 )
Change subject: {drivers,soc/intel/braswell}: Add C_ENVIRONMENT_BOOTBLOCK support ......................................................................
Patch Set 27:
(4 comments)
Will perform some more testing and update a new patch set.
https://review.coreboot.org/#/c/29662/27//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/29662/27//COMMIT_MSG@13 PS27, Line 13: romstage_c_entry
you have hooks to do this in the bootblock.
Comment of PS13 was to add this functions. I realize that bootblock takes care of init, but was not 100% sure about non-bootblock implementation.
https://review.coreboot.org/#/c/29662/27/src/drivers/intel/fsp1_1/car.c File src/drivers/intel/fsp1_1/car.c:
https://review.coreboot.org/#/c/29662/27/src/drivers/intel/fsp1_1/car.c@105 PS27, Line 105: car_soc_pre_console_init(); : car_mainboard_pre_console_init();
You generally want the console set up in the bootblock, not in the romstage with C_ENVIRONMENT_BOOTB […]
Same here. Should this functions not be placed around the console_init() to be compatible with other location where console_init() is used?
https://review.coreboot.org/#/c/29662/27/src/drivers/intel/fsp1_1/car.c@110 PS27, Line 110: car_soc_post_console_init(); : car_mainboard_post_console_init();
Similarly you have bootblock_soc_init() and bootblock_mainboard_init().
Will not add these calls here.
https://review.coreboot.org/#/c/29662/27/src/soc/intel/braswell/bootblock/bo... File src/soc/intel/braswell/bootblock/bootblock.c:
https://review.coreboot.org/#/c/29662/27/src/soc/intel/braswell/bootblock/bo... PS27, Line 50:
Where is this done now?
Adding bootblock_cpu_init function result into message that fucntion is not used. For this reason (and the fact that system is booting without problem) This function was removed.
For test purposed I have added function bootblock_soc_early_init to call this function. Will perform some more testing and add this to the patch set.