Change in coreboot[master]: ec/google/chromeec: ignore LIMIT_POWER based on command code in response

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Rizwan Qureshi (Code Review)

27 Dec 2019 27 Dec '19

6:53 p.m.

Rizwan Qureshi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37947 )

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

ec/google/chromeec: ignore LIMIT_POWER based on command code in response

Assume that LIMIT_POWER is not requested if the ec does not support it. Do this by checking the command code in the response message instead of return value.

BUG=b:146165519 BRANCH=None TEST=Boot puff with EC which does not support LIMIT_POWER param.

Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/ec/google/chromeec/ec.c 1 file changed, 10 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37947/1

diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index de43eb5..02b26a0 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -760,14 +760,19 @@ int rv;

rv = google_chromeec_command(&cmd); - if (rv == -EC_RES_INVALID_PARAM || rv == -EC_RES_INVALID_COMMAND) { - printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n"); - *limit_power = 0; - return 0; + + if (rv != 0 && (cmd.cmd_code == EC_RES_INVALID_COMMAND || + cmd.cmd_code == EC_RES_INVALID_PARAM)) { + printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n"); + *limit_power = 0; + return 0; + } else if (rv != 0) { + return -1; }

*limit_power = resp.get_param.value; - return rv; + + return 0; }

int google_chromeec_get_protocol_info(

Attachments:

attachment.htm (text/html — 3.7 KB)

Show replies by date

build bot (Jenkins) (Code Review)

27 Dec 27 Dec

6:53 p.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37947 )

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

Patch Set 1:

(8 comments)

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 766: printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n"); code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 766: printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n"); please, no spaces at the start of a line

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 767: *limit_power = 0; code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 767: *limit_power = 0; please, no spaces at the start of a line

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 768: return 0; code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 768: return 0; please, no spaces at the start of a line

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 769: } else if (rv != 0) { code indent should use tabs where possible

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 769: } else if (rv != 0) { please, no spaces at the start of a line

Edward O'Callaghan (Code Review)

28 Dec 28 Dec

7:22 a.m.

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37947 )

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

Patch Set 1: Code-Review+1

With the format fixes sorted out i'll +2.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Gerrit-Change-Number: 37947 Gerrit-PatchSet: 1 Gerrit-Owner: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kangheui Won khwon@chromium.org Gerrit-Reviewer: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Dec 2019 05:22:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Rizwan Qureshi (Code Review)

9:31 a.m.

Hello Edward O'Callaghan, Kangheui Won, Tim Wawrzynczak, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37947

to look at the new patch set (#2).

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

ec/google/chromeec: ignore LIMIT_POWER based on command code in response

Assume that LIMIT_POWER is not requested if the ec does not support it. Do this by checking the command code in the response message instead of return value.

BUG=b:146165519 BRANCH=None TEST=Boot puff with EC which does not support LIMIT_POWER param.

Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com --- M src/ec/google/chromeec/ec.c 1 file changed, 7 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/37947/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Gerrit-Change-Number: 37947 Gerrit-PatchSet: 2 Gerrit-Owner: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kangheui Won khwon@chromium.org Gerrit-Reviewer: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Rizwan Qureshi (Code Review)

9:36 a.m.

Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37947 )

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

Patch Set 2:

(8 comments)

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 766: printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n");

...

code indent should use tabs where possible

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 766: printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n");

...

please, no spaces at the start of a line

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 767: *limit_power = 0;

...

please, no spaces at the start of a line

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 767: *limit_power = 0;

...

code indent should use tabs where possible

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 768: return 0;

...

code indent should use tabs where possible

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 768: return 0;

...

please, no spaces at the start of a line

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 769: } else if (rv != 0) {

...

please, no spaces at the start of a line

Done

https://review.coreboot.org/c/coreboot/+/37947/1/src/ec/google/chromeec/ec.c... PS1, Line 769: } else if (rv != 0) {

...

code indent should use tabs where possible

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/37947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Gerrit-Change-Number: 37947 Gerrit-PatchSet: 2 Gerrit-Owner: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kangheui Won khwon@chromium.org Gerrit-Reviewer: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Dec 2019 07:36:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: comment

Edward O'Callaghan (Code Review)

10:33 a.m.

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37947 )

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

Patch Set 2: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Gerrit-Change-Number: 37947 Gerrit-PatchSet: 2 Gerrit-Owner: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kangheui Won khwon@chromium.org Gerrit-Reviewer: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Dec 2019 08:33:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Edward O'Callaghan (Code Review)

29 Dec 29 Dec

2:34 a.m.

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37947 )

Change subject: ec/google/chromeec: ignore LIMIT_POWER based on command code in response ......................................................................

ec/google/chromeec: ignore LIMIT_POWER based on command code in response

Assume that LIMIT_POWER is not requested if the ec does not support it. Do this by checking the command code in the response message instead of return value.

BUG=b:146165519 BRANCH=None TEST=Boot puff with EC which does not support LIMIT_POWER param.

Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Signed-off-by: Rizwan Qureshi rizwan.qureshi@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37947 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M src/ec/google/chromeec/ec.c 1 file changed, 7 insertions(+), 2 deletions(-)

Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved

diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index de43eb5..b6020a3 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -760,14 +760,19 @@ int rv;

rv = google_chromeec_command(&cmd); - if (rv == -EC_RES_INVALID_PARAM || rv == -EC_RES_INVALID_COMMAND) { + + if (rv != 0 && (cmd.cmd_code == EC_RES_INVALID_COMMAND || + cmd.cmd_code == EC_RES_INVALID_PARAM)) { printk(BIOS_INFO, "PARAM_LIMIT_POWER not supported by EC.\n"); *limit_power = 0; return 0; + } else if (rv != 0) { + return -1; }

*limit_power = resp.get_param.value; - return rv; + + return 0; }

int google_chromeec_get_protocol_info(

-- To view, visit https://review.coreboot.org/c/coreboot/+/37947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib2f5f69a53f204acebfab3e36aab2960eeec1204 Gerrit-Change-Number: 37947 Gerrit-PatchSet: 3 Gerrit-Owner: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Kangheui Won khwon@chromium.org Gerrit-Reviewer: Rizwan Qureshi rizwan.qureshi@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1847

days inactive

1849

days old

coreboot-gerrit@coreboot.org

6 comments

3 participants

tags (0)

participants (3)

build bot (Jenkins) (Code Review)
Edward O'Callaghan (Code Review)
Rizwan Qureshi (Code Review)