[PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Rafael J. Wysocki]

Hi All,

This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
will be reverted shortly, because it triggered issues on quite a few systems.

The last patch in the series is a counterpart of the above commit with a few
modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
printed to kernel logs to make them somewhat less confusing.

The previous patches are pre-requisite changes plus some cleanups.  The major
ones are [1-2/6] and [4/6] that are really needed for things to work as expected
after [6/6].

In addition to that, this patch from Hans: https://patchwork.kernel.org/patch/9762815/
is needed for USB wakeup on Bay Trail and Cherry Trail systems to work in general.

Thanks,
Rafael

[PATCH 1/6] ACPI / PM: Run wakeup notify handlers synchronously

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The work functions provided by the users of acpi_add_pm_notifier()
should be run synchronously before re-enabling the wakeup GPE in
case they are used to clear the status and/or disable the wakeup
signaling at the source.  Otherwise, which is the case currently in
the PCI bus type code, the same wakeup event may be signaled for
multiple times while the execution of the work function in response
to it has already been queued up.

Fortunately, acpi_add_pm_notifier() is only used by PCI and by
ACPI device PM code internally, so the change is relatively
straightforward to make.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   27 +++++++++++----------------
 drivers/pci/pci-acpi.c   |   17 +++++++----------
 include/acpi/acpi_bus.h  |    4 ++--
 3 files changed, 20 insertions(+), 28 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -400,8 +400,8 @@ static void acpi_pm_notify_handler(acpi_
 
 	if (adev->wakeup.flags.notifier_present) {
 		__pm_wakeup_event(adev->wakeup.ws, 0);
-		if (adev->wakeup.context.work.func)
-			queue_pm_work(&adev->wakeup.context.work);
+		if (adev->wakeup.context.func)
+			adev->wakeup.context.func(&adev->wakeup.context);
 	}
 
 	mutex_unlock(&acpi_pm_notifier_lock);
@@ -413,7 +413,7 @@ static void acpi_pm_notify_handler(acpi_
  * acpi_add_pm_notifier - Register PM notify handler for given ACPI device.
  * @adev: ACPI device to add the notify handler for.
  * @dev: Device to generate a wakeup event for while handling the notification.
- * @work_func: Work function to execute when handling the notification.
+ * @func: Work function to execute when handling the notification.
  *
  * NOTE: @adev need not be a run-wake or wakeup device to be a valid source of
  * PM wakeup events.  For example, wakeup events may be generated for bridges
@@ -421,11 +421,11 @@ static void acpi_pm_notify_handler(acpi_
  * bridge itself doesn't have a wakeup GPE associated with it.
  */
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
-				 void (*work_func)(struct work_struct *work))
+			void (*func)(struct acpi_device_wakeup_context *context))
 {
 	acpi_status status = AE_ALREADY_EXISTS;
 
-	if (!dev && !work_func)
+	if (!dev && !func)
 		return AE_BAD_PARAMETER;
 
 	mutex_lock(&acpi_pm_notifier_lock);
@@ -435,8 +435,7 @@ acpi_status acpi_add_pm_notifier(struct
 
 	adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
 	adev->wakeup.context.dev = dev;
-	if (work_func)
-		INIT_WORK(&adev->wakeup.context.work, work_func);
+	adev->wakeup.context.func = func;
 
 	status = acpi_install_notify_handler(adev->handle, ACPI_SYSTEM_NOTIFY,
 					     acpi_pm_notify_handler, NULL);
@@ -469,10 +468,7 @@ acpi_status acpi_remove_pm_notifier(stru
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	if (adev->wakeup.context.work.func) {
-		cancel_work_sync(&adev->wakeup.context.work);
-		adev->wakeup.context.work.func = NULL;
-	}
+	adev->wakeup.context.func = NULL;
 	adev->wakeup.context.dev = NULL;
 	wakeup_source_unregister(adev->wakeup.ws);
 
@@ -658,16 +654,15 @@ EXPORT_SYMBOL(acpi_pm_device_sleep_state
 
 /**
  * acpi_pm_notify_work_func - ACPI devices wakeup notification work function.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void acpi_pm_notify_work_func(struct work_struct *work)
+static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
 {
-	struct device *dev;
+	struct device *dev = context->dev;
 
-	dev = container_of(work, struct acpi_device_wakeup_context, work)->dev;
 	if (dev) {
 		pm_wakeup_event(dev, 0);
-		pm_runtime_resume(dev);
+		pm_request_resume(dev);
 	}
 }
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -319,7 +319,7 @@ struct acpi_device_wakeup_flags {
 };
 
 struct acpi_device_wakeup_context {
-	struct work_struct work;
+	void (*func)(struct acpi_device_wakeup_context *context);
 	struct device *dev;
 };
 
@@ -599,7 +599,7 @@ static inline bool acpi_device_always_pr
 
 #ifdef CONFIG_PM
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
-				 void (*work_func)(struct work_struct *work));
+			void (*func)(struct acpi_device_wakeup_context *context));
 acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
 int acpi_pm_device_sleep_state(struct device *, int *, int);
 int acpi_pm_device_run_wake(struct device *, bool);
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -395,29 +395,26 @@ bool pciehp_is_native(struct pci_dev *pd
 
 /**
  * pci_acpi_wake_bus - Root bus wakeup notification fork function.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void pci_acpi_wake_bus(struct work_struct *work)
+static void pci_acpi_wake_bus(struct acpi_device_wakeup_context *context)
 {
 	struct acpi_device *adev;
 	struct acpi_pci_root *root;
 
-	adev = container_of(work, struct acpi_device, wakeup.context.work);
+	adev = container_of(context, struct acpi_device, wakeup.context);
 	root = acpi_driver_data(adev);
 	pci_pme_wakeup_bus(root->bus);
 }
 
 /**
  * pci_acpi_wake_dev - PCI device wakeup notification work function.
- * @handle: ACPI handle of a device the notification is for.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void pci_acpi_wake_dev(struct work_struct *work)
+static void pci_acpi_wake_dev(struct acpi_device_wakeup_context *context)
 {
-	struct acpi_device_wakeup_context *context;
 	struct pci_dev *pci_dev;
 
-	context = container_of(work, struct acpi_device_wakeup_context, work);
 	pci_dev = to_pci_dev(context->dev);
 
 	if (pci_dev->pme_poll)
@@ -425,7 +422,7 @@ static void pci_acpi_wake_dev(struct wor
 
 	if (pci_dev->current_state == PCI_D3cold) {
 		pci_wakeup_event(pci_dev);
-		pm_runtime_resume(&pci_dev->dev);
+		pm_request_resume(&pci_dev->dev);
 		return;
 	}
 
@@ -434,7 +431,7 @@ static void pci_acpi_wake_dev(struct wor
 		pci_check_pme_status(pci_dev);
 
 	pci_wakeup_event(pci_dev);
-	pm_runtime_resume(&pci_dev->dev);
+	pm_request_resume(&pci_dev->dev);
 
 	pci_pme_wakeup_bus(pci_dev->subordinate);
 }

[PATCH 2/6] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
PCI USB controllers calls pci_back_from_sleep() which is unnecessary
and may become problematic.

It is unnecessary, because the PCI bus type carries out post-suspend
cleanup of all PCI devices during resume and that covers all things
done by the pci_back_from_sleep().  There is no reason why USB cannot
follow all of the other PCI devices in that respect.

It will become problematic after subsequent changes that make it
possible to go back to sleep again after executing dpm_resume_noirq()
if no valid system wakeup events have been detected at that point.
Namely, calling pci_back_from_sleep() at the _resume_noirq stage
will cause the wakeup status of the devices in question to be cleared
and if any of them has triggered system wakeup, that event may be
missed then.

For the above reasons, drop the pci_back_from_sleep() invocation
from hcd_pci_resume_noirq().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/usb/core/hcd-pci.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Index: linux-pm/drivers/usb/core/hcd-pci.c
===================================================================
--- linux-pm.orig/drivers/usb/core/hcd-pci.c
+++ linux-pm/drivers/usb/core/hcd-pci.c
@@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
 
 static int hcd_pci_resume_noirq(struct device *dev)
 {
-	struct pci_dev		*pci_dev = to_pci_dev(dev);
-
-	powermac_set_asic(pci_dev, 1);
-
-	/* Go back to D0 and disable remote wakeup */
-	pci_back_from_sleep(pci_dev);
+	powermac_set_asic(to_pci_dev(dev), 1);
 	return 0;
 }
 

[PATCH 3/6] ACPI / PM: Change log level of wakeup-related message

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Change the log level of the "System wakeup enabled/disabled by ACPI"
message in acpi_pm_device_sleep_wake() to "debug" to reduce to log
noise level.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -756,8 +756,8 @@ int acpi_pm_device_sleep_wake(struct dev
 
 	error = acpi_device_wakeup(adev, acpi_target_system_state(), enable);
 	if (!error)
-		dev_info(dev, "System wakeup %s by ACPI\n",
-				enable ? "enabled" : "disabled");
+		dev_dbg(dev, "System wakeup %s by ACPI\n",
+			enable ? "enabled" : "disabled");
 
 	return error;
 }

[PATCH 4/6] ACPI / PM: Clean up device wakeup enable/disable code

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The wakeup.flags.enabled flag in struct acpi_device is not used
consistently, as there is no reason why it should only apply
to the enabling/disabling of the wakeup GPE, so put the invocation
of acpi_enable_wakeup_device_power() under it too.

Moreover, it is not necessary to call
acpi_enable_wakeup_devices() and acpi_disable_wakeup_devices() for
suspend-to-idle, so don't do that.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   19 ++++++++-----------
 drivers/acpi/sleep.c     |    4 ++--
 2 files changed, 10 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -688,26 +688,23 @@ static int acpi_device_wakeup(struct acp
 		acpi_status res;
 		int error;
 
+		if (adev->wakeup.flags.enabled)
+			return 0;
+
 		error = acpi_enable_wakeup_device_power(adev, target_state);
 		if (error)
 			return error;
 
-		if (adev->wakeup.flags.enabled)
-			return 0;
-
 		res = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
-		if (ACPI_SUCCESS(res)) {
-			adev->wakeup.flags.enabled = 1;
-		} else {
+		if (ACPI_FAILURE(res)) {
 			acpi_disable_wakeup_device_power(adev);
 			return -EIO;
 		}
-	} else {
-		if (adev->wakeup.flags.enabled) {
-			acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
-			adev->wakeup.flags.enabled = 0;
-		}
+		adev->wakeup.flags.enabled = 1;
+	} else if (adev->wakeup.flags.enabled) {
+		acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
 		acpi_disable_wakeup_device_power(adev);
+		adev->wakeup.flags.enabled = 0;
 	}
 	return 0;
 }
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -658,19 +658,19 @@ static int acpi_freeze_begin(void)
 
 static int acpi_freeze_prepare(void)
 {
-	acpi_enable_wakeup_devices(ACPI_STATE_S0);
 	acpi_enable_all_wakeup_gpes();
 	acpi_os_wait_events_complete();
 	if (acpi_sci_irq_valid())
 		enable_irq_wake(acpi_sci_irq);
+
 	return 0;
 }
 
 static void acpi_freeze_restore(void)
 {
-	acpi_disable_wakeup_devices(ACPI_STATE_S0);
 	if (acpi_sci_irq_valid())
 		disable_irq_wake(acpi_sci_irq);
+
 	acpi_enable_all_runtime_gpes();
 }
 

[PATCH 5/6] PM / sleep: Print timing information if debug is enabled

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Avoid printing the device suspend/resume timing information if
CONFIG_PM_DEBUG is not set to reduce the log noise level.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -417,6 +417,7 @@ static void pm_dev_err(struct device *de
 		dev_name(dev), pm_verb(state.event), info, error);
 }
 
+#ifdef CONFIG_PM_DEBUG
 static void dpm_show_time(ktime_t starttime, pm_message_t state, char *info)
 {
 	ktime_t calltime;
@@ -433,6 +434,9 @@ static void dpm_show_time(ktime_t startt
 		info ?: "", info ? " " : "", pm_verb(state.event),
 		usecs / USEC_PER_MSEC, usecs % USEC_PER_MSEC);
 }
+#else
+static inline void dpm_show_time(ktime_t starttime, pm_message_t state, char *info) {}
+#endif /* CONFIG_PM_DEBUG */
 
 static int dpm_run_callback(pm_callback_t cb, struct device *dev,
 			    pm_message_t state, char *info)

[PATCH 6/6] ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The ACPI SCI (System Control Interrupt) is set up as a wakeup IRQ
during suspend-to-idle transitions and, consequently, any events
signaled through it wake up the system from that state.  However,
on some systems some of the events signaled via the ACPI SCI while
suspended to idle should not cause the system to wake up.  In fact,
quite often they should just be discarded.

Arguably, systems should not resume entirely on such events, but in
order to decide which events really should cause the system to resume
and which are spurious, it is necessary to resume up to the point
when ACPI SCIs are actually handled and processed, which is after
executing dpm_resume_noirq() in the system resume path.

For this reasons, add a loop around freeze_enter() in which the
platforms can process events signaled via multiplexed IRQ lines
like the ACPI SCI and add suspend-to-idle hooks that can be
used for this purpose to struct platform_freeze_ops.

In the ACPI case, the ->wake hook is used for checking if the SCI
has triggered while suspended and deferring the interrupt-induced
system wakeup until the events signaled through it are actually
processed sufficiently to decide whether or not the system should
resume.  In turn, the ->sync hook allows all of the relevant event
queues to be flushed so as to prevent events from being missed due
to race conditions.

In addition to that, some ACPI code processing wakeup events needs
to be modified to use the "hard" version of wakeup triggers, so that
it will cause a system resume to happen on device-induced wakeup
events even if the "soft" mechanism to prevent the system from
suspending is not enabled.  However, to preserve the existing
behavior with respect to suspend-to-RAM, this only is done in
the suspend-to-idle case and only if an SCI has occurred while
suspended.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/battery.c      |    2 +-
 drivers/acpi/button.c       |    5 +++--
 drivers/acpi/device_pm.c    |    9 ++++++++-
 drivers/acpi/internal.h     |    2 ++
 drivers/acpi/sleep.c        |   37 +++++++++++++++++++++++++++++++++++++
 drivers/base/power/main.c   |    5 -----
 drivers/base/power/wakeup.c |   18 ++++++++++++------
 include/acpi/acpi_bus.h     |    4 ++++
 include/linux/suspend.h     |    7 +++++--
 kernel/power/process.c      |    2 +-
 kernel/power/suspend.c      |   35 +++++++++++++++++++++++++++++------
 11 files changed, 102 insertions(+), 24 deletions(-)

Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -189,6 +189,8 @@ struct platform_suspend_ops {
 struct platform_freeze_ops {
 	int (*begin)(void);
 	int (*prepare)(void);
+	void (*wake)(void);
+	void (*sync)(void);
 	void (*restore)(void);
 	void (*end)(void);
 };
@@ -428,7 +430,8 @@ extern unsigned int pm_wakeup_irq;
 
 extern bool pm_wakeup_pending(void);
 extern void pm_system_wakeup(void);
-extern void pm_wakeup_clear(void);
+extern void pm_system_cancel_wakeup(void);
+extern void pm_wakeup_clear(bool reset);
 extern void pm_system_irq_wakeup(unsigned int irq_number);
 extern bool pm_get_wakeup_count(unsigned int *count, bool block);
 extern bool pm_save_wakeup_count(unsigned int count);
@@ -478,7 +481,7 @@ static inline int unregister_pm_notifier
 
 static inline bool pm_wakeup_pending(void) { return false; }
 static inline void pm_system_wakeup(void) {}
-static inline void pm_wakeup_clear(void) {}
+static inline void pm_wakeup_clear(bool reset) {}
 static inline void pm_system_irq_wakeup(unsigned int irq_number) {}
 
 static inline void lock_system_sleep(void) {}
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -72,6 +72,8 @@ static void freeze_begin(void)
 
 static void freeze_enter(void)
 {
+	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, true);
+
 	spin_lock_irq(&suspend_freeze_lock);
 	if (pm_wakeup_pending())
 		goto out;
@@ -84,11 +86,9 @@ static void freeze_enter(void)
 
 	/* Push all the CPUs into the idle loop. */
 	wake_up_all_idle_cpus();
-	pr_debug("PM: suspend-to-idle\n");
 	/* Make the current CPU wait so it can enter the idle loop too. */
 	wait_event(suspend_freeze_wait_head,
 		   suspend_freeze_state == FREEZE_STATE_WAKE);
-	pr_debug("PM: resume from suspend-to-idle\n");
 
 	cpuidle_pause();
 	put_online_cpus();
@@ -98,6 +98,31 @@ static void freeze_enter(void)
  out:
 	suspend_freeze_state = FREEZE_STATE_NONE;
 	spin_unlock_irq(&suspend_freeze_lock);
+
+	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, false);
+}
+
+static void s2idle_loop(void)
+{
+	pr_debug("PM: suspend-to-idle\n");
+
+	do {
+		freeze_enter();
+
+		if (freeze_ops && freeze_ops->wake)
+			freeze_ops->wake();
+
+		dpm_resume_noirq(PMSG_RESUME);
+		if (freeze_ops && freeze_ops->sync)
+			freeze_ops->sync();
+
+		if (pm_wakeup_pending())
+			break;
+
+		pm_wakeup_clear(false);
+	} while (!dpm_suspend_noirq(PMSG_SUSPEND));
+
+	pr_debug("PM: resume from suspend-to-idle\n");
 }
 
 void freeze_wake(void)
@@ -371,10 +396,8 @@ static int suspend_enter(suspend_state_t
 	 * all the devices are suspended.
 	 */
 	if (state == PM_SUSPEND_FREEZE) {
-		trace_suspend_resume(TPS("machine_suspend"), state, true);
-		freeze_enter();
-		trace_suspend_resume(TPS("machine_suspend"), state, false);
-		goto Platform_wake;
+		s2idle_loop();
+		goto Platform_early_resume;
 	}
 
 	error = disable_nonboot_cpus();
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -1095,11 +1095,6 @@ static int __device_suspend_noirq(struct
 	if (async_error)
 		goto Complete;
 
-	if (pm_wakeup_pending()) {
-		async_error = -EBUSY;
-		goto Complete;
-	}
-
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -28,8 +28,8 @@ bool events_check_enabled __read_mostly;
 /* First wakeup IRQ seen by the kernel in the last cycle. */
 unsigned int pm_wakeup_irq __read_mostly;
 
-/* If set and the system is suspending, terminate the suspend. */
-static bool pm_abort_suspend __read_mostly;
+/* If greater than 0 and the system is suspending, terminate the suspend. */
+static atomic_t pm_abort_suspend __read_mostly;
 
 /*
  * Combined counters of registered wakeup events and wakeup events in progress.
@@ -855,20 +855,26 @@ bool pm_wakeup_pending(void)
 		pm_print_active_wakeup_sources();
 	}
 
-	return ret || pm_abort_suspend;
+	return ret || atomic_read(&pm_abort_suspend) > 0;
 }
 
 void pm_system_wakeup(void)
 {
-	pm_abort_suspend = true;
+	atomic_inc(&pm_abort_suspend);
 	freeze_wake();
 }
 EXPORT_SYMBOL_GPL(pm_system_wakeup);
 
-void pm_wakeup_clear(void)
+void pm_system_cancel_wakeup(void)
+{
+	atomic_dec(&pm_abort_suspend);
+}
+
+void pm_wakeup_clear(bool reset)
 {
-	pm_abort_suspend = false;
 	pm_wakeup_irq = 0;
+	if (reset)
+		atomic_set(&pm_abort_suspend, 0);
 }
 
 void pm_system_irq_wakeup(unsigned int irq_number)
Index: linux-pm/kernel/power/process.c
===================================================================
--- linux-pm.orig/kernel/power/process.c
+++ linux-pm/kernel/power/process.c
@@ -132,7 +132,7 @@ int freeze_processes(void)
 	if (!pm_freezing)
 		atomic_inc(&system_freezing_cnt);
 
-	pm_wakeup_clear();
+	pm_wakeup_clear(true);
 	pr_info("Freezing user space processes ... ");
 	pm_freezing = true;
 	error = try_to_freeze_tasks(true);
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -650,6 +650,8 @@ static const struct platform_suspend_ops
 	.recover = acpi_pm_finish,
 };
 
+static bool s2idle_wakeup;
+
 static int acpi_freeze_begin(void)
 {
 	acpi_scan_lock_acquire();
@@ -666,6 +668,33 @@ static int acpi_freeze_prepare(void)
 	return 0;
 }
 
+static void acpi_freeze_wake(void)
+{
+	/*
+	 * If IRQD_WAKEUP_ARMED is not set for the SCI at this point, it means
+	 * that the SCI has triggered while suspended, so cancel the wakeup in
+	 * case it has not been a wakeup event (the GPEs will be checked later).
+	 */
+	if (acpi_sci_irq_valid() &&
+	    !irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) {
+		pm_system_cancel_wakeup();
+		s2idle_wakeup = true;
+	}
+}
+
+static void acpi_freeze_sync(void)
+{
+	/*
+	 * Process all pending events in case there are any wakeup ones.
+	 *
+	 * The EC driver uses the system workqueue, so that one needs to be
+	 * flushed too.
+	 */
+	acpi_os_wait_events_complete();
+	flush_scheduled_work();
+	s2idle_wakeup = false;
+}
+
 static void acpi_freeze_restore(void)
 {
 	if (acpi_sci_irq_valid())
@@ -682,6 +711,8 @@ static void acpi_freeze_end(void)
 static const struct platform_freeze_ops acpi_freeze_ops = {
 	.begin = acpi_freeze_begin,
 	.prepare = acpi_freeze_prepare,
+	.wake = acpi_freeze_wake,
+	.sync = acpi_freeze_sync,
 	.restore = acpi_freeze_restore,
 	.end = acpi_freeze_end,
 };
@@ -700,9 +731,15 @@ static void acpi_sleep_suspend_setup(voi
 }
 
 #else /* !CONFIG_SUSPEND */
+#define s2idle_wakeup	(false)
 static inline void acpi_sleep_suspend_setup(void) {}
 #endif /* !CONFIG_SUSPEND */
 
+bool acpi_s2idle_wakeup(void)
+{
+	return s2idle_wakeup;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static u32 saved_bm_rld;
 
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -24,6 +24,7 @@
 #include <linux/pm_qos.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/suspend.h>
 
 #include "internal.h"
 
@@ -385,6 +386,12 @@ EXPORT_SYMBOL(acpi_bus_power_manageable)
 #ifdef CONFIG_PM
 static DEFINE_MUTEX(acpi_pm_notifier_lock);
 
+void acpi_wakeup_event(struct device *dev)
+{
+	pm_wakeup_dev_event(dev, 0, acpi_s2idle_wakeup());
+}
+EXPORT_SYMBOL_GPL(acpi_wakeup_event);
+
 static void acpi_pm_notify_handler(acpi_handle handle, u32 val, void *not_used)
 {
 	struct acpi_device *adev;
@@ -399,7 +406,7 @@ static void acpi_pm_notify_handler(acpi_
 	mutex_lock(&acpi_pm_notifier_lock);
 
 	if (adev->wakeup.flags.notifier_present) {
-		__pm_wakeup_event(adev->wakeup.ws, 0);
+		pm_wakeup_ws_event(adev->wakeup.ws, 0, acpi_s2idle_wakeup());
 		if (adev->wakeup.context.func)
 			adev->wakeup.context.func(&adev->wakeup.context);
 	}
Index: linux-pm/drivers/acpi/button.c
===================================================================
--- linux-pm.orig/drivers/acpi/button.c
+++ linux-pm/drivers/acpi/button.c
@@ -217,7 +217,7 @@ static int acpi_lid_notify_state(struct
 	}
 
 	if (state)
-		pm_wakeup_event(&device->dev, 0);
+		acpi_wakeup_event(&device->dev);
 
 	ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device);
 	if (ret == NOTIFY_DONE)
@@ -402,7 +402,7 @@ static void acpi_button_notify(struct ac
 		} else {
 			int keycode;
 
-			pm_wakeup_event(&device->dev, 0);
+			acpi_wakeup_event(&device->dev);
 			if (button->suspended)
 				break;
 
@@ -534,6 +534,7 @@ static int acpi_button_add(struct acpi_d
 		lid_device = device;
 	}
 
+	device_init_wakeup(&device->dev, true);
 	printk(KERN_INFO PREFIX "%s [%s]\n", name, acpi_device_bid(device));
 	return 0;
 
Index: linux-pm/drivers/acpi/battery.c
===================================================================
--- linux-pm.orig/drivers/acpi/battery.c
+++ linux-pm/drivers/acpi/battery.c
@@ -782,7 +782,7 @@ static int acpi_battery_update(struct ac
 	if ((battery->state & ACPI_BATTERY_STATE_CRITICAL) ||
 	    (test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags) &&
             (battery->capacity_now <= battery->alarm)))
-		pm_wakeup_event(&battery->device->dev, 0);
+		acpi_wakeup_event(&battery->device->dev);
 
 	return result;
 }
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -198,8 +198,10 @@ void acpi_ec_remove_query_handler(struct
                                   Suspend/Resume
   -------------------------------------------------------------------------- */
 #ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT
+extern bool acpi_s2idle_wakeup(void);
 extern int acpi_sleep_init(void);
 #else
+static inline bool acpi_s2idle_wakeup(void) { return false; }
 static inline int acpi_sleep_init(void) { return -ENXIO; }
 #endif
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -598,12 +598,16 @@ static inline bool acpi_device_always_pr
 #endif
 
 #ifdef CONFIG_PM
+void acpi_wakeup_event(struct device *dev);
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
 			void (*func)(struct acpi_device_wakeup_context *context));
 acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
 int acpi_pm_device_sleep_state(struct device *, int *, int);
 int acpi_pm_device_run_wake(struct device *, bool);
 #else
+void acpi_wakeup_event(struct device *dev)
+{
+}
 static inline acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
 					       struct device *dev,
 				               void (*work_func)(struct work_struct *work))

Re: [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Hans de Goede]

Hi,

On 08-06-17 02:00, Rafael J. Wysocki wrote:
> Hi All,
> 
> This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> will be reverted shortly, because it triggered issues on quite a few systems.
> 
> The last patch in the series is a counterpart of the above commit with a few
> modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> printed to kernel logs to make them somewhat less confusing.
> 
> The previous patches are pre-requisite changes plus some cleanups.  The major
> ones are [1-2/6] and [4/6] that are really needed for things to work as expected
> after [6/6].
> 
> In addition to that, this patch from Hans: https://patchwork.kernel.org/patch/9762815/
> is needed for USB wakeup on Bay Trail and Cherry Trail systems to work in general.

Small correction, that patch currently only influences Cherry Trail devices, it has:

static const struct x86_cpu_id int0002_cpu_ids[] = {
/*
  * Limit ourselves to Cherry Trail for now, until testing shows we
  * need to handle the INT0002 device on Baytrail too.
  *     ICPU(INTEL_FAM6_ATOM_SILVERMONT1),       * Valleyview, Bay Trail *
  */
        ICPU(INTEL_FAM6_ATOM_AIRMONT),          /* Braswell, Cherry Trail */
        {}
};

If anyone sees any issues where the SCI (typically irq 9) gets a nobody
cared message on Bay Trail try the linked patch with the Bay Trail line
above uncommented.

Regards,

Hans

Re: [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Dominik Brodowski]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

Rafael,

On Thu, Jun 08, 2017 at 02:00:40AM +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> will be reverted shortly, because it triggered issues on quite a few systems.
> 
> The last patch in the series is a counterpart of the above commit with a few
> modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> printed to kernel logs to make them somewhat less confusing.
> 
> The previous patches are pre-requisite changes plus some cleanups.  The major
> ones are [1-2/6] and [4/6] that are really needed for things to work as expected
> after [6/6].
> 
> In addition to that, this patch from Hans: https://patchwork.kernel.org/patch/9762815/
> is needed for USB wakeup on Bay Trail and Cherry Trail systems to work in general.

with eed4d47efe95 reverted and this patch series applied, suspend-to-mem
works as expected. Thanks!

	Dominik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/6] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Rafael J. Wysocki]

On Thursday, June 08, 2017 10:42:02 AM Dominik Brodowski wrote:
> Rafael,
> 
> On Thu, Jun 08, 2017 at 02:00:40AM +0200, Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> > spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> > will be reverted shortly, because it triggered issues on quite a few systems.
> > 
> > The last patch in the series is a counterpart of the above commit with a few
> > modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> > printed to kernel logs to make them somewhat less confusing.
> > 
> > The previous patches are pre-requisite changes plus some cleanups.  The major
> > ones are [1-2/6] and [4/6] that are really needed for things to work as expected
> > after [6/6].
> > 
> > In addition to that, this patch from Hans: https://patchwork.kernel.org/patch/9762815/
> > is needed for USB wakeup on Bay Trail and Cherry Trail systems to work in general.
> 
> with eed4d47efe95 reverted and this patch series applied, suspend-to-mem
> works as expected. Thanks!

Cool, thanks for the confirmation!

Cheers,
Rafael

Re: [PATCH 2/6] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Alan Stern]

On Thu, 8 Jun 2017, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> and may become problematic.
> 
> It is unnecessary, because the PCI bus type carries out post-suspend
> cleanup of all PCI devices during resume and that covers all things
> done by the pci_back_from_sleep().  There is no reason why USB cannot
> follow all of the other PCI devices in that respect.
> 
> It will become problematic after subsequent changes that make it
> possible to go back to sleep again after executing dpm_resume_noirq()
> if no valid system wakeup events have been detected at that point.
> Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> will cause the wakeup status of the devices in question to be cleared
> and if any of them has triggered system wakeup, that event may be
> missed then.
> 
> For the above reasons, drop the pci_back_from_sleep() invocation
> from hcd_pci_resume_noirq().
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/usb/core/hcd-pci.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> Index: linux-pm/drivers/usb/core/hcd-pci.c
> ===================================================================
> --- linux-pm.orig/drivers/usb/core/hcd-pci.c
> +++ linux-pm/drivers/usb/core/hcd-pci.c
> @@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
>  
>  static int hcd_pci_resume_noirq(struct device *dev)
>  {
> -	struct pci_dev		*pci_dev = to_pci_dev(dev);
> -
> -	powermac_set_asic(pci_dev, 1);
> -
> -	/* Go back to D0 and disable remote wakeup */
> -	pci_back_from_sleep(pci_dev);
> +	powermac_set_asic(to_pci_dev(dev), 1);
>  	return 0;
>  }

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Re: [PATCH 2/6] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Rafael J. Wysocki]

On Thursday, June 08, 2017 11:24:55 AM Alan Stern wrote:
> On Thu, 8 Jun 2017, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> > PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> > and may become problematic.
> > 
> > It is unnecessary, because the PCI bus type carries out post-suspend
> > cleanup of all PCI devices during resume and that covers all things
> > done by the pci_back_from_sleep().  There is no reason why USB cannot
> > follow all of the other PCI devices in that respect.
> > 
> > It will become problematic after subsequent changes that make it
> > possible to go back to sleep again after executing dpm_resume_noirq()
> > if no valid system wakeup events have been detected at that point.
> > Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> > will cause the wakeup status of the devices in question to be cleared
> > and if any of them has triggered system wakeup, that event may be
> > missed then.
> > 
> > For the above reasons, drop the pci_back_from_sleep() invocation
> > from hcd_pci_resume_noirq().
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/usb/core/hcd-pci.c |    7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > Index: linux-pm/drivers/usb/core/hcd-pci.c
> > ===================================================================
> > --- linux-pm.orig/drivers/usb/core/hcd-pci.c
> > +++ linux-pm/drivers/usb/core/hcd-pci.c
> > @@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
> >  
> >  static int hcd_pci_resume_noirq(struct device *dev)
> >  {
> > -	struct pci_dev		*pci_dev = to_pci_dev(dev);
> > -
> > -	powermac_set_asic(pci_dev, 1);
> > -
> > -	/* Go back to D0 and disable remote wakeup */
> > -	pci_back_from_sleep(pci_dev);
> > +	powermac_set_asic(to_pci_dev(dev), 1);
> >  	return 0;
> >  }
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Thanks!

[PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Rafael J. Wysocki]

Hi All,

On Thursday, June 08, 2017 02:00:40 AM Rafael J. Wysocki wrote:
> Hi All,
> 
> This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> will be reverted shortly, because it triggered issues on quite a few systems.
> 
> The last patch in the series is a counterpart of the above commit with a few
> modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> printed to kernel logs to make them somewhat less confusing.

Here's a v2 which is very similar to the previous one, except for 3 things:
- There are few build fixes in the last patch.
- The patch from Hans mentioned previously is included now (as [7/8]).
- There is an additional PCI change related to config space saving/restoring
  and PME.

If anyone has any objections or concerns regarding this, please speak up now,
as I'm going to put it into linux-next shortly to allow it to receive some more
testing than commit eed4d47efe95 had had before it went it.

Thanks,
Rafael

[PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The work functions provided by the users of acpi_add_pm_notifier()
should be run synchronously before re-enabling the wakeup GPE in
case they are used to clear the status and/or disable the wakeup
signaling at the source.  Otherwise, which is the case currently in
the PCI bus type code, the same wakeup event may be signaled for
multiple times while the execution of the work function in response
to it has already been queued up.

Fortunately, acpi_add_pm_notifier() is only used by PCI and by
ACPI device PM code internally, so the change is relatively
straightforward to make.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   27 +++++++++++----------------
 drivers/pci/pci-acpi.c   |   17 +++++++----------
 include/acpi/acpi_bus.h  |    4 ++--
 3 files changed, 20 insertions(+), 28 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -400,8 +400,8 @@ static void acpi_pm_notify_handler(acpi_
 
 	if (adev->wakeup.flags.notifier_present) {
 		__pm_wakeup_event(adev->wakeup.ws, 0);
-		if (adev->wakeup.context.work.func)
-			queue_pm_work(&adev->wakeup.context.work);
+		if (adev->wakeup.context.func)
+			adev->wakeup.context.func(&adev->wakeup.context);
 	}
 
 	mutex_unlock(&acpi_pm_notifier_lock);
@@ -413,7 +413,7 @@ static void acpi_pm_notify_handler(acpi_
  * acpi_add_pm_notifier - Register PM notify handler for given ACPI device.
  * @adev: ACPI device to add the notify handler for.
  * @dev: Device to generate a wakeup event for while handling the notification.
- * @work_func: Work function to execute when handling the notification.
+ * @func: Work function to execute when handling the notification.
  *
  * NOTE: @adev need not be a run-wake or wakeup device to be a valid source of
  * PM wakeup events.  For example, wakeup events may be generated for bridges
@@ -421,11 +421,11 @@ static void acpi_pm_notify_handler(acpi_
  * bridge itself doesn't have a wakeup GPE associated with it.
  */
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
-				 void (*work_func)(struct work_struct *work))
+			void (*func)(struct acpi_device_wakeup_context *context))
 {
 	acpi_status status = AE_ALREADY_EXISTS;
 
-	if (!dev && !work_func)
+	if (!dev && !func)
 		return AE_BAD_PARAMETER;
 
 	mutex_lock(&acpi_pm_notifier_lock);
@@ -435,8 +435,7 @@ acpi_status acpi_add_pm_notifier(struct
 
 	adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
 	adev->wakeup.context.dev = dev;
-	if (work_func)
-		INIT_WORK(&adev->wakeup.context.work, work_func);
+	adev->wakeup.context.func = func;
 
 	status = acpi_install_notify_handler(adev->handle, ACPI_SYSTEM_NOTIFY,
 					     acpi_pm_notify_handler, NULL);
@@ -469,10 +468,7 @@ acpi_status acpi_remove_pm_notifier(stru
 	if (ACPI_FAILURE(status))
 		goto out;
 
-	if (adev->wakeup.context.work.func) {
-		cancel_work_sync(&adev->wakeup.context.work);
-		adev->wakeup.context.work.func = NULL;
-	}
+	adev->wakeup.context.func = NULL;
 	adev->wakeup.context.dev = NULL;
 	wakeup_source_unregister(adev->wakeup.ws);
 
@@ -658,16 +654,15 @@ EXPORT_SYMBOL(acpi_pm_device_sleep_state
 
 /**
  * acpi_pm_notify_work_func - ACPI devices wakeup notification work function.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void acpi_pm_notify_work_func(struct work_struct *work)
+static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
 {
-	struct device *dev;
+	struct device *dev = context->dev;
 
-	dev = container_of(work, struct acpi_device_wakeup_context, work)->dev;
 	if (dev) {
 		pm_wakeup_event(dev, 0);
-		pm_runtime_resume(dev);
+		pm_request_resume(dev);
 	}
 }
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -319,7 +319,7 @@ struct acpi_device_wakeup_flags {
 };
 
 struct acpi_device_wakeup_context {
-	struct work_struct work;
+	void (*func)(struct acpi_device_wakeup_context *context);
 	struct device *dev;
 };
 
@@ -599,7 +599,7 @@ static inline bool acpi_device_always_pr
 
 #ifdef CONFIG_PM
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
-				 void (*work_func)(struct work_struct *work));
+			void (*func)(struct acpi_device_wakeup_context *context));
 acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
 int acpi_pm_device_sleep_state(struct device *, int *, int);
 int acpi_pm_device_run_wake(struct device *, bool);
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -395,29 +395,26 @@ bool pciehp_is_native(struct pci_dev *pd
 
 /**
  * pci_acpi_wake_bus - Root bus wakeup notification fork function.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void pci_acpi_wake_bus(struct work_struct *work)
+static void pci_acpi_wake_bus(struct acpi_device_wakeup_context *context)
 {
 	struct acpi_device *adev;
 	struct acpi_pci_root *root;
 
-	adev = container_of(work, struct acpi_device, wakeup.context.work);
+	adev = container_of(context, struct acpi_device, wakeup.context);
 	root = acpi_driver_data(adev);
 	pci_pme_wakeup_bus(root->bus);
 }
 
 /**
  * pci_acpi_wake_dev - PCI device wakeup notification work function.
- * @handle: ACPI handle of a device the notification is for.
- * @work: Work item to handle.
+ * @context: Device wakeup context.
  */
-static void pci_acpi_wake_dev(struct work_struct *work)
+static void pci_acpi_wake_dev(struct acpi_device_wakeup_context *context)
 {
-	struct acpi_device_wakeup_context *context;
 	struct pci_dev *pci_dev;
 
-	context = container_of(work, struct acpi_device_wakeup_context, work);
 	pci_dev = to_pci_dev(context->dev);
 
 	if (pci_dev->pme_poll)
@@ -425,7 +422,7 @@ static void pci_acpi_wake_dev(struct wor
 
 	if (pci_dev->current_state == PCI_D3cold) {
 		pci_wakeup_event(pci_dev);
-		pm_runtime_resume(&pci_dev->dev);
+		pm_request_resume(&pci_dev->dev);
 		return;
 	}
 
@@ -434,7 +431,7 @@ static void pci_acpi_wake_dev(struct wor
 		pci_check_pme_status(pci_dev);
 
 	pci_wakeup_event(pci_dev);
-	pm_runtime_resume(&pci_dev->dev);
+	pm_request_resume(&pci_dev->dev);
 
 	pci_pme_wakeup_bus(pci_dev->subordinate);
 }

[PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
PCI USB controllers calls pci_back_from_sleep() which is unnecessary
and may become problematic.

It is unnecessary, because the PCI bus type carries out post-suspend
cleanup of all PCI devices during resume and that covers all things
done by the pci_back_from_sleep().  There is no reason why USB cannot
follow all of the other PCI devices in that respect.

It will become problematic after subsequent changes that make it
possible to go back to sleep again after executing dpm_resume_noirq()
if no valid system wakeup events have been detected at that point.
Namely, calling pci_back_from_sleep() at the _resume_noirq stage
will cause the wakeup status of the devices in question to be cleared
and if any of them has triggered system wakeup, that event may be
missed then.

For the above reasons, drop the pci_back_from_sleep() invocation
from hcd_pci_resume_noirq().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/core/hcd-pci.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Index: linux-pm/drivers/usb/core/hcd-pci.c
===================================================================
--- linux-pm.orig/drivers/usb/core/hcd-pci.c
+++ linux-pm/drivers/usb/core/hcd-pci.c
@@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
 
 static int hcd_pci_resume_noirq(struct device *dev)
 {
-	struct pci_dev		*pci_dev = to_pci_dev(dev);
-
-	powermac_set_asic(pci_dev, 1);
-
-	/* Go back to D0 and disable remote wakeup */
-	pci_back_from_sleep(pci_dev);
+	powermac_set_asic(to_pci_dev(dev), 1);
 	return 0;
 }
 

[PATCH v2 3/8] ACPI / PM: Change log level of wakeup-related message

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Change the log level of the "System wakeup enabled/disabled by ACPI"
message in acpi_pm_device_sleep_wake() to "debug" to reduce to log
noise level.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -756,8 +756,8 @@ int acpi_pm_device_sleep_wake(struct dev
 
 	error = acpi_device_wakeup(adev, acpi_target_system_state(), enable);
 	if (!error)
-		dev_info(dev, "System wakeup %s by ACPI\n",
-				enable ? "enabled" : "disabled");
+		dev_dbg(dev, "System wakeup %s by ACPI\n",
+			enable ? "enabled" : "disabled");
 
 	return error;
 }

[PATCH v2 4/8] ACPI / PM: Clean up device wakeup enable/disable code

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The wakeup.flags.enabled flag in struct acpi_device is not used
consistently, as there is no reason why it should only apply
to the enabling/disabling of the wakeup GPE, so put the invocation
of acpi_enable_wakeup_device_power() under it too.

Moreover, it is not necessary to call
acpi_enable_wakeup_devices() and acpi_disable_wakeup_devices() for
suspend-to-idle, so don't do that.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/device_pm.c |   19 ++++++++-----------
 drivers/acpi/sleep.c     |    4 ++--
 2 files changed, 10 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -688,26 +688,23 @@ static int acpi_device_wakeup(struct acp
 		acpi_status res;
 		int error;
 
+		if (adev->wakeup.flags.enabled)
+			return 0;
+
 		error = acpi_enable_wakeup_device_power(adev, target_state);
 		if (error)
 			return error;
 
-		if (adev->wakeup.flags.enabled)
-			return 0;
-
 		res = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
-		if (ACPI_SUCCESS(res)) {
-			adev->wakeup.flags.enabled = 1;
-		} else {
+		if (ACPI_FAILURE(res)) {
 			acpi_disable_wakeup_device_power(adev);
 			return -EIO;
 		}
-	} else {
-		if (adev->wakeup.flags.enabled) {
-			acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
-			adev->wakeup.flags.enabled = 0;
-		}
+		adev->wakeup.flags.enabled = 1;
+	} else if (adev->wakeup.flags.enabled) {
+		acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
 		acpi_disable_wakeup_device_power(adev);
+		adev->wakeup.flags.enabled = 0;
 	}
 	return 0;
 }
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -658,19 +658,19 @@ static int acpi_freeze_begin(void)
 
 static int acpi_freeze_prepare(void)
 {
-	acpi_enable_wakeup_devices(ACPI_STATE_S0);
 	acpi_enable_all_wakeup_gpes();
 	acpi_os_wait_events_complete();
 	if (acpi_sci_irq_valid())
 		enable_irq_wake(acpi_sci_irq);
+
 	return 0;
 }
 
 static void acpi_freeze_restore(void)
 {
-	acpi_disable_wakeup_devices(ACPI_STATE_S0);
 	if (acpi_sci_irq_valid())
 		disable_irq_wake(acpi_sci_irq);
+
 	acpi_enable_all_runtime_gpes();
 }
 

[PATCH v2 5/8] PM / sleep: Print timing information if debug is enabled

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Avoid printing the device suspend/resume timing information if
CONFIG_PM_DEBUG is not set to reduce the log noise level.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -417,6 +417,7 @@ static void pm_dev_err(struct device *de
 		dev_name(dev), pm_verb(state.event), info, error);
 }
 
+#ifdef CONFIG_PM_DEBUG
 static void dpm_show_time(ktime_t starttime, pm_message_t state, char *info)
 {
 	ktime_t calltime;
@@ -433,6 +434,9 @@ static void dpm_show_time(ktime_t startt
 		info ?: "", info ? " " : "", pm_verb(state.event),
 		usecs / USEC_PER_MSEC, usecs % USEC_PER_MSEC);
 }
+#else
+static inline void dpm_show_time(ktime_t starttime, pm_message_t state, char *info) {}
+#endif /* CONFIG_PM_DEBUG */
 
 static int dpm_run_callback(pm_callback_t cb, struct device *dev,
 			    pm_message_t state, char *info)

[PATCH v2 6/8] PCI / PM: Restore PME Enable if skipping wakeup setup

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The wakeup_prepared PCI device flag is used for preventing subsequent
changes of PCI device wakeup settings in the same way (e.g. enabling
device wakeup twice in a row).

However, in some cases PME Enable may be updated by things like PCI
configuration space restoration in the meantime and it may need to be
set again even though the rest of the settings need not change, so
modify __pci_enable_wake() to do that when it is about to return
early.

Also, it is reasonable to expect that __pci_enable_wake() will always
clear PME Status when invoked to disable device wakeup, so make it do
so even if it is going to return early then.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci.c |   26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -1805,6 +1805,23 @@ static void __pci_pme_active(struct pci_
 	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 }
 
+static void pci_pme_restore(struct pci_dev *dev)
+{
+	u16 pmcsr;
+
+	if (!dev->pme_support)
+		return;
+
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
+	if (dev->wakeup_prepared) {
+		pmcsr |= PCI_PM_CTRL_PME_ENABLE;
+	} else {
+		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
+		pmcsr |= PCI_PM_CTRL_PME_STATUS;
+	}
+	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
+}
+
 /**
  * pci_pme_active - enable or disable PCI device's PME# function
  * @dev: PCI device to handle.
@@ -1899,9 +1916,14 @@ int __pci_enable_wake(struct pci_dev *de
 	if (enable && !runtime && !device_may_wakeup(&dev->dev))
 		return -EINVAL;
 
-	/* Don't do the same thing twice in a row for one device. */
-	if (!!enable == !!dev->wakeup_prepared)
+	/*
+	 * Don't do the same thing twice in a row for one device, but restore
+	 * PME Enable in case it has been updated by config space restoration.
+	 */
+	if (!!enable == !!dev->wakeup_prepared) {
+		pci_pme_restore(dev);
 		return 0;
+	}
 
 	/*
 	 * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don

[PATCH v2 7/8] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Rafael J. Wysocki]

From: Hans de Goede <hdegoede@redhat.com>

Some peripherals on Bay Trail and Cherry Trail platforms signal a
Power Management Event (PME) to the Power Management Controller (PMC)
to wakeup the system. When this happens software needs to explicitly
clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
IRQ storm on IRQ 9.

This is modelled in ACPI through the INT0002 ACPI device, which is
called a "Virtual GPIO controller" in ACPI because it defines the
event handler to call when the PME triggers through _AEI and _L02
methods as would be done for a real GPIO interrupt in ACPI.

This commit adds a driver which registers the Virtual GPIOs expected
by the DSDT on these devices, letting gpiolib-acpi claim the
virtual GPIO and install a GPIO-interrupt handler which call the _L02
handler as it would for a real GPIO controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/Kconfig               |  19 +++
 drivers/platform/x86/Makefile              |   1 +
 drivers/platform/x86/intel_int0002_vgpio.c | 219 +++++++++++++++++++++++++++++
 3 files changed, 239 insertions(+)
 create mode 100644 drivers/platform/x86/intel_int0002_vgpio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 8489020ecf44..a3ccc3c795a5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -794,6 +794,25 @@ config INTEL_CHT_INT33FE
 	  This driver instantiates i2c-clients for these, so that standard
 	  i2c drivers for these chips can bind to the them.
 
+config INTEL_INT0002_VGPIO
+	tristate "Intel ACPI INT0002 Virtual GPIO driver"
+	depends on GPIOLIB && ACPI
+	select GPIOLIB_IRQCHIP
+	---help---
+	  Some peripherals on Bay Trail and Cherry Trail platforms signal a
+	  Power Management Event (PME) to the Power Management Controller (PMC)
+	  to wakeup the system. When this happens software needs to explicitly
+	  clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
+	  IRQ storm on IRQ 9.
+
+	  This is modelled in ACPI through the INT0002 ACPI device, which is
+	  called a "Virtual GPIO controller" in ACPI because it defines the
+	  event handler to call when the PME triggers through _AEI and _L02
+	  methods as would be done for a real GPIO interrupt in ACPI.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called intel_int0002_vgpio.
+
 config INTEL_HID_EVENT
 	tristate "INTEL HID Event"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 182a3ed6605a..ab22ce77fb66 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
 obj-$(CONFIG_TOSHIBA_HAPS)	+= toshiba_haps.o
 obj-$(CONFIG_TOSHIBA_WMI)	+= toshiba-wmi.o
 obj-$(CONFIG_INTEL_CHT_INT33FE)	+= intel_cht_int33fe.o
+obj-$(CONFIG_INTEL_INT0002_VGPIO) += intel_int0002_vgpio.o
 obj-$(CONFIG_INTEL_HID_EVENT)	+= intel-hid.o
 obj-$(CONFIG_INTEL_VBTN)	+= intel-vbtn.o
 obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c
new file mode 100644
index 000000000000..92dc230ef5b2
--- /dev/null
+++ b/drivers/platform/x86/intel_int0002_vgpio.c
@@ -0,0 +1,219 @@
+/*
+ * Intel INT0002 "Virtual GPIO" driver
+ *
+ * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * Author: Dyut Kumar Sil <dyut.k.sil@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Some peripherals on Bay Trail and Cherry Trail platforms signal a Power
+ * Management Event (PME) to the Power Management Controller (PMC) to wakeup
+ * the system. When this happens software needs to clear the PME bus 0 status
+ * bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
+ *
+ * This is modelled in ACPI through the INT0002 ACPI device, which is
+ * called a "Virtual GPIO controller" in ACPI because it defines the event
+ * handler to call when the PME triggers through _AEI and _L02 / _E02
+ * methods as would be done for a real GPIO interrupt in ACPI. Note this
+ * is a hack to define an AML event handler for the PME while using existing
+ * ACPI mechanisms, this is not a real GPIO at all.
+ *
+ * This driver will bind to the INT0002 device, and register as a GPIO
+ * controller, letting gpiolib-acpi.c call the _L02 handler as it would
+ * for a real GPIO controller.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bitmap.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
+#define DRV_NAME			"INT0002 Virtual GPIO"
+
+/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */
+#define GPE0A_PME_B0_VIRT_GPIO_PIN	2
+
+#define GPE0A_PME_B0_STS_BIT		BIT(13)
+#define GPE0A_PME_B0_EN_BIT		BIT(13)
+#define GPE0A_STS_PORT			0x420
+#define GPE0A_EN_PORT			0x428
+
+#define ICPU(model)	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id int0002_cpu_ids[] = {
+/*
+ * Limit ourselves to Cherry Trail for now, until testing shows we
+ * need to handle the INT0002 device on Baytrail too.
+ *	ICPU(INTEL_FAM6_ATOM_SILVERMONT1),	 * Valleyview, Bay Trail *
+ */
+	ICPU(INTEL_FAM6_ATOM_AIRMONT),		/* Braswell, Cherry Trail */
+	{}
+};
+
+/*
+ * As this is not a real GPIO at all, but just a hack to model an event in
+ * ACPI the get / set functions are dummy functions.
+ */
+
+static int int0002_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	return 0;
+}
+
+static void int0002_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+}
+
+static int int0002_gpio_direction_output(struct gpio_chip *chip,
+					 unsigned int offset, int value)
+{
+	return 0;
+}
+
+static void int0002_irq_ack(struct irq_data *data)
+{
+	outl(GPE0A_PME_B0_STS_BIT, GPE0A_STS_PORT);
+}
+
+static void int0002_irq_unmask(struct irq_data *data)
+{
+	u32 gpe_en_reg;
+
+	gpe_en_reg = inl(GPE0A_EN_PORT);
+	gpe_en_reg |= GPE0A_PME_B0_EN_BIT;
+	outl(gpe_en_reg, GPE0A_EN_PORT);
+}
+
+static void int0002_irq_mask(struct irq_data *data)
+{
+	u32 gpe_en_reg;
+
+	gpe_en_reg = inl(GPE0A_EN_PORT);
+	gpe_en_reg &= ~GPE0A_PME_B0_EN_BIT;
+	outl(gpe_en_reg, GPE0A_EN_PORT);
+}
+
+static irqreturn_t int0002_irq(int irq, void *data)
+{
+	struct gpio_chip *chip = data;
+	u32 gpe_sts_reg;
+
+	gpe_sts_reg = inl(GPE0A_STS_PORT);
+	if (!(gpe_sts_reg & GPE0A_PME_B0_STS_BIT))
+		return IRQ_NONE;
+
+	generic_handle_irq(irq_find_mapping(chip->irqdomain,
+					    GPE0A_PME_B0_VIRT_GPIO_PIN));
+
+	pm_system_wakeup();
+
+	return IRQ_HANDLED;
+}
+
+static struct irq_chip int0002_irqchip = {
+	.name			= DRV_NAME,
+	.irq_ack		= int0002_irq_ack,
+	.irq_mask		= int0002_irq_mask,
+	.irq_unmask		= int0002_irq_unmask,
+};
+
+static int int0002_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct x86_cpu_id *cpu_id;
+	struct gpio_chip *chip;
+	int irq, ret;
+
+	/* Menlow has a different INT0002 device? <sigh> */
+	cpu_id = x86_match_cpu(int0002_cpu_ids);
+	if (!cpu_id)
+		return -ENODEV;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Error getting IRQ: %d\n", irq);
+		return irq;
+	}
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->label = DRV_NAME;
+	chip->parent = dev;
+	chip->owner = THIS_MODULE;
+	chip->get = int0002_gpio_get;
+	chip->set = int0002_gpio_set;
+	chip->direction_input = int0002_gpio_get;
+	chip->direction_output = int0002_gpio_direction_output;
+	chip->base = -1;
+	chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1;
+	chip->irq_need_valid_mask = true;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL);
+	if (ret) {
+		dev_err(dev, "Error adding gpio chip: %d\n", ret);
+		return ret;
+	}
+
+	bitmap_clear(chip->irq_valid_mask, 0, GPE0A_PME_B0_VIRT_GPIO_PIN);
+
+	/*
+	 * We manually request the irq here instead of passing a flow-handler
+	 * to gpiochip_set_chained_irqchip, because the irq is shared.
+	 */
+	ret = devm_request_irq(dev, irq, int0002_irq,
+			       IRQF_SHARED | IRQF_NO_THREAD, "INT0002", chip);
+	if (ret) {
+		dev_err(dev, "Error requesting IRQ %d: %d\n", irq, ret);
+		return ret;
+	}
+
+	ret = gpiochip_irqchip_add(chip, &int0002_irqchip, 0, handle_edge_irq,
+				   IRQ_TYPE_NONE);
+	if (ret) {
+		dev_err(dev, "Error adding irqchip: %d\n", ret);
+		return ret;
+	}
+
+	gpiochip_set_chained_irqchip(chip, &int0002_irqchip, irq, NULL);
+
+	return 0;
+}
+
+static const struct acpi_device_id int0002_acpi_ids[] = {
+	{ "INT0002", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, int0002_acpi_ids);
+
+static struct platform_driver int0002_driver = {
+	.driver = {
+		.name			= DRV_NAME,
+		.acpi_match_table	= int0002_acpi_ids,
+	},
+	.probe	= int0002_probe,
+};
+
+module_platform_driver(int0002_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Intel INT0002 Virtual GPIO driver");
+MODULE_LICENSE("GPL");

[PATCH v2 8/8] ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The ACPI SCI (System Control Interrupt) is set up as a wakeup IRQ
during suspend-to-idle transitions and, consequently, any events
signaled through it wake up the system from that state.  However,
on some systems some of the events signaled via the ACPI SCI while
suspended to idle should not cause the system to wake up.  In fact,
quite often they should just be discarded.

Arguably, systems should not resume entirely on such events, but in
order to decide which events really should cause the system to resume
and which are spurious, it is necessary to resume up to the point
when ACPI SCIs are actually handled and processed, which is after
executing dpm_resume_noirq() in the system resume path.

For this reasons, add a loop around freeze_enter() in which the
platforms can process events signaled via multiplexed IRQ lines
like the ACPI SCI and add suspend-to-idle hooks that can be
used for this purpose to struct platform_freeze_ops.

In the ACPI case, the ->wake hook is used for checking if the SCI
has triggered while suspended and deferring the interrupt-induced
system wakeup until the events signaled through it are actually
processed sufficiently to decide whether or not the system should
resume.  In turn, the ->sync hook allows all of the relevant event
queues to be flushed so as to prevent events from being missed due
to race conditions.

In addition to that, some ACPI code processing wakeup events needs
to be modified to use the "hard" version of wakeup triggers, so that
it will cause a system resume to happen on device-induced wakeup
events even if the "soft" mechanism to prevent the system from
suspending is not enabled.  However, to preserve the existing
behavior with respect to suspend-to-RAM, this only is done in
the suspend-to-idle case and only if an SCI has occurred while
suspended.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/battery.c      |    2 +-
 drivers/acpi/button.c       |    5 +++--
 drivers/acpi/device_pm.c    |    9 ++++++++-
 drivers/acpi/internal.h     |    2 ++
 drivers/acpi/sleep.c        |   37 +++++++++++++++++++++++++++++++++++++
 drivers/base/power/main.c   |    5 -----
 drivers/base/power/wakeup.c |   18 ++++++++++++------
 include/acpi/acpi_bus.h     |    6 +++++-
 include/linux/suspend.h     |    7 +++++--
 kernel/power/process.c      |    2 +-
 kernel/power/suspend.c      |   35 +++++++++++++++++++++++++++++------
 11 files changed, 103 insertions(+), 25 deletions(-)

Index: linux-pm/include/linux/suspend.h
===================================================================
--- linux-pm.orig/include/linux/suspend.h
+++ linux-pm/include/linux/suspend.h
@@ -189,6 +189,8 @@ struct platform_suspend_ops {
 struct platform_freeze_ops {
 	int (*begin)(void);
 	int (*prepare)(void);
+	void (*wake)(void);
+	void (*sync)(void);
 	void (*restore)(void);
 	void (*end)(void);
 };
@@ -428,7 +430,8 @@ extern unsigned int pm_wakeup_irq;
 
 extern bool pm_wakeup_pending(void);
 extern void pm_system_wakeup(void);
-extern void pm_wakeup_clear(void);
+extern void pm_system_cancel_wakeup(void);
+extern void pm_wakeup_clear(bool reset);
 extern void pm_system_irq_wakeup(unsigned int irq_number);
 extern bool pm_get_wakeup_count(unsigned int *count, bool block);
 extern bool pm_save_wakeup_count(unsigned int count);
@@ -478,7 +481,7 @@ static inline int unregister_pm_notifier
 
 static inline bool pm_wakeup_pending(void) { return false; }
 static inline void pm_system_wakeup(void) {}
-static inline void pm_wakeup_clear(void) {}
+static inline void pm_wakeup_clear(bool reset) {}
 static inline void pm_system_irq_wakeup(unsigned int irq_number) {}
 
 static inline void lock_system_sleep(void) {}
Index: linux-pm/kernel/power/suspend.c
===================================================================
--- linux-pm.orig/kernel/power/suspend.c
+++ linux-pm/kernel/power/suspend.c
@@ -72,6 +72,8 @@ static void freeze_begin(void)
 
 static void freeze_enter(void)
 {
+	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, true);
+
 	spin_lock_irq(&suspend_freeze_lock);
 	if (pm_wakeup_pending())
 		goto out;
@@ -84,11 +86,9 @@ static void freeze_enter(void)
 
 	/* Push all the CPUs into the idle loop. */
 	wake_up_all_idle_cpus();
-	pr_debug("PM: suspend-to-idle\n");
 	/* Make the current CPU wait so it can enter the idle loop too. */
 	wait_event(suspend_freeze_wait_head,
 		   suspend_freeze_state == FREEZE_STATE_WAKE);
-	pr_debug("PM: resume from suspend-to-idle\n");
 
 	cpuidle_pause();
 	put_online_cpus();
@@ -98,6 +98,31 @@ static void freeze_enter(void)
  out:
 	suspend_freeze_state = FREEZE_STATE_NONE;
 	spin_unlock_irq(&suspend_freeze_lock);
+
+	trace_suspend_resume(TPS("machine_suspend"), PM_SUSPEND_FREEZE, false);
+}
+
+static void s2idle_loop(void)
+{
+	pr_debug("PM: suspend-to-idle\n");
+
+	do {
+		freeze_enter();
+
+		if (freeze_ops && freeze_ops->wake)
+			freeze_ops->wake();
+
+		dpm_resume_noirq(PMSG_RESUME);
+		if (freeze_ops && freeze_ops->sync)
+			freeze_ops->sync();
+
+		if (pm_wakeup_pending())
+			break;
+
+		pm_wakeup_clear(false);
+	} while (!dpm_suspend_noirq(PMSG_SUSPEND));
+
+	pr_debug("PM: resume from suspend-to-idle\n");
 }
 
 void freeze_wake(void)
@@ -371,10 +396,8 @@ static int suspend_enter(suspend_state_t
 	 * all the devices are suspended.
 	 */
 	if (state == PM_SUSPEND_FREEZE) {
-		trace_suspend_resume(TPS("machine_suspend"), state, true);
-		freeze_enter();
-		trace_suspend_resume(TPS("machine_suspend"), state, false);
-		goto Platform_wake;
+		s2idle_loop();
+		goto Platform_early_resume;
 	}
 
 	error = disable_nonboot_cpus();
Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -1095,11 +1095,6 @@ static int __device_suspend_noirq(struct
 	if (async_error)
 		goto Complete;
 
-	if (pm_wakeup_pending()) {
-		async_error = -EBUSY;
-		goto Complete;
-	}
-
 	if (dev->power.syscore || dev->power.direct_complete)
 		goto Complete;
 
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -28,8 +28,8 @@ bool events_check_enabled __read_mostly;
 /* First wakeup IRQ seen by the kernel in the last cycle. */
 unsigned int pm_wakeup_irq __read_mostly;
 
-/* If set and the system is suspending, terminate the suspend. */
-static bool pm_abort_suspend __read_mostly;
+/* If greater than 0 and the system is suspending, terminate the suspend. */
+static atomic_t pm_abort_suspend __read_mostly;
 
 /*
  * Combined counters of registered wakeup events and wakeup events in progress.
@@ -855,20 +855,26 @@ bool pm_wakeup_pending(void)
 		pm_print_active_wakeup_sources();
 	}
 
-	return ret || pm_abort_suspend;
+	return ret || atomic_read(&pm_abort_suspend) > 0;
 }
 
 void pm_system_wakeup(void)
 {
-	pm_abort_suspend = true;
+	atomic_inc(&pm_abort_suspend);
 	freeze_wake();
 }
 EXPORT_SYMBOL_GPL(pm_system_wakeup);
 
-void pm_wakeup_clear(void)
+void pm_system_cancel_wakeup(void)
+{
+	atomic_dec(&pm_abort_suspend);
+}
+
+void pm_wakeup_clear(bool reset)
 {
-	pm_abort_suspend = false;
 	pm_wakeup_irq = 0;
+	if (reset)
+		atomic_set(&pm_abort_suspend, 0);
 }
 
 void pm_system_irq_wakeup(unsigned int irq_number)
Index: linux-pm/kernel/power/process.c
===================================================================
--- linux-pm.orig/kernel/power/process.c
+++ linux-pm/kernel/power/process.c
@@ -132,7 +132,7 @@ int freeze_processes(void)
 	if (!pm_freezing)
 		atomic_inc(&system_freezing_cnt);
 
-	pm_wakeup_clear();
+	pm_wakeup_clear(true);
 	pr_info("Freezing user space processes ... ");
 	pm_freezing = true;
 	error = try_to_freeze_tasks(true);
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -650,6 +650,8 @@ static const struct platform_suspend_ops
 	.recover = acpi_pm_finish,
 };
 
+static bool s2idle_wakeup;
+
 static int acpi_freeze_begin(void)
 {
 	acpi_scan_lock_acquire();
@@ -666,6 +668,33 @@ static int acpi_freeze_prepare(void)
 	return 0;
 }
 
+static void acpi_freeze_wake(void)
+{
+	/*
+	 * If IRQD_WAKEUP_ARMED is not set for the SCI at this point, it means
+	 * that the SCI has triggered while suspended, so cancel the wakeup in
+	 * case it has not been a wakeup event (the GPEs will be checked later).
+	 */
+	if (acpi_sci_irq_valid() &&
+	    !irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq))) {
+		pm_system_cancel_wakeup();
+		s2idle_wakeup = true;
+	}
+}
+
+static void acpi_freeze_sync(void)
+{
+	/*
+	 * Process all pending events in case there are any wakeup ones.
+	 *
+	 * The EC driver uses the system workqueue, so that one needs to be
+	 * flushed too.
+	 */
+	acpi_os_wait_events_complete();
+	flush_scheduled_work();
+	s2idle_wakeup = false;
+}
+
 static void acpi_freeze_restore(void)
 {
 	if (acpi_sci_irq_valid())
@@ -682,6 +711,8 @@ static void acpi_freeze_end(void)
 static const struct platform_freeze_ops acpi_freeze_ops = {
 	.begin = acpi_freeze_begin,
 	.prepare = acpi_freeze_prepare,
+	.wake = acpi_freeze_wake,
+	.sync = acpi_freeze_sync,
 	.restore = acpi_freeze_restore,
 	.end = acpi_freeze_end,
 };
@@ -700,9 +731,15 @@ static void acpi_sleep_suspend_setup(voi
 }
 
 #else /* !CONFIG_SUSPEND */
+#define s2idle_wakeup	(false)
 static inline void acpi_sleep_suspend_setup(void) {}
 #endif /* !CONFIG_SUSPEND */
 
+bool acpi_s2idle_wakeup(void)
+{
+	return s2idle_wakeup;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static u32 saved_bm_rld;
 
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -24,6 +24,7 @@
 #include <linux/pm_qos.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/suspend.h>
 
 #include "internal.h"
 
@@ -385,6 +386,12 @@ EXPORT_SYMBOL(acpi_bus_power_manageable)
 #ifdef CONFIG_PM
 static DEFINE_MUTEX(acpi_pm_notifier_lock);
 
+void acpi_pm_wakeup_event(struct device *dev)
+{
+	pm_wakeup_dev_event(dev, 0, acpi_s2idle_wakeup());
+}
+EXPORT_SYMBOL_GPL(acpi_pm_wakeup_event);
+
 static void acpi_pm_notify_handler(acpi_handle handle, u32 val, void *not_used)
 {
 	struct acpi_device *adev;
@@ -399,7 +406,7 @@ static void acpi_pm_notify_handler(acpi_
 	mutex_lock(&acpi_pm_notifier_lock);
 
 	if (adev->wakeup.flags.notifier_present) {
-		__pm_wakeup_event(adev->wakeup.ws, 0);
+		pm_wakeup_ws_event(adev->wakeup.ws, 0, acpi_s2idle_wakeup());
 		if (adev->wakeup.context.func)
 			adev->wakeup.context.func(&adev->wakeup.context);
 	}
Index: linux-pm/drivers/acpi/button.c
===================================================================
--- linux-pm.orig/drivers/acpi/button.c
+++ linux-pm/drivers/acpi/button.c
@@ -217,7 +217,7 @@ static int acpi_lid_notify_state(struct
 	}
 
 	if (state)
-		pm_wakeup_event(&device->dev, 0);
+		acpi_pm_wakeup_event(&device->dev);
 
 	ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device);
 	if (ret == NOTIFY_DONE)
@@ -402,7 +402,7 @@ static void acpi_button_notify(struct ac
 		} else {
 			int keycode;
 
-			pm_wakeup_event(&device->dev, 0);
+			acpi_pm_wakeup_event(&device->dev);
 			if (button->suspended)
 				break;
 
@@ -534,6 +534,7 @@ static int acpi_button_add(struct acpi_d
 		lid_device = device;
 	}
 
+	device_init_wakeup(&device->dev, true);
 	printk(KERN_INFO PREFIX "%s [%s]\n", name, acpi_device_bid(device));
 	return 0;
 
Index: linux-pm/drivers/acpi/battery.c
===================================================================
--- linux-pm.orig/drivers/acpi/battery.c
+++ linux-pm/drivers/acpi/battery.c
@@ -782,7 +782,7 @@ static int acpi_battery_update(struct ac
 	if ((battery->state & ACPI_BATTERY_STATE_CRITICAL) ||
 	    (test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags) &&
             (battery->capacity_now <= battery->alarm)))
-		pm_wakeup_event(&battery->device->dev, 0);
+		acpi_pm_wakeup_event(&battery->device->dev);
 
 	return result;
 }
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -198,8 +198,10 @@ void acpi_ec_remove_query_handler(struct
                                   Suspend/Resume
   -------------------------------------------------------------------------- */
 #ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT
+extern bool acpi_s2idle_wakeup(void);
 extern int acpi_sleep_init(void);
 #else
+static inline bool acpi_s2idle_wakeup(void) { return false; }
 static inline int acpi_sleep_init(void) { return -ENXIO; }
 #endif
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -598,15 +598,19 @@ static inline bool acpi_device_always_pr
 #endif
 
 #ifdef CONFIG_PM
+void acpi_pm_wakeup_event(struct device *dev);
 acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
 			void (*func)(struct acpi_device_wakeup_context *context));
 acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
 int acpi_pm_device_sleep_state(struct device *, int *, int);
 int acpi_pm_device_run_wake(struct device *, bool);
 #else
+static inline void acpi_pm_wakeup_event(struct device *dev)
+{
+}
 static inline acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
 					       struct device *dev,
-				               void (*work_func)(struct work_struct *work))
+					       void (*func)(struct acpi_device_wakeup_context *context))
 {
 	return AE_SUPPORT;
 }

Re: [PATCH v2 5/8] PM / sleep: Print timing information if debug is enabled

[Joe Perches]

On Mon, 2017-06-12 at 22:51 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Avoid printing the device suspend/resume timing information if
> CONFIG_PM_DEBUG is not set to reduce the log noise level.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/base/power/main.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: linux-pm/drivers/base/power/main.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/main.c
> +++ linux-pm/drivers/base/power/main.c
> @@ -417,6 +417,7 @@ static void pm_dev_err(struct device *de
>  		dev_name(dev), pm_verb(state.event), info, error);
>  }
>  
> +#ifdef CONFIG_PM_DEBUG
>  static void dpm_show_time(ktime_t starttime, pm_message_t state, char *info)
>  {
>  	ktime_t calltime;
> @@ -433,6 +434,9 @@ static void dpm_show_time(ktime_t startt
>  		info ?: "", info ? " " : "", pm_verb(state.event),
>  		usecs / USEC_PER_MSEC, usecs % USEC_PER_MSEC);
>  }
> +#else
> +static inline void dpm_show_time(ktime_t starttime, pm_message_t state, char *info) {}
> +#endif /* CONFIG_PM_DEBUG */
>  
>  static int dpm_run_callback(pm_callback_t cb, struct device *dev,
>  			    pm_message_t state, char *info)
> 

trivia:

This style of code can be reduced a few lines of code
using a single definition.

static type func(args...)
{
#ifdef CONFIG_<FOO>
	[code ...]
#endif
}

and compilers will generate the same code for the
!defined CONFIG_<FOO> case.

This style can help avoid defects of updating one
function definition and not the other and only
compile testing the updated version.

Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Dominik Brodowski]

Rafael,

On Mon, Jun 12, 2017 at 10:46:47PM +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> On Thursday, June 08, 2017 02:00:40 AM Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> > spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> > will be reverted shortly, because it triggered issues on quite a few systems.
> > 
> > The last patch in the series is a counterpart of the above commit with a few
> > modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> > printed to kernel logs to make them somewhat less confusing.
> 
> Here's a v2 which is very similar to the previous one, except for 3 things:
> - There are few build fixes in the last patch.
> - The patch from Hans mentioned previously is included now (as [7/8]).
> - There is an additional PCI change related to config space saving/restoring
>   and PME.
> 
> If anyone has any objections or concerns regarding this, please speak up now,
> as I'm going to put it into linux-next shortly to allow it to receive some more
> testing than commit eed4d47efe95 had had before it went it.

suspend-to-mem works as expected, also with this v2 applied. Thanks!

	Dominik

Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Greg KH]

On Mon, Jun 12, 2017 at 10:49:40PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> and may become problematic.
> 
> It is unnecessary, because the PCI bus type carries out post-suspend
> cleanup of all PCI devices during resume and that covers all things
> done by the pci_back_from_sleep().  There is no reason why USB cannot
> follow all of the other PCI devices in that respect.
> 
> It will become problematic after subsequent changes that make it
> possible to go back to sleep again after executing dpm_resume_noirq()
> if no valid system wakeup events have been detected at that point.
> Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> will cause the wakeup status of the devices in question to be cleared
> and if any of them has triggered system wakeup, that event may be
> missed then.
> 
> For the above reasons, drop the pci_back_from_sleep() invocation
> from hcd_pci_resume_noirq().
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2 0/8] ACPI / PM: Suspend-to-idle rework to deal with spurious ACPI wakeups

[Rafael J. Wysocki]

On Tuesday, June 13, 2017 07:54:22 AM Dominik Brodowski wrote:
> Rafael,
> 
> On Mon, Jun 12, 2017 at 10:46:47PM +0200, Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > On Thursday, June 08, 2017 02:00:40 AM Rafael J. Wysocki wrote:
> > > Hi All,
> > > 
> > > This series is a replacement for commit eed4d47efe95 (ACPI / sleep: Ignore
> > > spurious SCI wakeups from suspend-to-idle) which is still there in 4.12-rc4 but
> > > will be reverted shortly, because it triggered issues on quite a few systems.
> > > 
> > > The last patch in the series is a counterpart of the above commit with a few
> > > modifications, mostly to avoid affecting suspend-to-RAM and to reorder messages
> > > printed to kernel logs to make them somewhat less confusing.
> > 
> > Here's a v2 which is very similar to the previous one, except for 3 things:
> > - There are few build fixes in the last patch.
> > - The patch from Hans mentioned previously is included now (as [7/8]).
> > - There is an additional PCI change related to config space saving/restoring
> >   and PME.
> > 
> > If anyone has any objections or concerns regarding this, please speak up now,
> > as I'm going to put it into linux-next shortly to allow it to receive some more
> > testing than commit eed4d47efe95 had had before it went it.
> 
> suspend-to-mem works as expected, also with this v2 applied. Thanks!

Thanks for the confirmation!

Rafael

Re: [PATCH v2 2/8] USB / PCI / PM: Allow the PCI core to do the resume cleanup

[Rafael J. Wysocki]

On Tuesday, June 13, 2017 10:52:52 AM Greg KH wrote:
> On Mon, Jun 12, 2017 at 10:49:40PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> > PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> > and may become problematic.
> > 
> > It is unnecessary, because the PCI bus type carries out post-suspend
> > cleanup of all PCI devices during resume and that covers all things
> > done by the pci_back_from_sleep().  There is no reason why USB cannot
> > follow all of the other PCI devices in that respect.
> > 
> > It will become problematic after subsequent changes that make it
> > possible to go back to sleep again after executing dpm_resume_noirq()
> > if no valid system wakeup events have been detected at that point.
> > Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> > will cause the wakeup status of the devices in question to be cleared
> > and if any of them has triggered system wakeup, that event may be
> > missed then.
> > 
> > For the above reasons, drop the pci_back_from_sleep() invocation
> > from hcd_pci_resume_noirq().
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks!

Re: [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously

[Bjorn Helgaas]

On Mon, Jun 12, 2017 at 10:48:41PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The work functions provided by the users of acpi_add_pm_notifier()
> should be run synchronously before re-enabling the wakeup GPE in
> case they are used to clear the status and/or disable the wakeup
> signaling at the source.  Otherwise, which is the case currently in
> the PCI bus type code, the same wakeup event may be signaled for
> multiple times while the execution of the work function in response
> to it has already been queued up.
> 
> Fortunately, acpi_add_pm_notifier() is only used by PCI and by
> ACPI device PM code internally, so the change is relatively
> straightforward to make.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/acpi/device_pm.c |   27 +++++++++++----------------
>  drivers/pci/pci-acpi.c   |   17 +++++++----------
>  include/acpi/acpi_bus.h  |    4 ++--
>  3 files changed, 20 insertions(+), 28 deletions(-)
> 
> Index: linux-pm/drivers/acpi/device_pm.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/device_pm.c
> +++ linux-pm/drivers/acpi/device_pm.c
> @@ -400,8 +400,8 @@ static void acpi_pm_notify_handler(acpi_
>  
>  	if (adev->wakeup.flags.notifier_present) {
>  		__pm_wakeup_event(adev->wakeup.ws, 0);
> -		if (adev->wakeup.context.work.func)
> -			queue_pm_work(&adev->wakeup.context.work);
> +		if (adev->wakeup.context.func)
> +			adev->wakeup.context.func(&adev->wakeup.context);
>  	}
>  
>  	mutex_unlock(&acpi_pm_notifier_lock);
> @@ -413,7 +413,7 @@ static void acpi_pm_notify_handler(acpi_
>   * acpi_add_pm_notifier - Register PM notify handler for given ACPI device.
>   * @adev: ACPI device to add the notify handler for.
>   * @dev: Device to generate a wakeup event for while handling the notification.
> - * @work_func: Work function to execute when handling the notification.
> + * @func: Work function to execute when handling the notification.
>   *
>   * NOTE: @adev need not be a run-wake or wakeup device to be a valid source of
>   * PM wakeup events.  For example, wakeup events may be generated for bridges
> @@ -421,11 +421,11 @@ static void acpi_pm_notify_handler(acpi_
>   * bridge itself doesn't have a wakeup GPE associated with it.
>   */
>  acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
> -				 void (*work_func)(struct work_struct *work))
> +			void (*func)(struct acpi_device_wakeup_context *context))
>  {
>  	acpi_status status = AE_ALREADY_EXISTS;
>  
> -	if (!dev && !work_func)
> +	if (!dev && !func)
>  		return AE_BAD_PARAMETER;
>  
>  	mutex_lock(&acpi_pm_notifier_lock);
> @@ -435,8 +435,7 @@ acpi_status acpi_add_pm_notifier(struct
>  
>  	adev->wakeup.ws = wakeup_source_register(dev_name(&adev->dev));
>  	adev->wakeup.context.dev = dev;
> -	if (work_func)
> -		INIT_WORK(&adev->wakeup.context.work, work_func);
> +	adev->wakeup.context.func = func;
>  
>  	status = acpi_install_notify_handler(adev->handle, ACPI_SYSTEM_NOTIFY,
>  					     acpi_pm_notify_handler, NULL);
> @@ -469,10 +468,7 @@ acpi_status acpi_remove_pm_notifier(stru
>  	if (ACPI_FAILURE(status))
>  		goto out;
>  
> -	if (adev->wakeup.context.work.func) {
> -		cancel_work_sync(&adev->wakeup.context.work);
> -		adev->wakeup.context.work.func = NULL;
> -	}
> +	adev->wakeup.context.func = NULL;
>  	adev->wakeup.context.dev = NULL;
>  	wakeup_source_unregister(adev->wakeup.ws);
>  
> @@ -658,16 +654,15 @@ EXPORT_SYMBOL(acpi_pm_device_sleep_state
>  
>  /**
>   * acpi_pm_notify_work_func - ACPI devices wakeup notification work function.
> - * @work: Work item to handle.
> + * @context: Device wakeup context.
>   */
> -static void acpi_pm_notify_work_func(struct work_struct *work)
> +static void acpi_pm_notify_work_func(struct acpi_device_wakeup_context *context)
>  {
> -	struct device *dev;
> +	struct device *dev = context->dev;
>  
> -	dev = container_of(work, struct acpi_device_wakeup_context, work)->dev;
>  	if (dev) {
>  		pm_wakeup_event(dev, 0);
> -		pm_runtime_resume(dev);
> +		pm_request_resume(dev);
>  	}
>  }
>  
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -319,7 +319,7 @@ struct acpi_device_wakeup_flags {
>  };
>  
>  struct acpi_device_wakeup_context {
> -	struct work_struct work;
> +	void (*func)(struct acpi_device_wakeup_context *context);
>  	struct device *dev;
>  };
>  
> @@ -599,7 +599,7 @@ static inline bool acpi_device_always_pr
>  
>  #ifdef CONFIG_PM
>  acpi_status acpi_add_pm_notifier(struct acpi_device *adev, struct device *dev,
> -				 void (*work_func)(struct work_struct *work));
> +			void (*func)(struct acpi_device_wakeup_context *context));
>  acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
>  int acpi_pm_device_sleep_state(struct device *, int *, int);
>  int acpi_pm_device_run_wake(struct device *, bool);
> Index: linux-pm/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-acpi.c
> +++ linux-pm/drivers/pci/pci-acpi.c
> @@ -395,29 +395,26 @@ bool pciehp_is_native(struct pci_dev *pd
>  
>  /**
>   * pci_acpi_wake_bus - Root bus wakeup notification fork function.
> - * @work: Work item to handle.
> + * @context: Device wakeup context.
>   */
> -static void pci_acpi_wake_bus(struct work_struct *work)
> +static void pci_acpi_wake_bus(struct acpi_device_wakeup_context *context)
>  {
>  	struct acpi_device *adev;
>  	struct acpi_pci_root *root;
>  
> -	adev = container_of(work, struct acpi_device, wakeup.context.work);
> +	adev = container_of(context, struct acpi_device, wakeup.context);
>  	root = acpi_driver_data(adev);
>  	pci_pme_wakeup_bus(root->bus);
>  }
>  
>  /**
>   * pci_acpi_wake_dev - PCI device wakeup notification work function.
> - * @handle: ACPI handle of a device the notification is for.
> - * @work: Work item to handle.
> + * @context: Device wakeup context.
>   */
> -static void pci_acpi_wake_dev(struct work_struct *work)
> +static void pci_acpi_wake_dev(struct acpi_device_wakeup_context *context)
>  {
> -	struct acpi_device_wakeup_context *context;
>  	struct pci_dev *pci_dev;
>  
> -	context = container_of(work, struct acpi_device_wakeup_context, work);
>  	pci_dev = to_pci_dev(context->dev);
>  
>  	if (pci_dev->pme_poll)
> @@ -425,7 +422,7 @@ static void pci_acpi_wake_dev(struct wor
>  
>  	if (pci_dev->current_state == PCI_D3cold) {
>  		pci_wakeup_event(pci_dev);
> -		pm_runtime_resume(&pci_dev->dev);
> +		pm_request_resume(&pci_dev->dev);
>  		return;
>  	}
>  
> @@ -434,7 +431,7 @@ static void pci_acpi_wake_dev(struct wor
>  		pci_check_pme_status(pci_dev);
>  
>  	pci_wakeup_event(pci_dev);
> -	pm_runtime_resume(&pci_dev->dev);
> +	pm_request_resume(&pci_dev->dev);
>  
>  	pci_pme_wakeup_bus(pci_dev->subordinate);
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/8] PCI / PM: Restore PME Enable if skipping wakeup setup

[Bjorn Helgaas]

On Mon, Jun 12, 2017 at 10:53:36PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The wakeup_prepared PCI device flag is used for preventing subsequent
> changes of PCI device wakeup settings in the same way (e.g. enabling
> device wakeup twice in a row).
> 
> However, in some cases PME Enable may be updated by things like PCI
> configuration space restoration in the meantime and it may need to be
> set again even though the rest of the settings need not change, so
> modify __pci_enable_wake() to do that when it is about to return
> early.
> 
> Also, it is reasonable to expect that __pci_enable_wake() will always
> clear PME Status when invoked to disable device wakeup, so make it do
> so even if it is going to return early then.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pci.c |   26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> Index: linux-pm/drivers/pci/pci.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci.c
> +++ linux-pm/drivers/pci/pci.c
> @@ -1805,6 +1805,23 @@ static void __pci_pme_active(struct pci_
>  	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
>  }
>  
> +static void pci_pme_restore(struct pci_dev *dev)
> +{
> +	u16 pmcsr;
> +
> +	if (!dev->pme_support)
> +		return;
> +
> +	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
> +	if (dev->wakeup_prepared) {
> +		pmcsr |= PCI_PM_CTRL_PME_ENABLE;
> +	} else {
> +		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
> +		pmcsr |= PCI_PM_CTRL_PME_STATUS;
> +	}
> +	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
> +}
> +
>  /**
>   * pci_pme_active - enable or disable PCI device's PME# function
>   * @dev: PCI device to handle.
> @@ -1899,9 +1916,14 @@ int __pci_enable_wake(struct pci_dev *de
>  	if (enable && !runtime && !device_may_wakeup(&dev->dev))
>  		return -EINVAL;
>  
> -	/* Don't do the same thing twice in a row for one device. */
> -	if (!!enable == !!dev->wakeup_prepared)
> +	/*
> +	 * Don't do the same thing twice in a row for one device, but restore
> +	 * PME Enable in case it has been updated by config space restoration.
> +	 */
> +	if (!!enable == !!dev->wakeup_prepared) {
> +		pci_pme_restore(dev);
>  		return 0;
> +	}
>  
>  	/*
>  	 * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don
> 

Re: [PATCH v2 1/8] ACPI / PM: Run wakeup notify handlers synchronously

[Rafael J. Wysocki]

On Wed, Jun 14, 2017 at 8:09 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Jun 12, 2017 at 10:48:41PM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The work functions provided by the users of acpi_add_pm_notifier()
>> should be run synchronously before re-enabling the wakeup GPE in
>> case they are used to clear the status and/or disable the wakeup
>> signaling at the source.  Otherwise, which is the case currently in
>> the PCI bus type code, the same wakeup event may be signaled for
>> multiple times while the execution of the work function in response
>> to it has already been queued up.
>>
>> Fortunately, acpi_add_pm_notifier() is only used by PCI and by
>> ACPI device PM code internally, so the change is relatively
>> straightforward to make.
>>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Thanks!