coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: amdfwtool: Cleanup the message of help
by Paul Menzel (Code Review) 02 May '21

02 May '21

Attention is currently required from: Bao Zheng. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52697 ) Change subject: amdfwtool: Cleanup the message of help ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52697/comment/72d6f4d8_2069c479 PS2, Line 7: Cleanup For the future, the verb is spelled with a space: Clean up. -- To view, visit https://review.coreboot.org/c/coreboot/+/52697 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id537d7c9b77641289274c1b2b6f606e2be37ac6b Gerrit-Change-Number: 52697 Gerrit-PatchSet: 2 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Comment-Date: Sun, 02 May 2021 13:04:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: aopen/dxplplusu: Add early GPIO settings
by Paul Menzel (Code Review) 02 May '21

02 May '21

Attention is currently required from: Kyösti Mälkki, Patrick Rudolph. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52815 ) Change subject: aopen/dxplplusu: Add early GPIO settings ...................................................................... Patch Set 1: (1 comment) File src/mainboard/aopen/dxplplusu/bootblock.c: https://review.coreboot.org/c/coreboot/+/52815/comment/a3b520c6_86ad0a56 PS1, Line 100: gpio_write(0x47, 0x86); /* LED1 invert open-drain*/ Nit: Could you please add a space before `*/`? -- To view, visit https://review.coreboot.org/c/coreboot/+/52815 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib211e9c4b487fadec3d3487f9d745f44d8ca4579 Gerrit-Change-Number: 52815 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sun, 02 May 2021 12:59:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: aopen/dxplplusu: Fix IOAPIC in ASL
by Paul Menzel (Code Review) 02 May '21

02 May '21

Attention is currently required from: Kyösti Mälkki. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52814 ) Change subject: aopen/dxplplusu: Fix IOAPIC in ASL ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/52814 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8a2abe2cf20952491d181766a3ca492de355fc88 Gerrit-Change-Number: 52814 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Sun, 02 May 2021 12:59:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Added CMOS option to disable ME
by Paul Menzel (Code Review) 02 May '21

02 May '21

Attention is currently required from: Star Labs, Patrick Rudolph. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52800 ) Change subject: Added CMOS option to disable ME ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52800/comment/cb64cdd4_33b2e88c PS3, Line 7: Added CMOS option to disable ME Please use a prefix, and also [use imperative mood in commit message summaries](https://chris.beams.io/posts/git-commit/). -- To view, visit https://review.coreboot.org/c/coreboot/+/52800 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I374db3b7c0ded71cdc18f27970252fec7220cc20 Gerrit-Change-Number: 52800 Gerrit-PatchSet: 3 Gerrit-Owner: Star Labs <admin(a)starlabs.systems> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Star Labs <admin(a)starlabs.systems> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sun, 02 May 2021 12:56:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lint: checkpatch: Ignore ASSIGN_IN_IF and UNNECESSARY_ELSE errors
by Nico Huber (Code Review) 02 May '21

02 May '21

Nico Huber 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 4: (1 comment) Patchset: PS3: > 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). Sorry for not responding, Gerrit's mail got caught in a filter /o\ > 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 like to think of the checkpatch warnings as a nice hint for inexperienced people. I don't recall having witnessed anyone taking them too seriously. On my own changes, I mostly ignore them without trouble. Examples from the past before checkpatch would not reflect the constitution of our community today. > > 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. Jenkins' comments are automatically tagged resolved and I treat them like that. It's just a comment that something _might_ be better written another way. Maybe we should adapt the comment texts to reflect that? If somebody would be waiting for style changes before starting a review, I would probably not want them to review my changes anyway. > > (Also, note that checkpatch already has a separate DEEP_INDENTATION warning for 6+ tabs if that's the primary concern here.) Should we make it 5+? ;) I don't think we need warnings for things that are obvious to everyone. It's rather the small nits that are seen more often than exceptionally bad style that I would prefer to have covered by a bot. -- 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: 4 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)mailbox.org> Gerrit-Comment-Date: Sun, 02 May 2021 11:03:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in libgfxinit[master]: dp aux: Add TODO and pre-condition for I2C writes
by Nico Huber (Code Review) 02 May '21

02 May '21

Attention is currently required from: Angel Pons. Hello Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/libgfxinit/+/52824 to review the following change. Change subject: dp aux: Add TODO and pre-condition for I2C writes ...................................................................... dp aux: Add TODO and pre-condition for I2C writes Change-Id: If7566fc7b701c8a36d9e8a0af5beb7a84a558ee3 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M common/hw-gfx-dp_aux_ch.adb 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/24/52824/1 diff --git a/common/hw-gfx-dp_aux_ch.adb b/common/hw-gfx-dp_aux_ch.adb index b8eb510..854a2bc 100644 --- a/common/hw-gfx-dp_aux_ch.adb +++ b/common/hw-gfx-dp_aux_ch.adb @@ -178,6 +178,15 @@ Length : in DP_Defs.Aux_Payload_Length; Data : in DP_Defs.Aux_Payload; Success : out Boolean) + with + -- TODO: The justification below turns out to be wrong. There is + -- another way to signal a deferred final ACK: + -- When multiple bytes are written and not all of them are + -- ack'ed yet, we should receive AUX-ACK/I2C-ACK with the + -- number of already written bytes as response. So far our + -- API only allows single-byte writes, so we can limit the + -- `Length` here. + Pre => Length <= 1 is Request : DP_Defs.Aux_Request; @@ -276,6 +285,8 @@ Length : in DP_Defs.Aux_Payload_Length; Data : in DP_Defs.Aux_Payload; Success : out Boolean) + with + Pre => Length <= 1 -- see I2C_Out_Packet() is Ignored_Success : Boolean; begin -- To view, visit https://review.coreboot.org/c/libgfxinit/+/52824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libgfxinit Gerrit-Branch: master Gerrit-Change-Id: If7566fc7b701c8a36d9e8a0af5beb7a84a558ee3 Gerrit-Change-Number: 52824 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in libgfxinit[master]: gfx dp_aux: Add I2C_{Read,Write}_Byte procedures
by Nico Huber (Code Review) 02 May '21

02 May '21

Attention is currently required from: Matt DeVillier, Angel Pons, Arthur Heymans. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/51122 ) Change subject: gfx dp_aux: Add I2C_{Read,Write}_Byte procedures ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/libgfxinit/+/51122 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libgfxinit Gerrit-Branch: master Gerrit-Change-Id: Ib66b073691282d0c89710b0591484d4123e039b7 Gerrit-Change-Number: 51122 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sun, 02 May 2021 08:09:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/common: Drop deprecated fixed BAR accessors
by Nico Huber (Code Review) 02 May '21

02 May '21

Attention is currently required from: Angel Pons, Arthur Heymans, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51885 ) Change subject: nb/intel/common: Drop deprecated fixed BAR accessors ...................................................................... Patch Set 10: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/51885 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4cb24ca71f3c3717ea50d147ddca74aaf0288fa Gerrit-Change-Number: 51885 Gerrit-PatchSet: 10 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sun, 02 May 2021 07:52:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/common: Turn `*bar_{read,write}*` macros into functions
by Nico Huber (Code Review) 02 May '21

02 May '21

Attention is currently required from: Angel Pons, Arthur Heymans, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52816 ) Change subject: nb/intel/common: Turn `*bar_{read,write}*` macros into functions ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52816 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa2ba208fba11c4a00f2b8a05eb1129a32c6c092 Gerrit-Change-Number: 52816 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sun, 02 May 2021 07:52:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in libgfxinit[master]: Add support to switch LSPCON modes
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Martin Roth, Matt DeVillier, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/51133 ) Change subject: Add support to switch LSPCON modes ...................................................................... Patch Set 9: (1 comment) File common/hw-gfx-dp_dual_mode.adb: https://review.coreboot.org/c/libgfxinit/+/51133/comment/d5d80282_57f7652b PS7, Line 118: Time.M_Delay (1); > I forgot what I meant with this comment. The loop below could be written like Timeout := Time.MS_From_Now (2_000); loop Read_LSPCON_Mode (Port, Mode, Success); exit when Success; ... Timed_Out := Time.Timed_Out (Timeout); exit when Timed_Out; end loop; -- To view, visit https://review.coreboot.org/c/libgfxinit/+/51133 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libgfxinit Gerrit-Branch: master Gerrit-Change-Id: Idfa9bdff47a6591000cd5092d64a4cd4f8dbdc8d Gerrit-Change-Number: 51133 Gerrit-PatchSet: 9 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 01 May 2021 22:09:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021