coreboot-gerrit April 2021

coreboot-gerrit@coreboot.org

1 participants
6269 discussions

Change in coreboot[master]: include/assert.h: Use mock_assert() for ENV_TEST targets
by Julius Werner (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Jakub Czapiga, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51804 ) Change subject: include/assert.h: Use mock_assert() for ENV_TEST targets ...................................................................... Patch Set 1: (2 comments) File src/include/assert.h: https://review.coreboot.org/c/coreboot/+/51804/comment/53289643_b3f0234d PS1, Line 12: #include <tests/test.h> Probably better to just unconditionally (re-)define the prototype for mock_assert() rather than conditionally include the header here. That way, you can use if (ENV_TEST) mock_assert(...) in the code and don't have to use so many preprocessor guards. https://review.coreboot.org/c/coreboot/+/51804/comment/91bb9dc5_501ddfef PS1, Line 67: #define assert(statement) mock_assert(statement, #statement, __FILE__, __LINE__) Please add this to all forms of assert (e.g. I'd say BUG() is just another form of assert for us), e.g. by inserting it before the FATAL_ASSERTS checks above. Maybe useful to factor the whole checking for FATAL_ASSERTS to do hlt() and checking for ENV_TEST to do mock_assert() out into a separate macro. -- To view, visit https://review.coreboot.org/c/coreboot/+/51804 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5e8dd1b198ee6fab61e2be3f92baf1178f79bf18 Gerrit-Change-Number: 51804 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.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-Attention: Jan Dabros <jsd(a)semihalf.com> Gerrit-Comment-Date: Fri, 02 Apr 2021 01:05:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: include/rules.h: Add ENV_TEST definition
by Julius Werner (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Martin Roth, Jakub Czapiga, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51803 ) Change subject: include/rules.h: Add ENV_TEST definition ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/51803 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib8f2932902a73a7dbe181adc82cc18437abb48e8 Gerrit-Change-Number: 51803 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.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: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com> Gerrit-Comment-Date: Fri, 02 Apr 2021 00:58:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/asurada: early-init eMMC
by Wenbin Mei (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Hung-Te Lin, Paul Menzel, Yu-Ping Wu. Wenbin Mei has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51973 ) Change subject: mb/google/asurada: early-init eMMC ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/51973/comment/8d5545ff_64a26915 PS1, Line 9: Early-init eMMC. > @wenbin please add this into commit message. […] Ack https://review.coreboot.org/c/coreboot/+/51973/comment/bbeb8f8a_b8fe26e3 PS1, Line 12: TEST=none > Would be nice to have a test. Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/51973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2f58d203e969dc1a13a479d7dc63b1b162a9ae3f Gerrit-Change-Number: 51973 Gerrit-PatchSet: 3 Gerrit-Owner: Wenbin Mei <wenbin.mei(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hung-Te Lin <hungte(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Fri, 02 Apr 2021 00:55:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] Rename do_printk() --> printk()
by Julius Werner (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Jason Glenesk, Nico Huber, Raul Rangel, Marshall Dawson, Kyösti Mälkki, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51887 ) Change subject: [RFC] Rename do_printk() --> printk() ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Does this -Wmisleading-indentation actually work (well)? That would certainly be an easy solution to the indentation debate too! (I'm curious how the printk macro threw it off though, just looks like a pretty normal macro to me, and we have zillions of those...) -- To view, visit https://review.coreboot.org/c/coreboot/+/51887 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie4eab935a367b5ad6b38225c4973d41d9f70ef10 Gerrit-Change-Number: 51887 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 02 Apr 2021 00:31:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lint: checkpatch: Ignore ASSIGN_IN_IF and UNNECESSARY_ELSE errors
by Julius Werner (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Nico Huber, Martin Roth. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51551 ) Change subject: lint: checkpatch: Ignore ASSIGN_IN_IF and UNNECESSARY_ELSE errors ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > Does this affect the comments on Gerrit? If so, I wouldn't want […] This will stop Jenkins from creating Gerrit comments to warn about these two things, yes (at least I hope it does, that's my goal with this patch). Removing UNNECESSARY_ELSE doesn't mean you *have* to always put the else, of course, it just means it's left up to the author and reviewers. Do you really think this warning is critical in protecting us from a deluge of over-indented code? Personally I don't recall seeing a lot of that before we introduced checkpatch, nor have I seen many patches recently where authors started out that way and then changed it due to the Jenkins warning. If you think it's a big concern, could you point to some examples? I'm just kinda tired of writing code that's perfectly fine according to our coding style and still get bothered about it by Jenkins, so I'm trying to reduce those cases. No matter how often we write "checkpatch is not authoritative" somewhere there's still always enough people who assume that it is or ask about it in reviews or otherwise create unnecessary churn (even if it's just when you don't know whether people are not giving you a +2 because they haven't reviewed the patch yet or they're implicitly waiting on you to "resolve" the spurious checkpatch comments). And the email spam also just gets annoying when you upload a lot of revisions for the same patch. (Also, note that checkpatch already has a separate DEEP_INDENTATION warning for 6+ tabs if that's the primary concern here.) -- To view, visit https://review.coreboot.org/c/coreboot/+/51551 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I130598057c1800277a129ae6b927e961d6e26e42 Gerrit-Change-Number: 51551 Gerrit-PatchSet: 3 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: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Fri, 02 Apr 2021 00:24:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: Print strapping IDs when adding them to coreboot table
by Julius Werner (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52011 ) Change subject: coreboot_tables: Print strapping IDs when adding them to coreboot table ...................................................................... Patch Set 1: (2 comments) File src/lib/coreboot_table.c: https://review.coreboot.org/c/coreboot/+/52011/comment/9c35e40f_626a1adc PS1, Line 308: uint64_t fw_config = fw_config_get(); > nit: const Done https://review.coreboot.org/c/coreboot/+/52011/comment/9bf40c7e_2bf3e47d PS1, Line 321: llx > PRIx64 As far as I'm aware we don't consider these mandatory in coreboot (they're hardcoded to the same values for all archs anyway), but fine. -- To view, visit https://review.coreboot.org/c/coreboot/+/52011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifdbfdd29d25a0937c27113ace776f7aec231a57d Gerrit-Change-Number: 52011 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Apr 2021 00:03:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: coreboot_tables: Print strapping IDs when adding them to coreboot table
by Julius Werner (Code Review) 02 Apr '21

02 Apr '21

Attention is currently required from: Furquan Shaikh, Julius Werner. Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52011 to look at the new patch set (#2). Change subject: coreboot_tables: Print strapping IDs when adding them to coreboot table ...................................................................... coreboot_tables: Print strapping IDs when adding them to coreboot table These used to be printed before CB:46605. Having them in the logs can be a huge timesaver when debugging logs sent to you by other people (especially from systems that don't boot all the way). Let's add them back. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Ifdbfdd29d25a0937c27113ace776f7aec231a57d --- M src/lib/coreboot_table.c 1 file changed, 12 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/52011/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifdbfdd29d25a0937c27113ace776f7aec231a57d Gerrit-Change-Number: 52011 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: include/cbfsglue.h: Use BIOS_INFO for LOG macro
by Julius Werner (Code Review) 01 Apr '21

01 Apr '21

Attention is currently required from: Wim Vervoorn. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52019 ) Change subject: include/cbfsglue.h: Use BIOS_INFO for LOG macro ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: Huh... why did I do that? The old commonlib/cbfs.c macros also used INFO for LOG(), and I think I tried to match those. Must have been a typo, honestly, don't know why else I would have used ERR there. -- To view, visit https://review.coreboot.org/c/coreboot/+/52019 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3827a7d65a9d70045a36fb8db4b2c129e1045122 Gerrit-Change-Number: 52019 Gerrit-PatchSet: 1 Gerrit-Owner: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-Comment-Date: Thu, 01 Apr 2021 23:58:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP] mb/amd/majolica: add DXIO and DDI descriptors
by Felix Held (Code Review) 01 Apr '21

01 Apr '21

Attention is currently required from: Jason Glenesk, Marshall Dawson. Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Matt Papageorge, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51956 to look at the new patch set (#7). Change subject: [WIP] mb/amd/majolica: add DXIO and DDI descriptors ...................................................................... [WIP] mb/amd/majolica: add DXIO and DDI descriptors TODO: The devices and function numbers of the PCIe bridges still need to be tested. The values used in the Majolica UEFI reference implementation weren't consistent with the PPR. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Signed-off-by: Matt Papageorge <matthewpapa07(a)gmail.com> Change-Id: I65c7e0ebf1e43fd4608d46bae8a176cfc3d0236b --- M src/mainboard/amd/majolica/port_descriptors.c 1 file changed, 110 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/51956/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/51956 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I65c7e0ebf1e43fd4608d46bae8a176cfc3d0236b Gerrit-Change-Number: 51956 Gerrit-PatchSet: 7 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/guybrush: Enable camera power GPIO
by Bhanu Prakash Maiya (Code Review) 01 Apr '21

01 Apr '21

Attention is currently required from: Martin Roth, Ivy Jian, Karthik Ramasubramanian. Bhanu Prakash Maiya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52017 ) Change subject: mb/google/guybrush: Enable camera power GPIO ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: LGTM -- To view, visit https://review.coreboot.org/c/coreboot/+/52017 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie894167e3c4f8efdb3710599c6ff3a9fc975adb6 Gerrit-Change-Number: 52017 Gerrit-PatchSet: 1 Gerrit-Owner: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-Reviewer: Bhanu Prakash Maiya <bhanumaiya(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 01 Apr 2021 23:39:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2021