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 <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/22033
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This commit is contained in:
Furquan Shaikh 2017-10-15 15:27:32 -07:00 committed by Aaron Durbin
parent e7d9248d9f
commit efe1e2d2d4
2 changed files with 15 additions and 3 deletions

View File

@ -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);

View File

@ -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();
}