Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
soc/amd/common: fix eSPI virtual wire polarity encoding
eSPI interrupts are active level high. The eSPI polarity register in the chipset inverts incoming signals if the corresonding bit is 0 in the register. Therefore, all active high (edge or level) virtual wire interrupts need to ensure they are not inverted.
And really the sender of the interrupts should be conforming to the the eSPI spec. As such inverting any signals should not be necessary, but this register in the chipset allows for fixing up those misbehaviors.
BUG=b:157984427
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: I7346bb0484506d96d7ab2e6d046ffa0571683a48 --- M src/soc/amd/common/block/include/amdblocks/espi.h 1 file changed, 5 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/43010/1
diff --git a/src/soc/amd/common/block/include/amdblocks/espi.h b/src/soc/amd/common/block/include/amdblocks/espi.h index e882af0..2bfef89 100644 --- a/src/soc/amd/common/block/include/amdblocks/espi.h +++ b/src/soc/amd/common/block/include/amdblocks/espi.h @@ -43,14 +43,11 @@ #define ESPI_OOB_CH_EN (1 << 1) #define ESPI_FLASH_CH_EN (1 << 0)
-/* - * Virtual wire interrupt polarity. If the interrupt is active level high or active falling - * edge, then controller expects its bit to be cleared in ESPI_RXVW_POLARITY whereas if the - * interrupt is active level low or active rising edge, then its bit needs to be set in - * ESPI_RXVW_POLARITY. - */ -#define ESPI_VW_IRQ_LEVEL_HIGH(x) (0 << (x)) -#define ESPI_VW_IRQ_LEVEL_LOW(x) (1 << (x)) +/* 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))
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
Patch Set 1:
a
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
Patch Set 1: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43010/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/espi.h:
https://review.coreboot.org/c/coreboot/+/43010/1/src/soc/amd/common/block/in... PS1, Line 46: eSPI interrupts are active level high signals Is this defined in the eSPI spec? I don't remember seeing that.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43010/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/espi.h:
https://review.coreboot.org/c/coreboot/+/43010/1/src/soc/amd/common/block/in... PS1, Line 46: eSPI interrupts are active level high signals
Is this defined in the eSPI spec? I don't remember seeing that.
yes. https://www.intel.com/content/dam/support/us/en/documents/software/chipset-s...
"Interrupt level high (‘1’) indicates interrupt assertion whereas interrupt level low (‘0’) indicates interrupt deassertion. Interrupt events virtual wires are active high."
Section 5.2.2.4 Interrupt Event has all the fun details.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43010/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/espi.h:
https://review.coreboot.org/c/coreboot/+/43010/1/src/soc/amd/common/block/in... PS1, Line 46: eSPI interrupts are active level high signals
yes. https://www.intel. […]
resolving
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43010 )
Change subject: soc/amd/common: fix eSPI virtual wire polarity encoding ......................................................................
soc/amd/common: fix eSPI virtual wire polarity encoding
eSPI interrupts are active level high. The eSPI polarity register in the chipset inverts incoming signals if the corresonding bit is 0 in the register. Therefore, all active high (edge or level) virtual wire interrupts need to ensure they are not inverted.
And really the sender of the interrupts should be conforming to the the eSPI spec. As such inverting any signals should not be necessary, but this register in the chipset allows for fixing up those misbehaviors.
BUG=b:157984427
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: I7346bb0484506d96d7ab2e6d046ffa0571683a48 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43010 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/common/block/include/amdblocks/espi.h 1 file changed, 5 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/common/block/include/amdblocks/espi.h b/src/soc/amd/common/block/include/amdblocks/espi.h index e882af0..2bfef89 100644 --- a/src/soc/amd/common/block/include/amdblocks/espi.h +++ b/src/soc/amd/common/block/include/amdblocks/espi.h @@ -43,14 +43,11 @@ #define ESPI_OOB_CH_EN (1 << 1) #define ESPI_FLASH_CH_EN (1 << 0)
-/* - * Virtual wire interrupt polarity. If the interrupt is active level high or active falling - * edge, then controller expects its bit to be cleared in ESPI_RXVW_POLARITY whereas if the - * interrupt is active level low or active rising edge, then its bit needs to be set in - * ESPI_RXVW_POLARITY. - */ -#define ESPI_VW_IRQ_LEVEL_HIGH(x) (0 << (x)) -#define ESPI_VW_IRQ_LEVEL_LOW(x) (1 << (x)) +/* 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))
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)
-
Raul Rangel (Code Review)