Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/26138 )
Change subject: soc/intel/common/{block, pch}: Move smihandler common functions into common code ......................................................................
Patch Set 38:
(1 comment)
https://review.coreboot.org/c/coreboot/+/26138/38/src/soc/intel/common/pch/s... File src/soc/intel/common/pch/smm/smihandler.c:
PS38:
I am curious why this is a separate file and the contents here were not moved to soc/intel/common/block/smm?
majorly because of APL/GLK not having exact same code as big core platform are using
For an example:
1. get_smm_save_state_ops function returns "em64t100_smm_ops" for APL/GLK and cores are using em64t101_smm_ops
https://github.com/coreboot/coreboot/blob/master/src/soc/intel/apollolake/sm...
const struct smm_save_state_ops *get_smm_save_state_ops(void) { return &em64t100_smm_ops; }
2. smihandler_soc_get_sci_mask function is doing the same operation in big core and small cores but bit definitions are not same
for APL/GLK below macro names are not same with cores SLP_SMI_STS, APM_SMI_STS. All small cores are using consistent macro name <something>_STS in pm.h
but on core platform we are using macro names like <something>_STS_BIT example: APM_STS_BIT and SMI_ON_SLP_EN_STS_BIT
Was thinking why we need to rename macros in APL/GLK to align with core platform. hence this file exist to maintain common core platform functions rather moving into common/block
Also, how was it decided that function smihandler_soc_get_sci_mask() should live in this file?
I hope to provide answer above, let me know if some confusion