Attention is currently required from: Raul Rangel, Jon Murphy, Tim Van Patten, Karthik Ramasubramanian.
Hello build bot (Jenkins), Raul Rangel, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/74112
to look at the new patch set (#31).
Change subject: mb/google/myst: Enable PCIe devices in devicetree
......................................................................
mb/google/myst: Enable PCIe devices in devicetree
Ensure that DXIO descriptors are updated using info from AMD and Myst
board schematics.
BUG=b:275960920,b:276744321
TEST=builds
Change-Id: Icdad785bcb90de036095bcc4219c15f55f4277fe
Signed-off-by: Jon Murphy <jpmurphy(a)google.com>
---
M src/mainboard/google/myst/port_descriptors.c
M src/mainboard/google/myst/variants/baseboard/devicetree.cb
M src/mainboard/google/myst/variants/baseboard/include/baseboard/variants.h
3 files changed, 135 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/74112/31
--
To view, visit https://review.coreboot.org/c/coreboot/+/74112
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icdad785bcb90de036095bcc4219c15f55f4277fe
Gerrit-Change-Number: 74112
Gerrit-PatchSet: 31
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Tim Van Patten <timvp(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Bora Guvendik, Hannah Williams, Subrata Banik, Jérémy Compostella, Paul Menzel.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74141 )
Change subject: soc/intel/cmn/pcie: Allow SoC to overwrite snoop/no-snoop latency
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
Sorry to interrupt again, I just had an odd thought:
Does Meteor Lake need this driver? It doesn't much and 90% of what it does
should already be handled by generic coreboot code.
If Meteor Lake would define its own driver, we wouldn't need an override
because it would have its own .get_ltr_max_latencies pointer.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74141
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic2b9a158d219e6c6e7f6e7f0ae0f093c1183b402
Gerrit-Change-Number: 74141
Gerrit-PatchSet: 5
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)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: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 07 Apr 2023 21:48:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Tarun Tuli, Subrata Banik, Christian Walter, Maximilian Brune, Angel Pons, Michael Niewöhner, Martin Roth, Lean Sheng Tan.
Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73902 )
Change subject: Revert "soc/intel/{tgl,adl}: Hook up D3ColdEnable UPD to D3COLD_SUPPORT"
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Isn't the only problem with the patch the "don't" in the help text? Surely it's easier to just remove that.
For sure, there is a devicetree option but the point of this patch was to tie ACPI and FSP together to one Kconfig option
--
To view, visit https://review.coreboot.org/c/coreboot/+/73902
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I56bab4d85d04e90cacfe77db59d0cde6a8a75949
Gerrit-Change-Number: 73902
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
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: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Fri, 07 Apr 2023 21:44:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Hannah Williams, Nico Huber, Subrata Banik, Jérémy Compostella, Paul Menzel.
Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74141 )
Change subject: soc/intel/cmn/pcie: Allow SoC to overwrite snoop/no-snoop latency
......................................................................
Patch Set 5:
(1 comment)
File src/soc/intel/common/block/pcie/pcie.c:
https://review.coreboot.org/c/coreboot/+/74141/comment/75219d28_0620cb3c
PS4, Line 14: __weak
> We usually use Kconfig for such overrides. Now that we have chipset […]
I will look into using Kconfig, thx for feedback
--
To view, visit https://review.coreboot.org/c/coreboot/+/74141
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic2b9a158d219e6c6e7f6e7f0ae0f093c1183b402
Gerrit-Change-Number: 74141
Gerrit-PatchSet: 5
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)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: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Fri, 07 Apr 2023 21:26:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72800 )
Change subject: soc/intel/{tgl,adl}: Hook up D3ColdEnable UPD to D3COLD_SUPPORT
......................................................................
Patch Set 8:
(1 comment)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/72800/comment/6c41b7fe_9d112bdc
PS4, Line 1004: Don't support D3Cold"
: default n if NO_S0IX_SUPPORT
> 1. […]
*hastily created a revert
--
To view, visit https://review.coreboot.org/c/coreboot/+/72800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I718952165daa6471f11e8025e745fe7c249d3b46
Gerrit-Change-Number: 72800
Gerrit-PatchSet: 8
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.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: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Fri, 07 Apr 2023 20:36:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72800 )
Change subject: soc/intel/{tgl,adl}: Hook up D3ColdEnable UPD to D3COLD_SUPPORT
......................................................................
Patch Set 8:
(1 comment)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/72800/comment/824bf776_0adb5fc3
PS4, Line 1004: Don't support D3Cold"
: default n if NO_S0IX_SUPPORT
> 1. You were the only one giving +2. […]
1. please check properly for the full log (by clicking show all entries), the build was passed before it was merged. the CI will run again after it was merged, as usual. I did not wait for “random period of time”, as per rules stated at least 24 hours should be waited, and per the whole log this patch was hanging there for more than 2 weeks before it was merged. I am not sure what else you are really looking for here.
2. basically, you jumped in suddenly stating a few things were wrong, and then hastily created a review, even before giving Sean time to respond to it. IMHO that was a bit rude, it’s like saying our patch discussion, review time didn’t matter at all, so let’s undo everything. that is not the right way it should go,
--
To view, visit https://review.coreboot.org/c/coreboot/+/72800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I718952165daa6471f11e8025e745fe7c249d3b46
Gerrit-Change-Number: 72800
Gerrit-PatchSet: 8
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.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: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Fri, 07 Apr 2023 20:34:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Sean Rhodes, Angel Pons, Martin Roth, Lean Sheng Tan.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73903 )
Change subject: Revert "device: Add Kconfig options for D3COLD_SUPPORT and NO_S0IX_SUPPORT"
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
Discussion on one of the changes: https://review.coreboot.org/c/coreboot/+/72800/4..8
--
To view, visit https://review.coreboot.org/c/coreboot/+/73903
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I56ce8f59f8548fc58bc2b3b07c1314e2eed7061c
Gerrit-Change-Number: 73903
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Fri, 07 Apr 2023 20:25:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Sean Rhodes.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72800 )
Change subject: soc/intel/{tgl,adl}: Hook up D3ColdEnable UPD to D3COLD_SUPPORT
......................................................................
Patch Set 8:
(1 comment)
File src/device/Kconfig:
https://review.coreboot.org/c/coreboot/+/72800/comment/7f324c7c_4cd7a5c7
PS4, Line 1004: Don't support D3Cold"
: default n if NO_S0IX_SUPPORT
> Michael, […]
1. You were the only one giving +2. You can't *assume* that changes have been reviewed just because some random period of time has passed. Review will always be signalled by +1 or +2. You even submitted the patches without waiting for the tests to finish. At least nothing broke...
2. As I said, it's not about a tiny problem here. Check the other comments. Anyway, IMHO the whole situation qualifies for a revert. However, I'm not the (only) one who is going to decide that. This will be handled by the review process as well and I'm sure other people will share their opinion soon.
--
To view, visit https://review.coreboot.org/c/coreboot/+/72800
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I718952165daa6471f11e8025e745fe7c249d3b46
Gerrit-Change-Number: 72800
Gerrit-PatchSet: 8
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.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: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Fri, 07 Apr 2023 20:25:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Jon Murphy, Tim Van Patten, Karthik Ramasubramanian.
Mark Hasemeyer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74112 )
Change subject: mb/google/myst: Enable PCIe devices in devicetree
......................................................................
Patch Set 30:
(2 comments)
File src/mainboard/google/myst/port_descriptors.c:
https://review.coreboot.org/c/coreboot/+/74112/comment/6e3c2d12_4e39cd87
PS30, Line 49: .gpio_group_id = GPIO_27,
Similar to comment below. SD_AUX_RST is on GPIO_29.
https://review.coreboot.org/c/coreboot/+/74112/comment/d5634bf9_5a4b0fc1
PS30, Line 64: .gpio_group_id = GPIO_6,
I'm not sure what this does, but on Skyrim GPIO_6 is tied to SSD_AUX_RST. It's now on GPIO_31 for Myst.
--
To view, visit https://review.coreboot.org/c/coreboot/+/74112
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icdad785bcb90de036095bcc4219c15f55f4277fe
Gerrit-Change-Number: 74112
Gerrit-PatchSet: 30
Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Tim Van Patten <timvp(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Fri, 07 Apr 2023 20:01:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment