Change in coreboot[master]: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_r...
Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37584 )
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values.
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 6 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/1
diff --git a/src/soc/intel/cannonlake/reset.c b/src/soc/intel/cannonlake/reset.c index 4758faf..6889bb0 100644 --- a/src/soc/intel/cannonlake/reset.c +++ b/src/soc/intel/cannonlake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (send_heci_reset_req_message(GLOBAL_RESET)) return;
/* global reset if CSE fail to reset */ diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index b60d888..f40e328 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -583,7 +583,7 @@ * Sends GLOBAL_RESET_REQ cmd to CSE.The reset type can be GLOBAL_RESET/ * HOST_RESET_ONLY/CSE_RESET_ONLY. */ -int send_heci_reset_req_message(uint8_t rst_type) +int send_heci_reset_req_message(enum rst_req_type rst_type) { int status; struct mkhi_hdr reply; @@ -602,27 +602,25 @@ }; size_t reply_size;
+ printk(BIOS_DEBUG, "HECI: Global Reset(Type:%d) Command\n", rst_type); if (!((rst_type == GLOBAL_RESET) || - (rst_type == HOST_RESET_ONLY) || (rst_type == CSE_RESET_ONLY))) - return -1; + (rst_type == HOST_RESET_ONLY) || (rst_type == CSE_RESET_ONLY))) { + printk(BIOS_ERR, "HECI: Unsupported reset type is requested\n"); + return 0; + }
heci_reset();
reply_size = sizeof(reply); memset(&reply, 0, reply_size);
- printk(BIOS_DEBUG, "HECI: Global Reset(Type:%d) Command\n", rst_type); if (rst_type == CSE_RESET_ONLY) - status = heci_send_receive(&msg, sizeof(msg), NULL, 0); + status = heci_send(&msg, sizeof(msg), BIOS_HOST_ADDR, HECI_MKHI_ADDR); else - status = heci_send_receive(&msg, sizeof(msg), &reply, - &reply_size); + status = heci_send_receive(&msg, sizeof(msg), &reply, &reply_size);
- if (status != 1) - return -1; - - printk(BIOS_DEBUG, "HECI: Global Reset success!\n"); - return 0; + printk(BIOS_DEBUG, "HECI: Global Reset %s!\n", status ? "success" : "failure"); + return status; }
/* Sends HMRFPO Enable command to CSE */ diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 53f5493..9ca1a5f 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -120,12 +120,19 @@ */ uint8_t wait_cse_sec_override_mode(void);
+/* Command GLOBAL_RESET_REQ Reset Types */ +enum rst_req_type { + GLOBAL_RESET = 1, + HOST_RESET_ONLY = 2, + CSE_RESET_ONLY = 3, +}; + /* - * Sends GLOBAL_RESET_REQ cmd to CSE.The reset type can be - * GLOBAL_RESET/HOST_RESET_ONLY/CSE_RESET_ONLY. - * Returns -1 on failure a 0 on success. + * Sends GLOBAL_RESET_REQ cmd to CSE. + * The reset type can be one of the above defined reset type. + * Returns 0 on failure a 1 on success. */ -int send_heci_reset_req_message(uint8_t rst_type); +int send_heci_reset_req_message(enum rst_req_type rst_type);
/* * Sends HMRFPO_ENABLE command. @@ -156,11 +163,6 @@ #define BIOS_HOST_ADDR 0x00 #define HECI_MKHI_ADDR 0x07
-/* Command GLOBAL_RESET_REQ Reset Types */ -#define GLOBAL_RESET 1 -#define HOST_RESET_ONLY 2 -#define CSE_RESET_ONLY 3 - /*HMRFPO Status types */ #define MKHI_HMRFPO_DISABLED 0 #define MKHI_HMRFPO_LOCKED 1 diff --git a/src/soc/intel/icelake/reset.c b/src/soc/intel/icelake/reset.c index 5526a42..5ac8f6f 100644 --- a/src/soc/intel/icelake/reset.c +++ b/src/soc/intel/icelake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (send_heci_reset_req_message(GLOBAL_RESET)) return;
/* global reset if CSE fail to reset */ diff --git a/src/soc/intel/skylake/reset.c b/src/soc/intel/skylake/reset.c index 8f5bf30..b16e11c 100644 --- a/src/soc/intel/skylake/reset.c +++ b/src/soc/intel/skylake/reset.c @@ -37,7 +37,7 @@
void do_global_reset(void) { - if (send_global_reset() != 0) { + if (!send_global_reset()) { /* If ME unable to reset platform then * force global reset using PMC CF9GR register*/ do_force_global_reset(); diff --git a/src/soc/intel/tigerlake/reset.c b/src/soc/intel/tigerlake/reset.c index 674cf68..d3d3340 100644 --- a/src/soc/intel/tigerlake/reset.c +++ b/src/soc/intel/tigerlake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (send_heci_reset_req_message(GLOBAL_RESET)) return;
/* global reset if CSE fail to reset */
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#2).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values.
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 6 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/2
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#4).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values.
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 6 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/4
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Patrick Georgi, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#6).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values.
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 6 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/6
Furquan Shaikh 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 6:
(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?
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 612: heci_reset Why is heci_reset() required here?
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?
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?
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 consistent.
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#7).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values.
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 6 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/7
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#8).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset().
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/me.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 7 files changed, 26 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/8
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#9).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset().
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/me.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 7 files changed, 26 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/9
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#15).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset().
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/me.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 7 files changed, 25 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/15
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#16).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset().
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/me.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 7 files changed, 25 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/16
Furquan Shaikh 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 17: Code-Review+2
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
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Sridhar Siricilla, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37584
to look at the new patch set (#21).
Change subject: soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset().
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/me.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 7 files changed, 25 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/37584/21
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 21:
(3 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:
Yes, it requires. I will push separate patch to address your comment.
The patch implements prerequisites https://review.coreboot.org/c/coreboot/+/38800/1
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 612: heci_reset
It resets HECI interface to ensure syncronization between CSE & Host for communication.
Done
https://review.coreboot.org/c/coreboot/+/37584/6/src/soc/intel/common/block/... PS6, Line 617: CSE_RESET_ONLY
Currently, no external doc talk about CSE_RESET_ONLY. […]
Done
Patrick Georgi has submitted 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() ......................................................................
soc/intel/{cnl,icl,skl,tgl,common}: Make changes to send_heci_reset_req_message()
Below changes have been implemented in send_heci_reset_req_message(): 1. Modify return values to align with other functions in the same file. 2. Add additional logging. 3. Replace macro definitions of reset types with ENUM. 4. Make changes to caller functions to sync with new return values. 5. Rename send_heci_reset_req_message() to cse_request_global_reset().
Test=Verified on hatch board.
Change-Id: I979b169a5bb3a5d4028ef030bcef2b8eeffe86e3 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37584 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h M src/soc/intel/icelake/reset.c M src/soc/intel/skylake/me.c M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/reset.c 7 files changed, 25 insertions(+), 26 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/reset.c b/src/soc/intel/cannonlake/reset.c index 4758faf..28211e3 100644 --- a/src/soc/intel/cannonlake/reset.c +++ b/src/soc/intel/cannonlake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (cse_request_global_reset(GLOBAL_RESET)) return;
/* global reset if CSE fail to reset */ diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index d323c76..c82f3bd 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -582,7 +582,7 @@ * Sends GLOBAL_RESET_REQ cmd to CSE.The reset type can be GLOBAL_RESET/ * HOST_RESET_ONLY/CSE_RESET_ONLY. */ -int send_heci_reset_req_message(uint8_t rst_type) +int cse_request_global_reset(enum rst_req_type rst_type) { int status; struct mkhi_hdr reply; @@ -601,27 +601,25 @@ }; size_t reply_size;
+ printk(BIOS_DEBUG, "HECI: Global Reset(Type:%d) Command\n", rst_type); if (!((rst_type == GLOBAL_RESET) || - (rst_type == HOST_RESET_ONLY) || (rst_type == CSE_RESET_ONLY))) - return -1; + (rst_type == HOST_RESET_ONLY) || (rst_type == CSE_RESET_ONLY))) { + printk(BIOS_ERR, "HECI: Unsupported reset type is requested\n"); + return 0; + }
heci_reset();
reply_size = sizeof(reply); memset(&reply, 0, reply_size);
- printk(BIOS_DEBUG, "HECI: Global Reset(Type:%d) Command\n", rst_type); if (rst_type == CSE_RESET_ONLY) - status = heci_send_receive(&msg, sizeof(msg), NULL, 0); + status = heci_send(&msg, sizeof(msg), BIOS_HOST_ADDR, HECI_MKHI_ADDR); else - status = heci_send_receive(&msg, sizeof(msg), &reply, - &reply_size); + status = heci_send_receive(&msg, sizeof(msg), &reply, &reply_size);
- if (status != 1) - return -1; - - printk(BIOS_DEBUG, "HECI: Global Reset success!\n"); - return 0; + printk(BIOS_DEBUG, "HECI: Global Reset %s!\n", status ? "success" : "failure"); + return status; }
/* Sends HMRFPO Enable command to CSE */ diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index 1377bd4..aff330a 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -112,12 +112,18 @@ */ uint8_t cse_wait_sec_override_mode(void);
+enum rst_req_type { + GLOBAL_RESET = 1, + HOST_RESET_ONLY = 2, + CSE_RESET_ONLY = 3, +}; + /* - * Sends GLOBAL_RESET_REQ cmd to CSE.The reset type can be - * GLOBAL_RESET/HOST_RESET_ONLY/CSE_RESET_ONLY. - * Returns -1 on failure and 0 on success. + * Sends GLOBAL_RESET_REQ cmd to CSE. + * The reset type can be one of the above defined reset type. + * Returns 0 on failure and 1 on success. */ -int send_heci_reset_req_message(uint8_t rst_type); +int cse_request_global_reset(enum rst_req_type rst_type);
/* * Send HMRFPO_ENABLE command. @@ -138,11 +144,6 @@ /* Fixed Address MEI Header's ME Address field value */ #define HECI_MKHI_ADDR 0x07
-/* Command GLOBAL_RESET_REQ Reset Types */ -#define GLOBAL_RESET 1 -#define HOST_RESET_ONLY 2 -#define CSE_RESET_ONLY 3 - /* HMRFPO Status types */ /* Host can't access ME region */ #define MKHI_HMRFPO_DISABLED 0 diff --git a/src/soc/intel/icelake/reset.c b/src/soc/intel/icelake/reset.c index 5526a42..d79ae45 100644 --- a/src/soc/intel/icelake/reset.c +++ b/src/soc/intel/icelake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (cse_request_global_reset(GLOBAL_RESET)) return;
/* global reset if CSE fail to reset */ diff --git a/src/soc/intel/skylake/me.c b/src/soc/intel/skylake/me.c index d53d91e..17a66bc 100644 --- a/src/soc/intel/skylake/me.c +++ b/src/soc/intel/skylake/me.c @@ -441,7 +441,7 @@ goto ret;
/* ME should be in Normal Mode for this command */ - status = send_heci_reset_req_message(GLOBAL_RESET); + status = cse_request_global_reset(GLOBAL_RESET); ret: return status; } diff --git a/src/soc/intel/skylake/reset.c b/src/soc/intel/skylake/reset.c index 8f5bf30..b16e11c 100644 --- a/src/soc/intel/skylake/reset.c +++ b/src/soc/intel/skylake/reset.c @@ -37,7 +37,7 @@
void do_global_reset(void) { - if (send_global_reset() != 0) { + if (!send_global_reset()) { /* If ME unable to reset platform then * force global reset using PMC CF9GR register*/ do_force_global_reset(); diff --git a/src/soc/intel/tigerlake/reset.c b/src/soc/intel/tigerlake/reset.c index 674cf68..11e411d 100644 --- a/src/soc/intel/tigerlake/reset.c +++ b/src/soc/intel/tigerlake/reset.c @@ -24,7 +24,7 @@ void do_global_reset(void) { /* Ask CSE to do the global reset */ - if (!send_heci_reset_req_message(GLOBAL_RESET)) + if (cse_request_global_reset(GLOBAL_RESET)) return;
/* global reset if CSE fail to reset */
Patrick Georgi 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 22:
In the future, consider writing more descriptive commit messages, please. Also, some of the changes could probably be spun out in separate commits.
-
Furquan Shaikh (Code Review) -
Patrick Georgi (Code Review)
-
Sridhar Siricilla (Code Review)