Hello Karthikeyan Ramasubramanian,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32447
to review the following change.
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
soc/intel/common: Add support to clear GPI IS & IE registers
Add support to reset the GPI Interrupt Status & Enable registers so that the system does not experience any interrupt storm from a GPI when it comes out of one of the sleep states.
BUG=b:130593883 BRANCH=None TEST=Ensure that the Interrupt status & enable registers are reset during the boot up. Ensure that the system boots fine to ChromeOS.
Change-Id: I99f36d88cbab8bb75f12ab1a4d06437f837841cb Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/common/block/gpio/gpio.c M src/soc/intel/common/block/include/intelblocks/gpio.h 2 files changed, 31 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/32447/1
diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c index 47e2817..f5dd47b 100644 --- a/src/soc/intel/common/block/gpio/gpio.c +++ b/src/soc/intel/common/block/gpio/gpio.c @@ -582,3 +582,28 @@ { return reg_val; } + +void gpi_clear_int_cfg(void) +{ + int i, group, num_groups; + uint32_t offset, sts_value; + size_t gpio_communities; + const struct pad_community *comm; + + comm = soc_gpio_get_community(&gpio_communities); + for (i = 0; i < gpio_communities; i++, comm++) { + num_groups = comm->num_gpi_regs; + for (group = 0; group < num_groups; group++) { + /* Clear the enable register */ + offset = comm->gpi_int_en_reg_0 + + group * sizeof(uint32_t); + pcr_write32(comm->port, offset, 0); + + /* Read and clear the set status register bits*/ + offset = comm->gpi_int_sts_reg_0 + + group * sizeof(uint32_t); + sts_value = pcr_read32(comm->port, offset); + pcr_write32(comm->port, offset, sts_value); + } + } +} diff --git a/src/soc/intel/common/block/include/intelblocks/gpio.h b/src/soc/intel/common/block/include/intelblocks/gpio.h index 147f689..4179293 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio.h @@ -209,5 +209,11 @@ uint32_t soc_gpio_pad_config_fixup(const struct pad_config *cfg, int dw_reg, uint32_t reg_val);
+/* + * Function to reset/clear the GPI Interrupt Enable & Status registers for + * all GPIO pad communities. + */ +void gpi_clear_int_cfg(void); + #endif #endif /* _SOC_INTELBLOCKS_GPIO_H_ */
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32447 )
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/gpio/gpio... File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/gpio/gpio... PS1, Line 598: comm->gpi_int_en_reg_0 + : group * sizeof(uint32_t) nit: You can also create macros like GPI_IE_OFFSET and GPI_IS_OFFSET similar to GPI_SMI_STS_OFFSET and GPI_SMI_EN_OFFSET
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio.h:
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/include/i... PS1, Line 216: gpi_clear_int_cfg Probably name this gpio_clear_int_cfg() to match the rest of the function APIs?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32447 )
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/gpio/gpio... File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/gpio/gpio... PS1, Line 598: comm->gpi_int_en_reg_0 + : group * sizeof(uint32_t)
nit: You can also create macros like GPI_IE_OFFSET and GPI_IS_OFFSET similar to GPI_SMI_STS_OFFSET a […]
Sure, will define those macros
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio.h:
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/include/i... PS1, Line 216: gpi_clear_int_cfg
Probably name this gpio_clear_int_cfg() to match the rest of the function APIs?
There are couple of functions here with gpi prefix:
gpi_clear_get_smi_status gpi_status_get
Hence defined it that way.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32447 )
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio.h:
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/include/i... PS1, Line 216: gpi_clear_int_cfg
There are couple of functions here with gpi prefix: […]
Aah okay.. SG.
Hello Patrick Rudolph, Karthikeyan Ramasubramanian, Kane Chen, Justin TerAvest, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32447
to look at the new patch set (#2).
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
soc/intel/common: Add support to clear GPI IS & IE registers
Add support to reset the GPI Interrupt Status & Enable registers so that the system does not experience any interrupt storm from a GPI when it comes out of one of the sleep states.
BUG=b:130593883 BRANCH=None TEST=Ensure that the Interrupt status & enable registers are reset during the boot up. Ensure that the system boots fine to ChromeOS.
Change-Id: I99f36d88cbab8bb75f12ab1a4d06437f837841cb Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/common/block/gpio/gpio.c M src/soc/intel/common/block/include/intelblocks/gpio.h 2 files changed, 33 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/32447/2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32447 )
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/gpio/gpio... File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/#/c/32447/1/src/soc/intel/common/block/gpio/gpio... PS1, Line 598: comm->gpi_int_en_reg_0 + : group * sizeof(uint32_t)
Sure, will define those macros
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32447 )
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32447 )
Change subject: soc/intel/common: Add support to clear GPI IS & IE registers ......................................................................
soc/intel/common: Add support to clear GPI IS & IE registers
Add support to reset the GPI Interrupt Status & Enable registers so that the system does not experience any interrupt storm from a GPI when it comes out of one of the sleep states.
BUG=b:130593883 BRANCH=None TEST=Ensure that the Interrupt status & enable registers are reset during the boot up. Ensure that the system boots fine to ChromeOS.
Change-Id: I99f36d88cbab8bb75f12ab1a4d06437f837841cb Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32447 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/common/block/gpio/gpio.c M src/soc/intel/common/block/include/intelblocks/gpio.h 2 files changed, 33 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c index 47e2817..3a0594c 100644 --- a/src/soc/intel/common/block/gpio/gpio.c +++ b/src/soc/intel/common/block/gpio/gpio.c @@ -62,6 +62,10 @@ ((group) * sizeof(uint32_t))) #define GPI_SMI_EN_OFFSET(comm, group) ((comm)->gpi_smi_en_reg_0 + \ ((group) * sizeof(uint32_t))) +#define GPI_IS_OFFSET(comm, group) ((comm)->gpi_int_sts_reg_0 + \ + ((group) * sizeof(uint32_t))) +#define GPI_IE_OFFSET(comm, group) ((comm)->gpi_int_en_reg_0 + \ + ((group) * sizeof(uint32_t)))
static inline size_t relative_pad_in_comm(const struct pad_community *comm, gpio_t gpio) @@ -582,3 +586,26 @@ { return reg_val; } + +void gpi_clear_int_cfg(void) +{ + int i, group, num_groups; + uint32_t sts_value; + size_t gpio_communities; + const struct pad_community *comm; + + comm = soc_gpio_get_community(&gpio_communities); + for (i = 0; i < gpio_communities; i++, comm++) { + num_groups = comm->num_gpi_regs; + for (group = 0; group < num_groups; group++) { + /* Clear the enable register */ + pcr_write32(comm->port, GPI_IE_OFFSET(comm, group), 0); + + /* Read and clear the set status register bits*/ + sts_value = pcr_read32(comm->port, + GPI_IS_OFFSET(comm, group)); + pcr_write32(comm->port, + GPI_IS_OFFSET(comm, group), sts_value); + } + } +} diff --git a/src/soc/intel/common/block/include/intelblocks/gpio.h b/src/soc/intel/common/block/include/intelblocks/gpio.h index 147f689..4179293 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio.h @@ -209,5 +209,11 @@ uint32_t soc_gpio_pad_config_fixup(const struct pad_config *cfg, int dw_reg, uint32_t reg_val);
+/* + * Function to reset/clear the GPI Interrupt Enable & Status registers for + * all GPIO pad communities. + */ +void gpi_clear_int_cfg(void); + #endif #endif /* _SOC_INTELBLOCKS_GPIO_H_ */