Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35226 )
Change subject: src/soc/intel/common/block/cse: Make hfsts1 common & add helper functions ......................................................................
Patch Set 25:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35226/25//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35226/25//COMMIT_MSG@9 PS25, Line 9: Host FW status 1 (FWSTS1/HFSTS1) register definition is common across SoCs,
Is this true even for small core? If yes, why was APL not updated to use this?
APL is just dumping all the FWSTS registers and not utilising the decoded FWSTS1.
https://review.coreboot.org/c/coreboot/+/35226/25/src/soc/intel/cannonlake/m... File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/c/coreboot/+/35226/25/src/soc/intel/cannonlake/m... PS25, Line 215: hfsts2
What about the other hfsts? Are they not common across SoCs?
Yes, they are differences in other hfsts registers across SoCs.
https://review.coreboot.org/c/coreboot/+/35226/25/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35226/25/src/soc/intel/common/block... PS25, Line 266: 'HECI_OP_MODE_SEC_OVERRIDE' for 15 seconds
How was the 15 second timeout decided?
no recommendation as such, used the same timeout as the one used for wait_heci_ready