From efe1e2d2d458897995552c173ad826b52afd768d Mon Sep 17 00:00:00 2001 From: Furquan Shaikh Date: Sun, 15 Oct 2017 15:27:32 -0700 Subject: soc/intel/common/block/pmc: Move pmc_disable_all_gpe to romstage Instead of disabling all GPEs during PMC init in bootblock, this change moves it to pmc_fill_power_state which allows romstage to correctly fill up GPE_EN registers in chipset_power_state. This is essential for correctly identifying the wake source. Disabling all GPEs was added recently in change 74145f76 (intel/common/pmc: Disable all GPEs during pmc_init) because keeping GPEs enabled in coreboot while enabling SMI could lead to side-effects as explained in the change. Moving pmc_disable_all_gpe to pmc_fill_power_state should be safe as that happens before SMI is enabled in coreboot. TEST=Verified that GPE-based wake source is correctly identified. Also, no issues observed while resuming from S3. Change-Id: I8e992ad09ffdefba62de11fa572e783715776bf1 Signed-off-by: Furquan Shaikh Reviewed-on: https://review.coreboot.org/22033 Reviewed-by: Aaron Durbin Tested-by: build bot (Jenkins) --- src/soc/intel/common/block/include/intelblocks/pmclib.h | 4 +++- src/soc/intel/common/block/pmc/pmclib.c | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/soc/intel/common/block/include/intelblocks/pmclib.h b/src/soc/intel/common/block/include/intelblocks/pmclib.h index 9dbac24f8c..89c3cdcd5d 100644 --- a/src/soc/intel/common/block/include/intelblocks/pmclib.h +++ b/src/soc/intel/common/block/include/intelblocks/pmclib.h @@ -149,7 +149,9 @@ struct chipset_power_state *pmc_get_power_state(void); /* * Reads and prints ACPI specific PM registers which are common across - * chipsets. Returns the previous sleep state which is one of ACPI_SX values. + * chipsets. Returns the previous sleep state which is one of ACPI_SX + * values. Additionally, it also disables all GPEs after GPE_EN + * registers are read. */ int pmc_fill_power_state(struct chipset_power_state *ps); diff --git a/src/soc/intel/common/block/pmc/pmclib.c b/src/soc/intel/common/block/pmc/pmclib.c index b8ec17dd58..7bddb4700c 100644 --- a/src/soc/intel/common/block/pmc/pmclib.c +++ b/src/soc/intel/common/block/pmc/pmclib.c @@ -410,6 +410,18 @@ int pmc_fill_power_state(struct chipset_power_state *ps) ps->prev_sleep_state = pmc_prev_sleep_state(ps); printk(BIOS_DEBUG, "prev_sleep_state %d\n", ps->prev_sleep_state); + /* + * GPEs need to be disabled before enabling SMI. Otherwise, it could + * lead to SMIs being triggered in coreboot preventing the progress of + * normal boot-up. However, GPEs should not be disabled as part of + * pmc_gpe_init which happens in bootblock. Otherwise, + * pmc_fill_power_state would read GPE0_EN registers as all 0s thus + * losing information about the wake source. Hence, + * pmc_disable_all_gpe() is placed here after GPE0_EN registers are + * saved in chipset_power_state. + */ + pmc_disable_all_gpe(); + return ps->prev_sleep_state; } @@ -558,6 +570,4 @@ void pmc_gpe_init(void) /* Set the routes in the GPIO communities as well. */ gpio_route_gpe(dw0, dw1, dw2); - - pmc_disable_all_gpe(); } -- cgit v1.2.3