ACPI: EC: PM: Avoid premature returns from acpi_s2idle_wake() (7b301750) · Commits · 戴 / test

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: ("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: ("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>

drivers/acpi/ec.c

+16 −8

Original line number	Diff line number	Diff line
		@@ -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 */

drivers/acpi/internal.h

+0 −1

Original line number	Diff line number	Diff line
		@@ -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

drivers/acpi/sleep.c

+2 −12

Original line number	Diff line number	Diff line
		@@ -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.

Admin message