Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37283 )
Change subject: soc/intel/common/block/cse: Modify handling of HMRFPO_ENABLE command ......................................................................
Patch Set 4:
(11 comments)
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@9 PS4, Line 9: Below changes have been done in send_hmrfpo_enable_msg():
Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@11 PS4, Line 11: operation mode is Temp Disable Mode.
Please remove the dot/period at the end, or at one to the third item.
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@12 PS4, Line 12: 2. Add check for response status.
Check response status
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@13 PS4, Line 13: 3. Add description for the send_hmrfpo_enable_msg()
Describe/document send_hmrfpo_enable_msg()
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@17 PS4, Line 17: EOP
What is that?
CSE does not honour HMRFPO_ENABLE command after it receives EOP message from CSE. Typically, Coreboot sends EOP(End of Post) message to CSE to indicate end of POST & start of OS load.
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@20 PS4, Line 20: 1. Ensure CSE to boot from BP1. When CSE boots from BP1, it will have
Ensure CSE boots from BP1. […]
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@21 PS4, Line 21: opmode-
Remove -.
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@22 PS4, Line 22: 2. Send HMRFPO_ENABLE command to CSE.
Remove line break.
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@26 PS4, Line 26: allow
allows
Done
https://review.coreboot.org/c/coreboot/+/37283/4//COMMIT_MSG@39 PS4, Line 39: TEST=Verfied on hatch board.
How did you verify this?
This is verified through CSE FW upgrade procedure. Please refer the patch:https://review.coreboot.org/c/coreboot/+/35403
https://review.coreboot.org/c/coreboot/+/37283/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37283/4/src/soc/intel/common/block/... PS4, Line 682: printk(BIOS_ERR, "HECI: Enable Failed, resp status:%d\n", resp.status);
Please add a space after the colon. […]
Done