[PATCH 2/2] Revert "ACPI: PM: s2idle: Avoid possible race related to the EC GPE"

Chris Wilson chris at chris-wilson.co.uk
Mon Feb 17 14:32:20 UTC 2020


This reverts commit e3728b50cd9be7d4b1469447cdf1feb93e3b7adb.
---
 drivers/acpi/sleep.c    | 44 ++++++++++++-----------------------------
 include/linux/suspend.h |  2 +-
 kernel/power/suspend.c  |  9 ++++-----
 3 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 2c695b196cd2..439880629839 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -990,28 +990,21 @@ static void acpi_s2idle_sync(void)
 	acpi_os_wait_events_complete(); /* synchronize Notify handling */
 }
 
-static bool acpi_s2idle_wake(void)
+static void acpi_s2idle_wake(void)
 {
-	if (!acpi_sci_irq_valid())
-		return pm_wakeup_pending();
-
-	while (pm_wakeup_pending()) {
-		/*
-		 * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the
-		 * SCI has not triggered while suspended, so bail out (the
-		 * wakeup is pending anyway and the SCI is not the source of
-		 * it).
-		 */
-		if (irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))
-			return true;
-
-		/*
-		 * If there are no EC events to process, the wakeup is regarded
-		 * as a genuine one.
-		 */
-		if (!acpi_ec_dispatch_gpe())
-			return true;
+	/*
+	 * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the SCI has
+	 * not triggered while suspended, so bail out.
+	 */
+	if (!acpi_sci_irq_valid() ||
+	    irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))
+		return;
 
+	/*
+	 * If there are EC events to process, the wakeup may be a spurious one
+	 * coming from the EC.
+	 */
+	if (acpi_ec_dispatch_gpe()) {
 		/*
 		 * Cancel the wakeup and process all pending events in case
 		 * there are any wakeup ones in there.
@@ -1024,19 +1017,8 @@ static bool acpi_s2idle_wake(void)
 
 		acpi_s2idle_sync();
 
-		/*
-		 * The SCI is in the "suspended" state now and it cannot produce
-		 * new wakeup events till the rearming below, so if any of them
-		 * are pending here, they must be resulting from the processing
-		 * of EC events above or coming from somewhere else.
-		 */
-		if (pm_wakeup_pending())
-			return true;
-
 		rearm_wake_irq(acpi_sci_irq);
 	}
-
-	return false;
 }
 
 static void acpi_s2idle_restore_early(void)
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 2b2055b035ee..4a230c2f1c31 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -191,7 +191,7 @@ struct platform_s2idle_ops {
 	int (*begin)(void);
 	int (*prepare)(void);
 	int (*prepare_late)(void);
-	bool (*wake)(void);
+	void (*wake)(void);
 	void (*restore_early)(void);
 	void (*restore)(void);
 	void (*end)(void);
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 8b1bb5ee7e5d..2c47280fbfc7 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -131,12 +131,11 @@ static void s2idle_loop(void)
 	 * to avoid them upfront.
 	 */
 	for (;;) {
-		if (s2idle_ops && s2idle_ops->wake) {
-			if (s2idle_ops->wake())
-				break;
-		} else if (pm_wakeup_pending()) {
+		if (s2idle_ops && s2idle_ops->wake)
+			s2idle_ops->wake();
+
+		if (pm_wakeup_pending())
 			break;
-		}
 
 		pm_wakeup_clear(false);
 
-- 
2.25.0



More information about the Intel-gfx-trybot mailing list