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,
What about the operation state, operation mode, working state and other fields? Do they all have the […]
Yes, rest of the bits remain same. The actual values represented by certain bits may differ, which have to be defined in SoC code. e.g, ICL CSME FW may define one extra operation mode than a CNL CSME.
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 273: if (stopwatch_expired(&sw))
It would be good to add a print indicating that timeout occurred. […]
WIll fix in a separate patch, Done
https://review.coreboot.org/c/coreboot/+/35226/25/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35226/25/src/soc/intel/common/block... PS25, Line 101: set_host_ready
This is a very generic name. […]
Will be done as separate patch, Done