Change in ...coreboot[master]: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage
Hello Karthikeyan Ramasubramanian,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32534
to review the following change.
Change subject: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage ......................................................................
soc/intel/apollolake: Reset GPI IS & IE registers at ramstage
Reset GPI Interrupt status and enable registers from ramstage instead of bootblock so that it applies to devices in field.
BUG=b:130593883 BRANCH=octopus 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: Ib11b580ceb23bd1fe789f549b667a8ced2d859a1 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/apollolake/bootblock/bootblock.c M src/soc/intel/apollolake/chip.c 2 files changed, 6 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/32534/1
diff --git a/src/soc/intel/apollolake/bootblock/bootblock.c b/src/soc/intel/apollolake/bootblock/bootblock.c index c791378..ac6903a 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -122,12 +122,3 @@ paging_enable_for_car("pdpt", "pt"); } } - -void bootblock_soc_init(void) -{ - /* - * Clear the GPI interrupt enable & status registers to avoid any - * interrupt storm during the kernel bootup. - */ - gpi_clear_int_cfg(); -} diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index c3de4ee..83af26e 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -399,6 +399,12 @@ /* Restore GPIO IRQ polarities back to previous settings. */ itss_restore_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
+ /* + * 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(); + /* override 'enabled' setting in device tree if needed */ pcie_override_devicetree_after_silicon_init();
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32534 )
Change subject: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32534/1/src/soc/intel/apollolake/chip.c File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/#/c/32534/1/src/soc/intel/apollolake/chip.c@406 PS1, Line 406: gpi_clear_int_cfg Can we do this before calling into fsp_silicon_init? Mainboard GPIO configuration is done before silicon init. So, it would be good to clear interrupt status and enable bits before that.
Hello Patrick Rudolph, Karthikeyan Ramasubramanian, Justin TerAvest, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32534
to look at the new patch set (#2).
Change subject: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage ......................................................................
soc/intel/apollolake: Reset GPI IS & IE registers at ramstage
Reset GPI Interrupt status and enable registers from ramstage instead of bootblock so that it applies to devices in field.
BUG=b:130593883 BRANCH=octopus 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: Ib11b580ceb23bd1fe789f549b667a8ced2d859a1 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/apollolake/bootblock/bootblock.c M src/soc/intel/apollolake/chip.c 2 files changed, 6 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/32534/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32534 )
Change subject: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage ......................................................................
Patch Set 2: Code-Review+2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32534 )
Change subject: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32534/1/src/soc/intel/apollolake/chip.c File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/#/c/32534/1/src/soc/intel/apollolake/chip.c@406 PS1, Line 406: gpi_clear_int_cfg
Can we do this before calling into fsp_silicon_init? Mainboard GPIO configuration is done before sil […]
Done
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32534 )
Change subject: soc/intel/apollolake: Reset GPI IS & IE registers at ramstage ......................................................................
soc/intel/apollolake: Reset GPI IS & IE registers at ramstage
Reset GPI Interrupt status and enable registers from ramstage instead of bootblock so that it applies to devices in field.
BUG=b:130593883 BRANCH=octopus 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: Ib11b580ceb23bd1fe789f549b667a8ced2d859a1 Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32534 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/apollolake/bootblock/bootblock.c M src/soc/intel/apollolake/chip.c 2 files changed, 6 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/apollolake/bootblock/bootblock.c b/src/soc/intel/apollolake/bootblock/bootblock.c index c791378..ac6903a 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -122,12 +122,3 @@ paging_enable_for_car("pdpt", "pt"); } } - -void bootblock_soc_init(void) -{ - /* - * Clear the GPI interrupt enable & status registers to avoid any - * interrupt storm during the kernel bootup. - */ - gpi_clear_int_cfg(); -} diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index 279551f..f6880a7 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -395,6 +395,12 @@ * default policy that doesn't honor boards' requirements. */ itss_snapshot_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
+ /* + * 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(); + fsp_silicon_init(romstage_handoff_is_resume());
/* Restore GPIO IRQ polarities back to previous settings. */
-
Furquan Shaikh (Code Review) -
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)