Change in coreboot[master]: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
......................................................................

src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit

Setting the default values for GPIO community power management,
causing issues in detecting tpm interrupts. So to avoid that gpio pm
had to be disabled in devicetree. But for s0ix it is needed. This
patch implements a workaround in asl code to enable gpio pm on s0ix
entry and disable it on s0ix exit.

This patch adds three methods platform specific methods

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep

3. MWAK to disable power management features for GPIO communities on
waking up

BUG=b:148892882
BRANCH=none
TEST="Booted with this change on volteer proto1 and checked for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 54 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/1

diff --git a/src/mainboard/google/volteer/dsdt.asl b/src/mainboard/google/volteer/dsdt.asl
index 640f7cd..ac4b0b9 100644
--- a/src/mainboard/google/volteer/dsdt.asl
+++ b/src/mainboard/google/volteer/dsdt.asl
@@ -32,11 +32,16 @@
    		#include <soc/intel/common/block/acpi/acpi/northbridge.asl>
    		#include <soc/intel/tigerlake/acpi/southbridge.asl>
    	}
+		/* Mainboard hooks */
+		#include "mainboard.asl"
    }

// Chrome OS specific
    #include <vendorcode/google/chromeos/acpi/chromeos.asl>

+	/* Low power idle table */
+	#include <soc/intel/tigerlake/acpi/lpit.asl>
+
    // Chrome OS Embedded Controller
    Scope (_SB.PCI0.LPCB)
    {
diff --git a/src/mainboard/google/volteer/mainboard.asl b/src/mainboard/google/volteer/mainboard.asl
new file mode 100644
index 0000000..69f5a23
--- /dev/null
+++ b/src/mainboard/google/volteer/mainboard.asl
@@ -0,0 +1,49 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <intelblocks/gpio.h>
+
+Method (LOCL, 1, Serialized)
+{
+	For (Local0 = 0, Local0 < 6, Local0++)
+	{
+		_SB.PCI0.CGPM (Local0, Arg0)
+	}
+}
+
+/*
+ * Method called from _PTS prior to system sleep state entry
+ * Enables dynamic clock gating for all 5 GPIO communities
+ */
+Method (MPTS, 1, Serialized)
+{
+	LOCL (MISCCFG_ENABLE_GPIO_PM_CONFIG)
+}
+
+/*
+ * Method called from _WAK prior to system sleep state wakeup
+ * Disables dynamic clock gating for all 5 GPIO communities
+ */
+Method (MWAK, 1, Serialized)
+{
+	LOCL (0)
+}
+
+/*
+ * S0ix Entry/Exit Notifications
+ * Called from _SB.LPID._DSM
+ */
+Method (MS0X, 1, Serialized)
+{
+	If (Arg0 == 1) {
+		/* S0ix Entry */
+		LOCL (MISCCFG_ENABLE_GPIO_PM_CONFIG)
+	} Else {
+		/* S0ix Exit */
+		LOCL (0)
+	}
+}

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
......................................................................

Patch Set 1: Code-Review+1

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
......................................................................

Patch Set 1:

(15 comments)

I do not understand, why this has to be done in ACPI/ASL, and not in coreboot proper?

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@7 
PS1, Line 7: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
Please make it a statement by adding a verb (in imperative mood).
> Add workaround …

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@6 
PS1, Line 6: 
           : src/
Remove.

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@10 
PS1, Line 10: causing
causes

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@10 
PS1, Line 10: tpm
TPM

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@10 
PS1, Line 10: gpio pm
GPIO PM

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@11 
PS1, Line 11: had
has

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@11 
PS1, Line 11: s0ix
S0ix

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@12 
PS1, Line 12: s0ix
S0ix

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@12 
PS1, Line 12: gpio pm
GPIO PM

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@12 
PS1, Line 12: asl
ASL

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@15 
PS1, Line 15: This patch adds three methods platform specific methods
Dot/period at the end.

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@15 
PS1, Line 15: methods
Duplicate. Remove.

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@21 
PS1, Line 21: preparing to sleep
Dot/period at the end.

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@24 
PS1, Line 24: waking up
Dot/period at the end.

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.asl:

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
PS1, Line 4:  *
Only one blank line?

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
PS1, Line 44: 
Shouldn't we comment that we don't really want this in the long run? And we should file a bug to remove this as workaround in the future as well.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
PS1, Line 44:
> Shouldn't we comment that we don't really want this in the long run? And we should file a bug to rem […]

Yes, our long term goal should be to get rid of GPIO PM config being set to 0 altogether. This will have to be coordinated along with the update to cr50 which generates the right pulses. Yes, we should have a bug to remove this workaround for the future.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
PS1, Line 44:
> Yes, our long term goal should be to get rid of GPIO PM config being set to 0 altogether. […]

Posted update on: b/148892882

Change in coreboot[master]: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Hello build bot (Jenkins), Shaunak Saha, Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro, Srinidhi N Kaushik,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40261

to look at the new patch set (#2).

Change subject: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
......................................................................

mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

Setting the default values for GPIO community power management, causes
issues in detecting tpm interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 55 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/2

Change in coreboot[master]: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Hello build bot (Jenkins), Shaunak Saha, Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro, Srinidhi N Kaushik,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40261

to look at the new patch set (#3).

Change subject: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
......................................................................

mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

Setting the default values for GPIO community power management, causes
issues in detecting tpm interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 55 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/3

Change in coreboot[master]: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Hello build bot (Jenkins), Shaunak Saha, Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro, Srinidhi N Kaushik,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40261

to look at the new patch set (#4).

Change subject: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
......................................................................

mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

Setting the default values for GPIO community power management, causes
issues in detecting TPM interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 55 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/4

Change in coreboot[master]: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
......................................................................

Patch Set 4: Code-Review+1

(14 comments)

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@7 
PS1, Line 7: src/mb/google/volteer: workaround for GPIO PM on s0ix entry/exit
> Please make it a statement by adding a verb (in imperative mood). […]

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@6 
PS1, Line 6: 
           : src/
> Remove.

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@10 
PS1, Line 10: causing
> causes

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@10 
PS1, Line 10: tpm
> TPM

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@10 
PS1, Line 10: gpio pm
> GPIO PM

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@11 
PS1, Line 11: s0ix
> S0ix

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@11 
PS1, Line 11: had
> has

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@12 
PS1, Line 12: asl
> ASL

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@12 
PS1, Line 12: s0ix
> S0ix

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@12 
PS1, Line 12: gpio pm
> GPIO PM

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@15 
PS1, Line 15: methods
> Duplicate. Remove.

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@15 
PS1, Line 15: This patch adds three methods platform specific methods
> Dot/period at the end.

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@21 
PS1, Line 21: preparing to sleep
> Dot/period at the end.

Done

https://review.coreboot.org/c/coreboot/+/40261/1//COMMIT_MSG@24 
PS1, Line 24: waking up
> Dot/period at the end.

Done

Change in coreboot[master]: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/4/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.asl:

https://review.coreboot.org/c/coreboot/+/40261/4/src/mainboard/google/voltee... 
PS4, Line 9: LOCL
Furquan, why did you let me get away with calling this LOCL before? 😄 LOCL was just short for "Local method" because I really couldn't come up with a better name. Maybe a better name would be PGPM (Program Gpio PM bits?). A quick grep shows that doesn't appear to be used.

Change in coreboot[master]: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/4/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.asl:

https://review.coreboot.org/c/coreboot/+/40261/4/src/mainboard/google/voltee... 
PS4, Line 9: LOCL
> Furquan, why did you let me get away with calling this LOCL before? 😄 LOCL was just short for "Loca […]

Haha yes, PGPM sounds better. Hopefully, we can get rid of this file soon once the cr50 pulse issue is resolved.

Change in coreboot[master]: mb/google/volteer: Work around for GPIO PM on S0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Hello build bot (Jenkins), Shaunak Saha, Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro, Srinidhi N Kaushik,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40261

to look at the new patch set (#7).

Change subject: mb/google/volteer: Work around for GPIO PM on S0ix entry/exit
......................................................................

mb/google/volteer: Work around for GPIO PM on S0ix entry/exit

Setting the default values for GPIO community power management, causes
issues in detecting TPM interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 55 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/7

Change in coreboot[master]: mb/google/volteer: Work around for GPIO PM on S0ix entry/exit

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around for GPIO PM on S0ix entry/exit
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40261/6//COMMIT_MSG@7 
PS6, Line 7: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
> By the way, *Add workaround* can be replaced by the shorter verb `Work around` (for future commits). […]

Done

Change in coreboot[master]: mb/google/volteer: Work around for GPIO PM on S0ix entry/exit

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around for GPIO PM on S0ix entry/exit
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40261/6//COMMIT_MSG@7 
PS6, Line 7: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
> Done

Sorry for the confusion, but *for* needs to be changed, if you use the verb in imperative mood. It’s *to work around someone/something*.

https://www.ldoceonline.com/dictionary/work-around

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Venkata Krishna Nimmagadda (Code Review)]

Hello build bot (Jenkins), Shaunak Saha, Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Nick Vaccaro, Srinidhi N Kaushik,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40261

to look at the new patch set (#9).

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

Setting the default values for GPIO community power management, causes
issues in detecting TPM interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 55 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/9

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 9: Code-Review+1

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 9: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/40261/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40261/6//COMMIT_MSG@7 
PS6, Line 7: mb/google/volteer: Add workaround for GPIO PM on S0ix entry/exit
> Sorry for the confusion, but *for* needs to be changed, if you use the verb in imperative mood. […]

Fixed it. Thank you for your time.

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Venkata Krishna Nimmagadda (Code Review)]

Hello build bot (Jenkins), Shaunak Saha, Furquan Shaikh, Wonkyu Kim, Caveh Jalali, Paul Menzel, Nick Vaccaro, Srinidhi N Kaushik,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40261

to look at the new patch set (#10).

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

Setting the default values for GPIO community power management, causes
issues in detecting TPM interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 54 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/40261/10

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 11:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/40261/1/src/mainboard/google/voltee... 
PS1, Line 44:
> Added relevant comment. […]

Ack

https://review.coreboot.org/c/coreboot/+/40261/6/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/40261/6/src/mainboard/google/voltee... 
PS6, Line 44: 	 */
> The `*/` goes on the line above. […]

Done

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 11: Code-Review+1

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 11: Code-Review+1

Has this been verified on a proto2?

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Venkata Krishna Nimmagadda (Code Review)]

Venkata Krishna Nimmagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 11:
> Patch Set 11: Code-Review+1
>
> Has this been verified on a proto2?

It is only verified on proto 1 as of now. I will try to get it verified on proto 2.

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Wonkyu Kim (Code Review)]

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 11: Code-Review+2

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 12: Code-Review+2

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[Duncan Laurie (Code Review)]

Duncan Laurie has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

Setting the default values for GPIO community power management, causes
issues in detecting TPM interrupts. So to avoid that GPIO PM has to be
disabled in devicetree. But for S0ix it is needed. This patch implements
a workaround in ASL code to enable GPIO PM on S0ix entry and disable it
on S0ix exit.

This patch adds the following three platform specific methods.

1. MS0X to enable power management features for GPIO communities on
entry and on exit, it disables them.

2. MPTS to enable power management features for GPIO communities when
preparing to sleep.

3. MWAK to disable power management features for GPIO communities on
waking up.

BUG=b:148892882
BRANCH=none
TEST="Boot with this change on volteer proto1 and check for GPIO
community config with debugger"

Signed-off-by: Venkata Krishna Nimmagadda 
Change-Id: If522c82c0069a4bf5738beb73a2b4f11ed6f51d3
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40261
Reviewed-by: Nick Vaccaro 
Reviewed-by: Venkata Krishna Nimmagadda 
Reviewed-by: Wonkyu Kim 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/volteer/dsdt.asl
A src/mainboard/google/volteer/mainboard.asl
2 files changed, 54 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nick Vaccaro: Looks good to me, approved
  Wonkyu Kim: Looks good to me, approved
  Venkata Krishna Nimmagadda: Looks good to me, but someone else must approve

diff --git a/src/mainboard/google/volteer/dsdt.asl b/src/mainboard/google/volteer/dsdt.asl
index af881ae..450835d 100644
--- a/src/mainboard/google/volteer/dsdt.asl
+++ b/src/mainboard/google/volteer/dsdt.asl
@@ -32,11 +32,17 @@
    		#include <soc/intel/common/block/acpi/acpi/northbridge.asl>
    		#include <soc/intel/tigerlake/acpi/southbridge.asl>
    	}
+		/* Mainboard hooks */
+		#include "mainboard.asl"
    }

// Chrome OS specific
    #include <vendorcode/google/chromeos/acpi/chromeos.asl>

+	/* Include Low power idle table for a short term workaround to enable
+	   S0ix. Once cr50 pulse width is fixed, this can be removed. */
+	#include <soc/intel/common/acpi/lpit.asl>
+
    // Chrome OS Embedded Controller
    Scope (_SB.PCI0.LPCB)
    {
diff --git a/src/mainboard/google/volteer/mainboard.asl b/src/mainboard/google/volteer/mainboard.asl
new file mode 100644
index 0000000..d58822d
--- /dev/null
+++ b/src/mainboard/google/volteer/mainboard.asl
@@ -0,0 +1,48 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <intelblocks/gpio.h>
+
+Method (PGPM, 1, Serialized)
+{
+	For (Local0 = 0, Local0 < 6, Local0++)
+	{
+		_SB.PCI0.CGPM (Local0, Arg0)
+	}
+}
+
+/*
+ * Method called from _PTS prior to system sleep state entry
+ * Enables dynamic clock gating for all 5 GPIO communities
+ */
+Method (MPTS, 1, Serialized)
+{
+	PGPM (MISCCFG_ENABLE_GPIO_PM_CONFIG)
+}
+
+/*
+ * Method called from _WAK prior to system sleep state wakeup
+ * Disables dynamic clock gating for all 5 GPIO communities
+ */
+Method (MWAK, 1, Serialized)
+{
+	PGPM (0)
+}
+
+/*
+ * S0ix Entry/Exit Notifications
+ * Called from _SB.LPID._DSM
+ */
+Method (MS0X, 1, Serialized)
+{
+	If (Arg0 == 1) {
+		/* S0ix Entry */
+		PGPM (MISCCFG_ENABLE_GPIO_PM_CONFIG)
+	} Else {
+		/* S0ix Exit */
+		PGPM (0)
+	}
+}

Change in coreboot[master]: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40261 )

Change subject: mb/google/volteer: Work around TPM issue by enabling GPIO PM in S0ix
......................................................................

Patch Set 13:

Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4
Emulation targets:
"QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2834
"QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2833
"QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2832
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/2831

Please note: This test is under development and might not be accurate at all!