Change in coreboot[master]: mb/google/dedede: Override GPIO PM configuration

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45857 )

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

mb/google/dedede: Override GPIO PM configuration

If Cr50 is running old firmware version and hence does not ensure long
interrupt pulses, override the GPIO PM configuration.

BUG=None
TEST=Build and boot waddledee to OS. Ensure that the suspend/resume
sequence works fine.

Change-Id: I455c51d4a63b1b5edadbf00c786ce61b0ba1ff00
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/mainboard.c
M src/mainboard/google/dedede/variants/baseboard/devicetree.cb
2 files changed, 20 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/45857/1

diff --git a/src/mainboard/google/dedede/mainboard.c b/src/mainboard/google/dedede/mainboard.c
index cb84e1f..150076f 100644
--- a/src/mainboard/google/dedede/mainboard.c
+++ b/src/mainboard/google/dedede/mainboard.c
@@ -4,9 +4,29 @@
 #include <bootstate.h>
 #include <baseboard/variants.h>
 #include <device/device.h>
+#include <drivers/spi/tpm/tpm.h>
 #include <ec/ec.h>
+#include <security/tpm/tss.h>
+#include <soc/ramstage.h>
 #include <vendorcode/google/chromeos/chromeos.h>

+void mainboard_update_soc_chip_config(struct soc_intel_jasperlake_config *cfg)
+{
+	int ret;
+	ret = tlcl_lib_init();
+	if (ret != VB2_SUCCESS) {
+		printk(BIOS_ERR, "tlcl_lib_init() failed: 0x%x\n", ret);
+		return;
+	}
+
+	if (!cr50_is_long_interrupt_pulse_enabled()) {
+		/* Disable GPIO PM to allow for shorter IRQ pulses */
+		printk(BIOS_INFO, "Override GPIO PM\n");
+		cfg->gpio_override_pm = 1;
+		memset(cfg->gpio_pm, 0, sizeof(cfg->gpio_pm));
+	}
+}
+
 __weak void variant_isst_override(void)
 {
    /*
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb
index 1f72eb1..3ced742 100644
--- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb
@@ -129,14 +129,6 @@
    # Select eDP for port A
    register "DdiPortAConfig" = "1"

-	# Disable PM to allow for shorter irq pulses
-	register "gpio_override_pm" = "1"
-	register "gpio_pm[0]" = "0"
-	register "gpio_pm[1]" = "0"
-	register "gpio_pm[2]" = "0"
-	register "gpio_pm[3]" = "0"
-	register "gpio_pm[4]" = "0"
-
    # Enable HPD for DDI ports B/C
    register "DdiPortBHpd" = "1"
    register "DdiPortCHpd" = "1"

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1: Code-Review+2

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45857/1//COMMIT_MSG@13 
PS1, Line 13: nsure that the suspend/resume
            : sequence works fine.
and no TPM errors during boot up and in OS?

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45857 )

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45857/1//COMMIT_MSG@13 
PS1, Line 13: nsure that the suspend/resume
            : sequence works fine.
> and no TPM errors during boot up and in OS?

I did reboot testing for 50 cycles and did not observe any errors. I also ensured that the DUT boots to OS in all those iterations and did not get into recovery mode.

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/45857/1//COMMIT_MSG@13 
PS1, Line 13: nsure that the suspend/resume
            : sequence works fine.
> I did reboot testing for 50 cycles and did not observe any errors. […]

Thanks Karthik!

[Tim Wawrzynczak (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1:

Are there any s0ix substates in JSL that need to be disabled as well along with GPIO PM bits?

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45857 )

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1:
> Patch Set 1:
>
> Are there any s0ix substates in JSL that need to be disabled as well along with GPIO PM bits?

Checked with Intel and confirmed there are no similar S0ix substates like in TGL.

[Tim Wawrzynczak (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 1: Code-Review+2

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Justin TerAvest, Tim Wawrzynczak, Aamir Bohra,

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

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

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

mb/google/dedede: Override GPIO PM configuration

If Cr50 is running old firmware version and hence does not ensure long
interrupt pulses, override the GPIO PM configuration.

BUG=None
TEST=Build and boot waddledee to OS. Ensure that any chip override
happens before FSP silicon parameter initialization. Ensure that the
suspend/resume sequence works fine. Ensure that the reboot sequence
works fine for 50 iterations.

Change-Id: I455c51d4a63b1b5edadbf00c786ce61b0ba1ff00
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/mainboard.c
M src/mainboard/google/dedede/variants/baseboard/devicetree.cb
2 files changed, 24 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/45857/2

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Justin TerAvest, Tim Wawrzynczak, Aamir Bohra,

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

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

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

mb/google/dedede: Override GPIO PM configuration

If Cr50 is running old firmware version and hence does not ensure long
interrupt pulses, override the GPIO PM configuration.

BUG=None
TEST=Build and boot waddledee to OS. Ensure that any chip override
happens before FSP silicon parameter initialization. Ensure that the
suspend/resume sequence works fine. Ensure that the reboot sequence
works fine for 50 iterations.

Change-Id: I455c51d4a63b1b5edadbf00c786ce61b0ba1ff00
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
M src/mainboard/google/dedede/variants/baseboard/devicetree.cb
3 files changed, 25 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/45857/3

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 3: Code-Review+2

[Tim Wawrzynczak (Code Review)]

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

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 3: Code-Review+2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45857 )

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 3: Code-Review+2

Thanks for the update!

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45857 )

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

Patch Set 3:
> Patch Set 3: Code-Review+2
>
> Thanks for the update!

No problem.

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45857 )

Change subject: mb/google/dedede: Override GPIO PM configuration
......................................................................

mb/google/dedede: Override GPIO PM configuration

If Cr50 is running old firmware version and hence does not ensure long
interrupt pulses, override the GPIO PM configuration.

BUG=None
TEST=Build and boot waddledee to OS. Ensure that any chip override
happens before FSP silicon parameter initialization. Ensure that the
suspend/resume sequence works fine. Ensure that the reboot sequence
works fine for 50 iterations.

Change-Id: I455c51d4a63b1b5edadbf00c786ce61b0ba1ff00
Signed-off-by: Karthikeyan Ramasubramanian 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45857
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Tim Wawrzynczak 
Reviewed-by: Nico Huber 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
M src/mainboard/google/dedede/variants/baseboard/devicetree.cb
3 files changed, 25 insertions(+), 8 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig
index e1b7136..2892dd1 100644
--- a/src/mainboard/google/dedede/Kconfig
+++ b/src/mainboard/google/dedede/Kconfig
@@ -1,6 +1,7 @@
 config BOARD_GOOGLE_BASEBOARD_DEDEDE
    def_bool n
    select BOARD_ROMSIZE_KB_16384 if !BOARD_ROMSIZE_KB_32768
+	select CR50_USE_LONG_INTERRUPT_PULSES
    select DPTF_USE_EISA_HID
    select DRIVERS_I2C_GENERIC
    select DRIVERS_I2C_HID
diff --git a/src/mainboard/google/dedede/mainboard.c b/src/mainboard/google/dedede/mainboard.c
index cb84e1f..45b2580 100644
--- a/src/mainboard/google/dedede/mainboard.c
+++ b/src/mainboard/google/dedede/mainboard.c
@@ -4,9 +4,31 @@
 #include <bootstate.h>
 #include <baseboard/variants.h>
 #include <device/device.h>
+#include <drivers/spi/tpm/tpm.h>
 #include <ec/ec.h>
+#include <security/tpm/tss.h>
+#include <soc/soc_chip.h>
 #include <vendorcode/google/chromeos/chromeos.h>

+static void mainboard_update_soc_chip_config(void)
+{
+	struct soc_intel_jasperlake_config *cfg = config_of_soc();
+	int ret;
+
+	ret = tlcl_lib_init();
+	if (ret != VB2_SUCCESS) {
+		printk(BIOS_ERR, "tlcl_lib_init() failed: 0x%x\n", ret);
+		return;
+	}
+
+	if (!cr50_is_long_interrupt_pulse_enabled()) {
+		/* Disable GPIO PM to allow for shorter IRQ pulses */
+		printk(BIOS_INFO, "Override GPIO PM\n");
+		cfg->gpio_override_pm = 1;
+		memset(cfg->gpio_pm, 0, sizeof(cfg->gpio_pm));
+	}
+}
+
 __weak void variant_isst_override(void)
 {
    /*
@@ -31,6 +53,8 @@

gpio_configure_pads_with_override(base_pads, base_num,
    	override_pads, override_num);
+
+	mainboard_update_soc_chip_config();
 }

static void mainboard_dev_init(struct device *dev)
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb
index 73b0c61..fe232e6 100644
--- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb
@@ -129,14 +129,6 @@
    # Select eDP for port A
    register "DdiPortAConfig" = "1"

-	# Disable PM to allow for shorter irq pulses
-	register "gpio_override_pm" = "1"
-	register "gpio_pm[0]" = "0"
-	register "gpio_pm[1]" = "0"
-	register "gpio_pm[2]" = "0"
-	register "gpio_pm[3]" = "0"
-	register "gpio_pm[4]" = "0"
-
    # Enable HPD for DDI ports B/C
    register "DdiPortBHpd" = "1"
    register "DdiPortCHpd" = "1"