Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41728 )
Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/si... File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/si... PS3, Line 41: fsp_handle_reset(status);
FSP requests System reset through "status" (return value) is not an error scenario.
If FSP insist system reset using Return value FSP_STATUS_RESET_REQUIRED_* then control won't come back to https://review.coreboot.org/c/coreboot/+/41728/3..4/src/drivers/intel/fsp2_0....
Assume FSP didn't request reset but there are some failure during FSP-S execution which results !EFI_SUCCESS those been handled between line 42-69. As per FSP spec, upon returning call from FSP to bootloader, we need to check return status idea is to create a helper function rather duplicating things for 'n' number of times in coreboot code to check FSP-S all possible API return status.
It can be due to the modified register settings take effect only post-reset. So, I suggest triggering system reset should be handled in the outside of function.
i don't understand how it might be helpful, i'm guessing you have been carried by function name. its been changed now :)
If FSP's reset request is considered as an error, then it should be handled differently like triggering system recovery to avoid back to back resets!
function name changed now
-
Subrata Banik (Code Review)