linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux PCI <linux-pci@vger.kernel.org>
Cc: Linux ACPI <linux-acpi@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Bjorn Helgaas <bhelgaas@google.com>
Subject: [PATCH 3/3] ACPI / PCI / PM: Rework acpi_pci_propagate_wakeup()
Date: Fri, 21 Jul 2017 14:42:06 +0200	[thread overview]
Message-ID: <4853865.5aRGCM4CdQ@aspire.rjw.lan> (raw)
In-Reply-To: <3116391.JNS1F4DjTg@aspire.rjw.lan>

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

The acpi_pci_propagate_wakeup() routine is there to handle cases in
which PCI bridges (or PCIe ports) are expected to signal wakeup
for devices below them, but currently it doesn't do that correctly.

The problem is that acpi_pci_propagate_wakeup() uses
acpi_pm_set_device_wakeup() for bridges and if that routine is
called for multiple times to disable wakeup for the same device,
it will disable it on the first invocation and the next calls
will have no effect (it works analogously when called to enable
wakeup, but that is not a problem).

Now, say acpi_pci_propagate_wakeup() has been called for two
different devices under the same bridge and it has called
acpi_pm_set_device_wakeup() for that bridge each time.  The
bridge is now enabled to generate wakeup signals.  Next,
suppose that one of the devices below it resumes and
acpi_pci_propagate_wakeup() is called to disable wakeup for that
device.  It will then call acpi_pm_set_device_wakeup() for the bridge
and that will effectively disable remote wakeup for all devices under
it even though some of them may still be suspended and remote wakeup
may be expected to work for them.

To address this (arguably theoretical) issue, allow
wakeup.enable_count under struct acpi_device to grow beyond 1 in
certain situations.  In particular, allow that to happen in
acpi_pci_propagate_wakeup() when wakeup is enabled or disabled
for PCI bridges, so that wakeup is actually disabled for the
bridge when all devices under it resume and not when just one
of them does that.

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

Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -682,19 +682,8 @@ static void acpi_pm_notify_work_func(str
 
 static DEFINE_MUTEX(acpi_wakeup_lock);
 
-/**
- * acpi_device_wakeup_enable - Enable wakeup functionality for device.
- * @adev: ACPI device to enable wakeup functionality for.
- * @target_state: State the system is transitioning into.
- *
- * Enable the GPE associated with @adev so that it can generate wakeup signals
- * for the device in response to external (remote) events and enable wakeup
- * power for it.
- *
- * Callers must ensure that @adev is a valid ACPI device node before executing
- * this function.
- */
-static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state)
+static int __acpi_device_wakeup_enable(struct acpi_device *adev,
+				       u32 target_state, int max_count)
 {
 	struct acpi_device_wakeup *wakeup = &adev->wakeup;
 	acpi_status status;
@@ -702,8 +691,12 @@ static int acpi_device_wakeup_enable(str
 
 	mutex_lock(&acpi_wakeup_lock);
 
-	if (wakeup->enable_count > 0)
-		goto out;
+	if (wakeup->enable_count > 0) {
+		if (wakeup->enable_count < max_count)
+			goto inc;
+		else
+			goto out;
+	}
 
 	error = acpi_enable_wakeup_device_power(adev, target_state);
 	if (error)
@@ -716,6 +709,7 @@ static int acpi_device_wakeup_enable(str
 		goto out;
 	}
 
+inc:
 	wakeup->enable_count++;
 
 out:
@@ -724,6 +718,23 @@ out:
 }
 
 /**
+ * acpi_device_wakeup_enable - Enable wakeup functionality for device.
+ * @adev: ACPI device to enable wakeup functionality for.
+ * @target_state: State the system is transitioning into.
+ *
+ * Enable the GPE associated with @adev so that it can generate wakeup signals
+ * for the device in response to external (remote) events and enable wakeup
+ * power for it.
+ *
+ * Callers must ensure that @adev is a valid ACPI device node before executing
+ * this function.
+ */
+static int acpi_device_wakeup_enable(struct acpi_device *adev, u32 target_state)
+{
+	return __acpi_device_wakeup_enable(adev, target_state, 1);
+}
+
+/**
  * acpi_device_wakeup_disable - Disable wakeup functionality for device.
  * @adev: ACPI device to disable wakeup functionality for.
  *
@@ -754,8 +765,9 @@ out:
  * acpi_pm_set_device_wakeup - Enable/disable remote wakeup for given device.
  * @dev: Device to enable/disable to generate wakeup events.
  * @enable: Whether to enable or disable the wakeup functionality.
+ * @max_count: Maximum value of the enable reference counter.
  */
-int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
+int __acpi_pm_set_device_wakeup(struct device *dev, bool enable, int max_count)
 {
 	struct acpi_device *adev;
 	int error;
@@ -775,13 +787,14 @@ int acpi_pm_set_device_wakeup(struct dev
 		return 0;
 	}
 
-	error = acpi_device_wakeup_enable(adev, acpi_target_system_state());
+	error = __acpi_device_wakeup_enable(adev, acpi_target_system_state(),
+					    max_count);
 	if (!error)
 		dev_dbg(dev, "Wakeup enabled by ACPI\n");
 
 	return error;
 }
-EXPORT_SYMBOL(acpi_pm_set_device_wakeup);
+EXPORT_SYMBOL(__acpi_pm_set_device_wakeup);
 
 /**
  * acpi_dev_pm_low_power - Put ACPI device into a low-power state.
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -605,7 +605,7 @@ acpi_status acpi_add_pm_notifier(struct
 acpi_status acpi_remove_pm_notifier(struct acpi_device *adev);
 bool acpi_pm_device_can_wakeup(struct device *dev);
 int acpi_pm_device_sleep_state(struct device *, int *, int);
-int acpi_pm_set_device_wakeup(struct device *dev, bool enable);
+int __acpi_pm_set_device_wakeup(struct device *dev, bool enable, int max_count);
 #else
 static inline void acpi_pm_wakeup_event(struct device *dev)
 {
@@ -632,12 +632,22 @@ static inline int acpi_pm_device_sleep_s
 	return (m >= ACPI_STATE_D0 && m <= ACPI_STATE_D3_COLD) ?
 		m : ACPI_STATE_D0;
 }
-static inline int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
+static inline int __acpi_pm_set_device_wakeup(struct device *dev, bool enable, int max_count)
 {
 	return -ENODEV;
 }
 #endif
 
+static inline int acpi_pm_set_device_wakeup(struct device *dev, bool enable)
+{
+	return __acpi_pm_set_device_wakeup(dev, enable, 1);
+}
+
+static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
+{
+	return __acpi_pm_set_device_wakeup(dev, enable, INT_MAX);
+}
+
 #ifdef CONFIG_ACPI_SLEEP
 u32 acpi_target_system_state(void);
 #else
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -573,7 +573,7 @@ static int acpi_pci_propagate_wakeup(str
 {
 	while (bus->parent) {
 		if (acpi_pm_device_can_wakeup(&bus->self->dev))
-			return acpi_pm_set_device_wakeup(&bus->self->dev, enable);
+			return acpi_pm_set_bridge_wakeup(&bus->self->dev, enable);
 
 		bus = bus->parent;
 	}
@@ -581,7 +581,7 @@ static int acpi_pci_propagate_wakeup(str
 	/* We have reached the root bus. */
 	if (bus->bridge) {
 		if (acpi_pm_device_can_wakeup(bus->bridge))
-			return acpi_pm_set_device_wakeup(bus->bridge, enable);
+			return acpi_pm_set_bridge_wakeup(bus->bridge, enable);
 	}
 	return 0;
 }


  parent reply	other threads:[~2017-07-21 12:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-21 12:36 [PATCH 0/3] PCI / ACPI / PM: Fix propagation of wakeup settings to bridges Rafael J. Wysocki
2017-07-21 12:38 ` [PATCH 1/3] PCI / PM: Skip bridges in pci_enable_wake() Rafael J. Wysocki
2017-07-25 12:44   ` Mika Westerberg
2017-07-31 20:53   ` Bjorn Helgaas
2017-07-31 20:59     ` Rafael J. Wysocki
2017-07-21 12:40 ` [PATCH 2/3] ACPI / PM: Split acpi_device_wakeup() Rafael J. Wysocki
2017-07-21 15:27   ` Andy Shevchenko
2017-07-21 20:49     ` Rafael J. Wysocki
2017-07-21 21:19       ` Andy Shevchenko
2017-07-21 21:16         ` Rafael J. Wysocki
2017-07-21 21:31           ` Andy Shevchenko
2017-07-21 21:25             ` Rafael J. Wysocki
2017-07-25 12:45   ` Mika Westerberg
2017-07-21 12:42 ` Rafael J. Wysocki [this message]
2017-07-21 15:45   ` [PATCH 3/3] ACPI / PCI / PM: Rework acpi_pci_propagate_wakeup() Andy Shevchenko
2017-07-21 20:44     ` Rafael J. Wysocki
2017-07-21 21:02       ` Rafael J. Wysocki
2017-07-21 21:30   ` [PATCH v2 " Rafael J. Wysocki
2017-07-21 21:43     ` Andy Shevchenko
2017-07-25 12:46     ` Mika Westerberg
2017-07-31 21:59     ` Bjorn Helgaas
2017-08-01  0:39       ` Rafael J. Wysocki
2017-08-01  0:56     ` [PATCH v3 " Rafael J. Wysocki
2017-07-28  0:34 ` [PATCH 0/3] PCI / ACPI / PM: Fix propagation of wakeup settings to bridges Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4853865.5aRGCM4CdQ@aspire.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=bhelgaas@google.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).