Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29662 )
Change subject: soc/intel/braswell: Add C_ENVIRONMENT_BOOTBLOCK support ......................................................................
Patch Set 9:
(5 comments)
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/Kconfig File src/soc/intel/braswell/Kconfig:
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/Kconfig@68 PS9, Line 68: VBOOT_STARTS_IN_ROMSTAGE
Could be changed.
Will change to VBOOT_STARTS_IN_BOOTBLOCK
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/Kconfig@70 PS9, Line 70: config BOOTBLOCK_CPU_INIT : string : default "soc/intel/braswell/bootblock/bootblock.c"
unused now
Ack
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/bootblock/boo... File src/soc/intel/braswell/bootblock/bootblock.c:
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/bootblock/boo... PS9, Line 76:
Microcode needs to be updated before running CAR. You can use cpu/intel/microcode/microcode_asm. […]
This code is not used in latest patch.
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/romstage/car_... File src/soc/intel/braswell/romstage/car_stage_entry.S:
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/romstage/car_... PS9, Line 24: #include "src/drivers/intel/fsp1_1/after_raminit.S"
It is better to link files than to include them like this.
This include file does not have entry point (yet) Now similar to other platforms
https://review.coreboot.org/#/c/29662/9/src/soc/intel/braswell/romstage/car_... PS9, Line 31: #if IS_ENABLED(CONFIG_POST_IO) : outb %al, $CONFIG_POST_IO_PORT : #else : post_code(POST_DEAD_CODE) : #endif
Why is this needed?
Will remove the post_code() call. This is just dummy code when CONFIG_POST_IO is not specified.