Attention is currently required from: Tim Wawrzynczak, Sridhar Siricilla, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62560 )
Change subject: soc/intel/common: Retry MEI CSE DISABLE command ......................................................................
Patch Set 3:
(3 comments)
File src/soc/intel/common/block/cse/cse_eop.c:
https://review.coreboot.org/c/coreboot/+/62560/comment/f65146e6_2658bfb7 PS3, Line 14: MAX_RETRY_EOP_MSG does it make sense to create enum as below, you can make this flexible to add short or long retry counts if needed in future?
enum cse_retry_count { CSE_NO_RETRY = 0, CSE_MAX_RETRY = 3, };
https://review.coreboot.org/c/coreboot/+/62560/comment/0631b46d_a865c6d4 PS3, Line 31: decode_heci_send_receive_error how this function is different than decode_eop_error() below?
I'm missing anything here?
https://review.coreboot.org/c/coreboot/+/62560/comment/39803506_ee104399 PS3, Line 77: cse_send_bus_disable_message it's almost same as cse_send_end_of_post_message()
try to create a function that takes another function pointer as argument and execute.
incase of cse_send_end_of_post_message(), function pointer would be cse_send_eop()
and in this example: cse_disable_mei_bus().
You don't need to write the same function in future as well.
Can you please think of creating a custom data structure that pass the command name and func_ptr().