Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35229 )
Change subject: src/soc/intel/common/block/cse: Add hmrfpo related functions to cse lib ......................................................................
Patch Set 19:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... PS19, Line 79: MKHI_HMRFPO_GROUP_ID Can you please make the names more consistent. Line 74 and line 79 use very different ways of defining these constants. Also, I would like to see related things grouped together appropriately.
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... PS19, Line 652: uint32_t fct_base; : uint32_t fct_limit; : uint8_t status; What are these fields?
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... PS19, Line 683: } Don't you care about the status?
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... PS19, Line 117: Send HMRFPO_ENABLE command. This should have a better explanation of what this command really means.
https://review.coreboot.org/c/coreboot/+/35229/19/src/soc/intel/common/block... PS19, Line 118: failure Does failure mean failure to send message or failure to enable?