Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37584 )
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
Patch Set 20:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 604:
Shouldn't this check the CWS and COP to ensure it is normal?
Yes, it requires. I will push separate patch to address your comment.
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 612: heci_reset
Why is heci_reset() required here?
It resets HECI interface to ensure syncronization between CSE & Host for communication.
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 615: memset(&reply, 0, reply_size);
Why is this required?
Done
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 617: CSE_RESET_ONLY
Can you please point me to the doc that talks about this?
Currently, no external doc talk about CSE_RESET_ONLY. We are working on documentation(Addendum to BWG). It will be available soon.
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 135: send_heci_reset_req_message
Since we are touching this, I think we should rename this to cse_request_reset() to make naming more […]
Done