mb/google/{zork,guybrush,skyrim},soc/amd/espi: Fix vw_irq_polarity

The default state for the IRQ lines when the eSPI controller comes out
of reset is high. This is because the IRQ lines are shared with the
other IRQ sources using AND gates. This means that in order to not cause
any spurious interrupts or miss any interrupts, the IO-APIC must use a
low polarity trigger.

On zork/guybrush/skyrim the eSPI IRQs are currently working as follows:
* On power on/resume the eSPI controller drives IRQ 1 high.
* eSPI controller gets configured to not invert IRQ 1.
* OS configures IO-APIC IRQ 1 as Edge/High.
* EC writes to HIKDO (Keyboard Data Out) which causes the EC to set IRQ1
  high.
* eSPI controller receives IRQ 1 high, doesn't invert it, and leaves IRQ
  1 as high. This results in missing the first interrupt.
* When the x86 reads from HIKDO, the EC deasserts IRQ1. This causes the
  eSPI controller to set IRQ1 to low. We are now primed to catch the
  next edge high interrupt. This is generally not a problem since the
  linux driver will probe the 8042 with interrupts off.

On S3/S0i3 resume since the eSPI controller comes out of reset driving
the IRQ lines high, we trigger a spurious IRQ since the IO-APIC is
configured to trigger on edge high. This results in the 8042 controller
getting incorrectly marked as a wake trigger.

By configuring the IO-APIC to use low polarity interrupts, we no longer
lose the first interrupt. This also means we can use a level interrupt
to match what the EC actually asserts.

We use the `Interrupt` keyword instead of the `IRQ` keyword in the ACPI
because the linux kernel will ignore the level/polarity parameters
for the `IRQ` keyword and default to `edge/high. `Interrupt` doesn't
have this problem.

The PIC is not currently configured anywhere and it defaults to an
edge/high trigger. We could add some code to configure the PICs trigger
register, but I don't think we need the functionality right now.

For zork and guybrush, this change is a no-op. eSPI is configured in
verstage which is located in RO, and we have already locked RO for
these devices. We will need to figure out how to properly set the
`vw_irq_polarity` for these devices.

BUG=b:218874489, b:160595155, b:184752352, b:157984427, b:238818104
TEST=On zork, guybrush and skyrim
$ suspend_stress_test --post_resume_command 'cat /sys/devices/platform/i8042/serio0/wakeup/wakeup35/active_count'
Verify keyboard works as expected and no interrupt storms are observed.

On morphius I verified keyboard and mouse work on windows as well.

Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I4608a7684e34ebb389e0e55ceba7e7441939afe7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54924
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>

5 files changed, 62 insertions, 10 deletions

diff --git a/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb b/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb
index 2d7eaec6c6..04e6e994be 100644
--- a/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/guybrush/variants/baseboard/devicetree.cb
@@ -42,7 +42,16 @@ chip soc/amd/cezanne
 		.oob_ch_en = 0,
 		.flash_ch_en = 0,
 
-		.vw_irq_polarity = ESPI_VW_IRQ_LEVEL_HIGH(1),
+		/*
+		 * b/218874489 - This should really be ESPI_VW_IRQ_LEVEL_HIGH,
+		 * but eSPI gets configured in verstage which is in RO.
+		 * We have already locked RO for guybrush devices so we need
+		 * make it so x86 coreboot re-initializes the vw_irq_polarity.
+		 * This leaves another problem, verstage also runs in S0i3, but
+		 * we don't run any other x86 coreboot stages, so we need to
+		 * figure out a way to reset the eSPI polarity.
+		 */
+		.vw_irq_polarity = ESPI_VW_IRQ_LEVEL_LOW(1),
 	}"
 
 	# Enable S0i3 support
diff --git a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/ec.h b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/ec.h
index ea6b93eef2..b8b466b6a8 100644
--- a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/ec.h
+++ b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/ec.h
@@ -64,6 +64,7 @@
 #define SIO_EC_MEMMAP_ENABLE /* EC Memory Map Resources */
 #define SIO_EC_HOST_ENABLE   /* EC Host Interface Resources */
 #define SIO_EC_ENABLE_PS2K   /* Enable PS/2 Keyboard */
+#define SIO_EC_PS2K_IRQ Interrupt (ResourceConsumer, Level, ActiveLow, Shared) {1}
 
 /* Enable EC sync interrupt */
 #define EC_ENABLE_SYNC_IRQ_GPIO
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb
index 68eb6ea588..69146041e8 100644
--- a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb
+++ b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb
@@ -227,7 +227,13 @@ chip soc/amd/picasso
 		.oob_ch_en = 0,
 		.flash_ch_en = 0,
 
-		.vw_irq_polarity = ESPI_VW_IRQ_LEVEL_HIGH(1) | ESPI_VW_IRQ_LEVEL_HIGH(12),
+		/*
+		 * b/160595155 - These should really be ESPI_VW_IRQ_LEVEL_HIGH,
+		 * but eSPI gets configured in verstage which is in RO.
+		 * We have already locked RO for zork devices so we need
+		 * make it so x86 coreboot re-initializes the vw_irq_polarity.
+		 */
+		.vw_irq_polarity = ESPI_VW_IRQ_LEVEL_LOW(1) | ESPI_VW_IRQ_LEVEL_LOW(12),
 	}"
 
 	register "i2c_scl_reset" = "GPIO_I2C2_SCL | GPIO_I2C3_SCL"
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
index 4bb42dea1c..96e66aff1c 100644
--- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
+++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb
@@ -225,7 +225,13 @@ chip soc/amd/picasso
 		.oob_ch_en = 0,
 		.flash_ch_en = 0,
 
-		.vw_irq_polarity = ESPI_VW_IRQ_LEVEL_HIGH(1) | ESPI_VW_IRQ_LEVEL_HIGH(12),
+		/*
+		 * b/160595155 - These should really be ESPI_VW_IRQ_LEVEL_HIGH,
+		 * but eSPI gets configured in verstage which is in RO.
+		 * We have already locked RO for zork devices so we need
+		 * make it so x86 coreboot re-initializes the vw_irq_polarity.
+		 */
+		.vw_irq_polarity = ESPI_VW_IRQ_LEVEL_LOW(1) | ESPI_VW_IRQ_LEVEL_LOW(12),
 	}"
 
 	register "i2c_scl_reset" = "GPIO_I2C2_SCL | GPIO_I2C3_SCL"
diff --git a/src/soc/amd/common/block/include/amdblocks/espi.h b/src/soc/amd/common/block/include/amdblocks/espi.h
index 971768853a..296ae4e1e3 100644
--- a/src/soc/amd/common/block/include/amdblocks/espi.h
+++ b/src/soc/amd/common/block/include/amdblocks/espi.h
@@ -36,13 +36,43 @@
 #define  ESPI_OOB_CH_EN				(1 << 1)
 #define  ESPI_FLASH_CH_EN			(1 << 0)
 
-/* Virtual wire interrupt polarity. eSPI interrupts are active level high signals. The
-   polarity register inverts the incoming signal if the associated bit with the irq is
-   0. */
-#define ESPI_VW_IRQ_LEVEL_HIGH(x)		(1 << (x))
-#define ESPI_VW_IRQ_LEVEL_LOW(x)		(0 << (x))
-#define ESPI_VW_IRQ_EDGE_HIGH(x)		(1 << (x))
-#define ESPI_VW_IRQ_EDGE_LOW(x)			(0 << (x))
+/*
+ * Internally the SoC uses active low signals for the IRQs. This means what when
+ * the eSPI controller comes out of reset, it's driving its IRQ lines high.
+ * In order to avoid any spurious interrupts the IO-APIC must be configured to
+ * trigger on active low signals. The PIC is only capable of triggering on
+ * active high signals so the hardware has an inverter that converts the signals
+ * before they feed into the PIC.
+ *
+ * +----------+         Active Low
+ * |          |             |            +--------+
+ * | IO-APIC  <------+      |      +-----+ LPC    |
+ * |          |      |  <---+--->  |     +--------+
+ * +----------+      |             |
+ *                   |   +-----+   |     +--------+
+ *                   +---+ AND <---+-----+ eSPI   |
+ *                   |   +-----+   |     +--------+
+ * +----------+      |             |
+ * |          |   +--v--+          |     +--------+
+ * |   PIC    <---+ NOT |          +-----+ PIR    |
+ * |          | ^ +-----+                +--------+
+ * +----------+ |
+ *              | Active High
+ *
+ * The eSPI controller has an inverter that is applied to incoming Virtual Wire
+ * IRQ messages. This allows eSPI peripherals to use active high signaling.
+ * If the peripheral uses active low signaling like the SoC does internally, the
+ * inverter can be disabled.
+ *
+ * The polarity register has the following behavior:
+ *   0: Invert the incoming VW IRQ before outputting to the AND gates.
+ *   1: Do not invert the incoming VW IRQ, but route it directly to the AND
+ *      gates.
+ */
+#define ESPI_VW_IRQ_LEVEL_HIGH(x)		(0 << (x))
+#define ESPI_VW_IRQ_LEVEL_LOW(x)		(1 << (x))
+#define ESPI_VW_IRQ_EDGE_HIGH(x)		(0 << (x))
+#define ESPI_VW_IRQ_EDGE_LOW(x)			(1 << (x))
 
 enum espi_io_mode {
 	ESPI_IO_MODE_SINGLE = ESPI_IO_MODE_VALUE(0),