Change in coreboot[master]: soc/intel/common: Add restore and enable GPIO PM

[EricR Lai (Code Review)]

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

soc/intel/common: Add restore and enable GPIO PM

Enable GPIO clock gating when enter s0ix/Sx and save the PM bits.
Restore the PM bits when exit s0ix/Sx. This reference CB:37685.

BUG=b:157280323
TEST=Check GPIO PM bits when enter/exit s0ix are expected

Signed-off-by: Eric Lai 
Change-Id: If5e4553d568a872de234a2d671118acbae0a6159
---
M src/soc/intel/common/acpi/gpio.asl
1 file changed, 42 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/41712/1

diff --git a/src/soc/intel/common/acpi/gpio.asl b/src/soc/intel/common/acpi/gpio.asl
index dbfa7af..7176334 100644
--- a/src/soc/intel/common/acpi/gpio.asl
+++ b/src/soc/intel/common/acpi/gpio.asl
@@ -16,3 +16,45 @@
    	PCRO (Local0, GPIO_MISCCFG,  Arg1 & MISCCFG_ENABLE_GPIO_PM_CONFIG)
    }
 }
+
+
+/* GPIO Power Management bits */
+Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
+
+/*
+ * Save GPIO Power Management bits
+ */
+Method (SGPM, 0, Serialized)
+{
+	For (Local0 = 0, Local0 < TOTAL_GPIO_COMM, Local0++)
+	{
+		Local1 = GPID (Local0)
+		GPMB[Local0] = PCRR (Local1, GPIO_MISCCFG)
+	}
+}
+
+/*
+ * Restore GPIO Power Management bits
+ */
+Method (RGPM, 0, Serialized)
+{
+	For (Local0 = 0, Local0 < TOTAL_GPIO_COMM, Local0++)
+	{
+		CGPM (Local0, DerefOf(GPMB[Local0]))
+	}
+}
+
+/*
+ * Save current setting of GPIO Power Management bits and
+ * enable all Power Management bits for all communities
+ */
+Method (EGPM, 0, Serialized)
+{
+	/* Save current setting and will restore it when resuming */
+	SGPM ()
+	/* Enable PM bits */
+	For (Local0 = 0, Local0 < TOTAL_GPIO_COMM, Local0++)
+	{
+		CGPM (Local0, MISCCFG_ENABLE_GPIO_PM_CONFIG)
+	}
+}

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 1:

Can anyone help try this on Volteer? But even I could enable GPIO PM, I still can't get SLP_S0 assert. Only enable GPIO PM in device tree can do it. More log please refer the issue.

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 1:
> Patch Set 1:
>
> Can anyone help try this on Volteer? But even I could enable GPIO PM, I still can't get SLP_S0 assert. Only enable GPIO PM in device tree can do it. More log please refer the issue.

I have similar observation for dedede. But if you configure it manually using iotools, the slp_s0 asserts.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 1:

I found the issue, And I solve this but it looks the SOC infra issue. I need to configure TOTAL_GPIO_COMM to 6.. Due to TGL community is skip 3... I am thinking the common way to fixed this.. And this is an issue for TGL soc_fill_gpio_pm_configuration function as well.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 1:

oh no issue in chip.c, gpio_soc_defs already shift the number. We only need to figure out ASL part.

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 1:
> Patch Set 1:
>
> oh no issue in chip.c, gpio_soc_defs already shift the number. We only need to figure out ASL part.

The issue is in devicetree PM config fill instead of disabling gpio PM for 0, 1, 2, 4, 5, it is disabling for 0, 1, 2, 3, 4, and asl does not restore it for COMM3.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 1:
> Patch Set 1:
> > Patch Set 1:
> >
> > oh no issue in chip.c, gpio_soc_defs already shift the number. We only need to figure out ASL part.
>
> The issue is in devicetree PM config fill instead of disabling gpio PM for 0, 1, 2, 4, 5, it is disabling for 0, 1, 2, 3, 4, and asl does not restore it for COMM3.

Yes, I am working on the fix of my patch. Idea is store the index in separate.

[EricR Lai (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, Subrata Banik, Selma Bensaid, Aamir Bohra, Bernardo Perez Priego, Patrick Rudolph, Venkata Krishna Nimmagadda,

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

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

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

soc/intel/common: Add restore and enable GPIO PM

Enable GPIO clock gating when enter s0ix/Sx and save the PM bits.
Restore the PM bits when exit s0ix/Sx. This reference CB:37685.

BUG=b:157280323
TEST=Check GPIO PM bits when enter/exit s0ix are expected

Signed-off-by: Eric Lai 
Change-Id: If5e4553d568a872de234a2d671118acbae0a6159
---
M src/soc/intel/common/acpi/gpio.asl
1 file changed, 53 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/41712/2

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 2:

I nailed the GPIO communities mapping. Please provide ideas if any.. I verified at my Deltan.

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 2:
> Patch Set 2:
>
> I nailed the GPIO communities mapping. Please provide ideas if any.. I verified at my Deltan.

Instead can you check with below changes + patch set 1(is deltan not using MS0x?) on TGL?
I checked below on JSL and verified it working.
https://review.coreboot.org/c/coreboot/+/41721

-               Case (0) {
+               Case (COMM_0) {
                        Local0 = PID_GPIOCOM0
                }
-               Case (1) {
+               Case (COMM_1) {
                        Local0 = PID_GPIOCOM1
                }
-               Case (2) {
+               Case (COMM_2) {
                        Local0 = PID_GPIOCOM2
                }
-               Case (4) {
+               Case (COMM_4) {
                        Local0 = PID_GPIOCOM4
                }
-               Case (5) {
+               Case (COMM_5) {
                        Local0 = PID_GPIOCOM5

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> >
> > I nailed the GPIO communities mapping. Please provide ideas if any.. I verified at my Deltan.
>
> Instead can you check with below changes + patch set 1(is deltan not using MS0x?) on TGL?
> I checked below on JSL and verified it working.
> https://review.coreboot.org/c/coreboot/+/41721
>           Case (0) {
>           Case (COMM_0) {
>                   Local0 = PID_GPIOCOM0
>           }
>           Case (1) {
>           Case (COMM_1) {
>                   Local0 = PID_GPIOCOM1
>           }
>           Case (2) {
>           Case (COMM_2) {
>                   Local0 = PID_GPIOCOM2
>           }
>           Case (4) {
>           Case (COMM_4) {
>                   Local0 = PID_GPIOCOM4
>           }
>           Case (5) {
>           Case (COMM_5) {
>                   Local0 = PID_GPIOCOM5

This is way is good... I can go back to patch 1, I did in CML. I already hooked the EGPM in the lpit.asl and platform.asl you don't need the board level hook...

[EricR Lai (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, Subrata Banik, Selma Bensaid, Aamir Bohra, Bernardo Perez Priego, Patrick Rudolph, Venkata Krishna Nimmagadda,

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

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

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

soc/intel/common: Add restore and enable GPIO PM

Enable GPIO clock gating when enter s0ix/Sx and save the PM bits.
Restore the PM bits when exit s0ix/Sx. This reference CB:37685.

BUG=b:157280323
TEST=Check GPIO PM bits when enter/exit s0ix are expected

Signed-off-by: Eric Lai 
Change-Id: If5e4553d568a872de234a2d671118acbae0a6159
---
M src/soc/intel/common/acpi/gpio.asl
1 file changed, 42 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/41712/3

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 3:

Please try remove the dedede and volteer board level hook then try my patch :) It should worked if you add the lpit.asl in your board level dsdt.asl.

[Paul Menzel (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41712/3//COMMIT_MSG@10 
PS3, Line 10: This reference CB:37685.
Please rephrase and, if committed already, add the git commit hash.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41712/3//COMMIT_MSG@10 
PS3, Line 10: This reference CB:37685.
> Please rephrase and, if committed already, add the git commit hash.

This is auto link to the CL. And how to add the hash? How many numbers for the hash?

[EricR Lai (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, Subrata Banik, Selma Bensaid, Aamir Bohra, Bernardo Perez Priego, Patrick Rudolph, Venkata Krishna Nimmagadda,

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

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

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

soc/intel/common: Add restore and enable GPIO PM

Enable GPIO clock gating when enter s0ix/Sx and save the PM bits.
Restore the PM bits when exit s0ix/Sx.

BUG=b:157280323
TEST=Check GPIO PM bits when enter/exit s0ix are expected

Signed-off-by: Eric Lai 
Change-Id: If5e4553d568a872de234a2d671118acbae0a6159
---
M src/soc/intel/common/acpi/gpio.asl
1 file changed, 42 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/41712/4

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/41712/3//COMMIT_MSG@10 
PS3, Line 10: This reference CB:37685.
> This is auto link to the CL. […]

I removed this line.

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
I'm surprised this compiles, the ACPI spec says " It is a compile time error for NumElements to be less than the number of elements defined in the PackageList". So I would expect you would need TOTAL_GPIO_COMM 0s in that Package... could you double check what the output from IASL (the AML) looks like?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> I'm surprised this compiles, the ACPI spec says " It is a compile time error for NumElements to be l […]

okay, but this is working fine to me. It may need use (buffer zero) or something, I can check it.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> okay, but this is working fine to me. It may need use (buffer zero) or something, I can check it.

@Tim, I can't find the complaint from Jenkins. GOOGLE_DELTAN built successfully. (took 45s). So you are just check the SPEC, I can check you back later...

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> @Tim, I can't find the complaint from Jenkins. GOOGLE_DELTAN built successfully. (took 45s). […]

It could just be an "optimization" that IASL does, and it correctly inserts all of the zeros... hopefully so.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> It could just be an "optimization" that IASL does, and it correctly inserts all of the zeros... […]

Just 0 here.
            Name (GPMB, Package (0x05)
            {
                Zero
            })

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> Just 0 here. […]

Do you see any kernel error messages?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)
> Patch Set 4:
>
> (1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> Do you see any kernel error messages?

No error. And I can print it by Printf ("%o", GPMB(Local0)), totally fine though.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> No error. And I can print it by Printf ("%o", GPMB(Local0)), totally fine though.

@Tim, here is the ACPI spec:

Example 3: This code allocates space for ten objects to be defined at runtime (see the Name and
Index term definitions).
Package (10) {}
Example 4: These package declarations will cause the compiler to emit a VarPackageOp AML
opcode.
Name (SIZE, 4)
Package (SIZE) {}
Package (1024) {}
Package (256) {}

This will be treated as runtime var, so I think I can remove the 0 and this won't confuse others.

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> @Tim, here is the ACPI spec: […]

sgtm!

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
File src/soc/intel/common/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/41712/4/src/soc/intel/common/acpi/g... 
PS4, Line 22: Name(GPMB, Package(TOTAL_GPIO_COMM) {0})
> sgtm!

Please help find someone test this on volteer. Even though I think it would be okay.

[EricR Lai (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie, Bora Guvendik, Subrata Banik, Selma Bensaid, Aamir Bohra, Bernardo Perez Priego, Patrick Rudolph, Venkata Krishna Nimmagadda,

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

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

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

soc/intel/common: Add restore and enable GPIO PM

Enable GPIO clock gating when enter s0ix/Sx and save the PM bits.
Restore the PM bits when exit s0ix/Sx.

BUG=b:157280323
TEST=Check GPIO PM bits when enter/exit s0ix are expected

Signed-off-by: Eric Lai 
Change-Id: If5e4553d568a872de234a2d671118acbae0a6159
---
M src/soc/intel/common/acpi/gpio.asl
1 file changed, 42 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/41712/5

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 5:

This patch can work on dedede and deltaur but you need remove board level hook. @Tim, I try the latest and still works fine at my side. Could you help review this?

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 5:
> Patch Set 5:
>
> This patch can work on dedede and deltaur but you need remove board level hook. @Tim, I try the latest and still works fine at my side. Could you help review this?

Eric, we're hoping that we don't need to use the LPIT & the associated ACPI methods to enable/disable GPIO PM. We are working on a cr50 + coreboot solution that should fix it.

Do you think you need this soon?

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5:
> >
> > This patch can work on dedede and deltaur but you need remove board level hook. @Tim, I try the latest and still works fine at my side. Could you help review this?
>
> Eric, we're hoping that we don't need to use the LPIT & the associated ACPI methods to enable/disable GPIO PM. We are working on a cr50 + coreboot solution that should fix it.
>
> Do you think you need this soon?

SGTM, but how do you plan to deal with factory H1 version 0.22? Sometimes this will cause H1 failed then enter recovery mode.

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5:
> > > Patch Set 5:
> > >
> > > This patch can work on dedede and deltaur but you need remove board level hook. @Tim, I try the latest and still works fine at my side. Could you help review this?
> >
> > Eric, we're hoping that we don't need to use the LPIT & the associated ACPI methods to enable/disable GPIO PM. We are working on a cr50 + coreboot solution that should fix it.
> >
> > Do you think you need this soon?
>
> SGTM, but how do you plan to deal with factory H1 version 0.22? Sometimes this will cause H1 failed then enter recovery mode.

So the plan is to:
1) Add a new function to cr50 which will makes its IRQ pulses >= 100us.
2) Check cr50 FW version:
  a) if cr50 FW < MAGIC_NEW_VERSION, then disable the s0ix substate that causes GPIO PM to kick in, this will mean GPIO will eat more power, but this should only be the first boot from the factory.
  b) if cr50 FW >= MAGIC_NEW_VERSION, we'll enable the long IRQ pulses, and leave the LPM mode in its lowest s0ix substate

[EricR Lai (Code Review)]

EricR Lai has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41712 )

Change subject: soc/intel/common: Add restore and enable GPIO PM
......................................................................

Abandoned