David Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
soc/intel/cannonlake: Clear the GPI IS & IE registers
Clear the GPI Interrupt Status & Enable registers to prevent any interrupt storms due to GPI.
BUG=b:138282962 TEST=Ensure that the Interrupt status & enable registers are reset during the boot up when the system is brought out of G3, S5 & S3. Ensure that the system boots fine to ChromeOS.
Change-Id: I2185355d0095601e0778b6bf47ae137cc53e4051 Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com --- M src/soc/intel/cannonlake/chip.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/34624/1
diff --git a/src/soc/intel/cannonlake/chip.c b/src/soc/intel/cannonlake/chip.c index 4e0dba5..fc511db 100644 --- a/src/soc/intel/cannonlake/chip.c +++ b/src/soc/intel/cannonlake/chip.c @@ -184,6 +184,12 @@
void soc_init_pre_device(void *chip_info) { + /* + * Clear the GPI interrupt status and enable registers. These + * registers do not get reset to default state when booting from S5. + */ + gpi_clear_int_cfg(); + /* Perform silicon specific init. */ fsp_silicon_init(romstage_handoff_is_resume());
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34624/1/src/soc/intel/cannonlake/ch... File src/soc/intel/cannonlake/chip.c:
https://review.coreboot.org/c/coreboot/+/34624/1/src/soc/intel/cannonlake/ch... PS1, Line 191: gpi_clear_int_cfg(); Why is this being cleared so late. Can this be done in bootblock instead?
Hello Patrick Rudolph, Karthik Ramasubramanian, Paul Fagerburg, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34624
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
soc/intel/cannonlake: Clear the GPI IS & IE registers
Clear the GPI Interrupt Status & Enable registers to prevent any interrupt storms due to GPI.
BUG=b:138282962 TEST=Ensure that the Interrupt status & enable registers are reset during the boot up when the system is brought out of G3, S5 & S3. Ensure that the system boots fine to ChromeOS.
Change-Id: I2185355d0095601e0778b6bf47ae137cc53e4051 Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/34624/2
Hello Patrick Rudolph, Karthik Ramasubramanian, Paul Fagerburg, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34624
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers
Clear the GPI Interrupt Status & Enable registers to prevent any interrupt storms due to GPI.
BUG=b:138282962 TEST=Ensure that the Interrupt status & enable registers are reset during the boot up when the system is brought out of G3, S5 & S3. Ensure that the system boots fine to ChromeOS.
Change-Id: I2185355d0095601e0778b6bf47ae137cc53e4051 Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/34624/3
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
(1 comment)
Thanks.
https://review.coreboot.org/c/coreboot/+/34624/1/src/soc/intel/cannonlake/ch... File src/soc/intel/cannonlake/chip.c:
https://review.coreboot.org/c/coreboot/+/34624/1/src/soc/intel/cannonlake/ch... PS1, Line 191: gpi_clear_int_cfg();
Why is this being cleared so late. […]
Done
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
Patch Set 3:
(1 comment)
Thanks.
I believe there are no devices deployed in the field already. Otherwise putting in bootblock may not help clear the GPI IS & IE registers on those devices.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
(1 comment)
Thanks.
I believe there are no devices deployed in the field already. Otherwise putting in bootblock may not help clear the GPI IS & IE registers on those devices.
That's right.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
what is the real problem statement here?
GPI Interrupt Enable (GPI_INT_EN_GPPC_A_0): This bit is used to enable/disable the generation of APIC interrupt when the corresponding GPI_INT_STS bit is set. 0 = disable interrupt generation 1 = enable interrupt generation
i don't think if this bit remain set alone won't cause any issue unless GPI_INT_STS bit is also set at that time?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
Patch Set 3:
what is the real problem statement here?
GPI Interrupt Enable (GPI_INT_EN_GPPC_A_0): This bit is used to enable/disable the generation of APIC interrupt when the corresponding GPI_INT_STS bit is set. 0 = disable interrupt generation 1 = enable interrupt generation
i don't think if this bit remain set alone won't cause any issue unless GPI_INT_STS bit is also set at that time?
Only when the system enters G3, these registers are reset. In other states, they are not reset. This becomes more of a problem when one SKU of a variant uses the concerned GPIO and a different SKU of the same variant does not use the concerned GPIO. For more context please refer to b/130593883.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
what is the real problem statement here?
GPI Interrupt Enable (GPI_INT_EN_GPPC_A_0): This bit is used to enable/disable the generation of APIC interrupt when the corresponding GPI_INT_STS bit is set. 0 = disable interrupt generation 1 = enable interrupt generation
i don't think if this bit remain set alone won't cause any issue unless GPI_INT_STS bit is also set at that time?
Only when the system enters G3, these registers are reset. In other states, they are not reset. This becomes more of a problem when one SKU of a variant uses the concerned GPIO and a different SKU of the same variant does not use the concerned GPIO. For more context please refer to b/130593883.
Thanks for the pointer, what is the PAD reset configuration for that GPIO PIN ? (where you are seeing its only resetting in G3)
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3: Code-Review+2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG@14 PS3, Line 14: boot up boot-up
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG@14 PS3, Line 14: G3 What is G3?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG@14 PS3, Line 14: G3
What is G3?
G3 is the ACPI power state "mechanical off"
Hello Patrick Rudolph, Karthik Ramasubramanian, Paul Fagerburg, Subrata Banik, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34624
to look at the new patch set (#4).
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
soc/intel/cannonlake: Clear the GPI IS & IE registers
Clear the GPI Interrupt Status & Enable registers to prevent any interrupt storms due to GPI.
BUG=b:138282962 TEST=Ensure that the Interrupt status & enable registers are reset during the boot-up when the system is brought out of G3, S5 & S3. Ensure that the system boots fine to ChromeOS.
Change-Id: I2185355d0095601e0778b6bf47ae137cc53e4051 Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com --- M src/soc/intel/cannonlake/chip.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/34624/4
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Refer to https://review.coreboot.org/c/coreboot/+/32534, clear the GPI IS & IE registers at ramstage.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 3:
Patch Set 3:
Patch Set 3:
what is the real problem statement here?
GPI Interrupt Enable (GPI_INT_EN_GPPC_A_0): This bit is used to enable/disable the generation of APIC interrupt when the corresponding GPI_INT_STS bit is set. 0 = disable interrupt generation 1 = enable interrupt generation
i don't think if this bit remain set alone won't cause any issue unless GPI_INT_STS bit is also set at that time?
Only when the system enters G3, these registers are reset. In other states, they are not reset. This becomes more of a problem when one SKU of a variant uses the concerned GPIO and a different SKU of the same variant does not use the concerned GPIO. For more context please refer to b/130593883.
Thanks for the pointer, what is the PAD reset configuration for that GPIO PIN ? (where you are seeing its only resetting in G3)
Looks like i don't have access b/130593883
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Refer to https://review.coreboot.org/c/coreboot/+/32534, clear the GPI IS & IE registers at ramstage.
yes, its been done in APL, wondering if its board W/A we are keeping in soc code or genuine soc issue.
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
(2 comments)
Patch Set 4:
Patch Set 3:
Patch Set 3:
Patch Set 3:
what is the real problem statement here?
GPI Interrupt Enable (GPI_INT_EN_GPPC_A_0): This bit is used to enable/disable the generation of APIC interrupt when the corresponding GPI_INT_STS bit is set. 0 = disable interrupt generation 1 = enable interrupt generation
i don't think if this bit remain set alone won't cause any issue unless GPI_INT_STS bit is also set at that time?
Only when the system enters G3, these registers are reset. In other states, they are not reset. This becomes more of a problem when one SKU of a variant uses the concerned GPIO and a different SKU of the same variant does not use the concerned GPIO. For more context please refer to b/130593883.
Thanks for the pointer, what is the PAD reset configuration for that GPIO PIN ? (where you are seeing its only resetting in G3)
Looks like i don't have access b/130593883
I have added you, please use pd account: subrata.banik@intel.corp-partner.google.com to access b/130593883. thanks.
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG@14 PS3, Line 14: G3
G3 is the ACPI power state "mechanical off"
Thanks.
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG@14 PS3, Line 14: boot up
boot-up
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4: Code-Review+2
looks like its APL learning carried forward'ed into CML as well. Its kind of WA in FW space to clear GPI enable/status
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
sorry, i missed that we are doing early gpio programming as well.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4: -Code-Review
Patch Set 4:
This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
I believe it's p2sb_enable_bar()
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4: -Code-Review
> Patch Set 4: > > This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware.
make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
I believe it's p2sb_enable_bar(), which is called from bootblock_soc_early_init(), so adding bootblock_mainboard_early_init() to hatch and moving this there probably makes sense
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
> Patch Set 4: -Code-Review > > > Patch Set 4: > > > > This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware. > > make sense. may be finalize.c ?
Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
I believe it's p2sb_enable_bar(), which is called from bootblock_soc_early_init(), so adding bootblock_mainboard_early_init() to hatch and moving this there probably makes sense
Yes that would work. But, in my opinion this change is required for all mainboards and would be good to keep in SoC code -- either in bootblock_soc_early_init() after bootblock_pch_early_init() or in bootblock_soc_init().
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
> Patch Set 4: > > > Patch Set 4: -Code-Review > > > > > Patch Set 4: > > > > > > This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware. > > > > make sense. may be finalize.c ? > > Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured?
Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
I believe it's p2sb_enable_bar(), which is called from bootblock_soc_early_init(), so adding bootblock_mainboard_early_init() to hatch and moving this there probably makes sense
Yes that would work. But, in my opinion this change is required for all mainboards and would be good to keep in SoC code -- either in bootblock_soc_early_init() after bootblock_pch_early_init() or in bootblock_soc_init().
Ah yes good point.
David Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake: Clear the GPI IS & IE registers ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
> Patch Set 4: > > > Patch Set 4: > > > > > Patch Set 4: -Code-Review > > > > > > > Patch Set 4: > > > > > > > > This can go into bootblock, not ramstage. We don't have any devices out in the field yet with locked R/O firmware. > > > > > > make sense. may be finalize.c ? > > > > Wouldn't it want to do this as soon as we come out of reset i.e. before any GPIOs are configured? > > Something like bootblock_soc_early_init ?
We need to ensure that the early BARs are set up correctly before accessing these registers. I believe it gets set in bootblock_soc_early_init().
https://github.com/coreboot/coreboot/blob/7706a04c603474400234cc72a27a610708... ?
I believe it's p2sb_enable_bar(), which is called from bootblock_soc_early_init(), so adding bootblock_mainboard_early_init() to hatch and moving this there probably makes sense
Yes that would work. But, in my opinion this change is required for all mainboards and would be good to keep in SoC code -- either in bootblock_soc_early_init() after bootblock_pch_early_init() or in bootblock_soc_init().
Ah yes good point.
Thanks all, this will go into bootblock(in bootblock_soc_init())
Hello Patrick Rudolph, Karthik Ramasubramanian, Paul Fagerburg, Subrata Banik, Tim Wawrzynczak, Paul Menzel, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34624
to look at the new patch set (#5).
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers
Clear the GPI Interrupt Status & Enable registers to prevent any interrupt storms due to GPI.
BUG=b:138282962 TEST=Ensure that the Interrupt status & enable registers are reset during the boot-up when the system is brought out of G3, S5 & S3. Ensure that the system boots fine to ChromeOS.
Change-Id: I2185355d0095601e0778b6bf47ae137cc53e4051 Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com --- M src/soc/intel/cannonlake/bootblock/bootblock.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/34624/5
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 5: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34624/3//COMMIT_MSG@14 PS3, Line 14: G3
Thanks.
Done
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34624 )
Change subject: soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers ......................................................................
soc/intel/cannonlake/bootblock: Clear the GPI IS & IE registers
Clear the GPI Interrupt Status & Enable registers to prevent any interrupt storms due to GPI.
BUG=b:138282962 TEST=Ensure that the Interrupt status & enable registers are reset during the boot-up when the system is brought out of G3, S5 & S3. Ensure that the system boots fine to ChromeOS.
Change-Id: I2185355d0095601e0778b6bf47ae137cc53e4051 Signed-off-by: David Wu david_wu@quanta.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34624 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Paul Fagerburg pfagerburg@chromium.org --- M src/soc/intel/cannonlake/bootblock/bootblock.c 1 file changed, 5 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Karthik Ramasubramanian: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved Paul Fagerburg: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/bootblock/bootblock.c b/src/soc/intel/cannonlake/bootblock/bootblock.c index 5555969..30c2266 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -59,6 +59,11 @@
void bootblock_soc_init(void) { + /* + * Clear the GPI interrupt status and enable registers. These + * registers do not get reset to default state when booting from S5. + */ + gpi_clear_int_cfg(); report_platform_info(); pch_early_init(); }
-
David Wu (Code Review) -
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Paul Fagerburg (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)