Change in coreboot[master]: vendorcode/google/chromesos/sar: Reduce severity of debug message

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Maulik V Vaghela (Code Review)

11 Nov 2020 11 Nov '20

8:46 a.m.

Maulik V Vaghela has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vendorcode/google/chromesos/sar: Reduce severity of debug message ......................................................................

vendorcode/google/chromesos/sar: Reduce severity of debug message

Coreboot might not store wifi SAR values in VPD and may store it in CBFS. Logging the debug message with 'error' severity may interfere with automated test tool.

Lowering severity to BIOS_DEBUG to avoid this issue

BUG=b:171931401 BRANCH=None TEST=Severity of message is reduced and we don't see it as an error

Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/vendorcode/google/chromeos/sar.c 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/47442/1

diff --git a/src/vendorcode/google/chromeos/sar.c b/src/vendorcode/google/chromeos/sar.c index 2f73d39..9bca423 100644 --- a/src/vendorcode/google/chromeos/sar.c +++ b/src/vendorcode/google/chromeos/sar.c @@ -71,7 +71,7 @@ /* Try to read the SAR limit entry from VPD */ if (!vpd_gets(wifi_sar_limit_key, wifi_sar_limit_str, buffer_size, VPD_RO_THEN_RW)) { - printk(BIOS_ERR, "Error: Could not locate '%s' in VPD.\n", + printk(BIOS_DEBUG, "Could not locate '%s' in VPD.\n", wifi_sar_limit_key);

if (!CONFIG(WIFI_SAR_CBFS))

Attachments:

attachment.htm (text/html — 2.9 KB)

Show replies by date

Maulik V Vaghela (Code Review)

11 Nov 11 Nov

8:52 a.m.

Hello build bot (Jenkins),

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

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

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

Change subject: vendorcode/google/chromesos/sar: Reduce severity of debug message ......................................................................

vendorcode/google/chromesos/sar: Reduce severity of debug message

coreboot might not store wifi SAR values in VPD and may store it in CBFS. Logging the debug message with 'error' severity may interfere with automated test tool.

Lowering severity to BIOS_DEBUG to avoid this issue

BUG=b:171931401 BRANCH=None TEST=Severity of message is reduced and we don't see it as an error

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/47442/2

Paul Menzel (Code Review)

10:40 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vendorcode/google/chromesos/sar: Reduce severity of debug message ......................................................................

Patch Set 2:

(7 comments)

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@7 PS2, Line 7: chromesos chromeos

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@7 PS2, Line 7: vendorcode/google/chromesos/sar: Reduce severity of debug message Maybe:

...

vc/google/chromeos/sar: Make SAR not found log a debug message

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@10 PS2, Line 10: debug message message

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@13 PS2, Line 13: Lowering severity to BIOS_DEBUG to avoid this issue Please add a dot/period at the end.

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@13 PS2, Line 13: to avoid avoids

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@13 PS2, Line 13: BIOS_DEBUG Why not `BIOS_INFO`?

https://review.coreboot.org/c/coreboot/+/47442/2/src/vendorcode/google/chrom... File src/vendorcode/google/chromeos/sar.c:

https://review.coreboot.org/c/coreboot/+/47442/2/src/vendorcode/google/chrom... PS2, Line 71: /* Try to read the SAR limit entry from VPD */ It’d be great, if you could sent a follow-up updating this comment too.

Maulik V Vaghela (Code Review)

12 Nov 12 Nov

11 a.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Hello build bot (Jenkins), Justin TerAvest, Ronak Kanabar, Aamir Bohra, Karthik Ramasubramanian,

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

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

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

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

vc/google/chromeos/sar: Make "SAR not found" log a debug message

coreboot might not store wifi SAR values in VPD and may store it in CBFS. Logging the message with 'error' severity may interfere with automated test tool.

Lowering severity to BIOS_DEBUG avoids this issue.

BUG=b:171931401 BRANCH=None TEST=Severity of message is reduced and we don't see it as an error

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/47442/3

Maulik V Vaghela (Code Review)

11:01 a.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

Patch Set 3:

(6 comments)

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@7 PS2, Line 7: chromesos

...

chromeos

Done

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@7 PS2, Line 7: vendorcode/google/chromesos/sar: Reduce severity of debug message

...

Maybe: […]

Done

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@10 PS2, Line 10: debug message

...

message

Done

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@13 PS2, Line 13: to avoid

...

avoids

Done

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@13 PS2, Line 13: BIOS_DEBUG

...

Why not `BIOS_INFO`?

I wanted to keep it debug, in case there is genuine issue of VPD not found.

https://review.coreboot.org/c/coreboot/+/47442/2//COMMIT_MSG@13 PS2, Line 13: Lowering severity to BIOS_DEBUG to avoid this issue

...

Please add a dot/period at the end.

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/47442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Gerrit-Change-Number: 47442 Gerrit-PatchSet: 3 Gerrit-Owner: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Justin TerAvest teravest@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 12 Nov 2020 10:01:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Maulik V Vaghela (Code Review)

3:32 p.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Hello build bot (Jenkins), Justin TerAvest, Ronak Kanabar, Aamir Bohra, Karthik Ramasubramanian,

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

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

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

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

vc/google/chromeos/sar: Make "SAR not found" log a debug message

coreboot might not store wifi SAR values in VPD and may store it in CBFS. Logging the message with 'error' severity may interfere with automated test tool.

Lowering severity to BIOS_DEBUG avoids this issue.

BUG=b:171931401 BRANCH=None TEST=Severity of message is reduced and we don't see it as an error

Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/drivers/wifi/generic/acpi.c M src/vendorcode/google/chromeos/sar.c 2 files changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/47442/4

Karthik Ramasubramanian (Code Review)

6:59 p.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/47442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Gerrit-Change-Number: 47442 Gerrit-PatchSet: 4 Gerrit-Owner: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Ronak Kanabar ronak.kanabar@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-Comment-Date: Thu, 12 Nov 2020 17:59:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Furquan Shaikh (Code Review)

7:04 p.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47442/4/src/vendorcode/google/chrom... File src/vendorcode/google/chromeos/sar.c:

https://review.coreboot.org/c/coreboot/+/47442/4/src/vendorcode/google/chrom... PS4, Line 71: /* Try to read the SAR limit entry from VPD */ Not for this change, but I think we should drop the support for SAR in VPD completely. This is not really recommended for any new platforms.

-- To view, visit https://review.coreboot.org/c/coreboot/+/47442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Gerrit-Change-Number: 47442 Gerrit-PatchSet: 4 Gerrit-Owner: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Ronak Kanabar ronak.kanabar@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-Comment-Date: Thu, 12 Nov 2020 18:04:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Maulik V Vaghela (Code Review)

19 Nov 19 Nov

7:04 p.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47442/2/src/vendorcode/google/chrom... File src/vendorcode/google/chromeos/sar.c:

https://review.coreboot.org/c/coreboot/+/47442/2/src/vendorcode/google/chrom... PS2, Line 71: /* Try to read the SAR limit entry from VPD */

...

It’d be great, if you could sent a follow-up updating this comment too.

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/47442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Gerrit-Change-Number: 47442 Gerrit-PatchSet: 4 Gerrit-Owner: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 19 Nov 2020 18:04:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: comment

Subrata Banik (Code Review)

20 Nov 20 Nov

9:36 a.m.

New subject: Change in coreboot[master]: vc/google/chromeos/sar: Make "SAR not found" log a debug message

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47442 )

Change subject: vc/google/chromeos/sar: Make "SAR not found" log a debug message ......................................................................

vc/google/chromeos/sar: Make "SAR not found" log a debug message

coreboot might not store wifi SAR values in VPD and may store it in CBFS. Logging the message with 'error' severity may interfere with automated test tool.

Lowering severity to BIOS_DEBUG avoids this issue.

BUG=b:171931401 BRANCH=None TEST=Severity of message is reduced and we don't see it as an error

Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47442 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com --- M src/drivers/wifi/generic/acpi.c M src/vendorcode/google/chromeos/sar.c 2 files changed, 2 insertions(+), 2 deletions(-)

Approvals: build bot (Jenkins): Verified Karthik Ramasubramanian: Looks good to me, approved

diff --git a/src/drivers/wifi/generic/acpi.c b/src/drivers/wifi/generic/acpi.c index cd5af4e..4440b81 100644 --- a/src/drivers/wifi/generic/acpi.c +++ b/src/drivers/wifi/generic/acpi.c @@ -53,7 +53,7 @@

/* Retrieve the sar limits data */ if (get_wifi_sar_limits(&sar_limits) < 0) { - printk(BIOS_ERR, "Error: failed from getting SAR limits!\n"); + printk(BIOS_DEBUG, "failed from getting SAR limits!\n"); return; }

if (!CONFIG(WIFI_SAR_CBFS))

-- To view, visit https://review.coreboot.org/c/coreboot/+/47442 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5c122a57cfe92b27e0291933618ca13d8e1889ba Gerrit-Change-Number: 47442 Gerrit-PatchSet: 5 Gerrit-Owner: Maulik V Vaghela maulik.v.vaghela@intel.com Gerrit-Reviewer: Aamir Bohra aamir.bohra@intel.com Gerrit-Reviewer: Furquan Shaikh furquan@google.com Gerrit-Reviewer: Justin TerAvest teravest@google.com Gerrit-Reviewer: Karthik Ramasubramanian kramasub@google.com Gerrit-Reviewer: Ronak Kanabar ronak.kanabar@intel.com Gerrit-Reviewer: Subrata Banik subrata.banik@intel.com Gerrit-Reviewer: Tim Wawrzynczak twawrzynczak@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1513

days inactive

1522

days old

coreboot-gerrit@coreboot.org

9 comments

5 participants

tags (0)

participants (5)

Furquan Shaikh (Code Review)
Karthik Ramasubramanian (Code Review)
Maulik V Vaghela (Code Review)
Paul Menzel (Code Review)
Subrata Banik (Code Review)