Shelley Chen has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/70230 )
Change subject: mb/google/herobrine: NVMe id determined by logical (not physical) bit
......................................................................
mb/google/herobrine: NVMe id determined by logical (not physical) bit
NVMe is determined by a logical bit 1, not the physical SKU pin.
Thus, (logical) sku_id & 0x2 == 0x2 would mean that the device has
NVMe enabled on it. Previously, I thought that it was tied to a
physical pin, but this is not correct.
BUG=b:254281839
BRANCH=None
TEST=flash and boot on villager and make sure that NVMe is not
initialized in coreboot.
Change-Id: Iaa75d2418d6a2351d874842e8678bd6ad3c92526
Signed-off-by: Shelley Chen <shchen(a)google.com>
---
M src/mainboard/google/herobrine/mainboard.c
1 file changed, 25 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/70230/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/70230
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa75d2418d6a2351d874842e8678bd6ad3c92526
Gerrit-Change-Number: 70230
Gerrit-PatchSet: 2
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Kangheui Won, Reka Norman, Tim Wawrzynczak, Paul Menzel, Angel Pons, Sridhar Siricilla, Arthur Heymans, Eric Lai, Ronak Kanabar, Andrey Petrov.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67669 )
Change subject: drivers/intel/fsp2_0: Update MRC cache in ramstage
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/67669/comment/6ca6caf8_20880949
PS1, Line 18:
> This won't work since FSP_INFO_HOB is not available on every FSP 2 platform. […]
Well, it seems really excessive but I can't come up with a better solution either. I assume there's no easy way to turn it around either and extract one of the versions that are available in the HOBs in romstage before running it (so we could just start using that one for the MRC driver instead)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/67669
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie6aa2dee83a3ab8913830746593935d36a034b8d
Gerrit-Change-Number: 67669
Gerrit-PatchSet: 2
Gerrit-Owner: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Reka Norman <rekanorman(a)google.com>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:54:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org>
Comment-In-Reply-To: Tim Wawrzynczak <inforichland(a)gmail.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/70230 )
Change subject: mb/google/herobrine: NVMe id determined by logical (not physical) bit
......................................................................
mb/google/herobrine: NVMe id determined by logical (not physical) bit
NVMe is determined by a logical bit 1, not the physical SKU pin.
Thus, (logical) sku_id & 0x2 == 0x2 would mean that the device has
NVMe enabled on it. Previously, I thought that it was tied to a
physical pin, but this is not correct.
BUG=b:254281839
BRANCH=None
TEST=flash and boot on villager and make sure that NVMe is not
initialized in coreboot.
Change-Id: Iaa75d2418d6a2351d874842e8678bd6ad3c92526
Signed-off-by: Shelley Chen <shchen(a)google.com>
---
M src/mainboard/google/herobrine/mainboard.c
1 file changed, 23 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/70230/1
diff --git a/src/mainboard/google/herobrine/mainboard.c b/src/mainboard/google/herobrine/mainboard.c
index c9bca73..62fad15 100644
--- a/src/mainboard/google/herobrine/mainboard.c
+++ b/src/mainboard/google/herobrine/mainboard.c
@@ -85,9 +85,8 @@
}
/*
- * Determine if board need to perform PCIe initialization. On Herobrine,
- * resistor strapping will be such that bit 0 will be assigned 2 (high Z) if it
- * is an NVMe enabled platform.
+ * Determine if board need to perform PCIe initialization. On Herobrine, If it
+ * is an NVMe enabled platform, logical sku_id & 2 will be true.
*/
bool mainboard_needs_pcie_init(void)
{
@@ -106,11 +105,7 @@
return true;
}
- if ((sku % 3) == 2)
- return true;
-
- /* Otherwise, eMMC */
- return false;
+ return !!(sku & 0x2);
}
static void mainboard_init(struct device *dev)
--
To view, visit https://review.coreboot.org/c/coreboot/+/70230
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa75d2418d6a2351d874842e8678bd6ad3c92526
Gerrit-Change-Number: 70230
Gerrit-PatchSet: 1
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Tarun Tuli, Eran Mitrani.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70166 )
Change subject: soc/intel/common: provide a list of D-states to enter LPM
......................................................................
Patch Set 5:
(1 comment)
File src/soc/intel/common/block/acpi/acpi.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-165024):
https://review.coreboot.org/c/coreboot/+/70166/comment/a10069ce_fc7439f5
PS5, Line 455: struct min_sleep_state * states_ar = get_min_sleep_state_array(&size);
"foo * bar" should be "foo *bar"
--
To view, visit https://review.coreboot.org/c/coreboot/+/70166
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I45eded3868a4987cb5eb0676c50378ac52ec3752
Gerrit-Change-Number: 70166
Gerrit-PatchSet: 5
Gerrit-Owner: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:46:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tarun Tuli, Eran Mitrani.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70166 )
Change subject: soc/intel/common: provide a list of D-states to enter LPM
......................................................................
Patch Set 4:
(1 comment)
File src/soc/intel/common/block/acpi/acpi.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-165023):
https://review.coreboot.org/c/coreboot/+/70166/comment/d73c3e40_ef828858
PS4, Line 455: struct min_sleep_state * states_ar = get_min_sleep_state_array(&size);
"foo * bar" should be "foo *bar"
--
To view, visit https://review.coreboot.org/c/coreboot/+/70166
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I45eded3868a4987cb5eb0676c50378ac52ec3752
Gerrit-Change-Number: 70166
Gerrit-PatchSet: 4
Gerrit-Owner: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:44:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tarun Tuli, Eran Mitrani.
Hello build bot (Jenkins), Tarun Tuli, Subrata Banik,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/70166
to look at the new patch set (#4).
Change subject: soc/intel/common: provide a list of D-states to enter LPM
......................................................................
soc/intel/common: provide a list of D-states to enter LPM
This was done previously for ADL. moving the code to common so
it can be leveraged for other platforms (e.g. MTL)
TEST=Built and tested on anahera by verifying SSDT contents
Change-Id: I45eded3868a4987cb5eb0676c50378ac52ec3752
Signed-off-by: Eran Mitrani <mitrani(a)google.com>
---
M src/include/acpi/acpi.h
M src/soc/intel/alderlake/acpi.c
M src/soc/intel/common/block/acpi/acpi.c
M src/soc/intel/common/block/include/intelblocks/acpi.h
4 files changed, 203 insertions(+), 156 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/70166/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/70166
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I45eded3868a4987cb5eb0676c50378ac52ec3752
Gerrit-Change-Number: 70166
Gerrit-PatchSet: 4
Gerrit-Owner: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Robert Zieba, Raul Rangel, Karthik Ramasubramanian.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69914 )
Change subject: device/xhci: Refactor functions to work with a non-const device tree
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/69914/comment/2d1b986d_e335561b
PS4, Line 7: non-const
const?
File src/device/xhci_const.c:
https://review.coreboot.org/c/coreboot/+/69914/comment/8ccd7d06_066874ae
PS4, Line 1: /* SPDX-License-Identifier: GPL-2.0-or-later */
Isn't this filename backwards? Isn't this the non-const file?
--
To view, visit https://review.coreboot.org/c/coreboot/+/69914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If5a74f9529d5dc6031ec968ef5f40a9cad5ffbc4
Gerrit-Change-Number: 69914
Gerrit-PatchSet: 4
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Robert Zieba <robertzieba(a)google.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:24:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Robert Zieba, Raul Rangel, Nico Huber, Angel Pons, Karthik Ramasubramanian, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67931 )
Change subject: cpu/x86/smm: Add PCI BAR store functionality
......................................................................
Patch Set 11:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/67931/comment/48bcf493_2f24a91a
PS11, Line 7: BAR
Should this be changed to resources?
File src/cpu/x86/Kconfig:
https://review.coreboot.org/c/coreboot/+/67931/comment/81ec4df8_73436dc5
PS11, Line 175: so they
: can't be tampered with.
:
"so SMM can tell if they've been moved"?
File src/cpu/x86/smm/pci_bar_store.c:
https://review.coreboot.org/c/coreboot/+/67931/comment/b634b357_9806ba21
PS11, Line 15: size_t num_slots,
Why isn't this using SMM_PCI_BAR_STORE_NUM_SLOTS?
https://review.coreboot.org/c/coreboot/+/67931/comment/e7179ae7_7eb0c52d
PS11, Line 47: SMM_PCI_BAR_STORE_NUM_BARS)
do we want a warning here?
File src/cpu/x86/smm/smm_module_handler.c:
https://review.coreboot.org/c/coreboot/+/67931/comment/f67bf87b_52565bc7
PS11, Line 11: #include <types.h>
Do you need to include smm.h for smm_runtime.pci_bar_store?
File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/67931/comment/d668b1c5_ee83436e
PS11, Line 69: bars
Can we change bars to resource or 'res' everywhere since it's no longer a BAR that's being saved?
--
To view, visit https://review.coreboot.org/c/coreboot/+/67931
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I23fb1e935dd1b89f1cc5c834cc2025f0fe5fda37
Gerrit-Change-Number: 67931
Gerrit-PatchSet: 11
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Robert Zieba <robertzieba(a)google.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:23:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Robert Zieba, Jason Nien, Karthik Ramasubramanian.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68326 )
Change subject: mb/google/guybrush: Store XHCI BARs
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/68326
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d0911df9e3102791bf7b5723ac38e2ba82a9db6
Gerrit-Change-Number: 68326
Gerrit-PatchSet: 11
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Robert Zieba <robertzieba(a)google.com>
Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:23:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Robert Zieba, Jason Glenesk, Raul Rangel, Matt DeVillier, Fred Reitberger, Karthik Ramasubramanian, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67939 )
Change subject: soc/amd/cezanne: Set up SoC-specific XHCI definitions
......................................................................
Patch Set 12: Code-Review+2
(1 comment)
File src/soc/amd/cezanne/Kconfig:
https://review.coreboot.org/c/coreboot/+/67939/comment/03b71ced_9f7f99e2
PS12, Line 76: select SOC_AMD_COMMON_BLOCK_XHCI
Same as other commit - mention this in the commit message.
--
To view, visit https://review.coreboot.org/c/coreboot/+/67939
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I15e9c06cd38ac858b861a4d19626664704af7541
Gerrit-Change-Number: 67939
Gerrit-PatchSet: 12
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Robert Zieba <robertzieba(a)google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 01 Dec 2022 21:22:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment