Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43011 )
Change subject: mb/google/zork: adjust eSPI virtual irq settings ......................................................................
mb/google/zork: adjust eSPI virtual irq settings
The eSPI polarity macros were reversed. Those are fixed so adjust the corresponding values related to the correct expectations of the IRQ path: eSPI virtual wire IRQs are active level high. The EC sends active level high virtual wire IRQs. The default interrupt encodings in ACPI for P2/S devices are active edge high. Therefore, there is no need to override anything.
BUG=b:157984427
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: Ia28d82cd9e432df98839f68bac4eae4447455e53 --- M src/mainboard/google/zork/variants/baseboard/devicetree.cb M src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h 2 files changed, 1 insertion(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/43011/1
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree.cb b/src/mainboard/google/zork/variants/baseboard/devicetree.cb index 3cc311b..beb2c71 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree.cb @@ -93,19 +93,11 @@ .oob_ch_en = 0, .flash_ch_en = 0,
- .vw_irq_polarity = ESPI_VW_IRQ_LEVEL_LOW(1) | ESPI_VW_IRQ_LEVEL_LOW(12), + .vw_irq_polarity = ESPI_VW_IRQ_LEVEL_HIGH(1) | ESPI_VW_IRQ_LEVEL_HIGH(12), }"
register "i2c_scl_reset" = "GPIO_I2C2_SCL | GPIO_I2C3_SCL"
- register "irq_override" = "{ - /* PS/2 keyboard IRQ1 override */ - {1, MP_IRQ_TRIGGER_LEVEL | MP_IRQ_POLARITY_HIGH}, - - /* PS/2 mouse IRQ12 override */ - {12, MP_IRQ_TRIGGER_LEVEL | MP_IRQ_POLARITY_HIGH}, - }" - device cpu_cluster 0 on device lapic 0 on end end diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h index 8ef0645..ff42989 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h @@ -59,8 +59,6 @@ #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 IRQ (Level, ActiveHigh, Exclusive) {1} -#define SIO_EC_PS2M_IRQ IRQ (Level, ActiveHigh, Exclusive) {12}
/* * Enable EC sync interrupt via GPIO controller, EC_SYNC_IRQ is defined in
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43011 )
Change subject: mb/google/zork: adjust eSPI virtual irq settings ......................................................................
Patch Set 1: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43011 )
Change subject: mb/google/zork: adjust eSPI virtual irq settings ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43011/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43011/1//COMMIT_MSG@13 PS1, Line 13: ACPI for P2/S devices are active edge high I'm pretty sure the kernel ignores the ACPI entries. It uses the bus defaults for them which means edge/high.
https://review.coreboot.org/c/coreboot/+/43011/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43011/1/src/mainboard/google/zork/v... PS1, Line 96: ESPI_VW_IRQ_LEVEL_HIGH I need to re read the PPR and decode the macros. Now sure how it was working with LOW here, and HIGH in the MADT.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43011 )
Change subject: mb/google/zork: adjust eSPI virtual irq settings ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43011/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43011/1//COMMIT_MSG@13 PS1, Line 13: ACPI for P2/S devices are active edge high
I'm pretty sure the kernel ignores the ACPI entries. […]
That's up to the kernel. By removing SIO_EC_PS2K_IRQ then the advertised IRQ resource is active edge high.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43011 )
Change subject: mb/google/zork: adjust eSPI virtual irq settings ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43011/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43011/1/src/mainboard/google/zork/v... PS1, Line 96: ESPI_VW_IRQ_LEVEL_HIGH
I need to re read the PPR and decode the macros. […]
resolving
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43011 )
Change subject: mb/google/zork: adjust eSPI virtual irq settings ......................................................................
mb/google/zork: adjust eSPI virtual irq settings
The eSPI polarity macros were reversed. Those are fixed so adjust the corresponding values related to the correct expectations of the IRQ path: eSPI virtual wire IRQs are active level high. The EC sends active level high virtual wire IRQs. The default interrupt encodings in ACPI for P2/S devices are active edge high. Therefore, there is no need to override anything.
BUG=b:157984427
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: Ia28d82cd9e432df98839f68bac4eae4447455e53 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43011 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/zork/variants/baseboard/devicetree.cb M src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h 2 files changed, 1 insertion(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree.cb b/src/mainboard/google/zork/variants/baseboard/devicetree.cb index 9324749..85ce4e6 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree.cb @@ -103,19 +103,11 @@ .oob_ch_en = 0, .flash_ch_en = 0,
- .vw_irq_polarity = ESPI_VW_IRQ_LEVEL_LOW(1) | ESPI_VW_IRQ_LEVEL_LOW(12), + .vw_irq_polarity = ESPI_VW_IRQ_LEVEL_HIGH(1) | ESPI_VW_IRQ_LEVEL_HIGH(12), }"
register "i2c_scl_reset" = "GPIO_I2C2_SCL | GPIO_I2C3_SCL"
- register "irq_override" = "{ - /* PS/2 keyboard IRQ1 override */ - {1, MP_IRQ_TRIGGER_LEVEL | MP_IRQ_POLARITY_HIGH}, - - /* PS/2 mouse IRQ12 override */ - {12, MP_IRQ_TRIGGER_LEVEL | MP_IRQ_POLARITY_HIGH}, - }" - device cpu_cluster 0 on device lapic 0 on end end diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h index 8ef0645..ff42989 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/ec.h @@ -59,8 +59,6 @@ #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 IRQ (Level, ActiveHigh, Exclusive) {1} -#define SIO_EC_PS2M_IRQ IRQ (Level, ActiveHigh, Exclusive) {12}
/* * Enable EC sync interrupt via GPIO controller, EC_SYNC_IRQ is defined in
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Raul Rangel (Code Review)