Some of the comments I provided might impact other SoC code. Please feel free to submit those changes as separate CLs.
13 comments:
File src/soc/intel/common/block/cse/cse.c:
Patch Set #4, Line 288: HECI_DELAY_READY
Just curious: How was this 15 second delay/wait time determined? I don't find any reference to it in BWG.
Since you are adding debug prints, it might be helpful to have a print that says how much time it took for cse to be ready in override mode here.
File src/soc/intel/common/block/include/intelblocks/cse.h:
MKHI
While you are cleaning up names, may be update this to cse_.
Patch Set #4, Line 111: is_cse_enabled
For consistency: cse_is_enabled()
nit: space before *
Patch Set #4, Line 121: wait_cse_sec_override_mode
For consistency: cse_wait_sec_override_mode()
Patch Set #4, Line 128: send_heci_reset_req_message
I think this should be: cse_request_global_reset() to match what BWG says. But I don't see Host reset or CSE reset only options in it. Can you please point me to the right doc?
Patch Set #4, Line 134: send_hmrfpo_enable_msg
For consistency: cse_hmrfpo_enable()
Patch Set #4, Line 141: send_hmrfpo_get_status_msg
For consistency: cse_hmrfpo_get_status()
Patch Set #4, Line 144: BIOS_HOST_ADDR
I think this should be HECI_HOST_ADDR for consistency.
/* Command GLOBAL_RESET_REQ Reset Types */
#define GLOBAL_RESET 1
#define HOST_RESET_ONLY 2
#define CSE_RESET_ONLY 3
These should be placed closer to send_heci_reset_req_message(). Actually, I think having an enum here might be helpful and then the param to send_heci_reset_req_message() can be of the enum type.
nit: space after *
To view, visit change 37282. To unsubscribe, or for help writing mail filters, visit settings.