Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45336 )
Change subject: soc/intel/common/basecode: Add Intel common reset code ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45336/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45336/5//COMMIT_MSG@7 PS5, Line 7: soc/intel/common/block
without any guard, its giving compilation error saying cse_request_global_reset unknown function reference.
I think we can add a check for CONFIG(SOC_INTEL_COMMON_BLOCK_CSE). In general, I think it is confusing that there are two different reset files under common and with very similar functionality.
Can we please make an attempt to consolidate this into a single place? I don't think the cse function call requires #if. You can have a C check: if (CONFIG(SOC_INTEL_COMMON_BLOCK_CSE)) cse_request_global_reset();
One thing that I think we would need to keep separate is the chipset_handle_reset which is used only by FSP2.0+. Considering that, I think we can have two files under soc/intel/common/basecode/reset:
reset.c - Provides implementation of global_reset, do_global_reset, do_board_reset. fsp_reset.c - Provides implementation of chipset_handle_reset. This file will get included only if PLATFORM_USES_FSP2_0.
Thoughts?