Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 )
Change subject: soc/intel/{common,skl,cnl,icl,apl,tgl}: Move HFSTS1 register definition to SoC ......................................................................
Patch Set 27: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/cannonlake/i... File src/soc/intel/cannonlake/include/soc/me.h:
https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/cannonlake/i... PS27, Line 34: u32 reserved0: 1; Isn't this debug_mode for CML? Does this need to be defined differently based on SOC_INTEL_COMETLAKE? I haven't checked what happens for WHL.
https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/common/block... PS27, Line 253: union me_hfsts1 hfs1; : hfs1.data = me_read_config32(PCI_ME_HFSTS1); : if (hfs1.fields.operation_mode == ME_HFS1_COM_NORMAL) : return true; : return false; nit: You can also add a helper function:
bool cse_check_hfs1_com(int mode) { union me_hfsts1 hfs1; hfs1.data = me_read_config32(PCI_ME_HFSTS1); return hfs1.fields.operation_mode == mode; }
bool cse_is_hfs1_com_normal(void) { return cse_check_hfs1_com(ME_HFS1_COM_NORMAL); }
bool cse_is_hfs1_com_secover_mei_msg(void) { return cse_check_hfs1_com(ME_HFS1_COM_SECOVER_MEI_MSG); }
bool cse_is_hfs1_com_soft_temp_disable(void) { return cse_check_hfs1_com(ME_HFS1_COM_SOFT_TEMP_DISABLE); }