ACPI: EC: PM: Avoid premature returns from acpi_s2idle_wake()

If the EC GPE status is not set after checking all of the other GPEs,
acpi_s2idle_wake() returns 'false', to indicate that the SCI event
that has just triggered is not a system wakeup one, but it does that
without canceling the pending wakeup and re-arming the SCI for system
wakeup which is a mistake, because it may cause s2idle_loop() to busy
spin until the next valid wakeup event.  [If that happens, the first
spurious wakeup is still pending after acpi_s2idle_wake() has
returned, so s2idle_enter() does nothing, acpi_s2idle_wake()
is called again and it sees that the SCI has triggered, but no GPEs
are active, so 'false' is returned again, and so on.]

Fix that by moving all of the GPE checking logic from
acpi_s2idle_wake() to acpi_ec_dispatch_gpe() and making the
latter return 'true' only if a non-EC GPE has triggered and
'false' otherwise, which will cause acpi_s2idle_wake() to
cancel the pending SCI wakeup and re-arm the SCI for system
wakeup regardless of the EC GPE status.

This also addresses a lockup observed on an Elitegroup EF20EA laptop
after attempting to wake it up from suspend-to-idle by a key press.

Fixes: d5406284ff80 ("ACPI: PM: s2idle: Refine active GPEs check")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=207603
Reported-by: Todd Brandt <todd.e.brandt@linux.intel.com>
Fixes: fdde0ff8590b ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system")
Link: https://lore.kernel.org/linux-acpi/CAB4CAwdqo7=MvyG_PE+PGVfeA17AHF5i5JucgaKqqMX6mjArbQ@mail.gmail.com/
Reported-by: Chris Chiu <chiu@endlessm.com>
Tested-by: Chris Chiu <chiu@endlessm.com>
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b4c0152e92aa124ac0d92a778cf49a448f4a03cd..145ec0b6f20b177d64bc40661ccd3aedd9896d42 100644 (file)
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1994,23 +1994,31 @@ void acpi_ec_set_gpe_wake_mask(u8 action)
                acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action);
 }
 
-bool acpi_ec_other_gpes_active(void)
-{
-       return acpi_any_gpe_status_set(first_ec ? first_ec->gpe : U32_MAX);
-}
-
 bool acpi_ec_dispatch_gpe(void)
 {
        u32 ret;
 
        if (!first_ec)
+               return acpi_any_gpe_status_set(U32_MAX);
+
+       /*
+        * Report wakeup if the status bit is set for any enabled GPE other
+        * than the EC one.
+        */
+       if (acpi_any_gpe_status_set(first_ec->gpe))
+               return true;
+
+       if (ec_no_wakeup)
                return false;
 
+       /*
+        * Dispatch the EC GPE in-band, but do not report wakeup in any case
+        * to allow the caller to process events properly after that.
+        */
        ret = acpi_dispatch_gpe(NULL, first_ec->gpe);
-       if (ret == ACPI_INTERRUPT_HANDLED) {
+       if (ret == ACPI_INTERRUPT_HANDLED)
                pm_pr_dbg("EC GPE dispatched\n");
-               return true;
-       }
+
        return false;
 }
 #endif /* CONFIG_PM_SLEEP */

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index e387517d3354605b7f248e55ebf04d3356379959..43411a7457cd7f4520eb80bd05aa73d724db616d 100644 (file)
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -202,7 +202,6 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
 
 #ifdef CONFIG_PM_SLEEP
 void acpi_ec_flush_work(void);
-bool acpi_ec_other_gpes_active(void);
 bool acpi_ec_dispatch_gpe(void);
 #endif
 

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 4edc8a3ce40fdc50039f0460953b3e422d193efe..3850704570c0c699e57a81401af8b5243b05e694 100644 (file)
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -1013,20 +1013,10 @@ static bool acpi_s2idle_wake(void)
                if (acpi_check_wakeup_handlers())
                        return true;
 
-               /*
-                * If the status bit is set for any enabled GPE other than the
-                * EC one, the wakeup is regarded as a genuine one.
-                */
-               if (acpi_ec_other_gpes_active())
+               /* Check non-EC GPE wakeups and dispatch the EC GPE. */
+               if (acpi_ec_dispatch_gpe())
                        return true;
 
-               /*
-                * If the EC GPE status bit has not been set, the wakeup is
-                * regarded as a spurious one.
-                */
-               if (!acpi_ec_dispatch_gpe())
-                       return false;
-
                /*
                 * Cancel the wakeup and process all pending events in case
                 * there are any wakeup ones in there.