coreboot-gerrit April 2021

coreboot-gerrit@coreboot.org

1 participants
6269 discussions

Change in coreboot[master]: chromeec: Fix google_chromeec_status_check timeout
by Paul Menzel (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Furquan Shaikh, Martin Roth. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51775 ) Change subject: chromeec: Fix google_chromeec_status_check timeout ...................................................................... Patch Set 6: (2 comments) File src/ec/google/chromeec/ec_lpc.c: https://review.coreboot.org/c/coreboot/+/51775/comment/5c430618_704fa7b7 PS4, Line 105: return 0; > This call happens pretty frequently. I'm not sure that's really useful. True, maybe only if it’s above a certain threshold. But could be done in the future to optimize stuff. https://review.coreboot.org/c/coreboot/+/51775/comment/a7d40852_8d140599 PS4, Line 108: > As mentioned above, I don't think that it's particularly useful, except during board bringup. Understood. -- To view, visit https://review.coreboot.org/c/coreboot/+/51775 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I363ff7453bcf81581884f92797629a6f96d42580 Gerrit-Change-Number: 51775 Gerrit-PatchSet: 6 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Tue, 06 Apr 2021 17:19:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: chromeec: Fix google_chromeec_status_check timeout
by Rob Barnes (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Furquan Shaikh, Martin Roth, Paul Menzel. Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51775 ) Change subject: chromeec: Fix google_chromeec_status_check timeout ...................................................................... Patch Set 6: (3 comments) File src/ec/google/chromeec/ec_lpc.c: https://review.coreboot.org/c/coreboot/+/51775/comment/7b8ca234_1545e496 PS4, Line 98: * > nit: spaces around * Done https://review.coreboot.org/c/coreboot/+/51775/comment/c9a98c91_2464a73f PS4, Line 105: return 0; > Print the stopwatch value in a spew message? This will be noisy. https://review.coreboot.org/c/coreboot/+/51775/comment/f5266df2_baafeed6 PS4, Line 108: > Please print a warning/error with the timeout value. All the calls to this function already print a timeout error. -- To view, visit https://review.coreboot.org/c/coreboot/+/51775 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I363ff7453bcf81581884f92797629a6f96d42580 Gerrit-Change-Number: 51775 Gerrit-PatchSet: 6 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 06 Apr 2021 17:17:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: chromeec: Fix google_chromeec_status_check timeout
by Rob Barnes (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Paul Menzel, Rob Barnes. Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Marshall Dawson, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51775 to look at the new patch set (#6). Change subject: chromeec: Fix google_chromeec_status_check timeout ...................................................................... chromeec: Fix google_chromeec_status_check timeout Rewrite google_chromeec_status_check to use stopwatch instead of a delay in a while loop. In practice the while loop ends up taking much longer than one second to timeout. Using stopwatch library will accurately timeout after one second. BUG=b:183524609 TEST=Build and run on guybrush BRANCH=None Change-Id: I363ff7453bcf81581884f92797629a6f96d42580 Signed-off-by: Rob Barnes <robbarnes(a)google.com> --- M src/ec/google/chromeec/ec_lpc.c 1 file changed, 13 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/51775/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/51775 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I363ff7453bcf81581884f92797629a6f96d42580 Gerrit-Change-Number: 51775 Gerrit-PatchSet: 6 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: chromeec: Fix google_chromeec_status_check timeout
by Rob Barnes (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Paul Menzel, Rob Barnes. Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Marshall Dawson, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51775 to look at the new patch set (#5). Change subject: chromeec: Fix google_chromeec_status_check timeout ...................................................................... chromeec: Fix google_chromeec_status_check timeout Rewrite google_chromeec_status_check to use stopwatch instead of a delay in a while loop. In practice the while loop ends up taking much longer than one second to timeout. Using stopwatch library will accurately timeout after one second. BUG=b:183524609 TEST=Build and run on guybrush BRANCH=None Change-Id: I363ff7453bcf81581884f92797629a6f96d42580 Signed-off-by: Rob Barnes <robbarnes(a)google.com> --- M 3rdparty/amd_blobs M 3rdparty/blobs M 3rdparty/intel-microcode M 3rdparty/qc_blobs M src/ec/google/chromeec/ec_lpc.c 5 files changed, 17 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/51775/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/51775 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I363ff7453bcf81581884f92797629a6f96d42580 Gerrit-Change-Number: 51775 Gerrit-PatchSet: 5 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/guybrush: PCIe GPIOs - enable enables, disable resets
by Furquan Shaikh (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Martin Roth. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52115 ) Change subject: mb/google/guybrush: PCIe GPIOs - enable enables, disable resets ...................................................................... Patch Set 2: (2 comments) File src/mainboard/google/guybrush/variants/baseboard/gpio.c: https://review.coreboot.org/c/coreboot/+/52115/comment/f0624b14_956e526e PS1, Line 54: HIGH > Until we handle the timings, which is not a part of this patch, this is the best way to go. […] Wouldn't you run into issues during the bringup if you are not ensuring that the timings are met? It is going to cause random behaviors on different boards and more issues to debug. Also, the bug you raised for "handle timings" (b/184598323), does not capture any of these steps that need to be performed. https://review.coreboot.org/c/coreboot/+/52115/comment/54c3d5ce_418bee74 PS1, Line 169: /* EN_PP3300_WLAN */ > The early gpio init is getting moved from bootblock to psp_verstage. I think we should have two separate tables - one for verstage on PSP and other for bootblock. The PCIe related GPIOs don't really need to be configured in verstage. Those are required only before FSP-M runs and so doing it in bootblock should be perfectly fine. Also, ensures that we are able to update this in field if required. -- To view, visit https://review.coreboot.org/c/coreboot/+/52115 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3807e02de1e9ae40b0a4162217afd6aabb5b04ed Gerrit-Change-Number: 52115 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Tue, 06 Apr 2021 16:57:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: tests/lib/malloc-test: Fix possible memory overrun
by Paul Fagerburg (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Jakub Czapiga. Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52136 ) Change subject: tests/lib/malloc-test: Fix possible memory overrun ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19f0718c657d565e515157e66367573e08f51254 Gerrit-Change-Number: 52136 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Comment-Date: Tue, 06 Apr 2021 16:56:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Makefile: export LANG LC_ALL TZ without using COREBOOT_EXPORTS
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Martin Roth. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51363 ) Change subject: Makefile: export LANG LC_ALL TZ without using COREBOOT_EXPORTS ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/51363 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea965abbce23bf6ec408ef587da0a4c4ebc65a27 Gerrit-Change-Number: 51363 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Tue, 06 Apr 2021 16:34:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/genbuild_h: add support to print SOURCE_DATE_EPOCH
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Martin Roth, Alexander Couzens. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51362 ) Change subject: util/genbuild_h: add support to print SOURCE_DATE_EPOCH ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Not _too_ happy about adding an option for that here (which will encourage adding all kinds of other options, yuck). We sometimes parse stuff out of build.h, eg in src/arch/arm64/Makefile.inc: BL31_MAKEARGS += BUILD_MESSAGE_TIMESTAMP='"$(shell sed -n 's/^.define COREBOOT_BUILD\>.*"$.*$".*/\1/p' $(obj)/build.h)"' -- To view, visit https://review.coreboot.org/c/coreboot/+/51362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa79d3e7df8101a1ba1b37a361d8992f7eab2d52 Gerrit-Change-Number: 51362 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Comment-Date: Tue, 06 Apr 2021 16:34:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: tests/lib/malloc-test: Fix possible memory overrun
by Jakub Czapiga (Code Review) 06 Apr '21

06 Apr '21

Jakub Czapiga has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52136 ) Change subject: tests/lib/malloc-test: Fix possible memory overrun ...................................................................... tests/lib/malloc-test: Fix possible memory overrun Change memset address to use actual buffer instead of pointer stored in symbol value. Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com> Change-Id: I19f0718c657d565e515157e66367573e08f51254 --- M tests/lib/malloc-test.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/52136/1 diff --git a/tests/lib/malloc-test.c b/tests/lib/malloc-test.c index fefacbb..b70de06 100644 --- a/tests/lib/malloc-test.c +++ b/tests/lib/malloc-test.c @@ -50,7 +50,7 @@ static int setup_calloc_test(void **state) { - memset(&_heap, 0xFF, TEST_HEAP_SZ); + memset(_test_heap, 0xFF, TEST_HEAP_SZ); return setup_test(state); } -- To view, visit https://review.coreboot.org/c/coreboot/+/52136 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19f0718c657d565e515157e66367573e08f51254 Gerrit-Change-Number: 52136 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: lint: checkpatch: Add SUSPICIOUS_CODE_INDENT test
by Patrick Georgi (Code Review) 06 Apr '21

06 Apr '21

Attention is currently required from: Nico Huber, Martin Roth, Julius Werner, Felix Held. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51838 ) Change subject: lint: checkpatch: Add SUSPICIOUS_CODE_INDENT test ...................................................................... Patch Set 2: Code-Review-1 (1 comment) Patchset: PS2: There's compiler tooling for that since gcc6 (-Wmisleading-indentation) and while it's also just heuristics, at least it's more widely used heuristics than this, so let's go that route? -- To view, visit https://review.coreboot.org/c/coreboot/+/51838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7bb90b56dfc7582271d2b82cb42a2c1df477054f Gerrit-Change-Number: 51838 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 06 Apr 2021 16:13:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2021