Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35224 )
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
Patch Set 11:
(3 comments)
Sorry, I am just looking at this patch train. Please address the comments as follow-up CLs.
https://review.coreboot.org/c/coreboot/+/35224/11/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/11/src/soc/intel/common/block... PS11, Line 463: BIOS_HOST_ADDR, HECI_MKHI_ADDR If these are hard-coded here, why give an option to the user to provide the addresses at all? Maybe in a follow up change, it might make sense to just move this into heci_send() directly and update its signature.
https://review.coreboot.org/c/coreboot/+/35224/11/src/soc/intel/common/block... PS11, Line 468: if (rcv_msg != NULL) { Isn't it an error if rcv_msg is NULL. Shouldn't it return 0?
https://review.coreboot.org/c/coreboot/+/35224/11/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/11/src/soc/intel/common/block... PS11, Line 42: Sends snd_msg of size snd_sz, and reads message into buffer pointed by : * rcv_msg of size rcv_sz Params should be explained better. Or at least refer to the comments for heci_receive() and heci_send().