Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/Makef... File src/soc/intel/common/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/Makef... PS10, Line 32: postcar fsp_reset.c is really required only in romstage and ramstage. postcar does not integrate any FSP component.
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... File src/soc/intel/common/fsp_reset.c:
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 3: #include <cf9_reset.h> Not required?
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 11: */ Probably add a #error here if FSP_STATUS_GLOBAL_RESET is not defined so that the user knows what needs to be done.
#ifndef FSP_STATUS_GLOBAL_RESET #error "Define FSP_STATUS_GLOBAL_RESET in SoC to match the FSP OEM status code for global reset" #endif
The other option if we want to avoid 0xffffffff being defined in SoC when global reset code is not present would be to add a Kconfig FSP_STATUS_GLOBAL_RESET in drivers/intel/fsp2_0/Kconfig which defaults to 0xffffffff.
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 22: Reorganize as: ``` if (status == FSP_STATUS_GLOBAL_RESET) { printk(BIOS_DEBUG, "GLOBAL RESET!\n"); global_reset(); }
printk(BIOS_ERR, "unhandled reset type %x\n", status); die("unknown reset type"); ```
Then you don't need to check for FSP_STATUS_GLOBAL_RESET separately.