Sridhar Siricilla 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 17:
(9 comments)
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 632: /* : * Sends HMRFPO Enable command to CSE : */
nit: multiline comment not needed.
Done. thanks
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 639: uint32_t nonce[2];
used for? Can we have description on the fields? […]
In client platforms, those are reserved fields.
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 654: uint32_t fct_base; : uint32_t fct_limit;
same.
Those are reserved fields in the client platforms.
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 660: struct hmrfpo_enable_resp resp;
define variables at start of the function
It must be here only due to structure "hmrfpo_enable_resp" declatration
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 661: size_t resp_size
same.
Not required!
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 662: union me_hfsts1 hfs1;
same
Not required!
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 669: * - Operation mode is normal or : * temporary disable mode.
indentation.
Done
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 672: (
nit: braces can go.
Done
https://review.coreboot.org/c/coreboot/+/35229/16/src/soc/intel/common/block... PS16, Line 715: struct hmrfpo_get_status_resp resp; : size_t resp_size
define variables at start of function?
Not required