Attention is currently required from: Felix Singer, Angel Pons.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56209 )
Change subject: mb/google/volteer/baseboard: Configure chipset_lockdown seperately
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56209/comment/251b17d7_af72be0a
PS2, Line 15:
: Thus, move `chipset_lockdown` out of `common_soc_config` in the
: baseboard devicetree and configure it seperately.
:
Should the `chipset_lockdown` be moved to chipset.cb: https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr…
I would expect majority of the boards to select this.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56209
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I595c042cf62680d61f60965710d382bfdcd81671
Gerrit-Change-Number: 56209
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:26:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jan Tatje, Nico Huber, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56071 )
Change subject: supermicro/x11-lga1151-series: Remove SkipExtGfxScan = 1
......................................................................
Patch Set 4:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56071/comment/6eb41959_e3854392
PS4, Line 7: Remove SkipExtGfxScan = 1
enable scanning for external graphics
https://review.coreboot.org/c/coreboot/+/56071/comment/abd066b4_ff66801a
PS4, Line 9: ,
by dropping SkipExtGfxScan=1 and setting PrimaryDisplay=PEG. This allows...
https://review.coreboot.org/c/coreboot/+/56071/comment/916ef858_da637107
PS4, Line 11: devicetree.
maybe add this? ', so all four boards can make use of of the text framebuffer and/or a graphics card in a PEG slot.'
File src/mainboard/supermicro/x11-lga1151-series/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/56071/comment/82eb0a83_e0071c6f
PS4, Line 3: # Additional FSP Configuration
nit: you can drop that if you wish. it doesn't (and didn't) provide any value
https://review.coreboot.org/c/coreboot/+/56071/comment/3e72c29e_6b0220b2
PS4, Line 4: # This board has an IGD with no output.
yeah, but this does not explain why we have PEG. What about adding "Display_PEG allows using the VGA text framebuffer and a graphics card in a PEG slot." or so?
https://review.coreboot.org/c/coreboot/+/56071/comment/e439af68_765f84b0
PS4, Line 43: off
would you mind adding a follow-up change that enables this and drops it from the variants?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56071
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I41249112c65927b61ca5f791f8eb8c3f3d204fce
Gerrit-Change-Number: 56071
Gerrit-PatchSet: 4
Gerrit-Owner: Jan Tatje <jan(a)jnt.io>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jan Tatje <jan(a)jnt.io>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:10:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Martin Roth, Marshall Dawson, Subrata Banik, Andrey Petrov, Patrick Rudolph, Nathaniel L Desimone.
Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56190 )
Change subject: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header
......................................................................
Patch Set 2:
(1 comment)
File src/drivers/intel/fsp2_0/include/fsp/info_header.h:
https://review.coreboot.org/c/coreboot/+/56190/comment/a6a39ac8_4c8b047f
PS2, Line 13: however, some FSP 2.0 variants have it as well.
> Thanks for the pointers Nick. […]
Re: less desirable approach.
In general, overriding any files in edk2 is considered to be a bad practice that we try to avoid it. In essence, we would have to copy our version of the IntelFsp2Pkg/Include/Guid/FspHeaderFile.h over the one in edk2 directory prior to FSP build. We also don't have support for it in the current build infrastructure. If we do implement override, we end up with a custom non-portable AMD solution that would have to be replicated for other vendors.
We could assign a custom revision to the FSP header version and check for it in coreboot but since we don't have the authority over issuance of FSP header revisions we will likely run into revision collisions.
Another option is to simply forgo sanity checking of the header length (which is what I believe is being done in looks_like_fsp_header()).
--
To view, visit https://review.coreboot.org/c/coreboot/+/56190
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie8422447b2cff0a6c536e13014905ffa15c70586
Gerrit-Change-Number: 56190
Gerrit-PatchSet: 2
Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.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: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:04:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56234 )
Change subject: soc/amd/cezanne: Start loading APOB asynchronously
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/56234
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4b5c1ef4cad571d1cbca33b1aff017a3cedc1bea
Gerrit-Change-Number: 56234
Gerrit-PatchSet: 2
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:02:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56233 )
Change subject: WIP: soc/amd/common/block/lpc: Don't disable the HOG bit
......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/common/block/lpc/lpc.c:
https://review.coreboot.org/c/coreboot/+/56233/comment/b1e8c680_8cb7d835
PS2, Line 69: // byte &= ~LPC_NOHOG;
is this commented out line going to confuse future generations?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56233
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If015869657f36d3533f4ab9ebd1f54b0d4eb283a
Gerrit-Change-Number: 56233
Gerrit-PatchSet: 2
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:01:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Eric Peers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56232 )
Change subject: soc/amd/common/apob: Add support for asynchronously reading APOB
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/56232
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I930d58b76eb4558bc4f48ed928c4d6538fefb1e5
Gerrit-Change-Number: 56232
Gerrit-PatchSet: 2
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 13 Jul 2021 00:00:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Sugnan Prabhu S.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55317 )
Change subject: mb/google/brya: Program Unused Cnvi BT related GPIOs to NC
......................................................................
Patch Set 6:
(1 comment)
File src/mainboard/google/brya/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/55317/comment/d6fe3e2a_430e4a87
PS6, Line 368: Put unused Cnvi BT UART lines in NC mode since we use USB mode
What is configuring these virtual GPIOs to UART lines? Is that the default state of the virtual GPIO?
--
To view, visit https://review.coreboot.org/c/coreboot/+/55317
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33a37ceb8a91603d2a193de5bdd1b6885eb3c319
Gerrit-Change-Number: 55317
Gerrit-PatchSet: 6
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
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-CC: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Comment-Date: Mon, 12 Jul 2021 23:58:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment