Change in coreboot[master]: soc/intel/common/block/cse: Add helper function heci_send_receive
Hello Rizwan Qureshi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to review the following change.
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: sridhar sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/1
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 7bd46ce..97989f7 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -458,6 +458,27 @@ return 0; }
+int heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg, + size_t *rcv_sz) +{ + if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, HECI_MKHI_ADDR)) { + printk(BIOS_ERR, "Heci Send Failed\n"); + goto failed; + } + + if (rcv_msg != NULL) { + if (!heci_receive(rcv_msg, rcv_sz)) { + printk(BIOS_ERR, "Heci receive Failed\n"); + goto failed; + } + } + + return 1; + +failed: + return 0; +} + /* * Attempt to reset the device. This is useful when host and ME are out * of sync during transmission or ME didn't understand the message. diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index d7c4d9f..371a781 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -37,6 +37,15 @@ */ int heci_send(const void *msg, size_t len, uint8_t host_addr, uint8_t cse_addr); + +/* + * Sends snd_msg of size snd_sz, and reads message into buffer pointed by + * rcv_msg of size rcv_sz + * Returns 0 on failure a 1 on success. + */ +int heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg, + size_t *rcv_sz); + /* * Attempt device reset. This is useful and perhaps only thing left to do when * CPU and CSE are out of sync or CSE fails to respond.
build bot (Jenkins) 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 1:
(28 comments)
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 462: size_t *rcv_sz) code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 462: size_t *rcv_sz) please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 464: if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, HECI_MKHI_ADDR)) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 464: if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, HECI_MKHI_ADDR)) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 465: printk(BIOS_ERR, "Heci Send Failed\n"); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 465: printk(BIOS_ERR, "Heci Send Failed\n"); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 466: goto failed; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 466: goto failed; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 467: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 467: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 469: if (rcv_msg != NULL) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 469: if (rcv_msg != NULL) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 470: if (!heci_receive(rcv_msg, rcv_sz)) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 470: if (!heci_receive(rcv_msg, rcv_sz)) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 471: printk(BIOS_ERR, "Heci receive Failed\n"); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 471: printk(BIOS_ERR, "Heci receive Failed\n"); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 472: goto failed; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 472: goto failed; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 473: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 473: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 474: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 474: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 476: return 1; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 476: return 1; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 479: return 0; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 479: return 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 47: size_t *rcv_sz); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 47: size_t *rcv_sz); please, no spaces at the start of a line
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#2).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response. * Fix all indentation comments
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: sridhar sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 28 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/2
Rizwan Qureshi 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 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35224/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35224/2//COMMIT_MSG@10 PS2, Line 10: * Fix all indentation comments not required, the gerrit patchset history knows what you have done with the patch
Subrata Banik 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 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 462: remove tab
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 466: goto failed; return 0 from here
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 472: goto failed; return 0 from here
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 42: remove space ?
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#3).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/3
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#4).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/4
Sridhar Siricilla 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 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35224/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35224/2//COMMIT_MSG@10 PS2, Line 10: * Fix all indentation comments
not required, the gerrit patchset history knows what you have done with the patch
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 462:
remove tab
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 466: goto failed;
return 0 from here
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 472: goto failed;
return 0 from here
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 42:
remove space ?
Done
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#5).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/5
Sridhar Siricilla 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 5:
(32 comments)
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 462: size_t *rcv_sz)
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 462: size_t *rcv_sz)
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 464: if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, HECI_MKHI_ADDR)) {
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 464: if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, HECI_MKHI_ADDR)) {
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 465: printk(BIOS_ERR, "Heci Send Failed\n");
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 465: printk(BIOS_ERR, "Heci Send Failed\n");
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 466: goto failed;
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 466: goto failed;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 467: }
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 467: }
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 469: if (rcv_msg != NULL) {
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 469: if (rcv_msg != NULL) {
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 470: if (!heci_receive(rcv_msg, rcv_sz)) {
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 470: if (!heci_receive(rcv_msg, rcv_sz)) {
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 471: printk(BIOS_ERR, "Heci receive Failed\n");
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 471: printk(BIOS_ERR, "Heci receive Failed\n");
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 472: goto failed;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 472: goto failed;
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 473: }
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 473: }
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 474: }
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 474: }
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 476: return 1;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 476: return 1;
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 479: return 0;
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 479: return 0;
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 462:
Done
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 466: goto failed;
Done
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 472: goto failed;
Done
Done
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/2/src/soc/intel/common/block/... PS2, Line 42:
Done
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 47: size_t *rcv_sz);
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/35224/1/src/soc/intel/common/block/... PS1, Line 47: size_t *rcv_sz);
please, no spaces at the start of a line
Done
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#6).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
TEST=Verified on CML RVP & Hatch board
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/6
Hello Patrick Rudolph, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#7).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
TEST=Verified sending and receiving reply HECI message on CML RVP & Hatch board
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/7
Angel Pons 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 7: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 462: size_t *rcv_sz) Minor: this fits on the previous line (line length limit is 96 characters)
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 465: Heci Send Failed To be consistent with the other messages in this file, please use "HECI: send failed\n"
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 471: Heci receive Failed "HECI: receive failed\n"
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 47: size_t *rcv_sz); This would also fit on the previous line
Hello Patrick Rudolph, Angel Pons, Subrata Banik, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#8).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
TEST=Verified sending and receiving reply HECI message on CML RVP & Hatch board
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 25 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/8
Sridhar Siricilla 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 8:
(3 comments)
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 462: size_t *rcv_sz)
Minor: this fits on the previous line (line length limit is 96 characters)
Done
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 465: Heci Send Failed
To be consistent with the other messages in this file, please use "HECI: send failed\n"
Done
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 471: Heci receive Failed
"HECI: receive failed\n"
Done
Subrata Banik 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 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35224/8/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/8/src/soc/intel/common/block/... PS8, Line 48: move this also in same line as 96 line now ?
Hello Patrick Rudolph, Subrata Banik, Angel Pons, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#9).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Add a helper function to send a heci message and receive the response.
TEST=Verified sending and receiving reply HECI message on CML RVP & Hatch board
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/9
Sridhar Siricilla 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 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35224/8/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/8/src/soc/intel/common/block/... PS8, Line 48:
move this also in same line as 96 line now ?
Done
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35224/7/src/soc/intel/common/block/... PS7, Line 47: size_t *rcv_sz);
This would also fit on the previous line
Done
Hello Patrick Rudolph, Subrata Banik, Angel Pons, Aamir Bohra, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35224
to look at the new patch set (#10).
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Aggregate sending and receiving HECI messages into a single function.
TEST=Verified sending and receiving reply HECI message on CML RVP & Hatch board
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35224/10
Subrata Banik 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 10: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35224 )
Change subject: soc/intel/common/block/cse: Add helper function heci_send_receive ......................................................................
soc/intel/common/block/cse: Add helper function heci_send_receive
Aggregate sending and receiving HECI messages into a single function.
TEST=Verified sending and receiving reply HECI message on CML RVP & Hatch board
Change-Id: Ic95239eef8591d3aadf56a857c97f3f1e12b16ac Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35224 Reviewed-by: Subrata Banik subrata.banik@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 24 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 446c5ac..9520242 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -458,6 +458,22 @@ return 0; }
+int heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg, size_t *rcv_sz) +{ + if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, HECI_MKHI_ADDR)) { + printk(BIOS_ERR, "HECI: send Failed\n"); + return 0; + } + + if (rcv_msg != NULL) { + if (!heci_receive(rcv_msg, rcv_sz)) { + printk(BIOS_ERR, "HECI: receive Failed\n"); + return 0; + } + } + return 1; +} + /* * Attempt to reset the device. This is useful when host and ME are out * of sync during transmission or ME didn't understand the message. diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index d7c4d9f..424d483 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -37,6 +37,14 @@ */ int heci_send(const void *msg, size_t len, uint8_t host_addr, uint8_t cse_addr); + +/* + * Sends snd_msg of size snd_sz, and reads message into buffer pointed by + * rcv_msg of size rcv_sz + * Returns 0 on failure a 1 on success. + */ +int heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg, size_t *rcv_sz); + /* * Attempt device reset. This is useful and perhaps only thing left to do when * CPU and CSE are out of sync or CSE fails to respond.
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().
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Rizwan Qureshi (Code Review)
-
Sridhar Siricilla (Code Review)
-
Subrata Banik (Code Review)