From 6d64155cc8eb03912e500fee5631156bf70e7ace Mon Sep 17 00:00:00 2001 From: Subrata Banik Date: Mon, 16 Jan 2023 15:05:02 +0530 Subject: [PATCH] soc/intel/cannonlake: Fix incorrect `prev_sleep_state` issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The patch fixes indication of incorrect `prev_sleep_state` on the next boot after global reset trigger. The existing code misses an important check about `if PCH doesn't set the WAK_STS` while checking power failure. As a result, every early warm/global reset is considered as power failure after looking into the PMC MMIO CON-A register alone (as ignoring the ACPI PM_CTRL.WAK_STS bit). As per the code comment this code logic is expected to check the power failure reason if PCH doesn't set the WAK_STS while waking from G3 state. TEST=Able to build and boot google/hatch. Without this patch: Observation: Resuming after a warm reset is considered as `prev_sleep_state 5` although the SLP_TYP is zero and WAK_STS bit is set. pm1_sts: 8100 pm1_en: 0000 pm1_cnt: 00000000 GEN_PMCON: d1215238 00002200 .... prev_sleep_state 5 With this patch: Observation: Resuming after a warm reset is considered as `prev_sleep_state 0`. It matches with the SLP_TYP is zero and WAK_STS bit is set. pm1_sts: 8100 pm1_en: 0000 pm1_cnt: 00000000 GEN_PMCON: d1215238 00002200 .... prev_sleep_state 0 Signed-off-by: Subrata Banik Change-Id: I05a2fab75c3d931651885db0003ab8c5748a1568 Reviewed-on: https://review.coreboot.org/c/coreboot/+/71934 Reviewed-by: Lean Sheng Tan Reviewed-by: Sean Rhodes Reviewed-by: Sridhar Siricilla Tested-by: build bot (Jenkins) --- src/soc/intel/cannonlake/pmutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/soc/intel/cannonlake/pmutil.c b/src/soc/intel/cannonlake/pmutil.c index 94725f6139..512b49b6ae 100644 --- a/src/soc/intel/cannonlake/pmutil.c +++ b/src/soc/intel/cannonlake/pmutil.c @@ -195,7 +195,7 @@ int soc_prev_sleep_state(const struct chipset_power_state *ps, int prev_sleep_st * S5 because the PCH does not set the WAK_STS bit when waking * from a true G3 state. */ - if (ps->gen_pmcon_a & (PWR_FLR | SUS_PWR_FLR)) + if (!(ps->pm1_sts & WAK_STS) && (ps->gen_pmcon_a & (PWR_FLR | SUS_PWR_FLR))) prev_sleep_state = ACPI_S5; /*