Attention is currently required from: Raul Rangel, Karthik Ramasubramanian.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52801 )
Change subject: mb/google/guybrush: Fix S0i3/S3 GPIO configuration
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/5e257997_896db26f
PS3, Line 40: SOC_SAR_INT_L
> I got the list from b/186011392#12.
Aah. I don't really see that as a wake requirement in go/cros-partner-docs though. If it is a new requirement, we should ensure that the requirement docs get updated too.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52801
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7f9d2c13503c01fb9d834c436dac723f2c3b24c
Gerrit-Change-Number: 52801
Gerrit-PatchSet: 3
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 03 May 2021 18:07:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Sumeet R Pawnikar, Patrick Rudolph.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52858 )
Change subject: soc/intel/alderlake: remove duplicate PL2 override
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52858
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1424f36fbe038d478f4b8f6257d78d4a3ede3258
Gerrit-Change-Number: 52858
Gerrit-PatchSet: 1
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
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-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 03 May 2021 17:59:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Karthik Ramasubramanian.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52801 )
Change subject: mb/google/guybrush: Fix S0i3/S3 GPIO configuration
......................................................................
Patch Set 3:
(2 comments)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/674e3460_5f0f6053
PS1, Line 20: PAD_SCI
> > I think we need to set this to PAD_GPI instead and make it so the kernel driver can set the wake b […]
I've left it as PAD_WAKE for now. I can do more testing once the wake kernel patch lands.
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/61bd4737_21aee4c0
PS3, Line 40: SOC_SAR_INT_L
> Is this really a wake source? Probably just a PAD_GPI is okay here?
I got the list from b/186011392#12.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52801
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7f9d2c13503c01fb9d834c436dac723f2c3b24c
Gerrit-Change-Number: 52801
Gerrit-PatchSet: 3
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 03 May 2021 17:27:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Sumeet R Pawnikar.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52859 )
Change subject: mb/google/brya: enable DPTF functionality for brya
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52859/comment/cce7e966_80000da4
PS1, Line 7: enable DPTF functionality for brya
Duplicate of CB:52691?
File src/mainboard/google/brya/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/52859/comment/14da6320_22926733
PS1, Line 123: chip drivers/intel/dptf
Shouldn't these changes go into overridetree?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52859
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I33266c85aa30869466034ccbab04a3c7820ae2b0
Gerrit-Change-Number: 52859
Gerrit-PatchSet: 1
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Comment-Date: Mon, 03 May 2021 17:25:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Karthik Ramasubramanian.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52801 )
Change subject: mb/google/guybrush: Fix S0i3/S3 GPIO configuration
......................................................................
Patch Set 3:
(2 comments)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/6f3c18f9_a2641754
PS1, Line 20: PAD_SCI
> I think we need to set this to PAD_GPI instead and make it so the kernel driver can set the wake bits. https://lore.kernel.org/patchwork/patch/1420135/
Yes, that can work. I had set the wake bits in coreboot because those were set for a specific sleep type. But, thinking about it some more, I think it shouldn't matter if S3|S0i3|S4 are set at the same time (at least from a Chrome OS perspective). We support the same wake source from S3 and S0i3. S4 is not used and so the setting doesn't really matter.
> Since the OS doesn't have a driver loaded for this, the IRQ never gets enabled, so the wake_status never gets cleared. This results in not being able to stay in S0i3.
That should be handled correctly once you have the appropriate kernel driver loaded for it i.e. gpio-keys driver.
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52801/comment/96290514_0445df6f
PS3, Line 40: SOC_SAR_INT_L
Is this really a wake source? Probably just a PAD_GPI is okay here?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52801
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7f9d2c13503c01fb9d834c436dac723f2c3b24c
Gerrit-Change-Number: 52801
Gerrit-PatchSet: 3
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 03 May 2021 17:23:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Sumeet R Pawnikar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52858 )
Change subject: soc/intel/alderlake: remove duplicate PL2 override
......................................................................
soc/intel/alderlake: remove duplicate PL2 override
PL2 override value is already declared under common code in power_limit.h file.
Removing this duplicate PL2 override from soc specific header file.
BRANCH=None
BUG=None
TEST=Built and tested on brya
Change-Id: I1424f36fbe038d478f4b8f6257d78d4a3ede3258
Signed-off-by: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
---
M src/soc/intel/alderlake/chip.h
1 file changed, 0 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/52858/1
diff --git a/src/soc/intel/alderlake/chip.h b/src/soc/intel/alderlake/chip.h
index f7412dc..b3094ba 100644
--- a/src/soc/intel/alderlake/chip.h
+++ b/src/soc/intel/alderlake/chip.h
@@ -157,8 +157,6 @@
/* HeciEnabled decides the state of Heci1 at end of boot
* Setting to 0 (default) disables Heci1 and hides the device from OS */
uint8_t HeciEnabled;
- /* PL2 Override value in Watts */
- uint32_t tdp_pl2_override;
/* Enable/Disable EIST. 1b:Enabled, 0b:Disabled */
uint8_t eist_enable;
--
To view, visit https://review.coreboot.org/c/coreboot/+/52858
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1424f36fbe038d478f4b8f6257d78d4a3ede3258
Gerrit-Change-Number: 52858
Gerrit-PatchSet: 1
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-MessageType: newchange
Attention is currently required from: Raul Rangel, Furquan Shaikh, Karthik Ramasubramanian.
Hello build bot (Jenkins), Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52801
to look at the new patch set (#3).
Change subject: mb/google/guybrush: Fix S0i3/S3 GPIO configuration
......................................................................
mb/google/guybrush: Fix S0i3/S3 GPIO configuration
Using PAD_WAKE is actually wrong. The wake bits are only supposed to be
set when using the GPIO controller to wake the system. coreboot's
current architecture relies on using GPEs to wake the system.
BUG=b:186011392
TEST=Wake system from S0i3 with EC and see GPE 3 increment.
Signed-off-by: Raul E Rangel <rrangel(a)chromium.org>
Change-Id: If7f9d2c13503c01fb9d834c436dac723f2c3b24c
---
M src/mainboard/google/guybrush/variants/baseboard/gpio.c
1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/52801/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/52801
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7f9d2c13503c01fb9d834c436dac723f2c3b24c
Gerrit-Change-Number: 52801
Gerrit-PatchSet: 3
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: newpatchset
Jason Glenesk has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/52038 )
Change subject: soc/amd/common/block: Add support to get upep constraints
......................................................................
Abandoned
placed by other cl
--
To view, visit https://review.coreboot.org/c/coreboot/+/52038
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia593908c41ae5b74fa1d73123c60df7f75a1a743
Gerrit-Change-Number: 52038
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Akshu Agrawal <akshu.agrawal(a)amd.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt Papageorge <matthewpapa07(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)mailbox.org>
Gerrit-MessageType: abandon