From 74bce48f1d445053b217913df79a77dfe286e893 Mon Sep 17 00:00:00 2001 From: Raul E Rangel Date: Mon, 24 May 2021 16:38:50 -0600 Subject: 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 Change-Id: I4608a7684e34ebb389e0e55ceba7e7441939afe7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/54924 Tested-by: build bot (Jenkins) Reviewed-by: Karthik Ramasubramanian --- .../google/guybrush/variants/baseboard/devicetree.cb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'src/mainboard/google/guybrush') 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 -- cgit v1.2.3