Skip to contentSkip to conversation

PCI run-time PM support (rev. 2)

PATCH In review Rafael J. Wysocki 5 participants 64 messages Dec 27, 2009 – Jan 8, 2010

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

cover PCI run-time PM support (rev. 2)

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

[1/12] - Add function for checking PME status of devices

[2/12] - Modify wake-up enable propagation so that it's done for PCIe devices
too (this one is in the Jesse's tree already, but it's reproduced here for completness)

[3/12] - PCIe PME root port service driver

[4/12] - "Don't use MSIs for PME signaling" switch for PCIe

[5/12] - ACPI GPE refcounting, from Matthew

[6/12] - ACPI drivers support for GPE refcounting, from Matthew

[7/12] - ACPI removal of the old GPE API, from Matthew

[8/12] - ACPI add fields for handling run-wake devices

[9/12][New] - ACPI add helper function for enabling/disabling wake-up power

[10/12] - PCI / ACPI platform support for run-time power management

[11/12] - Runtime PM callbacks for the PCI bus type

[12/12] - Runtime PM support for r8169 (experimental)

If there are no objections, I think the patches [1-11/12] are ready for the upstream.

Thanks,
Rafael

#1 lore raw

PATCH 1/12 PCI PM: Add function for checking PME status of devices

From: Rafael J. Wysocki <[email protected]>

Add function pci_check_pme_status() that will check the PME status bit of given device and clear it along with the PME enable bit. It will be necessary for PCI run-time power management.

Based on a patch from Shaohua Li <[email protected]>

---
 drivers/pci/pci.c |   35 +++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h |    1 +
 2 files changed, 36 insertions(+)
drivers/pci/pci.h +1 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -49,6 +49,7 @@ struct pci_platform_pm_ops {
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
+extern bool pci_check_pme_status(struct pci_dev *dev);
 extern void pci_pm_init(struct pci_dev *dev);
 extern void platform_pci_wakeup_init(struct pci_dev *dev);
 extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
drivers/pci/pci.c +35 -0
Index: linux-2.6/drivers/pci/pci.c
===================================================================
@@ -1180,6 +1180,41 @@ int pci_set_pcie_reset_state(struct pci_
 }
 
 /**
+ * pci_check_pme_status - Check if given device has generated PME.
+ * @dev: Device to check.
+ *
+ * Check the PME status of the device and if set, clear it and clear PME enable
+ * (if set).  Return 'true' if PME status and PME enable were both set or
+ * 'false' otherwise.
+ */
+bool pci_check_pme_status(struct pci_dev *dev)
+{
+	int pmcsr_pos;
+	u16 pmcsr;
+	bool ret = false;
+
+	if (!dev->pm_cap)
+		return false;
+
+	pmcsr_pos = dev->pm_cap + PCI_PM_CTRL;
+	pci_read_config_word(dev, pmcsr_pos, &pmcsr);
+	if (!(pmcsr & PCI_PM_CTRL_PME_STATUS))
+		return false;
+
+	/* Clear PME status. */
+	pmcsr |= PCI_PM_CTRL_PME_STATUS;
+	if (pmcsr & PCI_PM_CTRL_PME_ENABLE) {
+		/* Disable PME to avoid interrupt flood. */
+		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
+		ret = true;
+	}
+
+	pci_write_config_word(dev, pmcsr_pos, pmcsr);
+
+	return ret;
+}
+
+/**
  * pci_pme_capable - check the capability of PCI device to generate PME#
  * @dev: PCI device to handle.
  * @state: PCI state from which device will issue PME#.
#2 lore raw

Re: [PATCH 1/12] PCI PM: Add function for checking PME status of devices

Quoted 72 lines · On Sun, 27 Dec 2009 20:59:06 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Add function pci_check_pme_status() that will check the PME status bit of given device and clear it along with the PME enable bit. It will be necessary for PCI run-time power management.

Based on a patch from Shaohua Li <[email protected]>

---
 drivers/pci/pci.c |   35 +++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h |    1 +
 2 files changed, 36 insertions(+)

Index: linux-2.6/drivers/pci/pci.h
===================================================================

drivers/pci/pci.h +0 -0
@@ -49,6 +49,7 @@ struct pci_platform_pm_ops {
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev,

pci_power_t state); extern void pci_disable_enabled_device(struct pci_dev *dev); +extern bool pci_check_pme_status(struct pci_dev *dev);
extern void pci_pm_init(struct pci_dev *dev);
extern void platform_pci_wakeup_init(struct pci_dev *dev); extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
Index: linux-2.6/drivers/pci/pci.c
===================================================================

drivers/pci/pci.c +4 -0
@@ -1180,6 +1180,41 @@ int pci_set_pcie_reset_state(struct pci_
 }
 
 /**
+ * pci_check_pme_status - Check if given device has generated PME.
+ * @dev: Device to check.
+ *
+ * Check the PME status of the device and if set, clear it and clear

PME enable
+ * (if set). Return 'true' if PME status and PME enable were both set or
+ * 'false' otherwise.
+ */

drivers/pci/pci.c +28 -0
+bool pci_check_pme_status(struct pci_dev *dev)
+{
+	int pmcsr_pos;
+	u16 pmcsr;
+	bool ret = false;
+
+	if (!dev->pm_cap)
+		return false;
+
+	pmcsr_pos = dev->pm_cap + PCI_PM_CTRL;
+	pci_read_config_word(dev, pmcsr_pos, &pmcsr);
+	if (!(pmcsr & PCI_PM_CTRL_PME_STATUS))
+		return false;
+
+	/* Clear PME status. */
+	pmcsr |= PCI_PM_CTRL_PME_STATUS;
+	if (pmcsr & PCI_PM_CTRL_PME_ENABLE) {
+		/* Disable PME to avoid interrupt flood. */
+		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
+		ret = true;
+	}
+
+	pci_write_config_word(dev, pmcsr_pos, pmcsr);
+
+	return ret;
+}
+
+/**
  * pci_pme_capable - check the capability of PCI device to generate
Quoted 5 lines

PME#
* @dev: PCI device to handle.
* @state: PCI state from which device will issue PME#.

Looks fine to me; would be especially nice if we could somehow communicate that this function was ack'ing any outstanding PME event in the function name too.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#3 lore raw

Re: [PATCH 1/12] PCI PM: Add function for checking PME status of devices

In reply to Rafael J. Wysocki
Quoted 72 lines · On Sun, 27 Dec 2009 20:59:06 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Add function pci_check_pme_status() that will check the PME status bit of given device and clear it along with the PME enable bit. It will be necessary for PCI run-time power management.

Based on a patch from Shaohua Li <[email protected]>

---
 drivers/pci/pci.c |   35 +++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h |    1 +
 2 files changed, 36 insertions(+)

Index: linux-2.6/drivers/pci/pci.h
===================================================================

drivers/pci/pci.h +0 -0
@@ -49,6 +49,7 @@ struct pci_platform_pm_ops {
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev,

pci_power_t state); extern void pci_disable_enabled_device(struct pci_dev *dev); +extern bool pci_check_pme_status(struct pci_dev *dev);
extern void pci_pm_init(struct pci_dev *dev);
extern void platform_pci_wakeup_init(struct pci_dev *dev); extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
Index: linux-2.6/drivers/pci/pci.c
===================================================================

drivers/pci/pci.c +4 -0
@@ -1180,6 +1180,41 @@ int pci_set_pcie_reset_state(struct pci_
 }
 
 /**
+ * pci_check_pme_status - Check if given device has generated PME.
+ * @dev: Device to check.
+ *
+ * Check the PME status of the device and if set, clear it and clear

PME enable
+ * (if set). Return 'true' if PME status and PME enable were both set or
+ * 'false' otherwise.
+ */

drivers/pci/pci.c +28 -0
+bool pci_check_pme_status(struct pci_dev *dev)
+{
+	int pmcsr_pos;
+	u16 pmcsr;
+	bool ret = false;
+
+	if (!dev->pm_cap)
+		return false;
+
+	pmcsr_pos = dev->pm_cap + PCI_PM_CTRL;
+	pci_read_config_word(dev, pmcsr_pos, &pmcsr);
+	if (!(pmcsr & PCI_PM_CTRL_PME_STATUS))
+		return false;
+
+	/* Clear PME status. */
+	pmcsr |= PCI_PM_CTRL_PME_STATUS;
+	if (pmcsr & PCI_PM_CTRL_PME_ENABLE) {
+		/* Disable PME to avoid interrupt flood. */
+		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
+		ret = true;
+	}
+
+	pci_write_config_word(dev, pmcsr_pos, pmcsr);
+
+	return ret;
+}
+
+/**
  * pci_pme_capable - check the capability of PCI device to generate
Quoted 5 lines

PME#
* @dev: PCI device to handle.
* @state: PCI state from which device will issue PME#.

Looks fine to me; would be especially nice if we could somehow communicate that this function was ack'ing any outstanding PME event in the function name too.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#4 lore raw

PATCH 2/12 PCI / PM: Propagate wake-up enable for PCIe devices too

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Having read the PM part of the PCIe 2.0 specification more carefully I think that it was a mistake to restrict the wake-up enable propagation to non-PCIe devices, because if we do not request control of the root ports' PME registers via OSC, PCIe PME is supposed to be handled by the platform, just like the non-PCIe PME. Even if we do that, the wake-up propagation is done to allow the devices to wake up the system from sleep states which involves the platform anyway, so it won't hurt.

---
 drivers/pci/pci-acpi.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/pci/pci-acpi.c +2 -8
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
@@ -112,11 +112,7 @@ static bool acpi_pci_can_wakeup(struct p
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus, bool enable)
 {
 	while (bus->parent) {
-		struct pci_dev *bridge = bus->self;
-		int ret;
-
-		ret = acpi_pm_device_sleep_wake(&bridge->dev, enable);
-		if (!ret || pci_is_pcie(bridge))
+		if (!acpi_pm_device_sleep_wake(&bus->self->dev, enable))
 			return;
 		bus = bus->parent;
 	}
@@ -131,9 +127,7 @@ static int acpi_pci_sleep_wake(struct pc
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
-	if (!pci_is_pcie(dev))
-		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
-
+	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }
 
#5 lore raw

PATCH 2/12 PCI / PM: Propagate wake-up enable for PCIe devices too

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Having read the PM part of the PCIe 2.0 specification more carefully I think that it was a mistake to restrict the wake-up enable propagation to non-PCIe devices, because if we do not request control of the root ports' PME registers via OSC, PCIe PME is supposed to be handled by the platform, just like the non-PCIe PME. Even if we do that, the wake-up propagation is done to allow the devices to wake up the system from sleep states which involves the platform anyway, so it won't hurt.

---
 drivers/pci/pci-acpi.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
drivers/pci/pci-acpi.c +2 -8
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
@@ -112,11 +112,7 @@ static bool acpi_pci_can_wakeup(struct p
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus, bool enable)
 {
 	while (bus->parent) {
-		struct pci_dev *bridge = bus->self;
-		int ret;
-
-		ret = acpi_pm_device_sleep_wake(&bridge->dev, enable);
-		if (!ret || pci_is_pcie(bridge))
+		if (!acpi_pm_device_sleep_wake(&bus->self->dev, enable))
 			return;
 		bus = bus->parent;
 	}
@@ -131,9 +127,7 @@ static int acpi_pci_sleep_wake(struct pc
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
-	if (!pci_is_pcie(dev))
-		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
-
+	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }
 
#6 lore raw

Re: [PATCH 2/12] PCI / PM: Propagate wake-up enable for PCIe devices too

Quoted 35 lines · On Sun, 27 Dec 2009 21:00:33 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Having read the PM part of the PCIe 2.0 specification more carefully I think that it was a mistake to restrict the wake-up enable propagation to non-PCIe devices, because if we do not request control of the root ports' PME registers via OSC, PCIe PME is supposed to be handled by the platform, just like the non-PCIe PME. Even if we do that, the wake-up propagation is done to allow the devices to wake up the system from sleep states which involves the platform anyway, so it won't hurt.

---
 drivers/pci/pci-acpi.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================

drivers/pci/pci-acpi.c +1 -5
@@ -112,11 +112,7 @@ static bool acpi_pci_can_wakeup(struct p
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus,
 bool enable) {
 	while (bus->parent) {
-		struct pci_dev *bridge = bus->self;
-		int ret;
-
-		ret = acpi_pm_device_sleep_wake(&bridge->dev,
 enable);
-		if (!ret || pci_is_pcie(bridge))
+		if (!acpi_pm_device_sleep_wake(&bus->self->dev,
 enable)) return;
 		bus = bus->parent;
drivers/pci/pci-acpi.c +0 -0
 	}

drivers/pci/pci-acpi.c static int acpi_pci_sleep_wake(struct pc · line 132

Quoted 9 lines
drivers/pci/pci-acpi.c +1 -3
@@ -131,9 +127,7 @@ static int acpi_pci_sleep_wake(struct pc
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
-	if (!pci_is_pcie(dev))
-		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
-
+	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }

Seems ok to try it out, but as usual with this sort of thing, the real question is, "What does Windows do?", since wakeup enable ACPI support for PCIe devices is likely broken unless Windows uses it.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#7 lore raw

Re: [PATCH 2/12] PCI / PM: Propagate wake-up enable for PCIe devices too

In reply to Rafael J. Wysocki
Quoted 35 lines · On Sun, 27 Dec 2009 21:00:33 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Having read the PM part of the PCIe 2.0 specification more carefully I think that it was a mistake to restrict the wake-up enable propagation to non-PCIe devices, because if we do not request control of the root ports' PME registers via OSC, PCIe PME is supposed to be handled by the platform, just like the non-PCIe PME. Even if we do that, the wake-up propagation is done to allow the devices to wake up the system from sleep states which involves the platform anyway, so it won't hurt.

---
 drivers/pci/pci-acpi.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================

drivers/pci/pci-acpi.c +1 -5
@@ -112,11 +112,7 @@ static bool acpi_pci_can_wakeup(struct p
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus,
 bool enable) {
 	while (bus->parent) {
-		struct pci_dev *bridge = bus->self;
-		int ret;
-
-		ret = acpi_pm_device_sleep_wake(&bridge->dev,
 enable);
-		if (!ret || pci_is_pcie(bridge))
+		if (!acpi_pm_device_sleep_wake(&bus->self->dev,
 enable)) return;
 		bus = bus->parent;
drivers/pci/pci-acpi.c +0 -0
 	}

drivers/pci/pci-acpi.c static int acpi_pci_sleep_wake(struct pc · line 132

Quoted 9 lines
drivers/pci/pci-acpi.c +1 -3
@@ -131,9 +127,7 @@ static int acpi_pci_sleep_wake(struct pc
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
-	if (!pci_is_pcie(dev))
-		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
-
+	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }

Seems ok to try it out, but as usual with this sort of thing, the real question is, "What does Windows do?", since wakeup enable ACPI support for PCIe devices is likely broken unless Windows uses it.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#8 lore raw

Re: [PATCH 2/12] PCI / PM: Propagate wake-up enable for PCIe devices too

Quoted 41 lines · Jesse Barnes
Quoted 38 lines · On Sun, 27 Dec 2009 21:00:33 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Having read the PM part of the PCIe 2.0 specification more carefully I think that it was a mistake to restrict the wake-up enable propagation to non-PCIe devices, because if we do not request control of the root ports' PME registers via OSC, PCIe PME is supposed to be handled by the platform, just like the non-PCIe PME. Even if we do that, the wake-up propagation is done to allow the devices to wake up the system from sleep states which involves the platform anyway, so it won't hurt.

---
 drivers/pci/pci-acpi.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================

drivers/pci/pci-acpi.c +1 -5
@@ -112,11 +112,7 @@ static bool acpi_pci_can_wakeup(struct p
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus,
 bool enable) {
 	while (bus->parent) {
-		struct pci_dev *bridge = bus->self;
-		int ret;
-
-		ret = acpi_pm_device_sleep_wake(&bridge->dev,
 enable);
-		if (!ret || pci_is_pcie(bridge))
+		if (!acpi_pm_device_sleep_wake(&bus->self->dev,
 enable)) return;
 		bus = bus->parent;
drivers/pci/pci-acpi.c +0 -0
 	}
@@ -131,9 +127,7 @@ static int acpi_pci_sleep_wake(struct pc
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
Quoted 10 lines

drivers/pci/pci-acpi.c static int acpi_pci_sleep_wake(struct pc · line 132

Quoted 6 lines
drivers/pci/pci-acpi.c +1 -3
-	if (!pci_is_pcie(dev))
-		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
-
+	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }

Seems ok to try it out, but as usual with this sort of thing, the real question is, "What does Windows do?", since wakeup enable ACPI support for PCIe devices is likely broken unless Windows uses it.

Well, that's in the Linus' tree already, so I guess we're going to learn. ;-)

Seriously, I think that Windows actually uses ACPI wake-up support for PCIe devices. On my Toshiba Portege R500 the BIOS doesn't even allow us to use the native PCIe PME mechanism directly, so the patch is correct IMO.

Rafael

#9 lore raw

Re: [PATCH 2/12] PCI / PM: Propagate wake-up enable for PCIe devices too

In reply to Jesse Barnes
Quoted 41 lines · Jesse Barnes
Quoted 38 lines · On Sun, 27 Dec 2009 21:00:33 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Having read the PM part of the PCIe 2.0 specification more carefully I think that it was a mistake to restrict the wake-up enable propagation to non-PCIe devices, because if we do not request control of the root ports' PME registers via OSC, PCIe PME is supposed to be handled by the platform, just like the non-PCIe PME. Even if we do that, the wake-up propagation is done to allow the devices to wake up the system from sleep states which involves the platform anyway, so it won't hurt.

---
 drivers/pci/pci-acpi.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================

drivers/pci/pci-acpi.c +1 -5
@@ -112,11 +112,7 @@ static bool acpi_pci_can_wakeup(struct p
 static void acpi_pci_propagate_wakeup_enable(struct pci_bus *bus,
 bool enable) {
 	while (bus->parent) {
-		struct pci_dev *bridge = bus->self;
-		int ret;
-
-		ret = acpi_pm_device_sleep_wake(&bridge->dev,
 enable);
-		if (!ret || pci_is_pcie(bridge))
+		if (!acpi_pm_device_sleep_wake(&bus->self->dev,
 enable)) return;
 		bus = bus->parent;
drivers/pci/pci-acpi.c +0 -0
 	}
@@ -131,9 +127,7 @@ static int acpi_pci_sleep_wake(struct pc
 	if (acpi_pci_can_wakeup(dev))
 		return acpi_pm_device_sleep_wake(&dev->dev, enable);
 
Quoted 10 lines

drivers/pci/pci-acpi.c static int acpi_pci_sleep_wake(struct pc · line 132

Quoted 6 lines
drivers/pci/pci-acpi.c +1 -3
-	if (!pci_is_pcie(dev))
-		acpi_pci_propagate_wakeup_enable(dev->bus, enable);
-
+	acpi_pci_propagate_wakeup_enable(dev->bus, enable);
 	return 0;
 }

Seems ok to try it out, but as usual with this sort of thing, the real question is, "What does Windows do?", since wakeup enable ACPI support for PCIe devices is likely broken unless Windows uses it.

Well, that's in the Linus' tree already, so I guess we're going to learn. ;-)

Seriously, I think that Windows actually uses ACPI wake-up support for PCIe devices. On my Toshiba Portege R500 the BIOS doesn't even allow us to use the native PCIe PME mechanism directly, so the patch is correct IMO.

Rafael

#10 lore raw

PATCH 3/12 PCI PM: PCIe PME root port service driver (rev. 5)

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

PCIe native PME detection mechanism is based on interrupts generated by root ports or event collectors every time a PCIe device sends a PME message upstream.

Once a PME message has been sent by an endpoint device and received by its root port (or event collector in the case of root complex integrated endpoints), the Requester ID from the message header is registered in the root port's Root Status register. At the same time, the PME Status bit of the Root Status register is set to indicate that there's a PME to handle. If PCIe PME interrupt is enabled for the root port, it generates an interrupt once the PME Status has been set. After receiving the interrupt, the kernel can identify the PCIe device that generated the PME using the Requester ID from the root port's Root Status register. [For details, see PCI Express Base Specification, Rev. 2.0.]

Implement a driver for the PCIe PME root port service working in accordance with the above description.

Based on a patch from Shaohua Li <[email protected]>.

---
 Documentation/kernel-parameters.txt  |    6 
 drivers/pci/pcie/Kconfig             |    4 
 drivers/pci/pcie/Makefile            |    2 
 drivers/pci/pcie/pme/Makefile        |    8 
 drivers/pci/pcie/pme/pcie_pme.c      |  493 +++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/pme/pcie_pme.h      |   28 +
 drivers/pci/pcie/pme/pcie_pme_acpi.c |   54 +++
 include/linux/pci.h                  |    1 
 8 files changed, 596 insertions(+)
drivers/pci/pcie/Kconfig +4 -0
Index: linux-2.6/drivers/pci/pcie/Kconfig
===================================================================
@@ -46,3 +46,7 @@ config PCIEASPM_DEBUG
 	help
 	  This enables PCI Express ASPM debug support. It will add per-device
 	  interface to control ASPM.
+
+config PCIE_PME
+	def_bool y
+	depends on PCIEPORTBUS && PM_RUNTIME && EXPERIMENTAL
drivers/pci/pcie/Makefile +2 -0
Index: linux-2.6/drivers/pci/pcie/Makefile
===================================================================
@@ -11,3 +11,5 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv
 
 # Build PCI Express AER if needed
 obj-$(CONFIG_PCIEAER)		+= aer/
+
+obj-$(CONFIG_PCIE_PME) += pme/
drivers/pci/pcie/pme/pcie_pme.c new file +493 -0

493 lines not on this page · raw message

drivers/pci/pcie/pme/pcie_pme_acpi.c new file +54 -0

54 lines not on this page · raw message

drivers/pci/pcie/pme/Makefile new file +8 -0
Index: linux-2.6/drivers/pci/pcie/pme/Makefile
===================================================================
@@ -0,0 +1,8 @@
+#
+# Makefile for PCI-Express Root Port PME signaling driver
+#
+
+obj-$(CONFIG_PCIE_PME) += pmedriver.o
+
+pmedriver-objs := pcie_pme.o
+pmedriver-$(CONFIG_ACPI) += pcie_pme_acpi.o
drivers/pci/pcie/pme/pcie_pme.h new file +28 -0
Index: linux-2.6/drivers/pci/pcie/pme/pcie_pme.h
===================================================================
@@ -0,0 +1,28 @@
+/*
+ * drivers/pci/pcie/pme/pcie_pme.h
+ *
+ * PCI Express Root Port PME signaling support
+ *
+ * Copyright (C) 2009 Rafael J. Wysocki <[email protected]>, Novell Inc.
+ */
+
+#ifndef _PCIE_PME_H_
+#define _PCIE_PME_H_
+
+struct pcie_device;
+
+#ifdef CONFIG_ACPI
+extern int pcie_pme_acpi_setup(struct pcie_device *srv);
+
+static inline int pcie_pme_platform_notify(struct pcie_device *srv)
+{
+	return pcie_pme_acpi_setup(srv);
+}
+#else /* !CONFIG_ACPI */
+static inline int pcie_pme_platform_notify(struct pcie_device *srv)
+{
+	return 0;
+}
+#endif /* !CONFIG_ACPI */
+
+#endif
include/linux/pci.h +1 -0
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -239,6 +239,7 @@ struct pci_dev {
 					   configuration space */
 	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
 					   can be generated */
+	unsigned int	pme_interrupt:1;
 	unsigned int	d1_support:1;	/* Low power state D1 is supported */
 	unsigned int	d2_support:1;	/* Low power state D2 is supported */
 	unsigned int	no_d1d2:1;	/* Only allow D0 and D3 */
Documentation/kernel-parameters.txt +6 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -1986,6 +1986,12 @@ and is between 256 and 4096 characters. 
 		force	Enable ASPM even on devices that claim not to support it.
 			WARNING: Forcing ASPM on may cause system lockups.
 
+	pcie_pme=	[PCIE,PM] Native PCIe PME signaling options:
+		off	Do not use native PCIe PME signaling.
+		force	Use native PCIe PME signaling even if the BIOS refuses
+			to allow the kernel to control the relevant PCIe config
+			registers.
+
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
 	pd.		[PARIDE]
#11 lore raw

PATCH 3/12 PCI PM: PCIe PME root port service driver (rev. 5)

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

PCIe native PME detection mechanism is based on interrupts generated by root ports or event collectors every time a PCIe device sends a PME message upstream.

Once a PME message has been sent by an endpoint device and received by its root port (or event collector in the case of root complex integrated endpoints), the Requester ID from the message header is registered in the root port's Root Status register. At the same time, the PME Status bit of the Root Status register is set to indicate that there's a PME to handle. If PCIe PME interrupt is enabled for the root port, it generates an interrupt once the PME Status has been set. After receiving the interrupt, the kernel can identify the PCIe device that generated the PME using the Requester ID from the root port's Root Status register. [For details, see PCI Express Base Specification, Rev. 2.0.]

Implement a driver for the PCIe PME root port service working in accordance with the above description.

Based on a patch from Shaohua Li <[email protected]>.

---
 Documentation/kernel-parameters.txt  |    6 
 drivers/pci/pcie/Kconfig             |    4 
 drivers/pci/pcie/Makefile            |    2 
 drivers/pci/pcie/pme/Makefile        |    8 
 drivers/pci/pcie/pme/pcie_pme.c      |  493 +++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/pme/pcie_pme.h      |   28 +
 drivers/pci/pcie/pme/pcie_pme_acpi.c |   54 +++
 include/linux/pci.h                  |    1 
 8 files changed, 596 insertions(+)
drivers/pci/pcie/Kconfig +4 -0
Index: linux-2.6/drivers/pci/pcie/Kconfig
===================================================================
@@ -46,3 +46,7 @@ config PCIEASPM_DEBUG
 	help
 	  This enables PCI Express ASPM debug support. It will add per-device
 	  interface to control ASPM.
+
+config PCIE_PME
+	def_bool y
+	depends on PCIEPORTBUS && PM_RUNTIME && EXPERIMENTAL
drivers/pci/pcie/Makefile +2 -0
Index: linux-2.6/drivers/pci/pcie/Makefile
===================================================================
@@ -11,3 +11,5 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv
 
 # Build PCI Express AER if needed
 obj-$(CONFIG_PCIEAER)		+= aer/
+
+obj-$(CONFIG_PCIE_PME) += pme/
drivers/pci/pcie/pme/pcie_pme.c new file +493 -0

493 lines not on this page · raw message

drivers/pci/pcie/pme/pcie_pme_acpi.c new file +54 -0

54 lines not on this page · raw message

drivers/pci/pcie/pme/Makefile new file +8 -0
Index: linux-2.6/drivers/pci/pcie/pme/Makefile
===================================================================
@@ -0,0 +1,8 @@
+#
+# Makefile for PCI-Express Root Port PME signaling driver
+#
+
+obj-$(CONFIG_PCIE_PME) += pmedriver.o
+
+pmedriver-objs := pcie_pme.o
+pmedriver-$(CONFIG_ACPI) += pcie_pme_acpi.o
drivers/pci/pcie/pme/pcie_pme.h new file +28 -0
Index: linux-2.6/drivers/pci/pcie/pme/pcie_pme.h
===================================================================
@@ -0,0 +1,28 @@
+/*
+ * drivers/pci/pcie/pme/pcie_pme.h
+ *
+ * PCI Express Root Port PME signaling support
+ *
+ * Copyright (C) 2009 Rafael J. Wysocki <[email protected]>, Novell Inc.
+ */
+
+#ifndef _PCIE_PME_H_
+#define _PCIE_PME_H_
+
+struct pcie_device;
+
+#ifdef CONFIG_ACPI
+extern int pcie_pme_acpi_setup(struct pcie_device *srv);
+
+static inline int pcie_pme_platform_notify(struct pcie_device *srv)
+{
+	return pcie_pme_acpi_setup(srv);
+}
+#else /* !CONFIG_ACPI */
+static inline int pcie_pme_platform_notify(struct pcie_device *srv)
+{
+	return 0;
+}
+#endif /* !CONFIG_ACPI */
+
+#endif
include/linux/pci.h +1 -0
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -239,6 +239,7 @@ struct pci_dev {
 					   configuration space */
 	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
 					   can be generated */
+	unsigned int	pme_interrupt:1;
 	unsigned int	d1_support:1;	/* Low power state D1 is supported */
 	unsigned int	d2_support:1;	/* Low power state D2 is supported */
 	unsigned int	no_d1d2:1;	/* Only allow D0 and D3 */
Documentation/kernel-parameters.txt +6 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -1986,6 +1986,12 @@ and is between 256 and 4096 characters. 
 		force	Enable ASPM even on devices that claim not to support it.
 			WARNING: Forcing ASPM on may cause system lockups.
 
+	pcie_pme=	[PCIE,PM] Native PCIe PME signaling options:
+		off	Do not use native PCIe PME signaling.
+		force	Use native PCIe PME signaling even if the BIOS refuses
+			to allow the kernel to control the relevant PCIe config
+			registers.
+
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
 	pd.		[PARIDE]
#12 lore raw

Re: [PATCH 3/12] PCI PM: PCIe PME root port service driver (rev. 5)

Quoted 18 lines · On Sun, 27 Dec 2009 21:01:24 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

PCIe native PME detection mechanism is based on interrupts generated by root ports or event collectors every time a PCIe device sends a PME message upstream.

Once a PME message has been sent by an endpoint device and received by its root port (or event collector in the case of root complex integrated endpoints), the Requester ID from the message header is registered in the root port's Root Status register. At the same time, the PME Status bit of the Root Status register is set to indicate that there's a PME to handle. If PCIe PME interrupt is enabled for the root port, it generates an interrupt once the PME Status has been set. After receiving the interrupt, the kernel can identify the PCIe device that generated the PME using the Requester ID from the root port's Root Status register. [For details, see PCI Express Base Specification, Rev. 2.0.]

Quoted 4 lines

Implement a driver for the PCIe PME root port service working in accordance with the above description.

Based on a patch from Shaohua Li <[email protected]>.

This one looks pretty nice.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#13 lore raw

Re: [PATCH 3/12] PCI PM: PCIe PME root port service driver (rev. 5)

In reply to Rafael J. Wysocki
Quoted 18 lines · On Sun, 27 Dec 2009 21:01:24 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

PCIe native PME detection mechanism is based on interrupts generated by root ports or event collectors every time a PCIe device sends a PME message upstream.

Once a PME message has been sent by an endpoint device and received by its root port (or event collector in the case of root complex integrated endpoints), the Requester ID from the message header is registered in the root port's Root Status register. At the same time, the PME Status bit of the Root Status register is set to indicate that there's a PME to handle. If PCIe PME interrupt is enabled for the root port, it generates an interrupt once the PME Status has been set. After receiving the interrupt, the kernel can identify the PCIe device that generated the PME using the Requester ID from the root port's Root Status register. [For details, see PCI Express Base Specification, Rev. 2.0.]

Quoted 4 lines

Implement a driver for the PCIe PME root port service working in accordance with the above description.

Based on a patch from Shaohua Li <[email protected]>.

This one looks pretty nice.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#14 lore raw

PATCH 4/12 PCI PM: Make it possible to force using INTx for PCIe PME signaling

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Apparently, some machines may have problems with PCI run-time power management if MSIs are used for the hative PCIe PME signaling. In particular, on the MSI Wind U-100 PCIe PME interrupts are not generated by a PCIe root port after a resume from suspend to RAM, if the system wake-up was triggered by a PME from the device attached to this port. [It doesn't help to free the interrupt on suspend and request it back on resume, even if that is done along with disabling the MSI and re-enabling it, respectively.] However, if INTx interrupts are used for this purpose on the same machine, everything works just fine.

For this reason, add a kernel command line switch allowing one to request that MSIs be not used for the native PCIe PME signaling, introduce a DMI table allowing us to blacklist machines that need this switch to be set by default and put the MSI Wind U-100 into this table.

---
 Documentation/kernel-parameters.txt |    2 ++
 drivers/pci/pcie/pme/pcie_pme.c     |   14 +++++++++++++-
 drivers/pci/pcie/portdrv.h          |   17 +++++++++++++++++
 drivers/pci/pcie/portdrv_core.c     |   12 ++++++++++--
 drivers/pci/pcie/portdrv_pci.c      |   27 +++++++++++++++++++++++++++
 5 files changed, 69 insertions(+), 3 deletions(-)
drivers/pci/pcie/pme/pcie_pme.c +13 -1
Index: linux-2.6/drivers/pci/pcie/pme/pcie_pme.c
===================================================================
@@ -53,12 +53,22 @@ static bool pcie_pme_disabled;
  */
 static bool pcie_pme_force_enable;
 
+/*
+ * If this switch is set, MSI will not be used for PCIe PME signaling.  This
+ * causes the PCIe port driver to use INTx interrupts only, but it turns out
+ * that using MSI for PCIe PME signaling doesn't play well with PCIe PME-based
+ * wake-up from system sleep states.
+ */
+bool pcie_pme_msi_disabled;
+
 static int __init pcie_pme_setup(char *str)
 {
 	if (!strcmp(str, "off"))
 		pcie_pme_disabled = true;
 	else if (!strcmp(str, "force"))
 		pcie_pme_force_enable = true;
+	else if (!strcmp(str, "nomsi"))
+		pcie_pme_msi_disabled = true;
 	return 1;
 }
 __setup("pcie_pme=", pcie_pme_setup);
@@ -73,7 +83,9 @@ __setup("pcie_pme=", pcie_pme_setup);
  */
 static bool pcie_pme_platform_setup(struct pcie_device *srv)
 {
-	return !pcie_pme_platform_notify(srv) || pcie_pme_force_enable;
+	if (!pcie_pme_platform_notify(srv))
+		return true;
+	return pcie_pme_force_enable;
 }
 
 struct pcie_pme_service_data {
drivers/pci/pcie/portdrv_core.c +10 -2
Index: linux-2.6/drivers/pci/pcie/portdrv_core.c
===================================================================
@@ -186,16 +186,24 @@ static int pcie_port_enable_msix(struct 
  */
 static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 {
-	int i, irq;
+	int i, irq = -1;
+
+	/* We have to use INTx if MSI cannot be used for PCIe PME. */
+	if ((mask & PCIE_PORT_SERVICE_PME) && pcie_pme_no_msi()) {
+		if (dev->pin)
+			irq = dev->irq;
+		goto no_msi;
+	}
 
 	/* Try to use MSI-X if supported */
 	if (!pcie_port_enable_msix(dev, irqs, mask))
 		return 0;
+
 	/* We're not going to use MSI-X, so try MSI and fall back to INTx */
-	irq = -1;
 	if (!pci_enable_msi(dev) || dev->pin)
 		irq = dev->irq;
 
+ no_msi:
 	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
 		irqs[i] = irq;
 	irqs[PCIE_PORT_SERVICE_VC_SHIFT] = -1;
drivers/pci/pcie/portdrv_pci.c +27 -0
Index: linux-2.6/drivers/pci/pcie/portdrv_pci.c
===================================================================
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/pcieport_if.h>
 #include <linux/aer.h>
+#include <linux/dmi.h>
 
 #include "portdrv.h"
 #include "aer/aerdrv.h"
@@ -274,10 +275,36 @@ static struct pci_driver pcie_portdriver
 	.driver.pm 	= PCIE_PORTDRV_PM_OPS,
 };
 
+static int __init dmi_pcie_pme_disable_msi(const struct dmi_system_id *d)
+{
+	pr_notice("%s detected: will not use MSI for PCIe PME signaling\n",
+			d->ident);
+	pcie_pme_disable_msi();
+	return 0;
+}
+
+static struct dmi_system_id __initdata pcie_portdrv_dmi_table[] = {
+	/*
+	 * Boxes that should not use MSI for PCIe PME signaling.
+	 */
+	{
+	 .callback = dmi_pcie_pme_disable_msi,
+	 .ident = "MSI Wind U-100",
+	 .matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR,
+		     		"MICRO-STAR INTERNATIONAL CO., LTD"),
+		     DMI_MATCH(DMI_PRODUCT_NAME, "U-100"),
+		     },
+	 },
+	 {}
+};
+
 static int __init pcie_portdrv_init(void)
 {
 	int retval;
 
+	dmi_check_system(pcie_portdrv_dmi_table);
+
 	retval = pcie_port_bus_register();
 	if (retval) {
 		printk(KERN_WARNING "PCIE: bus_register error: %d\n", retval);
drivers/pci/pcie/portdrv.h +17 -0
Index: linux-2.6/drivers/pci/pcie/portdrv.h
===================================================================
@@ -30,4 +30,21 @@ extern void pcie_port_device_remove(stru
 extern int __must_check pcie_port_bus_register(void);
 extern void pcie_port_bus_unregister(void);
 
+#ifdef CONFIG_PCIE_PME
+extern bool pcie_pme_msi_disabled;
+
+static inline void pcie_pme_disable_msi(void)
+{
+	pcie_pme_msi_disabled = true;
+}
+
+static inline bool pcie_pme_no_msi(void)
+{
+	return pcie_pme_msi_disabled;
+}
+#else /* !CONFIG_PCIE_PME */
+static inline void pcie_pme_disable_msi(void) {}
+static inline bool pcie_pme_no_msi(void) { return false; }
+#endif /* !CONFIG_PCIE_PME */
+
 #endif /* _PORTDRV_H_ */
Documentation/kernel-parameters.txt +2 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -1991,6 +1991,8 @@ and is between 256 and 4096 characters. 
 		force	Use native PCIe PME signaling even if the BIOS refuses
 			to allow the kernel to control the relevant PCIe config
 			registers.
+		nomsi	Do not use MSI for native PCIe PME signaling (this makes
+			all PCIe root ports use INTx for everything).
 
 	pcmv=		[HW,PCMCIA] BadgePAD 4
 
#15 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

Quoted 13 lines · On Sun, 27 Dec 2009 21:02:22 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Apparently, some machines may have problems with PCI run-time power management if MSIs are used for the hative PCIe PME signaling. In particular, on the MSI Wind U-100 PCIe PME interrupts are not generated by a PCIe root port after a resume from suspend to RAM, if the system wake-up was triggered by a PME from the device attached to this port. [It doesn't help to free the interrupt on suspend and request it back on resume, even if that is done along with disabling the MSI and re-enabling it, respectively.] However, if INTx interrupts are used for this purpose on the same machine, everything works just fine.

Quoted 8 lines

For this reason, add a kernel command line switch allowing one to request that MSIs be not used for the native PCIe PME signaling, introduce a DMI table allowing us to blacklist machines that need this switch to be set by default and put the MSI Wind U-100 into this table.

---

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#16 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

In reply to Rafael J. Wysocki
Quoted 13 lines · On Sun, 27 Dec 2009 21:02:22 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Apparently, some machines may have problems with PCI run-time power management if MSIs are used for the hative PCIe PME signaling. In particular, on the MSI Wind U-100 PCIe PME interrupts are not generated by a PCIe root port after a resume from suspend to RAM, if the system wake-up was triggered by a PME from the device attached to this port. [It doesn't help to free the interrupt on suspend and request it back on resume, even if that is done along with disabling the MSI and re-enabling it, respectively.] However, if INTx interrupts are used for this purpose on the same machine, everything works just fine.

Quoted 8 lines

For this reason, add a kernel command line switch allowing one to request that MSIs be not used for the native PCIe PME signaling, introduce a DMI table allowing us to blacklist machines that need this switch to be set by default and put the MSI Wind U-100 into this table.

---

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#17 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

Quoted 2 lines · Jesse Barnes

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Yeah, that was my concern. We debugged this a little on IRC and didn't get anywhere, though, so probably need to find someone who knows more about MSIs...

Signature · 1 line
Matthew Garrett | [email protected]
#18 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

In reply to Jesse Barnes
Quoted 2 lines · Jesse Barnes

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Yeah, that was my concern. We debugged this a little on IRC and didn't get anywhere, though, so probably need to find someone who knows more about MSIs...

Signature · 1 line
Matthew Garrett | [email protected]
#19 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

Quoted 8 lines · Matthew Garrett
Quoted 2 lines · Jesse Barnes

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Yeah, that was my concern. We debugged this a little on IRC and didn't get anywhere, though, so probably need to find someone who knows more about MSIs...

I'd rather say someone who knows more about ICH7. :-)

MSIs work just fine after resume for the other devices in the same box, so I'd say it's a chipset issue.

Rafael

reviewer
#20 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

Quoted 11 lines
Quoted 6 lines
Quoted 2 lines

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Yeah, that was my concern. We debugged this a little on IRC and didn't get anywhere, though, so probably need to find someone who knows more about MSIs...

I'd rather say someone who knows more about ICH7. :-)

MSIs work just fine after resume for the other devices in the same box, so I'd say it's a chipset issue.

Hmmm, do all the ICH7 boxes behave the same way?

Yesterday MSI announced a netbook running Suse/Moblin, so there is hope for more technical work w/ MSI to support Linux...

It is a bit worrysome to go out of the gate to need a DMI blacklist for relatively new hardware.

cheers,
Len Brown, Intel Open Source Technology Center

#21 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

Quoted 2 lines · Len Brown

Yesterday MSI announced a netbook running Suse/Moblin, so there is hope for more technical work w/ MSI to support Linux...

Yes, we are currently working on a patch for the existing msi driver to get some of the keys to work a bit better. It's still going through review back-and-forth between me and the developer, and will be posted soon.

thanks,

greg k-h

#22 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

In reply to Len Brown
Quoted 2 lines · Len Brown

Yesterday MSI announced a netbook running Suse/Moblin, so there is hope for more technical work w/ MSI to support Linux...

Yes, we are currently working on a patch for the existing msi driver to get some of the keys to work a bit better. It's still going through review back-and-forth between me and the developer, and will be posted soon.

thanks,

greg k-h

reviewer
#23 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

In reply to Rafael J. Wysocki
Quoted 11 lines
Quoted 6 lines
Quoted 2 lines

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Yeah, that was my concern. We debugged this a little on IRC and didn't get anywhere, though, so probably need to find someone who knows more about MSIs...

I'd rather say someone who knows more about ICH7. :-)

MSIs work just fine after resume for the other devices in the same box, so I'd say it's a chipset issue.

Hmmm, do all the ICH7 boxes behave the same way?

Yesterday MSI announced a netbook running Suse/Moblin, so there is hope for more technical work w/ MSI to support Linux...

It is a bit worrysome to go out of the gate to need a DMI blacklist for relatively new hardware.

cheers,
Len Brown, Intel Open Source Technology Center

#24 lore raw

Re: [PATCH 4/12] PCI PM: Make it possible to force using INTx for PCIe PME signaling

In reply to Matthew Garrett
Quoted 8 lines · Matthew Garrett
Quoted 2 lines · Jesse Barnes

This scares me a little... seems like we're not restoring some state on resume that's needed for MSIs to work? Looks fine though.

Yeah, that was my concern. We debugged this a little on IRC and didn't get anywhere, though, so probably need to find someone who knows more about MSIs...

I'd rather say someone who knows more about ICH7. :-)

MSIs work just fine after resume for the other devices in the same box, so I'd say it's a chipset issue.

Rafael

#25 lore raw

PATCH 5/12 ACPI: Add infrastructure for refcounting GPE consumers

In reply to Rafael J. Wysocki

From: Matthew Garrett <[email protected]>

ACPI GPEs may map to multiple devices. The current GPE interface only provides a mechanism for enabling and disabling GPEs, making it difficult to change the state of GPEs at runtime without extensive cooperation between devices. Add an API to allow devices to indicate whether or not they want their device's GPE to be enabled for both runtime and wakeup events.

---
drivers/acpi/acpica/aclocal.h | 2 +

 drivers/acpi/acpica/aclocal.h  |    2 
 drivers/acpi/acpica/evxfevnt.c |  161 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpixf.h          |    8 ++
 3 files changed, 171 insertions(+)
drivers/acpi/acpica/aclocal.h +2 -0
Index: linux-2.6/drivers/acpi/acpica/aclocal.h
===================================================================
@@ -426,6 +426,8 @@ struct acpi_gpe_event_info {
 	struct acpi_gpe_register_info *register_info;	/* Backpointer to register info */
 	u8 flags;		/* Misc info about this GPE */
 	u8 gpe_number;		/* This GPE */
+	u8 runtime_count;
+	u8 wakeup_count;
 };
 
 /* Information about a GPE register pair, one per each status/enable pair in an array */
drivers/acpi/acpica/evxfevnt.c +161 -0

167 lines not on this page · raw message

include/acpi/acpixf.h +8 -0
Index: linux-2.6/include/acpi/acpixf.h
===================================================================
@@ -283,6 +283,14 @@ acpi_status acpi_get_event_status(u32 ev
  */
 acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type);
 
+acpi_status acpi_ref_runtime_gpe(acpi_handle gpe_device, u32 gpe_number);
+
+acpi_status acpi_unref_runtime_gpe(acpi_handle gpe_device, u32 gpe_number);
+
+acpi_status acpi_ref_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number);
+
+acpi_status acpi_unref_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number);
+
 acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number);
 
 acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number);
#26 lore raw

PATCH 5/12 ACPI: Add infrastructure for refcounting GPE consumers

In reply to Rafael J. Wysocki

From: Matthew Garrett <[email protected]>

ACPI GPEs may map to multiple devices. The current GPE interface only provides a mechanism for enabling and disabling GPEs, making it difficult to change the state of GPEs at runtime without extensive cooperation between devices. Add an API to allow devices to indicate whether or not they want their device's GPE to be enabled for both runtime and wakeup events.

---
drivers/acpi/acpica/aclocal.h | 2 +

 drivers/acpi/acpica/aclocal.h  |    2 
 drivers/acpi/acpica/evxfevnt.c |  161 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpixf.h          |    8 ++
 3 files changed, 171 insertions(+)
drivers/acpi/acpica/aclocal.h +2 -0
Index: linux-2.6/drivers/acpi/acpica/aclocal.h
===================================================================
@@ -426,6 +426,8 @@ struct acpi_gpe_event_info {
 	struct acpi_gpe_register_info *register_info;	/* Backpointer to register info */
 	u8 flags;		/* Misc info about this GPE */
 	u8 gpe_number;		/* This GPE */
+	u8 runtime_count;
+	u8 wakeup_count;
 };
 
 /* Information about a GPE register pair, one per each status/enable pair in an array */
drivers/acpi/acpica/evxfevnt.c +161 -0

167 lines not on this page · raw message

include/acpi/acpixf.h +8 -0
Index: linux-2.6/include/acpi/acpixf.h
===================================================================
@@ -283,6 +283,14 @@ acpi_status acpi_get_event_status(u32 ev
  */
 acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type);
 
+acpi_status acpi_ref_runtime_gpe(acpi_handle gpe_device, u32 gpe_number);
+
+acpi_status acpi_unref_runtime_gpe(acpi_handle gpe_device, u32 gpe_number);
+
+acpi_status acpi_ref_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number);
+
+acpi_status acpi_unref_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number);
+
 acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number);
 
 acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number);
#27 lore raw

PATCH 6/12 ACPI: Add support for new refcounted GPE API to drivers

In reply to Rafael J. Wysocki

From: Matthew Garrett <[email protected]>

Add GPE refcounting support to ACPI drivers that need it. This will currently do little until the core is changed over to use the new behaviour.

Signed-off-by; Rafael J. Wysocki <[email protected]>
---
drivers/acpi/button.c | 12 ++++++++++++

 drivers/acpi/button.c |   12 ++++++++++++
 drivers/acpi/ec.c     |    4 +++-
 drivers/acpi/wakeup.c |    6 ++++++
 3 files changed, 21 insertions(+), 1 deletion(-)
drivers/acpi/button.c +12 -0
Index: linux-2.6/drivers/acpi/button.c
===================================================================
@@ -427,6 +427,10 @@ static int acpi_button_add(struct acpi_d
 				  ACPI_GPE_TYPE_WAKE_RUN);
 		acpi_enable_gpe(device->wakeup.gpe_device,
 				device->wakeup.gpe_number);
+		acpi_ref_runtime_gpe(device->wakeup.gpe_device,
+				     device->wakeup.gpe_number);
+		acpi_ref_wakeup_gpe(device->wakeup.gpe_device,
+				    device->wakeup.gpe_number);
 		device->wakeup.state.enabled = 1;
 	}
 
@@ -446,6 +450,14 @@ static int acpi_button_remove(struct acp
 {
 	struct acpi_button *button = acpi_driver_data(device);
 
+	if (device->wakeup.flags.valid) {
+		acpi_unref_runtime_gpe(device->wakeup.gpe_device,
+			     device->wakeup.gpe_number);
+		acpi_unref_wakeup_gpe(device->wakeup.gpe_device,
+				      device->wakeup.gpe_number);
+		device->wakeup.state.enabled = 0;
+	}
+
 	acpi_button_remove_fs(device);
 	input_unregister_device(button->input);
 	kfree(button);
drivers/acpi/ec.c +3 -1
Index: linux-2.6/drivers/acpi/ec.c
===================================================================
@@ -755,7 +755,7 @@ static int ec_install_handlers(struct ac
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 	acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME);
-	acpi_enable_gpe(NULL, ec->gpe);
+	acpi_ref_runtime_gpe(NULL, ec->gpe);
 	status = acpi_install_address_space_handler(ec->handle,
 						    ACPI_ADR_SPACE_EC,
 						    &acpi_ec_space_handler,
@@ -772,6 +772,7 @@ static int ec_install_handlers(struct ac
 		} else {
 			acpi_remove_gpe_handler(NULL, ec->gpe,
 				&acpi_ec_gpe_handler);
+                        acpi_unref_runtime_gpe(NULL, ec->gpe);
 			return -ENODEV;
 		}
 	}
@@ -782,6 +783,7 @@ static int ec_install_handlers(struct ac
 
 static void ec_remove_handlers(struct acpi_ec *ec)
 {
+	acpi_unref_runtime_gpe(NULL, ec->gpe);
 	if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
 				ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
 		pr_err(PREFIX "failed to remove space handler\n");
drivers/acpi/wakeup.c +6 -0
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
@@ -81,6 +81,8 @@ void acpi_enable_wakeup_device(u8 sleep_
 		if (!dev->wakeup.flags.run_wake)
 			acpi_enable_gpe(dev->wakeup.gpe_device,
 					dev->wakeup.gpe_number);
+		acpi_ref_wakeup_gpe(dev->wakeup.gpe_device,
+				    dev->wakeup.gpe_number);
 	}
 }
 
@@ -121,6 +123,8 @@ void acpi_disable_wakeup_device(u8 sleep
 			acpi_clear_gpe(dev->wakeup.gpe_device,
 				       dev->wakeup.gpe_number, ACPI_NOT_ISR);
 		}
+		acpi_unref_wakeup_gpe(dev->wakeup.gpe_device,
+				    dev->wakeup.gpe_number);
 	}
 }
 
@@ -141,6 +145,8 @@ int __init acpi_wakeup_device_init(void)
 				  ACPI_GPE_TYPE_WAKE_RUN);
 		acpi_enable_gpe(dev->wakeup.gpe_device,
 				dev->wakeup.gpe_number);
+		acpi_ref_wakeup_gpe(dev->wakeup.gpe_device,
+				    dev->wakeup.gpe_number);
 		dev->wakeup.state.enabled = 1;
 	}
 	mutex_unlock(&acpi_device_lock);
#28 lore raw

PATCH 7/12 ACPI: Remove old GPE API and transition code entirely to new one

In reply to Rafael J. Wysocki

From: Matthew Garrett <[email protected]>

Remove the old GPE type handling entirely, which gets rid of various quirks like the implicit disabling with GPE type setting. This requires a small amount of rework in order to ensure that non-wake GPEs are enabled by default to preserve existing behaviour.

---
drivers/acpi/acpica/acevents.h | 6 +--

 drivers/acpi/acpica/acevents.h |    6 -
 drivers/acpi/acpica/evgpe.c    |  147 +++--------------------------------------
 drivers/acpi/acpica/evgpeblk.c |   69 ++++++-------------
 drivers/acpi/acpica/evxface.c  |   14 ---
 drivers/acpi/acpica/evxfevnt.c |   48 -------------
 drivers/acpi/button.c          |    5 -
 drivers/acpi/ec.c              |    2 
 drivers/acpi/wakeup.c          |   55 ++-------------
 include/acpi/actypes.h         |   32 ++------
 9 files changed, 59 insertions(+), 319 deletions(-)
drivers/acpi/acpica/acevents.h +1 -5
Index: linux-2.6/drivers/acpi/acpica/acevents.h
===================================================================
@@ -76,8 +76,7 @@ acpi_ev_queue_notify_request(struct acpi
  * evgpe - GPE handling and dispatch
  */
 acpi_status
-acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info,
-				u8 type);
+acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info);
 
 acpi_status
 acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
@@ -122,9 +121,6 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_eve
 u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list);
 
 acpi_status
-acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type);
-
-acpi_status
 acpi_ev_check_for_wake_only_gpe(struct acpi_gpe_event_info *gpe_event_info);
 
 acpi_status acpi_ev_gpe_initialize(void);
drivers/acpi/acpica/evgpe.c +14 -133

194 lines not on this page · raw message

drivers/acpi/acpica/evgpeblk.c +22 -47

112 lines not on this page · raw message

drivers/acpi/acpica/evxface.c +0 -14
Index: linux-2.6/drivers/acpi/acpica/evxface.c
===================================================================
@@ -617,13 +617,6 @@ acpi_install_gpe_handler(acpi_handle gpe
 	handler->context = context;
 	handler->method_node = gpe_event_info->dispatch.method_node;
 
-	/* Disable the GPE before installing the handler */
-
-	status = acpi_ev_disable_gpe(gpe_event_info);
-	if (ACPI_FAILURE(status)) {
-		goto unlock_and_exit;
-	}
-
 	/* Install the handler */
 
 	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
@@ -707,13 +700,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_
 		goto unlock_and_exit;
 	}
 
-	/* Disable the GPE before removing the handler */
-
-	status = acpi_ev_disable_gpe(gpe_event_info);
-	if (ACPI_FAILURE(status)) {
-		goto unlock_and_exit;
-	}
-
 	/* Make sure all deferred tasks are completed */
 
 	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
drivers/acpi/acpica/evxfevnt.c +2 -46

66 lines not on this page · raw message

drivers/acpi/button.c +0 -5
Index: linux-2.6/drivers/acpi/button.c
===================================================================
@@ -422,11 +422,6 @@ static int acpi_button_add(struct acpi_d
 
 	if (device->wakeup.flags.valid) {
 		/* Button's GPE is run-wake GPE */
-		acpi_set_gpe_type(device->wakeup.gpe_device,
-				  device->wakeup.gpe_number,
-				  ACPI_GPE_TYPE_WAKE_RUN);
-		acpi_enable_gpe(device->wakeup.gpe_device,
-				device->wakeup.gpe_number);
 		acpi_ref_runtime_gpe(device->wakeup.gpe_device,
 				     device->wakeup.gpe_number);
 		acpi_ref_wakeup_gpe(device->wakeup.gpe_device,
drivers/acpi/ec.c +1 -1
Index: linux-2.6/drivers/acpi/ec.c
===================================================================
@@ -754,7 +754,7 @@ static int ec_install_handlers(struct ac
 				  &acpi_ec_gpe_handler, ec);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
-	acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME);
+
 	acpi_ref_runtime_gpe(NULL, ec->gpe);
 	status = acpi_install_address_space_handler(ec->handle,
 						    ACPI_ADR_SPACE_EC,
drivers/acpi/wakeup.c +9 -46

77 lines not on this page · raw message

include/acpi/actypes.h +10 -22
Index: linux-2.6/include/acpi/actypes.h
===================================================================
@@ -668,41 +668,29 @@ typedef u32 acpi_event_status;
 
 /*
  * GPE info flags - Per GPE
- * +-+-+-+---+---+-+
- * |7|6|5|4:3|2:1|0|
- * +-+-+-+---+---+-+
- *  | | |  |   |  |
- *  | | |  |   |  +--- Interrupt type: Edge or Level Triggered
- *  | | |  |   +--- Type: Wake-only, Runtime-only, or wake/runtime
+ * +-+-+-+---+-+-+-+
+ * |7|6|5|4:3|2|1|0|
+ * +-+-+-+---+-+-+-+
+ *  | | |  |  | | |
+ *  | | |  |  | | +--- Interrupt type: Edge or Level Triggered
+ *  | | |  |  | +--- GPE can wake the system
+ *  | | |  |  +--- Unused 
  *  | | |  +--- Type of dispatch -- to method, handler, or none
- *  | | +--- Enabled for runtime?
- *  | +--- Enabled for wake?
+ *  | | +--- Unused
+ *  | +--- Unused
  *  +--- Unused
  */
 #define ACPI_GPE_XRUPT_TYPE_MASK        (u8) 0x01
 #define ACPI_GPE_LEVEL_TRIGGERED        (u8) 0x01
 #define ACPI_GPE_EDGE_TRIGGERED         (u8) 0x00
 
-#define ACPI_GPE_TYPE_MASK              (u8) 0x06
-#define ACPI_GPE_TYPE_WAKE_RUN          (u8) 0x06
-#define ACPI_GPE_TYPE_WAKE              (u8) 0x02
-#define ACPI_GPE_TYPE_RUNTIME           (u8) 0x04	/* Default */
+#define ACPI_GPE_CAN_WAKE		(u8) 0x02
 
 #define ACPI_GPE_DISPATCH_MASK          (u8) 0x18
 #define ACPI_GPE_DISPATCH_HANDLER       (u8) 0x08
 #define ACPI_GPE_DISPATCH_METHOD        (u8) 0x10
 #define ACPI_GPE_DISPATCH_NOT_USED      (u8) 0x00	/* Default */
 
-#define ACPI_GPE_RUN_ENABLE_MASK        (u8) 0x20
-#define ACPI_GPE_RUN_ENABLED            (u8) 0x20
-#define ACPI_GPE_RUN_DISABLED           (u8) 0x00	/* Default */
-
-#define ACPI_GPE_WAKE_ENABLE_MASK       (u8) 0x40
-#define ACPI_GPE_WAKE_ENABLED           (u8) 0x40
-#define ACPI_GPE_WAKE_DISABLED          (u8) 0x00	/* Default */
-
-#define ACPI_GPE_ENABLE_MASK            (u8) 0x60	/* Both run/wake */
-
 /*
  * Flags for GPE and Lock interfaces
  */
#29 lore raw

PATCH 7/12 ACPI: Remove old GPE API and transition code entirely to new one

In reply to Rafael J. Wysocki

From: Matthew Garrett <[email protected]>

Remove the old GPE type handling entirely, which gets rid of various quirks like the implicit disabling with GPE type setting. This requires a small amount of rework in order to ensure that non-wake GPEs are enabled by default to preserve existing behaviour.

---
drivers/acpi/acpica/acevents.h | 6 +--

 drivers/acpi/acpica/acevents.h |    6 -
 drivers/acpi/acpica/evgpe.c    |  147 +++--------------------------------------
 drivers/acpi/acpica/evgpeblk.c |   69 ++++++-------------
 drivers/acpi/acpica/evxface.c  |   14 ---
 drivers/acpi/acpica/evxfevnt.c |   48 -------------
 drivers/acpi/button.c          |    5 -
 drivers/acpi/ec.c              |    2 
 drivers/acpi/wakeup.c          |   55 ++-------------
 include/acpi/actypes.h         |   32 ++------
 9 files changed, 59 insertions(+), 319 deletions(-)
drivers/acpi/acpica/acevents.h +1 -5
Index: linux-2.6/drivers/acpi/acpica/acevents.h
===================================================================
@@ -76,8 +76,7 @@ acpi_ev_queue_notify_request(struct acpi
  * evgpe - GPE handling and dispatch
  */
 acpi_status
-acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info,
-				u8 type);
+acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info);
 
 acpi_status
 acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
@@ -122,9 +121,6 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_eve
 u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list);
 
 acpi_status
-acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type);
-
-acpi_status
 acpi_ev_check_for_wake_only_gpe(struct acpi_gpe_event_info *gpe_event_info);
 
 acpi_status acpi_ev_gpe_initialize(void);
drivers/acpi/acpica/evgpe.c +14 -133

194 lines not on this page · raw message

drivers/acpi/acpica/evgpeblk.c +22 -47

112 lines not on this page · raw message

drivers/acpi/acpica/evxface.c +0 -14
Index: linux-2.6/drivers/acpi/acpica/evxface.c
===================================================================
@@ -617,13 +617,6 @@ acpi_install_gpe_handler(acpi_handle gpe
 	handler->context = context;
 	handler->method_node = gpe_event_info->dispatch.method_node;
 
-	/* Disable the GPE before installing the handler */
-
-	status = acpi_ev_disable_gpe(gpe_event_info);
-	if (ACPI_FAILURE(status)) {
-		goto unlock_and_exit;
-	}
-
 	/* Install the handler */
 
 	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
@@ -707,13 +700,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_
 		goto unlock_and_exit;
 	}
 
-	/* Disable the GPE before removing the handler */
-
-	status = acpi_ev_disable_gpe(gpe_event_info);
-	if (ACPI_FAILURE(status)) {
-		goto unlock_and_exit;
-	}
-
 	/* Make sure all deferred tasks are completed */
 
 	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
drivers/acpi/acpica/evxfevnt.c +2 -46

66 lines not on this page · raw message

drivers/acpi/button.c +0 -5
Index: linux-2.6/drivers/acpi/button.c
===================================================================
@@ -422,11 +422,6 @@ static int acpi_button_add(struct acpi_d
 
 	if (device->wakeup.flags.valid) {
 		/* Button's GPE is run-wake GPE */
-		acpi_set_gpe_type(device->wakeup.gpe_device,
-				  device->wakeup.gpe_number,
-				  ACPI_GPE_TYPE_WAKE_RUN);
-		acpi_enable_gpe(device->wakeup.gpe_device,
-				device->wakeup.gpe_number);
 		acpi_ref_runtime_gpe(device->wakeup.gpe_device,
 				     device->wakeup.gpe_number);
 		acpi_ref_wakeup_gpe(device->wakeup.gpe_device,
drivers/acpi/ec.c +1 -1
Index: linux-2.6/drivers/acpi/ec.c
===================================================================
@@ -754,7 +754,7 @@ static int ec_install_handlers(struct ac
 				  &acpi_ec_gpe_handler, ec);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
-	acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME);
+
 	acpi_ref_runtime_gpe(NULL, ec->gpe);
 	status = acpi_install_address_space_handler(ec->handle,
 						    ACPI_ADR_SPACE_EC,
drivers/acpi/wakeup.c +9 -46

77 lines not on this page · raw message

include/acpi/actypes.h +10 -22
Index: linux-2.6/include/acpi/actypes.h
===================================================================
@@ -668,41 +668,29 @@ typedef u32 acpi_event_status;
 
 /*
  * GPE info flags - Per GPE
- * +-+-+-+---+---+-+
- * |7|6|5|4:3|2:1|0|
- * +-+-+-+---+---+-+
- *  | | |  |   |  |
- *  | | |  |   |  +--- Interrupt type: Edge or Level Triggered
- *  | | |  |   +--- Type: Wake-only, Runtime-only, or wake/runtime
+ * +-+-+-+---+-+-+-+
+ * |7|6|5|4:3|2|1|0|
+ * +-+-+-+---+-+-+-+
+ *  | | |  |  | | |
+ *  | | |  |  | | +--- Interrupt type: Edge or Level Triggered
+ *  | | |  |  | +--- GPE can wake the system
+ *  | | |  |  +--- Unused 
  *  | | |  +--- Type of dispatch -- to method, handler, or none
- *  | | +--- Enabled for runtime?
- *  | +--- Enabled for wake?
+ *  | | +--- Unused
+ *  | +--- Unused
  *  +--- Unused
  */
 #define ACPI_GPE_XRUPT_TYPE_MASK        (u8) 0x01
 #define ACPI_GPE_LEVEL_TRIGGERED        (u8) 0x01
 #define ACPI_GPE_EDGE_TRIGGERED         (u8) 0x00
 
-#define ACPI_GPE_TYPE_MASK              (u8) 0x06
-#define ACPI_GPE_TYPE_WAKE_RUN          (u8) 0x06
-#define ACPI_GPE_TYPE_WAKE              (u8) 0x02
-#define ACPI_GPE_TYPE_RUNTIME           (u8) 0x04	/* Default */
+#define ACPI_GPE_CAN_WAKE		(u8) 0x02
 
 #define ACPI_GPE_DISPATCH_MASK          (u8) 0x18
 #define ACPI_GPE_DISPATCH_HANDLER       (u8) 0x08
 #define ACPI_GPE_DISPATCH_METHOD        (u8) 0x10
 #define ACPI_GPE_DISPATCH_NOT_USED      (u8) 0x00	/* Default */
 
-#define ACPI_GPE_RUN_ENABLE_MASK        (u8) 0x20
-#define ACPI_GPE_RUN_ENABLED            (u8) 0x20
-#define ACPI_GPE_RUN_DISABLED           (u8) 0x00	/* Default */
-
-#define ACPI_GPE_WAKE_ENABLE_MASK       (u8) 0x40
-#define ACPI_GPE_WAKE_ENABLED           (u8) 0x40
-#define ACPI_GPE_WAKE_DISABLED          (u8) 0x00	/* Default */
-
-#define ACPI_GPE_ENABLE_MASK            (u8) 0x60	/* Both run/wake */
-
 /*
  * Flags for GPE and Lock interfaces
  */
#30 lore raw

PATCH 8/12 ACPI / PM: Add more run-time wake-up fields

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Use the run_wake flag to mark all devices for which run-time wake-up events may be generated by the platform. Introduce a new wake-up flag, special, for marking devices that should be permanently enabled to generate run-time events. Also, introduce a reference counter for run-wake devices and a function that will initialize all of the run-time wake-up fields for given device.

---
 drivers/acpi/button.c   |    2 ++
 drivers/acpi/scan.c     |   37 +++++++++++++++++++++++++++----------
 drivers/acpi/wakeup.c   |    2 +-
 include/acpi/acpi_bus.h |    2 ++
 4 files changed, 32 insertions(+), 11 deletions(-)
drivers/acpi/wakeup.c +1 -1
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
@@ -106,7 +106,7 @@ int __init acpi_wakeup_device_init(void)
 						       struct acpi_device,
 						       wakeup_list);
 		/* In case user doesn't load button driver */
-		if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled)
+		if (!dev->wakeup.flags.special || dev->wakeup.state.enabled)
 			continue;
 		acpi_ref_wakeup_gpe(dev->wakeup.gpe_device,
 				    dev->wakeup.gpe_number);
include/acpi/acpi_bus.h +2 -0
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -242,6 +242,7 @@ struct acpi_device_perf {
 struct acpi_device_wakeup_flags {
 	u8 valid:1;		/* Can successfully enable wakeup? */
 	u8 run_wake:1;		/* Run-Wake GPE devices */
+	u8 special:1;		/* Run-wake devices that are always enabled */
 };
 
 struct acpi_device_wakeup_state {
@@ -256,6 +257,7 @@ struct acpi_device_wakeup {
 	struct acpi_device_wakeup_state state;
 	struct acpi_device_wakeup_flags flags;
 	int prepare_count;
+	int run_wake_count;
 };
 
 /* Device */
drivers/acpi/button.c +2 -0
Index: linux-2.6/drivers/acpi/button.c
===================================================================
@@ -426,6 +426,7 @@ static int acpi_button_add(struct acpi_d
 				     device->wakeup.gpe_number);
 		acpi_ref_wakeup_gpe(device->wakeup.gpe_device,
 				    device->wakeup.gpe_number);
+		device->wakeup.run_wake_count++;
 		device->wakeup.state.enabled = 1;
 	}
 
@@ -450,6 +451,7 @@ static int acpi_button_remove(struct acp
 			     device->wakeup.gpe_number);
 		acpi_unref_wakeup_gpe(device->wakeup.gpe_device,
 				      device->wakeup.gpe_number);
+		device->wakeup.run_wake_count--;
 		device->wakeup.state.enabled = 0;
 	}
 
drivers/acpi/scan.c +27 -10
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
@@ -741,19 +741,39 @@ acpi_bus_extract_wakeup_device_power_pac
 	return AE_OK;
 }
 
-static int acpi_bus_get_wakeup_device_flags(struct acpi_device *device)
+static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
 {
-	acpi_status status = 0;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *package = NULL;
-	int psw_error;
-
 	struct acpi_device_id button_device_ids[] = {
 		{"PNP0C0D", 0},
 		{"PNP0C0C", 0},
 		{"PNP0C0E", 0},
 		{"", 0},
 	};
+	acpi_status status;
+	acpi_event_status event_status;
+
+	device->wakeup.run_wake_count = 0;
+
+	/* Power button, Lid switch always enable wakeup */
+	if (!acpi_match_device_ids(device, button_device_ids)) {
+		device->wakeup.flags.run_wake = 1;
+		device->wakeup.flags.special = 1;
+		return;
+	}
+
+	status = acpi_get_gpe_status(NULL, device->wakeup.gpe_number,
+					ACPI_NOT_ISR, &event_status);
+	if (status == AE_OK)
+		device->wakeup.flags.run_wake =
+				!!(event_status & ACPI_EVENT_FLAG_HANDLE);
+}
+
+static int acpi_bus_get_wakeup_device_flags(struct acpi_device *device)
+{
+	acpi_status status = 0;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *package = NULL;
+	int psw_error;
 
 	/* _PRW */
 	status = acpi_evaluate_object(device->handle, "_PRW", NULL, &buffer);
@@ -773,6 +793,7 @@ static int acpi_bus_get_wakeup_device_fl
 
 	device->wakeup.flags.valid = 1;
 	device->wakeup.prepare_count = 0;
+	acpi_bus_set_run_wake_flags(device);
 	/* Call _PSW/_DSW object to disable its ability to wake the sleeping
 	 * system for the ACPI device with the _PRW object.
 	 * The _PSW object is depreciated in ACPI 3.0 and is replaced by _DSW.
@@ -784,10 +805,6 @@ static int acpi_bus_get_wakeup_device_fl
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				"error in _DSW or _PSW evaluation\n"));
 
-	/* Power button, Lid switch always enable wakeup */
-	if (!acpi_match_device_ids(device, button_device_ids))
-		device->wakeup.flags.run_wake = 1;
-
 end:
 	if (ACPI_FAILURE(status))
 		device->flags.wake_capable = 0;
reviewer Acked-by on v1 8/12
#31 lore raw

Re: [PATCH 8/12] ACPI / PM: Add more run-time wake-up fields

Quoted 4 lines

Use the run_wake flag to mark all devices for which run-time wake-up events may be generated by the platform. Introduce a new wake-up flag, special, for marking devices that should be permanently enabled to generate run-time events.

how about calling the flag "always_enabled" rather than "special"?

otherwise,

reviewer Acked-by on v1 8/12
#32 lore raw

Re: [PATCH 8/12] ACPI / PM: Add more run-time wake-up fields

In reply to Rafael J. Wysocki
Quoted 4 lines

Use the run_wake flag to mark all devices for which run-time wake-up events may be generated by the platform. Introduce a new wake-up flag, special, for marking devices that should be permanently enabled to generate run-time events.

how about calling the flag "always_enabled" rather than "special"?

otherwise,

#33 lore raw

Re: [PATCH 8/12] ACPI / PM: Add more run-time wake-up fields

Quoted 6 lines · Len Brown
Quoted 4 lines

Use the run_wake flag to mark all devices for which run-time wake-up events may be generated by the platform. Introduce a new wake-up flag, special, for marking devices that should be permanently enabled to generate run-time events.

how about calling the flag "always_enabled" rather than "special"?

OK, I'll do that.

Quoted 2 lines

otherwise,

Thanks!

#34 lore raw

Re: [PATCH 8/12] ACPI / PM: Add more run-time wake-up fields

In reply to Len Brown
Quoted 6 lines · Len Brown
Quoted 4 lines

Use the run_wake flag to mark all devices for which run-time wake-up events may be generated by the platform. Introduce a new wake-up flag, special, for marking devices that should be permanently enabled to generate run-time events.

how about calling the flag "always_enabled" rather than "special"?

OK, I'll do that.

Quoted 2 lines

otherwise,

Thanks!

#35 lore raw

PATCH 8/12 ACPI / PM: Add more run-time wake-up fields

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Use the run_wake flag to mark all devices for which run-time wake-up events may be generated by the platform. Introduce a new wake-up flag, special, for marking devices that should be permanently enabled to generate run-time events. Also, introduce a reference counter for run-wake devices and a function that will initialize all of the run-time wake-up fields for given device.

---
 drivers/acpi/button.c   |    2 ++
 drivers/acpi/scan.c     |   37 +++++++++++++++++++++++++++----------
 drivers/acpi/wakeup.c   |    2 +-
 include/acpi/acpi_bus.h |    2 ++
 4 files changed, 32 insertions(+), 11 deletions(-)
drivers/acpi/wakeup.c +1 -1
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
@@ -106,7 +106,7 @@ int __init acpi_wakeup_device_init(void)
 						       struct acpi_device,
 						       wakeup_list);
 		/* In case user doesn't load button driver */
-		if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled)
+		if (!dev->wakeup.flags.special || dev->wakeup.state.enabled)
 			continue;
 		acpi_ref_wakeup_gpe(dev->wakeup.gpe_device,
 				    dev->wakeup.gpe_number);
include/acpi/acpi_bus.h +2 -0
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -242,6 +242,7 @@ struct acpi_device_perf {
 struct acpi_device_wakeup_flags {
 	u8 valid:1;		/* Can successfully enable wakeup? */
 	u8 run_wake:1;		/* Run-Wake GPE devices */
+	u8 special:1;		/* Run-wake devices that are always enabled */
 };
 
 struct acpi_device_wakeup_state {
@@ -256,6 +257,7 @@ struct acpi_device_wakeup {
 	struct acpi_device_wakeup_state state;
 	struct acpi_device_wakeup_flags flags;
 	int prepare_count;
+	int run_wake_count;
 };
 
 /* Device */
drivers/acpi/button.c +2 -0
Index: linux-2.6/drivers/acpi/button.c
===================================================================
@@ -426,6 +426,7 @@ static int acpi_button_add(struct acpi_d
 				     device->wakeup.gpe_number);
 		acpi_ref_wakeup_gpe(device->wakeup.gpe_device,
 				    device->wakeup.gpe_number);
+		device->wakeup.run_wake_count++;
 		device->wakeup.state.enabled = 1;
 	}
 
@@ -450,6 +451,7 @@ static int acpi_button_remove(struct acp
 			     device->wakeup.gpe_number);
 		acpi_unref_wakeup_gpe(device->wakeup.gpe_device,
 				      device->wakeup.gpe_number);
+		device->wakeup.run_wake_count--;
 		device->wakeup.state.enabled = 0;
 	}
 
drivers/acpi/scan.c +27 -10
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
@@ -741,19 +741,39 @@ acpi_bus_extract_wakeup_device_power_pac
 	return AE_OK;
 }
 
-static int acpi_bus_get_wakeup_device_flags(struct acpi_device *device)
+static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
 {
-	acpi_status status = 0;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *package = NULL;
-	int psw_error;
-
 	struct acpi_device_id button_device_ids[] = {
 		{"PNP0C0D", 0},
 		{"PNP0C0C", 0},
 		{"PNP0C0E", 0},
 		{"", 0},
 	};
+	acpi_status status;
+	acpi_event_status event_status;
+
+	device->wakeup.run_wake_count = 0;
+
+	/* Power button, Lid switch always enable wakeup */
+	if (!acpi_match_device_ids(device, button_device_ids)) {
+		device->wakeup.flags.run_wake = 1;
+		device->wakeup.flags.special = 1;
+		return;
+	}
+
+	status = acpi_get_gpe_status(NULL, device->wakeup.gpe_number,
+					ACPI_NOT_ISR, &event_status);
+	if (status == AE_OK)
+		device->wakeup.flags.run_wake =
+				!!(event_status & ACPI_EVENT_FLAG_HANDLE);
+}
+
+static int acpi_bus_get_wakeup_device_flags(struct acpi_device *device)
+{
+	acpi_status status = 0;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *package = NULL;
+	int psw_error;
 
 	/* _PRW */
 	status = acpi_evaluate_object(device->handle, "_PRW", NULL, &buffer);
@@ -773,6 +793,7 @@ static int acpi_bus_get_wakeup_device_fl
 
 	device->wakeup.flags.valid = 1;
 	device->wakeup.prepare_count = 0;
+	acpi_bus_set_run_wake_flags(device);
 	/* Call _PSW/_DSW object to disable its ability to wake the sleeping
 	 * system for the ACPI device with the _PRW object.
 	 * The _PSW object is depreciated in ACPI 3.0 and is replaced by _DSW.
@@ -784,10 +805,6 @@ static int acpi_bus_get_wakeup_device_fl
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				"error in _DSW or _PSW evaluation\n"));
 
-	/* Power button, Lid switch always enable wakeup */
-	if (!acpi_match_device_ids(device, button_device_ids))
-		device->wakeup.flags.run_wake = 1;
-
 end:
 	if (ACPI_FAILURE(status))
 		device->flags.wake_capable = 0;
#36 lore raw

PATCH 9/12 ACPI / PM: Introduce acpi_pm_wakeup_power()

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

PCI run-time power management generally requires us to set up wake-up power for devices that are supposed to generate wake-up events, but at the moment we only have acpi_pm_device_sleep_wake() allowing us only to do that during system-wide power transitions. For this reason introduce another helper function, acpi_pm_wakeup_power(), allowing us to enable/disable wake-up power for a device at any time and rework acpi_pm_device_sleep_wake() to use it too.

---
 drivers/acpi/sleep.c    |   22 ++++++++++++++++------
 include/acpi/acpi_bus.h |   11 ++++++++---
 2 files changed, 24 insertions(+), 9 deletions(-)
kernel/power/Kconfig +5 -0
Index: linux-2.6/kernel/power/Kconfig
===================================================================
@@ -236,3 +236,8 @@ config PM_RUNTIME
 	  and the bus type drivers of the buses the devices are on are
 	  responsible for the actual handling of the autosuspend requests and
 	  wake-up events.
+
+config PM_WAKEUP
+	bool
+	depends on SUSPEND || HIBERNATION || PM_RUNTIME
+	default y
drivers/acpi/sleep.c +16 -6
Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
@@ -634,7 +634,7 @@ int acpi_suspend(u32 acpi_state)
 	return -EINVAL;
 }
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM_WAKEUP
 /**
  *	acpi_pm_device_sleep_state - return preferred power state of ACPI device
  *		in the system sleep state given by %acpi_target_sleep_state
@@ -720,6 +720,18 @@ int acpi_pm_device_sleep_state(struct de
 }
 
 /**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{
+	return enable ?
+		acpi_enable_wakeup_device_power(dev, acpi_target_sleep_state) :
+		acpi_disable_wakeup_device_power(dev);
+}
+
+/**
  *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
  *                                  capability of given device
  *	@dev: device to handle
@@ -740,16 +752,14 @@ int acpi_pm_device_sleep_wake(struct dev
 		return -ENODEV;
 	}
 
-	error = enable ?
-		acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
-		acpi_disable_wakeup_device_power(adev);
+	error = acpi_pm_wakeup_power(adev, enable);
 	if (!error)
-		dev_info(dev, "wake-up capability %s by ACPI\n",
+		dev_info(dev, "wake-up power %s by ACPI\n",
 				enable ? "enabled" : "disabled");
 
 	return error;
 }
-#endif
+#endif /* CONFIG_PM_WAKEUP */
 
 static void acpi_power_off_prepare(void)
 {
include/acpi/acpi_bus.h +8 -3
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -388,21 +388,26 @@ acpi_handle acpi_get_pci_rootbridge_hand
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
 #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM_WAKEUP
 int acpi_pm_device_sleep_state(struct device *, int *);
+int acpi_pm_wakeup_power(struct acpi_device *, bool);
 int acpi_pm_device_sleep_wake(struct device *, bool);
-#else /* !CONFIG_PM_SLEEP */
+#else /* !CONFIG_PM_WAKEUP */
 static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
 {
 	if (p)
 		*p = ACPI_STATE_D0;
 	return ACPI_STATE_D3;
 }
+static inline int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{
+	return -ENODEV;
+}
 static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
 {
 	return -ENODEV;
 }
-#endif /* !CONFIG_PM_SLEEP */
+#endif /* !CONFIG_PM_WAKEUP */
 
 #endif				/* CONFIG_ACPI */
 
#37 lore raw

PATCH 9/12 ACPI / PM: Introduce acpi_pm_wakeup_power()

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

PCI run-time power management generally requires us to set up wake-up power for devices that are supposed to generate wake-up events, but at the moment we only have acpi_pm_device_sleep_wake() allowing us only to do that during system-wide power transitions. For this reason introduce another helper function, acpi_pm_wakeup_power(), allowing us to enable/disable wake-up power for a device at any time and rework acpi_pm_device_sleep_wake() to use it too.

---
 drivers/acpi/sleep.c    |   22 ++++++++++++++++------
 include/acpi/acpi_bus.h |   11 ++++++++---
 2 files changed, 24 insertions(+), 9 deletions(-)
kernel/power/Kconfig +5 -0
Index: linux-2.6/kernel/power/Kconfig
===================================================================
@@ -236,3 +236,8 @@ config PM_RUNTIME
 	  and the bus type drivers of the buses the devices are on are
 	  responsible for the actual handling of the autosuspend requests and
 	  wake-up events.
+
+config PM_WAKEUP
+	bool
+	depends on SUSPEND || HIBERNATION || PM_RUNTIME
+	default y
drivers/acpi/sleep.c +16 -6
Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
@@ -634,7 +634,7 @@ int acpi_suspend(u32 acpi_state)
 	return -EINVAL;
 }
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM_WAKEUP
 /**
  *	acpi_pm_device_sleep_state - return preferred power state of ACPI device
  *		in the system sleep state given by %acpi_target_sleep_state
@@ -720,6 +720,18 @@ int acpi_pm_device_sleep_state(struct de
 }
 
 /**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{
+	return enable ?
+		acpi_enable_wakeup_device_power(dev, acpi_target_sleep_state) :
+		acpi_disable_wakeup_device_power(dev);
+}
+
+/**
  *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
  *                                  capability of given device
  *	@dev: device to handle
@@ -740,16 +752,14 @@ int acpi_pm_device_sleep_wake(struct dev
 		return -ENODEV;
 	}
 
-	error = enable ?
-		acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
-		acpi_disable_wakeup_device_power(adev);
+	error = acpi_pm_wakeup_power(adev, enable);
 	if (!error)
-		dev_info(dev, "wake-up capability %s by ACPI\n",
+		dev_info(dev, "wake-up power %s by ACPI\n",
 				enable ? "enabled" : "disabled");
 
 	return error;
 }
-#endif
+#endif /* CONFIG_PM_WAKEUP */
 
 static void acpi_power_off_prepare(void)
 {
include/acpi/acpi_bus.h +8 -3
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -388,21 +388,26 @@ acpi_handle acpi_get_pci_rootbridge_hand
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
 #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM_WAKEUP
 int acpi_pm_device_sleep_state(struct device *, int *);
+int acpi_pm_wakeup_power(struct acpi_device *, bool);
 int acpi_pm_device_sleep_wake(struct device *, bool);
-#else /* !CONFIG_PM_SLEEP */
+#else /* !CONFIG_PM_WAKEUP */
 static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
 {
 	if (p)
 		*p = ACPI_STATE_D0;
 	return ACPI_STATE_D3;
 }
+static inline int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{
+	return -ENODEV;
+}
 static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
 {
 	return -ENODEV;
 }
-#endif /* !CONFIG_PM_SLEEP */
+#endif /* !CONFIG_PM_WAKEUP */
 
 #endif				/* CONFIG_ACPI */
 
#38 lore raw

Re: [PATCH 9/12] ACPI / PM: Introduce acpi_pm_wakeup_power()

drivers/acpi/sleep.c int acpi_pm_device_sleep_state(struct de · line 728

Quoted 8 lines · On Sun, 27 Dec 2009 21:06:26 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

/**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */

drivers/acpi/sleep.c +2 -0
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{

I know we've got these all over now, but functions that just take a bool are generally hard to read when you just look at the call site. If it was called "acpi_pm_set_wakeup_power" and then took an on/off enum it would be really easy to see, from the callsite, what was going on.

It's a fairly minor complaint, but it's something that's always bugged me about the PCI PM code in particular.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#39 lore raw

Re: [PATCH 9/12] ACPI / PM: Introduce acpi_pm_wakeup_power()

Quoted 17 lines · Jesse Barnes

drivers/acpi/sleep.c int acpi_pm_device_sleep_state(struct de · line 728

Quoted 8 lines · On Sun, 27 Dec 2009 21:06:26 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

/**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */

drivers/acpi/sleep.c +2 -0
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{

I know we've got these all over now, but functions that just take a bool are generally hard to read when you just look at the call site. If it was called "acpi_pm_set_wakeup_power" and then took an on/off enum it would be really easy to see, from the callsite, what was going on.

Quoted 2 lines

It's a fairly minor complaint, but it's something that's always bugged me about the PCI PM code in particular.

Well, in this particular case acpi_pm_wakeup_power() uses a bool, because acpi_pm_device_sleep_wake() (which is a caller of it) does. IMO it won't be logical to use something else just here.

Also, as you noticed above, this follows a convention used not only in the PCI PM, but generally in the core PM code. Although we could change this convention, I'm not really sure that would be worth the effort.

Rafael

#40 lore raw

Re: [PATCH 9/12] ACPI / PM: Introduce acpi_pm_wakeup_power()

Quoted 21 lines · Rafael J. Wysocki
Quoted 17 lines · Jesse Barnes

drivers/acpi/sleep.c int acpi_pm_device_sleep_state(struct de · line 728

Quoted 8 lines · On Sun, 27 Dec 2009 21:06:26 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

/**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */

drivers/acpi/sleep.c +2 -0
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{

I know we've got these all over now, but functions that just take a bool are generally hard to read when you just look at the call site. If it was called "acpi_pm_set_wakeup_power" and then took an on/off enum it would be really easy to see, from the callsite, what was going on.

Quoted 2 lines

It's a fairly minor complaint, but it's something that's always bugged me about the PCI PM code in particular.

Quoted 7 lines

Well, in this particular case acpi_pm_wakeup_power() uses a bool, because acpi_pm_device_sleep_wake() (which is a caller of it) does. IMO it won't be logical to use something else just here.

Also, as you noticed above, this follows a convention used not only in the PCI PM, but generally in the core PM code. Although we could change this convention, I'm not really sure that would be worth the effort.

That said, it looks like we can drop this (ie. [9/12]) patch altogether if the next one is changed slightly.

I'll post the modified [10/12] shortly.

Rafael

#41 lore raw

Re: [PATCH 9/12] ACPI / PM: Introduce acpi_pm_wakeup_power()

In reply to Rafael J. Wysocki
Quoted 21 lines · Rafael J. Wysocki
Quoted 17 lines · Jesse Barnes

drivers/acpi/sleep.c int acpi_pm_device_sleep_state(struct de · line 728

Quoted 8 lines · On Sun, 27 Dec 2009 21:06:26 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

/**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */

drivers/acpi/sleep.c +2 -0
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{

I know we've got these all over now, but functions that just take a bool are generally hard to read when you just look at the call site. If it was called "acpi_pm_set_wakeup_power" and then took an on/off enum it would be really easy to see, from the callsite, what was going on.

Quoted 2 lines

It's a fairly minor complaint, but it's something that's always bugged me about the PCI PM code in particular.

Quoted 7 lines

Well, in this particular case acpi_pm_wakeup_power() uses a bool, because acpi_pm_device_sleep_wake() (which is a caller of it) does. IMO it won't be logical to use something else just here.

Also, as you noticed above, this follows a convention used not only in the PCI PM, but generally in the core PM code. Although we could change this convention, I'm not really sure that would be worth the effort.

That said, it looks like we can drop this (ie. [9/12]) patch altogether if the next one is changed slightly.

I'll post the modified [10/12] shortly.

Rafael

#42 lore raw

Re: [PATCH 9/12] ACPI / PM: Introduce acpi_pm_wakeup_power()

In reply to Jesse Barnes
Quoted 17 lines · Jesse Barnes

drivers/acpi/sleep.c int acpi_pm_device_sleep_state(struct de · line 728

Quoted 8 lines · On Sun, 27 Dec 2009 21:06:26 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

/**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */

drivers/acpi/sleep.c +2 -0
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{

I know we've got these all over now, but functions that just take a bool are generally hard to read when you just look at the call site. If it was called "acpi_pm_set_wakeup_power" and then took an on/off enum it would be really easy to see, from the callsite, what was going on.

Quoted 2 lines

It's a fairly minor complaint, but it's something that's always bugged me about the PCI PM code in particular.

Well, in this particular case acpi_pm_wakeup_power() uses a bool, because acpi_pm_device_sleep_wake() (which is a caller of it) does. IMO it won't be logical to use something else just here.

Also, as you noticed above, this follows a convention used not only in the PCI PM, but generally in the core PM code. Although we could change this convention, I'm not really sure that would be worth the effort.

Rafael

#43 lore raw

Re: [PATCH 9/12] ACPI / PM: Introduce acpi_pm_wakeup_power()

In reply to Rafael J. Wysocki

drivers/acpi/sleep.c int acpi_pm_device_sleep_state(struct de · line 728

Quoted 8 lines · On Sun, 27 Dec 2009 21:06:26 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

/**
+ * acpi_pm_wakeup_power - Enable/disable device wake-up power.
+ * @dev: ACPI device to handle.
+ * @enable: Whether to enable or disable the wake-up power of the device.
+ */

drivers/acpi/sleep.c +2 -0
+int acpi_pm_wakeup_power(struct acpi_device *dev, bool enable)
+{

I know we've got these all over now, but functions that just take a bool are generally hard to read when you just look at the call site. If it was called "acpi_pm_set_wakeup_power" and then took an on/off enum it would be really easy to see, from the callsite, what was going on.

It's a fairly minor complaint, but it's something that's always bugged me about the PCI PM code in particular.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#44 lore raw

PATCH 10/12 PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 6)

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Developed in cooperation with Matthew Garrett <[email protected]>.

---
 drivers/acpi/pci_bind.c            |   14 +
 drivers/acpi/pci_root.c            |    8 
 drivers/pci/hotplug/acpiphp_glue.c |   23 --
 drivers/pci/pci-acpi.c             |  341 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c                  |   67 +++++++
 drivers/pci/pci.h                  |    7 
 include/acpi/acpi_bus.h            |    1 
 include/linux/pci-acpi.h           |   10 +
 include/linux/pci.h                |    1 
 9 files changed, 457 insertions(+), 15 deletions(-)
drivers/pci/pci.h +7 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -35,6 +35,10 @@ int pci_probe_reset_function(struct pci_
  *
  * @sleep_wake: enables/disables the system wake up capability of given device
  *
+ * @run_wake: enables/disables the platform to generate run-time wake-up events
+ *		for given device (the device's wake-up capability has to be
+ *		enabled by @sleep_wake for this feature to work)
+ *
  * If given platform is generally capable of power managing PCI devices, all of
  * these callbacks are mandatory.
  */
@@ -44,12 +48,15 @@ struct pci_platform_pm_ops {
 	pci_power_t (*choose_state)(struct pci_dev *dev);
 	bool (*can_wakeup)(struct pci_dev *dev);
 	int (*sleep_wake)(struct pci_dev *dev, bool enable);
+	int (*run_wake)(struct pci_dev *dev, bool enable);
 };
 
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
 extern bool pci_check_pme_status(struct pci_dev *dev);
+extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
+extern void pci_pme_wakeup_bus(struct pci_bus *bus);
 extern void pci_pm_init(struct pci_dev *dev);
 extern void platform_pci_wakeup_init(struct pci_dev *dev);
 extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
drivers/pci/pci-acpi.c +341 -0

361 lines not on this page · raw message

drivers/pci/pci.c +67 -0

91 lines not on this page · raw message

include/linux/pci-acpi.h +10 -0
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
@@ -11,6 +11,16 @@
 #include <linux/acpi.h>
 
 #ifdef CONFIG_ACPI
+extern acpi_status pci_acpi_add_hp_notifier(acpi_handle handle,
+					     acpi_notify_handler handler,
+					     void *context);
+extern acpi_status pci_acpi_remove_hp_notifier(acpi_handle handle,
+					       acpi_notify_handler handler);
+extern acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
+					     struct pci_dev *pci_dev,
+					     struct pci_bus *pci_bus);
+extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
drivers/acpi/pci_bind.c +13 -1
Index: linux-2.6/drivers/acpi/pci_bind.c
===================================================================
@@ -26,7 +26,9 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/acpi.h>
+#include <linux/pm_runtime.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
@@ -38,7 +40,13 @@ static int acpi_pci_unbind(struct acpi_d
 	struct pci_dev *dev;
 
 	dev = acpi_get_pci_dev(device->handle);
-	if (!dev || !dev->subordinate)
+	if (!dev)
+		goto out;
+
+	device_set_run_wake(&dev->dev, false);
+	pci_acpi_remove_pm_notifier(device);
+
+	if (!dev->subordinate)
 		goto out;
 
 	acpi_pci_irq_del_prt(dev->subordinate);
@@ -62,6 +70,10 @@ static int acpi_pci_bind(struct acpi_dev
 	if (!dev)
 		return 0;
 
+	pci_acpi_add_pm_notifier(device, dev, dev->subordinate);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(&dev->dev, true);
+
 	/*
 	 * Install the 'bind' function to facilitate callbacks for
 	 * children of the P2P bridge.
drivers/pci/hotplug/acpiphp_glue.c +9 -14
Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
@@ -236,8 +236,7 @@ register_slot(acpi_handle handle, u32 lv
 
 	/* install notify handler */
 	if (!(newfunc->flags & FUNC_HAS_DCK)) {
-		status = acpi_install_notify_handler(handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(handle,
 					     handle_hotplug_event_func,
 					     newfunc);
 
@@ -288,14 +287,12 @@ static void init_bridge_misc(struct acpi
 	/* install notify handler */
 	if (bridge->type != BRIDGE_TYPE_HOST) {
 		if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-			status = acpi_remove_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+			status = pci_acpi_remove_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func);
 			if (ACPI_FAILURE(status))
 				err("failed to remove notify handler\n");
 		}
-		status = acpi_install_notify_handler(bridge->handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->handle,
 					     handle_hotplug_event_bridge,
 					     bridge);
 
@@ -505,15 +502,14 @@ static void cleanup_bridge(struct acpiph
 	acpi_status status;
 	acpi_handle handle = bridge->handle;
 
-	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+	status = pci_acpi_remove_hp_notifier(handle,
 					    handle_hotplug_event_bridge);
 	if (ACPI_FAILURE(status))
 		err("failed to remove notify handler\n");
 
 	if ((bridge->type != BRIDGE_TYPE_HOST) &&
 	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
-		status = acpi_install_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func,
 						bridge->func);
 		if (ACPI_FAILURE(status))
@@ -529,8 +525,7 @@ static void cleanup_bridge(struct acpiph
 				unregister_dock_notifier(&func->nb);
 			}
 			if (!(func->flags & FUNC_HAS_DCK)) {
-				status = acpi_remove_notify_handler(func->handle,
-						ACPI_SYSTEM_NOTIFY,
+				status = pci_acpi_remove_hp_notifier(func->handle,
 						handle_hotplug_event_func);
 				if (ACPI_FAILURE(status))
 					err("failed to remove notify handler\n");
@@ -592,7 +587,7 @@ static void remove_bridge(acpi_handle ha
 	if (bridge)
 		cleanup_bridge(bridge);
 	else
-		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+		pci_acpi_remove_hp_notifier(handle,
 					   handle_hotplug_event_bridge);
 }
 
@@ -1278,8 +1273,8 @@ find_root_bridges(acpi_handle handle, u3
 	int *count = (int *)context;
 
 	if (acpi_is_root_bridge(handle)) {
-		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				handle_hotplug_event_bridge, NULL);
+		pci_acpi_add_hp_notifier(handle,
+					handle_hotplug_event_bridge, NULL);
 			(*count)++;
 	}
 	return AE_OK ;
include/linux/pci.h +1 -0
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -752,6 +752,7 @@ int pci_wake_from_d3(struct pci_dev *dev
 pci_power_t pci_target_state(struct pci_dev *dev);
 int pci_prepare_to_sleep(struct pci_dev *dev);
 int pci_back_from_sleep(struct pci_dev *dev);
+bool pci_dev_run_wake(struct pci_dev *dev);
 
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
include/acpi/acpi_bus.h +1 -0
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -282,6 +282,7 @@ struct acpi_device {
 	struct device dev;
 	struct acpi_bus_ops bus_ops;	/* workaround for different code path for hotplug */
 	enum acpi_bus_removal_type removal_type;	/* indicate for different removal type */
+	void *bus_data;
 };
 
 static inline void *acpi_driver_data(struct acpi_device *d)
drivers/acpi/pci_root.c +8 -0
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
@@ -30,6 +30,7 @@
 #include <linux/proc_fs.h>
 #include <linux/spinlock.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/acpi.h>
@@ -528,6 +529,10 @@ static int __devinit acpi_pci_root_add(s
 	if (flags != base_flags)
 		acpi_pci_osc_support(root, flags);
 
+	pci_acpi_add_pm_notifier(device, NULL, root->bus);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(root->bus->bridge, true);
+
 	return 0;
 
 end:
@@ -549,6 +554,9 @@ static int acpi_pci_root_remove(struct a
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 
+	device_set_run_wake(root->bus->bridge, false);
+	pci_acpi_remove_pm_notifier(device);
+
 	kfree(root);
 	return 0;
 }
#45 lore raw

PATCH 10/12 PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 6)

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Developed in cooperation with Matthew Garrett <[email protected]>.

---
 drivers/acpi/pci_bind.c            |   14 +
 drivers/acpi/pci_root.c            |    8 
 drivers/pci/hotplug/acpiphp_glue.c |   23 --
 drivers/pci/pci-acpi.c             |  341 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c                  |   67 +++++++
 drivers/pci/pci.h                  |    7 
 include/acpi/acpi_bus.h            |    1 
 include/linux/pci-acpi.h           |   10 +
 include/linux/pci.h                |    1 
 9 files changed, 457 insertions(+), 15 deletions(-)
drivers/pci/pci.h +7 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -35,6 +35,10 @@ int pci_probe_reset_function(struct pci_
  *
  * @sleep_wake: enables/disables the system wake up capability of given device
  *
+ * @run_wake: enables/disables the platform to generate run-time wake-up events
+ *		for given device (the device's wake-up capability has to be
+ *		enabled by @sleep_wake for this feature to work)
+ *
  * If given platform is generally capable of power managing PCI devices, all of
  * these callbacks are mandatory.
  */
@@ -44,12 +48,15 @@ struct pci_platform_pm_ops {
 	pci_power_t (*choose_state)(struct pci_dev *dev);
 	bool (*can_wakeup)(struct pci_dev *dev);
 	int (*sleep_wake)(struct pci_dev *dev, bool enable);
+	int (*run_wake)(struct pci_dev *dev, bool enable);
 };
 
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
 extern bool pci_check_pme_status(struct pci_dev *dev);
+extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
+extern void pci_pme_wakeup_bus(struct pci_bus *bus);
 extern void pci_pm_init(struct pci_dev *dev);
 extern void platform_pci_wakeup_init(struct pci_dev *dev);
 extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
drivers/pci/pci-acpi.c +341 -0

361 lines not on this page · raw message

drivers/pci/pci.c +67 -0

91 lines not on this page · raw message

include/linux/pci-acpi.h +10 -0
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
@@ -11,6 +11,16 @@
 #include <linux/acpi.h>
 
 #ifdef CONFIG_ACPI
+extern acpi_status pci_acpi_add_hp_notifier(acpi_handle handle,
+					     acpi_notify_handler handler,
+					     void *context);
+extern acpi_status pci_acpi_remove_hp_notifier(acpi_handle handle,
+					       acpi_notify_handler handler);
+extern acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
+					     struct pci_dev *pci_dev,
+					     struct pci_bus *pci_bus);
+extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
drivers/acpi/pci_bind.c +13 -1
Index: linux-2.6/drivers/acpi/pci_bind.c
===================================================================
@@ -26,7 +26,9 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/acpi.h>
+#include <linux/pm_runtime.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
@@ -38,7 +40,13 @@ static int acpi_pci_unbind(struct acpi_d
 	struct pci_dev *dev;
 
 	dev = acpi_get_pci_dev(device->handle);
-	if (!dev || !dev->subordinate)
+	if (!dev)
+		goto out;
+
+	device_set_run_wake(&dev->dev, false);
+	pci_acpi_remove_pm_notifier(device);
+
+	if (!dev->subordinate)
 		goto out;
 
 	acpi_pci_irq_del_prt(dev->subordinate);
@@ -62,6 +70,10 @@ static int acpi_pci_bind(struct acpi_dev
 	if (!dev)
 		return 0;
 
+	pci_acpi_add_pm_notifier(device, dev, dev->subordinate);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(&dev->dev, true);
+
 	/*
 	 * Install the 'bind' function to facilitate callbacks for
 	 * children of the P2P bridge.
drivers/pci/hotplug/acpiphp_glue.c +9 -14
Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
@@ -236,8 +236,7 @@ register_slot(acpi_handle handle, u32 lv
 
 	/* install notify handler */
 	if (!(newfunc->flags & FUNC_HAS_DCK)) {
-		status = acpi_install_notify_handler(handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(handle,
 					     handle_hotplug_event_func,
 					     newfunc);
 
@@ -288,14 +287,12 @@ static void init_bridge_misc(struct acpi
 	/* install notify handler */
 	if (bridge->type != BRIDGE_TYPE_HOST) {
 		if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-			status = acpi_remove_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+			status = pci_acpi_remove_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func);
 			if (ACPI_FAILURE(status))
 				err("failed to remove notify handler\n");
 		}
-		status = acpi_install_notify_handler(bridge->handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->handle,
 					     handle_hotplug_event_bridge,
 					     bridge);
 
@@ -505,15 +502,14 @@ static void cleanup_bridge(struct acpiph
 	acpi_status status;
 	acpi_handle handle = bridge->handle;
 
-	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+	status = pci_acpi_remove_hp_notifier(handle,
 					    handle_hotplug_event_bridge);
 	if (ACPI_FAILURE(status))
 		err("failed to remove notify handler\n");
 
 	if ((bridge->type != BRIDGE_TYPE_HOST) &&
 	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
-		status = acpi_install_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func,
 						bridge->func);
 		if (ACPI_FAILURE(status))
@@ -529,8 +525,7 @@ static void cleanup_bridge(struct acpiph
 				unregister_dock_notifier(&func->nb);
 			}
 			if (!(func->flags & FUNC_HAS_DCK)) {
-				status = acpi_remove_notify_handler(func->handle,
-						ACPI_SYSTEM_NOTIFY,
+				status = pci_acpi_remove_hp_notifier(func->handle,
 						handle_hotplug_event_func);
 				if (ACPI_FAILURE(status))
 					err("failed to remove notify handler\n");
@@ -592,7 +587,7 @@ static void remove_bridge(acpi_handle ha
 	if (bridge)
 		cleanup_bridge(bridge);
 	else
-		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+		pci_acpi_remove_hp_notifier(handle,
 					   handle_hotplug_event_bridge);
 }
 
@@ -1278,8 +1273,8 @@ find_root_bridges(acpi_handle handle, u3
 	int *count = (int *)context;
 
 	if (acpi_is_root_bridge(handle)) {
-		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				handle_hotplug_event_bridge, NULL);
+		pci_acpi_add_hp_notifier(handle,
+					handle_hotplug_event_bridge, NULL);
 			(*count)++;
 	}
 	return AE_OK ;
include/linux/pci.h +1 -0
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -752,6 +752,7 @@ int pci_wake_from_d3(struct pci_dev *dev
 pci_power_t pci_target_state(struct pci_dev *dev);
 int pci_prepare_to_sleep(struct pci_dev *dev);
 int pci_back_from_sleep(struct pci_dev *dev);
+bool pci_dev_run_wake(struct pci_dev *dev);
 
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
include/acpi/acpi_bus.h +1 -0
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -282,6 +282,7 @@ struct acpi_device {
 	struct device dev;
 	struct acpi_bus_ops bus_ops;	/* workaround for different code path for hotplug */
 	enum acpi_bus_removal_type removal_type;	/* indicate for different removal type */
+	void *bus_data;
 };
 
 static inline void *acpi_driver_data(struct acpi_device *d)
drivers/acpi/pci_root.c +8 -0
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
@@ -30,6 +30,7 @@
 #include <linux/proc_fs.h>
 #include <linux/spinlock.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/acpi.h>
@@ -528,6 +529,10 @@ static int __devinit acpi_pci_root_add(s
 	if (flags != base_flags)
 		acpi_pci_osc_support(root, flags);
 
+	pci_acpi_add_pm_notifier(device, NULL, root->bus);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(root->bus->bridge, true);
+
 	return 0;
 
 end:
@@ -549,6 +554,9 @@ static int acpi_pci_root_remove(struct a
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 
+	device_set_run_wake(root->bus->bridge, false);
+	pci_acpi_remove_pm_notifier(device);
+
 	kfree(root);
 	return 0;
 }
#46 lore raw

Re: [PATCH 10/12] PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 6)

Quoted 34 lines · On Sun, 27 Dec 2009 21:07:36 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Quoted 3 lines

Developed in cooperation with Matthew Garrett <[email protected]>.

Looks pretty good, though my comment about the bool taking functions applies here too.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#47 lore raw

Re: [PATCH 10/12] PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 6)

Quoted 37 lines · Jesse Barnes
Quoted 34 lines · On Sun, 27 Dec 2009 21:07:36 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Quoted 6 lines
Quoted 3 lines

Developed in cooperation with Matthew Garrett <[email protected]>.

Looks pretty good, though my comment about the bool taking functions applies here too.

Hmm, I'm not sure if we can avoid that in this particular patch.

Since the .run_wake() callback in struct pci_platform_pm_ops takes the second argument (the fact that it's a bool doesn't really matter here IMhO), we need to propagate it all the way up to acpi_dev_run_wake_enable() (renamed to acpi_dev_run_wake() in the patch below) in pci-acpi.c.

Anyway, as I said in the previous message, patch [9/12] can be dropped if the $subject one is modified slightly, so the updated [10/12] is appended below.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 7)

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Developed in cooperation with Matthew Garrett <[email protected]>.

---
 drivers/acpi/internal.h            |    2 
 drivers/acpi/pci_bind.c            |   14 +
 drivers/acpi/pci_root.c            |    8 
 drivers/pci/hotplug/acpiphp_glue.c |   23 --
 drivers/pci/pci-acpi.c             |  341 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c                  |   67 +++++++
 drivers/pci/pci.h                  |    7 
 include/acpi/acpi_bus.h            |    4 
 include/linux/pci-acpi.h           |   10 +
 include/linux/pci.h                |    1 
 10 files changed, 460 insertions(+), 17 deletions(-)
drivers/pci/pci.h +7 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -35,6 +35,10 @@ int pci_probe_reset_function(struct pci_
  *
  * @sleep_wake: enables/disables the system wake up capability of given device
  *
+ * @run_wake: enables/disables the platform to generate run-time wake-up events
+ *		for given device (the device's wake-up capability has to be
+ *		enabled by @sleep_wake for this feature to work)
+ *
  * If given platform is generally capable of power managing PCI devices, all of
  * these callbacks are mandatory.
  */
@@ -44,12 +48,15 @@ struct pci_platform_pm_ops {
 	pci_power_t (*choose_state)(struct pci_dev *dev);
 	bool (*can_wakeup)(struct pci_dev *dev);
 	int (*sleep_wake)(struct pci_dev *dev, bool enable);
+	int (*run_wake)(struct pci_dev *dev, bool enable);
 };
 
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
 extern bool pci_check_pme_status(struct pci_dev *dev);
+extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
+extern void pci_pme_wakeup_bus(struct pci_bus *bus);
 extern void pci_pm_init(struct pci_dev *dev);
 extern void platform_pci_wakeup_init(struct pci_dev *dev);
 extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
drivers/pci/pci-acpi.c +341 -0

361 lines not on this page · raw message

drivers/pci/pci.c +67 -0

91 lines not on this page · raw message

include/linux/pci-acpi.h +10 -0
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
@@ -11,6 +11,16 @@
 #include <linux/acpi.h>
 
 #ifdef CONFIG_ACPI
+extern acpi_status pci_acpi_add_hp_notifier(acpi_handle handle,
+					     acpi_notify_handler handler,
+					     void *context);
+extern acpi_status pci_acpi_remove_hp_notifier(acpi_handle handle,
+					       acpi_notify_handler handler);
+extern acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
+					     struct pci_dev *pci_dev,
+					     struct pci_bus *pci_bus);
+extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
drivers/acpi/pci_bind.c +13 -1
Index: linux-2.6/drivers/acpi/pci_bind.c
===================================================================
@@ -26,7 +26,9 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/acpi.h>
+#include <linux/pm_runtime.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
@@ -38,7 +40,13 @@ static int acpi_pci_unbind(struct acpi_d
 	struct pci_dev *dev;
 
 	dev = acpi_get_pci_dev(device->handle);
-	if (!dev || !dev->subordinate)
+	if (!dev)
+		goto out;
+
+	device_set_run_wake(&dev->dev, false);
+	pci_acpi_remove_pm_notifier(device);
+
+	if (!dev->subordinate)
 		goto out;
 
 	acpi_pci_irq_del_prt(dev->subordinate);
@@ -62,6 +70,10 @@ static int acpi_pci_bind(struct acpi_dev
 	if (!dev)
 		return 0;
 
+	pci_acpi_add_pm_notifier(device, dev, dev->subordinate);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(&dev->dev, true);
+
 	/*
 	 * Install the 'bind' function to facilitate callbacks for
 	 * children of the P2P bridge.
drivers/pci/hotplug/acpiphp_glue.c +9 -14
Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
@@ -236,8 +236,7 @@ register_slot(acpi_handle handle, u32 lv
 
 	/* install notify handler */
 	if (!(newfunc->flags & FUNC_HAS_DCK)) {
-		status = acpi_install_notify_handler(handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(handle,
 					     handle_hotplug_event_func,
 					     newfunc);
 
@@ -288,14 +287,12 @@ static void init_bridge_misc(struct acpi
 	/* install notify handler */
 	if (bridge->type != BRIDGE_TYPE_HOST) {
 		if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-			status = acpi_remove_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+			status = pci_acpi_remove_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func);
 			if (ACPI_FAILURE(status))
 				err("failed to remove notify handler\n");
 		}
-		status = acpi_install_notify_handler(bridge->handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->handle,
 					     handle_hotplug_event_bridge,
 					     bridge);
 
@@ -505,15 +502,14 @@ static void cleanup_bridge(struct acpiph
 	acpi_status status;
 	acpi_handle handle = bridge->handle;
 
-	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+	status = pci_acpi_remove_hp_notifier(handle,
 					    handle_hotplug_event_bridge);
 	if (ACPI_FAILURE(status))
 		err("failed to remove notify handler\n");
 
 	if ((bridge->type != BRIDGE_TYPE_HOST) &&
 	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
-		status = acpi_install_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func,
 						bridge->func);
 		if (ACPI_FAILURE(status))
@@ -529,8 +525,7 @@ static void cleanup_bridge(struct acpiph
 				unregister_dock_notifier(&func->nb);
 			}
 			if (!(func->flags & FUNC_HAS_DCK)) {
-				status = acpi_remove_notify_handler(func->handle,
-						ACPI_SYSTEM_NOTIFY,
+				status = pci_acpi_remove_hp_notifier(func->handle,
 						handle_hotplug_event_func);
 				if (ACPI_FAILURE(status))
 					err("failed to remove notify handler\n");
@@ -592,7 +587,7 @@ static void remove_bridge(acpi_handle ha
 	if (bridge)
 		cleanup_bridge(bridge);
 	else
-		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+		pci_acpi_remove_hp_notifier(handle,
 					   handle_hotplug_event_bridge);
 }
 
@@ -1278,8 +1273,8 @@ find_root_bridges(acpi_handle handle, u3
 	int *count = (int *)context;
 
 	if (acpi_is_root_bridge(handle)) {
-		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				handle_hotplug_event_bridge, NULL);
+		pci_acpi_add_hp_notifier(handle,
+					handle_hotplug_event_bridge, NULL);
 			(*count)++;
 	}
 	return AE_OK ;
include/linux/pci.h +1 -0
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -756,6 +756,7 @@ int pci_wake_from_d3(struct pci_dev *dev
 pci_power_t pci_target_state(struct pci_dev *dev);
 int pci_prepare_to_sleep(struct pci_dev *dev);
 int pci_back_from_sleep(struct pci_dev *dev);
+bool pci_dev_run_wake(struct pci_dev *dev);
 
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
include/acpi/acpi_bus.h +4 -0
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -282,6 +282,7 @@ struct acpi_device {
 	struct device dev;
 	struct acpi_bus_ops bus_ops;	/* workaround for different code path for hotplug */
 	enum acpi_bus_removal_type removal_type;	/* indicate for different removal type */
+	void *bus_data;
 };
 
 static inline void *acpi_driver_data(struct acpi_device *d)
@@ -388,6 +389,9 @@ acpi_handle acpi_get_pci_rootbridge_hand
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
 #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
 
+int acpi_enable_wakeup_device_power(struct acpi_device *dev, int state);
+int acpi_disable_wakeup_device_power(struct acpi_device *dev);
+
 #ifdef CONFIG_PM_SLEEP
 int acpi_pm_device_sleep_state(struct device *, int *);
 int acpi_pm_device_sleep_wake(struct device *, bool);
drivers/acpi/pci_root.c +8 -0
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
@@ -30,6 +30,7 @@
 #include <linux/proc_fs.h>
 #include <linux/spinlock.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/acpi.h>
@@ -528,6 +529,10 @@ static int __devinit acpi_pci_root_add(s
 	if (flags != base_flags)
 		acpi_pci_osc_support(root, flags);
 
+	pci_acpi_add_pm_notifier(device, NULL, root->bus);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(root->bus->bridge, true);
+
 	return 0;
 
 end:
@@ -549,6 +554,9 @@ static int acpi_pci_root_remove(struct a
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 
+	device_set_run_wake(root->bus->bridge, false);
+	pci_acpi_remove_pm_notifier(device);
+
 	kfree(root);
 	return 0;
 }
drivers/acpi/internal.h +0 -2
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
@@ -36,8 +36,6 @@ static inline int acpi_debug_init(void) 
 int acpi_power_init(void);
 int acpi_device_sleep_wake(struct acpi_device *dev,
                            int enable, int sleep_state, int dev_state);
-int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state);
-int acpi_disable_wakeup_device_power(struct acpi_device *dev);
 int acpi_power_get_inferred_state(struct acpi_device *device);
 int acpi_power_transition(struct acpi_device *device, int state);
 extern int acpi_power_nocheck;
#48 lore raw

Re: [PATCH 10/12] PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 6)

In reply to Jesse Barnes
Quoted 37 lines · Jesse Barnes
Quoted 34 lines · On Sun, 27 Dec 2009 21:07:36 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Quoted 6 lines
Quoted 3 lines

Developed in cooperation with Matthew Garrett <[email protected]>.

Looks pretty good, though my comment about the bool taking functions applies here too.

Hmm, I'm not sure if we can avoid that in this particular patch.

Since the .run_wake() callback in struct pci_platform_pm_ops takes the second argument (the fact that it's a bool doesn't really matter here IMhO), we need to propagate it all the way up to acpi_dev_run_wake_enable() (renamed to acpi_dev_run_wake() in the patch below) in pci-acpi.c.

Anyway, as I said in the previous message, patch [9/12] can be dropped if the $subject one is modified slightly, so the updated [10/12] is appended below.

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 7)

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Developed in cooperation with Matthew Garrett <[email protected]>.

---
 drivers/acpi/internal.h            |    2 
 drivers/acpi/pci_bind.c            |   14 +
 drivers/acpi/pci_root.c            |    8 
 drivers/pci/hotplug/acpiphp_glue.c |   23 --
 drivers/pci/pci-acpi.c             |  341 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c                  |   67 +++++++
 drivers/pci/pci.h                  |    7 
 include/acpi/acpi_bus.h            |    4 
 include/linux/pci-acpi.h           |   10 +
 include/linux/pci.h                |    1 
 10 files changed, 460 insertions(+), 17 deletions(-)
drivers/pci/pci.h +7 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -35,6 +35,10 @@ int pci_probe_reset_function(struct pci_
  *
  * @sleep_wake: enables/disables the system wake up capability of given device
  *
+ * @run_wake: enables/disables the platform to generate run-time wake-up events
+ *		for given device (the device's wake-up capability has to be
+ *		enabled by @sleep_wake for this feature to work)
+ *
  * If given platform is generally capable of power managing PCI devices, all of
  * these callbacks are mandatory.
  */
@@ -44,12 +48,15 @@ struct pci_platform_pm_ops {
 	pci_power_t (*choose_state)(struct pci_dev *dev);
 	bool (*can_wakeup)(struct pci_dev *dev);
 	int (*sleep_wake)(struct pci_dev *dev, bool enable);
+	int (*run_wake)(struct pci_dev *dev, bool enable);
 };
 
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
 extern bool pci_check_pme_status(struct pci_dev *dev);
+extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
+extern void pci_pme_wakeup_bus(struct pci_bus *bus);
 extern void pci_pm_init(struct pci_dev *dev);
 extern void platform_pci_wakeup_init(struct pci_dev *dev);
 extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
drivers/pci/pci-acpi.c +341 -0

361 lines not on this page · raw message

drivers/pci/pci.c +67 -0

91 lines not on this page · raw message

include/linux/pci-acpi.h +10 -0
Index: linux-2.6/include/linux/pci-acpi.h
===================================================================
@@ -11,6 +11,16 @@
 #include <linux/acpi.h>
 
 #ifdef CONFIG_ACPI
+extern acpi_status pci_acpi_add_hp_notifier(acpi_handle handle,
+					     acpi_notify_handler handler,
+					     void *context);
+extern acpi_status pci_acpi_remove_hp_notifier(acpi_handle handle,
+					       acpi_notify_handler handler);
+extern acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
+					     struct pci_dev *pci_dev,
+					     struct pci_bus *pci_bus);
+extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
drivers/acpi/pci_bind.c +13 -1
Index: linux-2.6/drivers/acpi/pci_bind.c
===================================================================
@@ -26,7 +26,9 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/pci.h>
+#include <linux/pci-acpi.h>
 #include <linux/acpi.h>
+#include <linux/pm_runtime.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
@@ -38,7 +40,13 @@ static int acpi_pci_unbind(struct acpi_d
 	struct pci_dev *dev;
 
 	dev = acpi_get_pci_dev(device->handle);
-	if (!dev || !dev->subordinate)
+	if (!dev)
+		goto out;
+
+	device_set_run_wake(&dev->dev, false);
+	pci_acpi_remove_pm_notifier(device);
+
+	if (!dev->subordinate)
 		goto out;
 
 	acpi_pci_irq_del_prt(dev->subordinate);
@@ -62,6 +70,10 @@ static int acpi_pci_bind(struct acpi_dev
 	if (!dev)
 		return 0;
 
+	pci_acpi_add_pm_notifier(device, dev, dev->subordinate);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(&dev->dev, true);
+
 	/*
 	 * Install the 'bind' function to facilitate callbacks for
 	 * children of the P2P bridge.
drivers/pci/hotplug/acpiphp_glue.c +9 -14
Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
@@ -236,8 +236,7 @@ register_slot(acpi_handle handle, u32 lv
 
 	/* install notify handler */
 	if (!(newfunc->flags & FUNC_HAS_DCK)) {
-		status = acpi_install_notify_handler(handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(handle,
 					     handle_hotplug_event_func,
 					     newfunc);
 
@@ -288,14 +287,12 @@ static void init_bridge_misc(struct acpi
 	/* install notify handler */
 	if (bridge->type != BRIDGE_TYPE_HOST) {
 		if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) {
-			status = acpi_remove_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+			status = pci_acpi_remove_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func);
 			if (ACPI_FAILURE(status))
 				err("failed to remove notify handler\n");
 		}
-		status = acpi_install_notify_handler(bridge->handle,
-					     ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->handle,
 					     handle_hotplug_event_bridge,
 					     bridge);
 
@@ -505,15 +502,14 @@ static void cleanup_bridge(struct acpiph
 	acpi_status status;
 	acpi_handle handle = bridge->handle;
 
-	status = acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+	status = pci_acpi_remove_hp_notifier(handle,
 					    handle_hotplug_event_bridge);
 	if (ACPI_FAILURE(status))
 		err("failed to remove notify handler\n");
 
 	if ((bridge->type != BRIDGE_TYPE_HOST) &&
 	    ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func)) {
-		status = acpi_install_notify_handler(bridge->func->handle,
-						ACPI_SYSTEM_NOTIFY,
+		status = pci_acpi_add_hp_notifier(bridge->func->handle,
 						handle_hotplug_event_func,
 						bridge->func);
 		if (ACPI_FAILURE(status))
@@ -529,8 +525,7 @@ static void cleanup_bridge(struct acpiph
 				unregister_dock_notifier(&func->nb);
 			}
 			if (!(func->flags & FUNC_HAS_DCK)) {
-				status = acpi_remove_notify_handler(func->handle,
-						ACPI_SYSTEM_NOTIFY,
+				status = pci_acpi_remove_hp_notifier(func->handle,
 						handle_hotplug_event_func);
 				if (ACPI_FAILURE(status))
 					err("failed to remove notify handler\n");
@@ -592,7 +587,7 @@ static void remove_bridge(acpi_handle ha
 	if (bridge)
 		cleanup_bridge(bridge);
 	else
-		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+		pci_acpi_remove_hp_notifier(handle,
 					   handle_hotplug_event_bridge);
 }
 
@@ -1278,8 +1273,8 @@ find_root_bridges(acpi_handle handle, u3
 	int *count = (int *)context;
 
 	if (acpi_is_root_bridge(handle)) {
-		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-				handle_hotplug_event_bridge, NULL);
+		pci_acpi_add_hp_notifier(handle,
+					handle_hotplug_event_bridge, NULL);
 			(*count)++;
 	}
 	return AE_OK ;
include/linux/pci.h +1 -0
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -756,6 +756,7 @@ int pci_wake_from_d3(struct pci_dev *dev
 pci_power_t pci_target_state(struct pci_dev *dev);
 int pci_prepare_to_sleep(struct pci_dev *dev);
 int pci_back_from_sleep(struct pci_dev *dev);
+bool pci_dev_run_wake(struct pci_dev *dev);
 
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
include/acpi/acpi_bus.h +4 -0
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
@@ -282,6 +282,7 @@ struct acpi_device {
 	struct device dev;
 	struct acpi_bus_ops bus_ops;	/* workaround for different code path for hotplug */
 	enum acpi_bus_removal_type removal_type;	/* indicate for different removal type */
+	void *bus_data;
 };
 
 static inline void *acpi_driver_data(struct acpi_device *d)
@@ -388,6 +389,9 @@ acpi_handle acpi_get_pci_rootbridge_hand
 struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle);
 #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
 
+int acpi_enable_wakeup_device_power(struct acpi_device *dev, int state);
+int acpi_disable_wakeup_device_power(struct acpi_device *dev);
+
 #ifdef CONFIG_PM_SLEEP
 int acpi_pm_device_sleep_state(struct device *, int *);
 int acpi_pm_device_sleep_wake(struct device *, bool);
drivers/acpi/pci_root.c +8 -0
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
@@ -30,6 +30,7 @@
 #include <linux/proc_fs.h>
 #include <linux/spinlock.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/acpi.h>
@@ -528,6 +529,10 @@ static int __devinit acpi_pci_root_add(s
 	if (flags != base_flags)
 		acpi_pci_osc_support(root, flags);
 
+	pci_acpi_add_pm_notifier(device, NULL, root->bus);
+	if (device->wakeup.flags.run_wake)
+		device_set_run_wake(root->bus->bridge, true);
+
 	return 0;
 
 end:
@@ -549,6 +554,9 @@ static int acpi_pci_root_remove(struct a
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 
+	device_set_run_wake(root->bus->bridge, false);
+	pci_acpi_remove_pm_notifier(device);
+
 	kfree(root);
 	return 0;
 }
drivers/acpi/internal.h +0 -2
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
@@ -36,8 +36,6 @@ static inline int acpi_debug_init(void) 
 int acpi_power_init(void);
 int acpi_device_sleep_wake(struct acpi_device *dev,
                            int enable, int sleep_state, int dev_state);
-int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state);
-int acpi_disable_wakeup_device_power(struct acpi_device *dev);
 int acpi_power_get_inferred_state(struct acpi_device *device);
 int acpi_power_transition(struct acpi_device *device, int state);
 extern int acpi_power_nocheck;
#49 lore raw

Re: [PATCH 10/12] PCI / ACPI / PM: Platform support for PCI PME wake-up (rev. 6)

In reply to Rafael J. Wysocki
Quoted 34 lines · On Sun, 27 Dec 2009 21:07:36 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Although the majority of PCI devices can generate PMEs that in principle may be used to wake up devices suspended at run time, platform support is generally necessary to convert PMEs into wake-up events that can be delivered to the kernel. If ACPI is used for this purpose, a PME generated by a PCI device will trigger the ACPI GPE associated with the device to generate an ACPI wake-up event that we can set up a handler for, provided that everything is configured correctly.

Unfortunately, the subset of PCI devices that have GPEs associated with them is quite limited and the other devices have to rely on the GPEs associated with their upstream bridges and, possibly, the root bridge to generate ACPI wake-up events in response to PMEs from them. Moreover, ACPI-based PCI hotplug also uses ACPI notify handlers that in general may conflict with the PM notify handlers, unless this issue is specifically taken care of.

Add ACPI platform support for PCI PME wake-up:
o Add a framework making is possible to use ACPI system notify handlers for both PM and hotplug at the same time and to take the wake-up GPE sharing into account.
o Add new PCI platform callback ->run_wake() to struct pci_platform_pm_ops allowing us to enable/disable the platform to generate wake-up events for given device. Implemet this callback for the ACPI platform.
o Define ACPI wake-up handlers for PCI devices and PCI buses and make the PCI-ACPI binding code register wake-up notifiers for devices associated with wake-up GPEs.
o Add function pci_dev_run_wake() which can be used by PCI drivers to check if given device is capable of generating wake-up events at run time.

Quoted 3 lines

Developed in cooperation with Matthew Garrett <[email protected]>.

Looks pretty good, though my comment about the bool taking functions applies here too.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#50 lore raw

PATCH 11/12 PCI PM: Run-time callbacks for PCI bus type (rev. 2)

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Introduce run-time PM callbacks for the PCI bus type. Make the new callbacks work in analogy with the existing system sleep PM callbacks, so that the drivers already converted to struct dev_pm_ops can use their suspend and resume routines for run-time PM without modifications.

---
 drivers/pci/pci-driver.c |  105 ++++++++++++++++++++++++++++++++++++++++++++---
 drivers/pci/pci.c        |   43 ++++++++++++++++---
 drivers/pci/pci.h        |    1 
 include/linux/pci.h      |    9 +++-
 kernel/power/Kconfig     |    5 ++
 5 files changed, 151 insertions(+), 12 deletions(-)
kernel/power/Kconfig +5 -0
Index: linux-2.6/kernel/power/Kconfig
===================================================================
@@ -241,3 +241,8 @@ config PM_WAKEUP
 	bool
 	depends on SUSPEND || HIBERNATION || PM_RUNTIME
 	default y
+
+config PM_OPS
+	bool
+	depends on PM_SLEEP || PM_RUNTIME
+	default y
drivers/pci/pci-driver.c +100 -5

154 lines not on this page · raw message

drivers/pci/pci.c +37 -6

89 lines not on this page · raw message

include/linux/pci.h +8 -1
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -747,13 +747,20 @@ int pci_set_power_state(struct pci_dev *
 pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
 bool pci_pme_capable(struct pci_dev *dev, pci_power_t state);
 void pci_pme_active(struct pci_dev *dev, bool enable);
-int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable);
+int __pci_enable_wake(struct pci_dev *dev, pci_power_t state,
+		      bool runtime, bool enable);
 int pci_wake_from_d3(struct pci_dev *dev, bool enable);
 pci_power_t pci_target_state(struct pci_dev *dev);
 int pci_prepare_to_sleep(struct pci_dev *dev);
 int pci_back_from_sleep(struct pci_dev *dev);
 bool pci_dev_run_wake(struct pci_dev *dev);
 
+static inline int pci_enable_wake(struct pci_dev *dev, pci_power_t state,
+				  bool enable)
+{
+	return __pci_enable_wake(dev, state, false, enable);
+}
+
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
 #ifdef CONFIG_HOTPLUG
drivers/pci/pci.h +1 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -55,6 +55,7 @@ extern int pci_set_platform_pm(struct pc
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
 extern bool pci_check_pme_status(struct pci_dev *dev);
+extern int pci_finish_runtime_suspend(struct pci_dev *dev);
 extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
 extern void pci_pme_wakeup_bus(struct pci_bus *bus);
 extern void pci_pm_init(struct pci_dev *dev);
#51 lore raw

Re: [PATCH 11/12] PCI PM: Run-time callbacks for PCI bus type (rev. 2)

Quoted 10 lines · On Sun, 27 Dec 2009 21:08:24 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Introduce run-time PM callbacks for the PCI bus type. Make the new callbacks work in analogy with the existing system sleep PM callbacks, so that the drivers already converted to struct dev_pm_ops can use their suspend and resume routines for run-time PM without modifications.

---

Looks good.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#52 lore raw

Re: [PATCH 11/12] PCI PM: Run-time callbacks for PCI bus type (rev. 2)

In reply to Rafael J. Wysocki
Quoted 10 lines · On Sun, 27 Dec 2009 21:08:24 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

From: Rafael J. Wysocki <[email protected]>

Introduce run-time PM callbacks for the PCI bus type. Make the new callbacks work in analogy with the existing system sleep PM callbacks, so that the drivers already converted to struct dev_pm_ops can use their suspend and resume routines for run-time PM without modifications.

---

Looks good.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#53 lore raw

PATCH 11/12 PCI PM: Run-time callbacks for PCI bus type (rev. 2)

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Introduce run-time PM callbacks for the PCI bus type. Make the new callbacks work in analogy with the existing system sleep PM callbacks, so that the drivers already converted to struct dev_pm_ops can use their suspend and resume routines for run-time PM without modifications.

---
 drivers/pci/pci-driver.c |  105 ++++++++++++++++++++++++++++++++++++++++++++---
 drivers/pci/pci.c        |   43 ++++++++++++++++---
 drivers/pci/pci.h        |    1 
 include/linux/pci.h      |    9 +++-
 kernel/power/Kconfig     |    5 ++
 5 files changed, 151 insertions(+), 12 deletions(-)
kernel/power/Kconfig +5 -0
Index: linux-2.6/kernel/power/Kconfig
===================================================================
@@ -241,3 +241,8 @@ config PM_WAKEUP
 	bool
 	depends on SUSPEND || HIBERNATION || PM_RUNTIME
 	default y
+
+config PM_OPS
+	bool
+	depends on PM_SLEEP || PM_RUNTIME
+	default y
drivers/pci/pci-driver.c +100 -5

154 lines not on this page · raw message

drivers/pci/pci.c +37 -6

89 lines not on this page · raw message

include/linux/pci.h +8 -1
Index: linux-2.6/include/linux/pci.h
===================================================================
@@ -747,13 +747,20 @@ int pci_set_power_state(struct pci_dev *
 pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
 bool pci_pme_capable(struct pci_dev *dev, pci_power_t state);
 void pci_pme_active(struct pci_dev *dev, bool enable);
-int pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable);
+int __pci_enable_wake(struct pci_dev *dev, pci_power_t state,
+		      bool runtime, bool enable);
 int pci_wake_from_d3(struct pci_dev *dev, bool enable);
 pci_power_t pci_target_state(struct pci_dev *dev);
 int pci_prepare_to_sleep(struct pci_dev *dev);
 int pci_back_from_sleep(struct pci_dev *dev);
 bool pci_dev_run_wake(struct pci_dev *dev);
 
+static inline int pci_enable_wake(struct pci_dev *dev, pci_power_t state,
+				  bool enable)
+{
+	return __pci_enable_wake(dev, state, false, enable);
+}
+
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
 #ifdef CONFIG_HOTPLUG
drivers/pci/pci.h +1 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -55,6 +55,7 @@ extern int pci_set_platform_pm(struct pc
 extern void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 extern void pci_disable_enabled_device(struct pci_dev *dev);
 extern bool pci_check_pme_status(struct pci_dev *dev);
+extern int pci_finish_runtime_suspend(struct pci_dev *dev);
 extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
 extern void pci_pme_wakeup_bus(struct pci_bus *bus);
 extern void pci_pm_init(struct pci_dev *dev);
#54 lore raw

PATCH 12/12 PM / r8169: Add simplified run-time PM support

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Use the PCI run-time power management framework to add simplified run-time PM support to the r8169 driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

---
The patch is still missing an important part, which is that the driver should turn the power management on as soon as it's probed, not only after someone calls open() on it. Nevertheless, the patch is fully functional in this form.

Rafael

---
 drivers/net/r8169.c |  135 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 108 insertions(+), 27 deletions(-)
drivers/net/r8169.c +108 -27
Index: linux-2.6/drivers/net/r8169.c
===================================================================
@@ -23,6 +23,7 @@
 #include <linux/tcp.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/system.h>
 #include <asm/io.h>
@@ -504,6 +505,8 @@ struct rtl8169_private {
 
 	struct mii_if_info mii;
 	struct rtl8169_counters counters;
+	u32 saved_wolopts;
+	bool exiting;
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <[email protected]>");
@@ -743,6 +746,8 @@ static void rtl8169_check_link_status(st
 
 	spin_lock_irqsave(&tp->lock, flags);
 	if (tp->link_ok(ioaddr)) {
+		/* This is to cancel a scheduled suspend if there's one. */
+		pm_request_resume(&tp->pci_dev->dev);
 		netif_carrier_on(dev);
 		if (netif_msg_ifup(tp))
 			printk(KERN_INFO PFX "%s: link up\n", dev->name);
@@ -750,48 +755,54 @@ static void rtl8169_check_link_status(st
 		if (netif_msg_ifdown(tp))
 			printk(KERN_INFO PFX "%s: link down\n", dev->name);
 		netif_carrier_off(dev);
+		pm_schedule_suspend(&tp->pci_dev->dev, 100);
 	}
 	spin_unlock_irqrestore(&tp->lock, flags);
 }
 
-static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
+
+static u32 __rtl8169_get_wol(struct rtl8169_private *tp)
 {
-	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
 	u8 options;
-
-	wol->wolopts = 0;
-
-#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
-	wol->supported = WAKE_ANY;
-
-	spin_lock_irq(&tp->lock);
+	u32 wolopts = 0;
 
 	options = RTL_R8(Config1);
 	if (!(options & PMEnable))
-		goto out_unlock;
+		return 0;
 
 	options = RTL_R8(Config3);
 	if (options & LinkUp)
-		wol->wolopts |= WAKE_PHY;
+		wolopts |= WAKE_PHY;
 	if (options & MagicPacket)
-		wol->wolopts |= WAKE_MAGIC;
+		wolopts |= WAKE_MAGIC;
 
 	options = RTL_R8(Config5);
 	if (options & UWF)
-		wol->wolopts |= WAKE_UCAST;
+		wolopts |= WAKE_UCAST;
 	if (options & BWF)
-		wol->wolopts |= WAKE_BCAST;
+		wolopts |= WAKE_BCAST;
 	if (options & MWF)
-		wol->wolopts |= WAKE_MCAST;
+		wolopts |= WAKE_MCAST;
 
-out_unlock:
-	spin_unlock_irq(&tp->lock);
+	return wolopts;
 }
 
-static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
+
+	wol->supported = WAKE_ANY;
+	wol->wolopts = __rtl8169_get_wol(tp);
+
+	spin_unlock_irq(&tp->lock);
+}
+
+static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
+{
 	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned int i;
 	static const struct {
@@ -808,23 +819,29 @@ static int rtl8169_set_wol(struct net_de
 		{ WAKE_ANY,   Config5, LanWake }
 	};
 
-	spin_lock_irq(&tp->lock);
-
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 
 	for (i = 0; i < ARRAY_SIZE(cfg); i++) {
 		u8 options = RTL_R8(cfg[i].reg) & ~cfg[i].mask;
-		if (wol->wolopts & cfg[i].opt)
+		if (wolopts & cfg[i].opt)
 			options |= cfg[i].mask;
 		RTL_W8(cfg[i].reg, options);
 	}
 
 	RTL_W8(Cfg9346, Cfg9346_Lock);
+}
+
+static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
 
 	if (wol->wolopts)
 		tp->features |= RTL_FEATURE_WOL;
 	else
 		tp->features &= ~RTL_FEATURE_WOL;
+	__rtl8169_set_wol(tp, wol->wolopts);
 	device_set_wakeup_enable(&tp->pci_dev->dev, wol->wolopts);
 
 	spin_unlock_irq(&tp->lock);
@@ -3000,6 +3017,7 @@ rtl8169_init_one(struct pci_dev *pdev, c
 	tp->dev = dev;
 	tp->pci_dev = pdev;
 	tp->msg_enable = netif_msg_init(debug.msg_enable, R8169_MSG_DEFAULT);
+	tp->exiting = false;
 
 	mii = &tp->mii;
 	mii->dev = dev;
@@ -3232,6 +3250,8 @@ static void __devexit rtl8169_remove_one
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
+	tp->exiting = true;
+
 	flush_scheduled_work();
 
 	unregister_netdev(dev);
@@ -3258,7 +3278,6 @@ static int rtl8169_open(struct net_devic
 	struct pci_dev *pdev = tp->pci_dev;
 	int retval = -ENOMEM;
 
-
 	rtl8169_set_rxbufsize(tp, dev);
 
 	/*
@@ -3295,6 +3314,13 @@ static int rtl8169_open(struct net_devic
 
 	rtl8169_request_timer(dev);
 
+	tp->saved_wolopts = 0;
+
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	rtl8169_check_link_status(dev, tp, tp->mmio_addr);
 out:
 	return retval;
@@ -4727,6 +4753,14 @@ static int rtl8169_close(struct net_devi
 	struct rtl8169_private *tp = netdev_priv(dev);
 	struct pci_dev *pdev = tp->pci_dev;
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_get_noresume(&pdev->dev);
+		pm_runtime_resume(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+	}
+
 	/* update counters before going down */
 	rtl8169_update_counters(dev);
 
@@ -4844,21 +4878,65 @@ static int rtl8169_suspend(struct device
 	return 0;
 }
 
+static void __rtl8169_resume(struct net_device *dev)
+{
+	netif_device_attach(dev);
+	rtl8169_schedule_work(dev, rtl8169_reset_task);
+}
+
 static int rtl8169_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct net_device *dev = pci_get_drvdata(pdev);
 
-	if (!netif_running(dev))
-		goto out;
+	if (netif_running(dev))
+		__rtl8169_resume(dev);
 
-	netif_device_attach(dev);
+	return 0;
+}
+
+static int rtl8169_runtime_suspend(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	dev_dbg(&pdev->dev, "suspending\n");
+
+	spin_lock_irq(&tp->lock);
+	tp->saved_wolopts = __rtl8169_get_wol(tp);
+	__rtl8169_set_wol(tp, WAKE_ANY);
+	spin_unlock_irq(&tp->lock);
+
+	rtl8169_net_suspend(dev);
+
+	return 0;
+}
+
+static int rtl8169_runtime_resume(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	dev_dbg(&pdev->dev, "resuming\n");
+
+	spin_lock_irq(&tp->lock);
+	__rtl8169_set_wol(tp, tp->saved_wolopts);
+	tp->saved_wolopts = 0;
+	spin_unlock_irq(&tp->lock);
+
+	if (!tp->exiting)
+		__rtl8169_resume(dev);
 
-	rtl8169_schedule_work(dev, rtl8169_reset_task);
-out:
 	return 0;
 }
 
+static int rtl8169_runtime_idle(struct device *device)
+{
+	return -EBUSY;
+}
+
 static const struct dev_pm_ops rtl8169_pm_ops = {
 	.suspend = rtl8169_suspend,
 	.resume = rtl8169_resume,
@@ -4866,6 +4944,9 @@ static const struct dev_pm_ops rtl8169_p
 	.thaw = rtl8169_resume,
 	.poweroff = rtl8169_suspend,
 	.restore = rtl8169_resume,
+	.runtime_suspend = rtl8169_runtime_suspend,
+	.runtime_resume = rtl8169_runtime_resume,
+	.runtime_idle = rtl8169_runtime_idle,
 };
 
 #define RTL8169_PM_OPS	(&rtl8169_pm_ops)
#55 lore raw

PATCH 12/12 PM / r8169: Add simplified run-time PM support (rev. 2)

Quoted 8 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Use the PCI run-time power management framework to add simplified run-time PM support to the r8169 driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

Appended is an improved (in my opinion) version of this patch that enables the runtime PM in _init(), so that the device can be put into the low power state before _open() or after _close().

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / PM / r8169: Add simplified run-time PM support (rev. 2)

Use the PCI run-time power management framework to add simplified run-time PM support to the r8169 driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

---
 drivers/net/r8169.c |  151 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 124 insertions(+), 27 deletions(-)
drivers/net/r8169.c +124 -27
Index: linux-2.6/drivers/net/r8169.c
===================================================================
@@ -23,6 +23,7 @@
 #include <linux/tcp.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/system.h>
 #include <asm/io.h>
@@ -504,6 +505,7 @@ struct rtl8169_private {
 
 	struct mii_if_info mii;
 	struct rtl8169_counters counters;
+	u32 saved_wolopts;
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <[email protected]>");
@@ -743,6 +745,8 @@ static void rtl8169_check_link_status(st
 
 	spin_lock_irqsave(&tp->lock, flags);
 	if (tp->link_ok(ioaddr)) {
+		/* This is to cancel a scheduled suspend if there's one. */
+		pm_request_resume(&tp->pci_dev->dev);
 		netif_carrier_on(dev);
 		if (netif_msg_ifup(tp))
 			printk(KERN_INFO PFX "%s: link up\n", dev->name);
@@ -750,48 +754,54 @@ static void rtl8169_check_link_status(st
 		if (netif_msg_ifdown(tp))
 			printk(KERN_INFO PFX "%s: link down\n", dev->name);
 		netif_carrier_off(dev);
+		pm_schedule_suspend(&tp->pci_dev->dev, 100);
 	}
 	spin_unlock_irqrestore(&tp->lock, flags);
 }
 
-static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
+
+static u32 __rtl8169_get_wol(struct rtl8169_private *tp)
 {
-	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
 	u8 options;
-
-	wol->wolopts = 0;
-
-#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
-	wol->supported = WAKE_ANY;
-
-	spin_lock_irq(&tp->lock);
+	u32 wolopts = 0;
 
 	options = RTL_R8(Config1);
 	if (!(options & PMEnable))
-		goto out_unlock;
+		return 0;
 
 	options = RTL_R8(Config3);
 	if (options & LinkUp)
-		wol->wolopts |= WAKE_PHY;
+		wolopts |= WAKE_PHY;
 	if (options & MagicPacket)
-		wol->wolopts |= WAKE_MAGIC;
+		wolopts |= WAKE_MAGIC;
 
 	options = RTL_R8(Config5);
 	if (options & UWF)
-		wol->wolopts |= WAKE_UCAST;
+		wolopts |= WAKE_UCAST;
 	if (options & BWF)
-		wol->wolopts |= WAKE_BCAST;
+		wolopts |= WAKE_BCAST;
 	if (options & MWF)
-		wol->wolopts |= WAKE_MCAST;
+		wolopts |= WAKE_MCAST;
 
-out_unlock:
-	spin_unlock_irq(&tp->lock);
+	return wolopts;
 }
 
-static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
+
+	wol->supported = WAKE_ANY;
+	wol->wolopts = __rtl8169_get_wol(tp);
+
+	spin_unlock_irq(&tp->lock);
+}
+
+static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
+{
 	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned int i;
 	static const struct {
@@ -808,23 +818,29 @@ static int rtl8169_set_wol(struct net_de
 		{ WAKE_ANY,   Config5, LanWake }
 	};
 
-	spin_lock_irq(&tp->lock);
-
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 
 	for (i = 0; i < ARRAY_SIZE(cfg); i++) {
 		u8 options = RTL_R8(cfg[i].reg) & ~cfg[i].mask;
-		if (wol->wolopts & cfg[i].opt)
+		if (wolopts & cfg[i].opt)
 			options |= cfg[i].mask;
 		RTL_W8(cfg[i].reg, options);
 	}
 
 	RTL_W8(Cfg9346, Cfg9346_Lock);
+}
+
+static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
 
 	if (wol->wolopts)
 		tp->features |= RTL_FEATURE_WOL;
 	else
 		tp->features &= ~RTL_FEATURE_WOL;
+	__rtl8169_set_wol(tp, wol->wolopts);
 	device_set_wakeup_enable(&tp->pci_dev->dev, wol->wolopts);
 
 	spin_unlock_irq(&tp->lock);
@@ -3210,6 +3226,11 @@ rtl8169_init_one(struct pci_dev *pdev, c
 
 	device_set_wakeup_enable(&pdev->dev, tp->features & RTL_FEATURE_WOL);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 out:
 	return rc;
 
@@ -3232,10 +3253,18 @@ static void __devexit rtl8169_remove_one
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	flush_scheduled_work();
 
 	unregister_netdev(dev);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/* restore original MAC address */
 	rtl_rar_set(tp, dev->perm_addr);
 
@@ -3258,6 +3287,7 @@ static int rtl8169_open(struct net_devic
 	struct pci_dev *pdev = tp->pci_dev;
 	int retval = -ENOMEM;
 
+	pm_runtime_get_sync(&pdev->dev);
 
 	rtl8169_set_rxbufsize(tp, dev);
 
@@ -3268,7 +3298,7 @@ static int rtl8169_open(struct net_devic
 	tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
 					       &tp->TxPhyAddr);
 	if (!tp->TxDescArray)
-		goto out;
+		goto err_pm_runtime_put;
 
 	tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
 					       &tp->RxPhyAddr);
@@ -3295,6 +3325,9 @@ static int rtl8169_open(struct net_devic
 
 	rtl8169_request_timer(dev);
 
+	tp->saved_wolopts = 0;
+	pm_runtime_put_noidle(&pdev->dev);
+
 	rtl8169_check_link_status(dev, tp, tp->mmio_addr);
 out:
 	return retval;
@@ -3304,9 +3337,13 @@ err_release_ring_2:
 err_free_rx_1:
 	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
 			    tp->RxPhyAddr);
+	tp->RxDescArray = NULL;
 err_free_tx_0:
 	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
 			    tp->TxPhyAddr);
+	tp->TxDescArray = NULL;
+err_pm_runtime_put:
+	pm_runtime_put_noidle(&pdev->dev);
 	goto out;
 }
 
@@ -4727,6 +4764,8 @@ static int rtl8169_close(struct net_devi
 	struct rtl8169_private *tp = netdev_priv(dev);
 	struct pci_dev *pdev = tp->pci_dev;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	/* update counters before going down */
 	rtl8169_update_counters(dev);
 
@@ -4741,6 +4780,8 @@ static int rtl8169_close(struct net_devi
 	tp->TxDescArray = NULL;
 	tp->RxDescArray = NULL;
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 }
 
@@ -4844,18 +4885,71 @@ static int rtl8169_suspend(struct device
 	return 0;
 }
 
+static void __rtl8169_resume(struct net_device *dev)
+{
+	netif_device_attach(dev);
+	rtl8169_schedule_work(dev, rtl8169_reset_task);
+}
+
 static int rtl8169_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct net_device *dev = pci_get_drvdata(pdev);
 
-	if (!netif_running(dev))
-		goto out;
+	if (netif_running(dev))
+		__rtl8169_resume(dev);
 
-	netif_device_attach(dev);
+	return 0;
+}
 
-	rtl8169_schedule_work(dev, rtl8169_reset_task);
-out:
+static int rtl8169_runtime_suspend(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->TxDescArray)
+		return 0;
+
+	spin_lock_irq(&tp->lock);
+	tp->saved_wolopts = __rtl8169_get_wol(tp);
+	__rtl8169_set_wol(tp, WAKE_ANY);
+	spin_unlock_irq(&tp->lock);
+
+	rtl8169_net_suspend(dev);
+
+	return 0;
+}
+
+static int rtl8169_runtime_resume(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->TxDescArray)
+		return 0;
+
+	spin_lock_irq(&tp->lock);
+	__rtl8169_set_wol(tp, tp->saved_wolopts);
+	tp->saved_wolopts = 0;
+	spin_unlock_irq(&tp->lock);
+
+	__rtl8169_resume(dev);
+
+	return 0;
+}
+
+static int rtl8169_runtime_idle(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (tp->TxDescArray)
+		return -EBUSY;
+
+	pm_schedule_suspend(device, MSEC_PER_SEC);
 	return 0;
 }
 
@@ -4866,6 +4960,9 @@ static const struct dev_pm_ops rtl8169_p
 	.thaw = rtl8169_resume,
 	.poweroff = rtl8169_suspend,
 	.restore = rtl8169_resume,
+	.runtime_suspend = rtl8169_runtime_suspend,
+	.runtime_resume = rtl8169_runtime_resume,
+	.runtime_idle = rtl8169_runtime_idle,
 };
 
 #define RTL8169_PM_OPS	(&rtl8169_pm_ops)
#56 lore raw

PATCH 12/12 PM / r8169: Add simplified run-time PM support (rev. 2)

In reply to Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Use the PCI run-time power management framework to add simplified run-time PM support to the r8169 driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

Appended is an improved (in my opinion) version of this patch that enables the runtime PM in _init(), so that the device can be put into the low power state before _open() or after _close().

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / PM / r8169: Add simplified run-time PM support (rev. 2)

Use the PCI run-time power management framework to add simplified run-time PM support to the r8169 driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

---
 drivers/net/r8169.c |  151 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 124 insertions(+), 27 deletions(-)
drivers/net/r8169.c +124 -27
Index: linux-2.6/drivers/net/r8169.c
===================================================================
@@ -23,6 +23,7 @@
 #include <linux/tcp.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/system.h>
 #include <asm/io.h>
@@ -504,6 +505,7 @@ struct rtl8169_private {
 
 	struct mii_if_info mii;
 	struct rtl8169_counters counters;
+	u32 saved_wolopts;
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <[email protected]>");
@@ -743,6 +745,8 @@ static void rtl8169_check_link_status(st
 
 	spin_lock_irqsave(&tp->lock, flags);
 	if (tp->link_ok(ioaddr)) {
+		/* This is to cancel a scheduled suspend if there's one. */
+		pm_request_resume(&tp->pci_dev->dev);
 		netif_carrier_on(dev);
 		if (netif_msg_ifup(tp))
 			printk(KERN_INFO PFX "%s: link up\n", dev->name);
@@ -750,48 +754,54 @@ static void rtl8169_check_link_status(st
 		if (netif_msg_ifdown(tp))
 			printk(KERN_INFO PFX "%s: link down\n", dev->name);
 		netif_carrier_off(dev);
+		pm_schedule_suspend(&tp->pci_dev->dev, 100);
 	}
 	spin_unlock_irqrestore(&tp->lock, flags);
 }
 
-static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
+
+static u32 __rtl8169_get_wol(struct rtl8169_private *tp)
 {
-	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
 	u8 options;
-
-	wol->wolopts = 0;
-
-#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
-	wol->supported = WAKE_ANY;
-
-	spin_lock_irq(&tp->lock);
+	u32 wolopts = 0;
 
 	options = RTL_R8(Config1);
 	if (!(options & PMEnable))
-		goto out_unlock;
+		return 0;
 
 	options = RTL_R8(Config3);
 	if (options & LinkUp)
-		wol->wolopts |= WAKE_PHY;
+		wolopts |= WAKE_PHY;
 	if (options & MagicPacket)
-		wol->wolopts |= WAKE_MAGIC;
+		wolopts |= WAKE_MAGIC;
 
 	options = RTL_R8(Config5);
 	if (options & UWF)
-		wol->wolopts |= WAKE_UCAST;
+		wolopts |= WAKE_UCAST;
 	if (options & BWF)
-		wol->wolopts |= WAKE_BCAST;
+		wolopts |= WAKE_BCAST;
 	if (options & MWF)
-		wol->wolopts |= WAKE_MCAST;
+		wolopts |= WAKE_MCAST;
 
-out_unlock:
-	spin_unlock_irq(&tp->lock);
+	return wolopts;
 }
 
-static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
+
+	wol->supported = WAKE_ANY;
+	wol->wolopts = __rtl8169_get_wol(tp);
+
+	spin_unlock_irq(&tp->lock);
+}
+
+static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
+{
 	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned int i;
 	static const struct {
@@ -808,23 +818,29 @@ static int rtl8169_set_wol(struct net_de
 		{ WAKE_ANY,   Config5, LanWake }
 	};
 
-	spin_lock_irq(&tp->lock);
-
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 
 	for (i = 0; i < ARRAY_SIZE(cfg); i++) {
 		u8 options = RTL_R8(cfg[i].reg) & ~cfg[i].mask;
-		if (wol->wolopts & cfg[i].opt)
+		if (wolopts & cfg[i].opt)
 			options |= cfg[i].mask;
 		RTL_W8(cfg[i].reg, options);
 	}
 
 	RTL_W8(Cfg9346, Cfg9346_Lock);
+}
+
+static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
 
 	if (wol->wolopts)
 		tp->features |= RTL_FEATURE_WOL;
 	else
 		tp->features &= ~RTL_FEATURE_WOL;
+	__rtl8169_set_wol(tp, wol->wolopts);
 	device_set_wakeup_enable(&tp->pci_dev->dev, wol->wolopts);
 
 	spin_unlock_irq(&tp->lock);
@@ -3210,6 +3226,11 @@ rtl8169_init_one(struct pci_dev *pdev, c
 
 	device_set_wakeup_enable(&pdev->dev, tp->features & RTL_FEATURE_WOL);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 out:
 	return rc;
 
@@ -3232,10 +3253,18 @@ static void __devexit rtl8169_remove_one
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	flush_scheduled_work();
 
 	unregister_netdev(dev);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/* restore original MAC address */
 	rtl_rar_set(tp, dev->perm_addr);
 
@@ -3258,6 +3287,7 @@ static int rtl8169_open(struct net_devic
 	struct pci_dev *pdev = tp->pci_dev;
 	int retval = -ENOMEM;
 
+	pm_runtime_get_sync(&pdev->dev);
 
 	rtl8169_set_rxbufsize(tp, dev);
 
@@ -3268,7 +3298,7 @@ static int rtl8169_open(struct net_devic
 	tp->TxDescArray = pci_alloc_consistent(pdev, R8169_TX_RING_BYTES,
 					       &tp->TxPhyAddr);
 	if (!tp->TxDescArray)
-		goto out;
+		goto err_pm_runtime_put;
 
 	tp->RxDescArray = pci_alloc_consistent(pdev, R8169_RX_RING_BYTES,
 					       &tp->RxPhyAddr);
@@ -3295,6 +3325,9 @@ static int rtl8169_open(struct net_devic
 
 	rtl8169_request_timer(dev);
 
+	tp->saved_wolopts = 0;
+	pm_runtime_put_noidle(&pdev->dev);
+
 	rtl8169_check_link_status(dev, tp, tp->mmio_addr);
 out:
 	return retval;
@@ -3304,9 +3337,13 @@ err_release_ring_2:
 err_free_rx_1:
 	pci_free_consistent(pdev, R8169_RX_RING_BYTES, tp->RxDescArray,
 			    tp->RxPhyAddr);
+	tp->RxDescArray = NULL;
 err_free_tx_0:
 	pci_free_consistent(pdev, R8169_TX_RING_BYTES, tp->TxDescArray,
 			    tp->TxPhyAddr);
+	tp->TxDescArray = NULL;
+err_pm_runtime_put:
+	pm_runtime_put_noidle(&pdev->dev);
 	goto out;
 }
 
@@ -4727,6 +4764,8 @@ static int rtl8169_close(struct net_devi
 	struct rtl8169_private *tp = netdev_priv(dev);
 	struct pci_dev *pdev = tp->pci_dev;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	/* update counters before going down */
 	rtl8169_update_counters(dev);
 
@@ -4741,6 +4780,8 @@ static int rtl8169_close(struct net_devi
 	tp->TxDescArray = NULL;
 	tp->RxDescArray = NULL;
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 }
 
@@ -4844,18 +4885,71 @@ static int rtl8169_suspend(struct device
 	return 0;
 }
 
+static void __rtl8169_resume(struct net_device *dev)
+{
+	netif_device_attach(dev);
+	rtl8169_schedule_work(dev, rtl8169_reset_task);
+}
+
 static int rtl8169_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct net_device *dev = pci_get_drvdata(pdev);
 
-	if (!netif_running(dev))
-		goto out;
+	if (netif_running(dev))
+		__rtl8169_resume(dev);
 
-	netif_device_attach(dev);
+	return 0;
+}
 
-	rtl8169_schedule_work(dev, rtl8169_reset_task);
-out:
+static int rtl8169_runtime_suspend(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->TxDescArray)
+		return 0;
+
+	spin_lock_irq(&tp->lock);
+	tp->saved_wolopts = __rtl8169_get_wol(tp);
+	__rtl8169_set_wol(tp, WAKE_ANY);
+	spin_unlock_irq(&tp->lock);
+
+	rtl8169_net_suspend(dev);
+
+	return 0;
+}
+
+static int rtl8169_runtime_resume(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (!tp->TxDescArray)
+		return 0;
+
+	spin_lock_irq(&tp->lock);
+	__rtl8169_set_wol(tp, tp->saved_wolopts);
+	tp->saved_wolopts = 0;
+	spin_unlock_irq(&tp->lock);
+
+	__rtl8169_resume(dev);
+
+	return 0;
+}
+
+static int rtl8169_runtime_idle(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (tp->TxDescArray)
+		return -EBUSY;
+
+	pm_schedule_suspend(device, MSEC_PER_SEC);
 	return 0;
 }
 
@@ -4866,6 +4960,9 @@ static const struct dev_pm_ops rtl8169_p
 	.thaw = rtl8169_resume,
 	.poweroff = rtl8169_suspend,
 	.restore = rtl8169_resume,
+	.runtime_suspend = rtl8169_runtime_suspend,
+	.runtime_resume = rtl8169_runtime_resume,
+	.runtime_idle = rtl8169_runtime_idle,
 };
 
 #define RTL8169_PM_OPS	(&rtl8169_pm_ops)
#57 lore raw

PATCH 12/12 PM / r8169: Add simplified run-time PM support

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Use the PCI run-time power management framework to add simplified run-time PM support to the r8169 driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

---
The patch is still missing an important part, which is that the driver should turn the power management on as soon as it's probed, not only after someone calls open() on it. Nevertheless, the patch is fully functional in this form.

Rafael

---
 drivers/net/r8169.c |  135 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 108 insertions(+), 27 deletions(-)
drivers/net/r8169.c +108 -27
Index: linux-2.6/drivers/net/r8169.c
===================================================================
@@ -23,6 +23,7 @@
 #include <linux/tcp.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/system.h>
 #include <asm/io.h>
@@ -504,6 +505,8 @@ struct rtl8169_private {
 
 	struct mii_if_info mii;
 	struct rtl8169_counters counters;
+	u32 saved_wolopts;
+	bool exiting;
 };
 
 MODULE_AUTHOR("Realtek and the Linux r8169 crew <[email protected]>");
@@ -743,6 +746,8 @@ static void rtl8169_check_link_status(st
 
 	spin_lock_irqsave(&tp->lock, flags);
 	if (tp->link_ok(ioaddr)) {
+		/* This is to cancel a scheduled suspend if there's one. */
+		pm_request_resume(&tp->pci_dev->dev);
 		netif_carrier_on(dev);
 		if (netif_msg_ifup(tp))
 			printk(KERN_INFO PFX "%s: link up\n", dev->name);
@@ -750,48 +755,54 @@ static void rtl8169_check_link_status(st
 		if (netif_msg_ifdown(tp))
 			printk(KERN_INFO PFX "%s: link down\n", dev->name);
 		netif_carrier_off(dev);
+		pm_schedule_suspend(&tp->pci_dev->dev, 100);
 	}
 	spin_unlock_irqrestore(&tp->lock, flags);
 }
 
-static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
+
+static u32 __rtl8169_get_wol(struct rtl8169_private *tp)
 {
-	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
 	u8 options;
-
-	wol->wolopts = 0;
-
-#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
-	wol->supported = WAKE_ANY;
-
-	spin_lock_irq(&tp->lock);
+	u32 wolopts = 0;
 
 	options = RTL_R8(Config1);
 	if (!(options & PMEnable))
-		goto out_unlock;
+		return 0;
 
 	options = RTL_R8(Config3);
 	if (options & LinkUp)
-		wol->wolopts |= WAKE_PHY;
+		wolopts |= WAKE_PHY;
 	if (options & MagicPacket)
-		wol->wolopts |= WAKE_MAGIC;
+		wolopts |= WAKE_MAGIC;
 
 	options = RTL_R8(Config5);
 	if (options & UWF)
-		wol->wolopts |= WAKE_UCAST;
+		wolopts |= WAKE_UCAST;
 	if (options & BWF)
-		wol->wolopts |= WAKE_BCAST;
+		wolopts |= WAKE_BCAST;
 	if (options & MWF)
-		wol->wolopts |= WAKE_MCAST;
+		wolopts |= WAKE_MCAST;
 
-out_unlock:
-	spin_unlock_irq(&tp->lock);
+	return wolopts;
 }
 
-static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+static void rtl8169_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
+
+	wol->supported = WAKE_ANY;
+	wol->wolopts = __rtl8169_get_wol(tp);
+
+	spin_unlock_irq(&tp->lock);
+}
+
+static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
+{
 	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned int i;
 	static const struct {
@@ -808,23 +819,29 @@ static int rtl8169_set_wol(struct net_de
 		{ WAKE_ANY,   Config5, LanWake }
 	};
 
-	spin_lock_irq(&tp->lock);
-
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 
 	for (i = 0; i < ARRAY_SIZE(cfg); i++) {
 		u8 options = RTL_R8(cfg[i].reg) & ~cfg[i].mask;
-		if (wol->wolopts & cfg[i].opt)
+		if (wolopts & cfg[i].opt)
 			options |= cfg[i].mask;
 		RTL_W8(cfg[i].reg, options);
 	}
 
 	RTL_W8(Cfg9346, Cfg9346_Lock);
+}
+
+static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	spin_lock_irq(&tp->lock);
 
 	if (wol->wolopts)
 		tp->features |= RTL_FEATURE_WOL;
 	else
 		tp->features &= ~RTL_FEATURE_WOL;
+	__rtl8169_set_wol(tp, wol->wolopts);
 	device_set_wakeup_enable(&tp->pci_dev->dev, wol->wolopts);
 
 	spin_unlock_irq(&tp->lock);
@@ -3000,6 +3017,7 @@ rtl8169_init_one(struct pci_dev *pdev, c
 	tp->dev = dev;
 	tp->pci_dev = pdev;
 	tp->msg_enable = netif_msg_init(debug.msg_enable, R8169_MSG_DEFAULT);
+	tp->exiting = false;
 
 	mii = &tp->mii;
 	mii->dev = dev;
@@ -3232,6 +3250,8 @@ static void __devexit rtl8169_remove_one
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
+	tp->exiting = true;
+
 	flush_scheduled_work();
 
 	unregister_netdev(dev);
@@ -3258,7 +3278,6 @@ static int rtl8169_open(struct net_devic
 	struct pci_dev *pdev = tp->pci_dev;
 	int retval = -ENOMEM;
 
-
 	rtl8169_set_rxbufsize(tp, dev);
 
 	/*
@@ -3295,6 +3314,13 @@ static int rtl8169_open(struct net_devic
 
 	rtl8169_request_timer(dev);
 
+	tp->saved_wolopts = 0;
+
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	rtl8169_check_link_status(dev, tp, tp->mmio_addr);
 out:
 	return retval;
@@ -4727,6 +4753,14 @@ static int rtl8169_close(struct net_devi
 	struct rtl8169_private *tp = netdev_priv(dev);
 	struct pci_dev *pdev = tp->pci_dev;
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_get_noresume(&pdev->dev);
+		pm_runtime_resume(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+	}
+
 	/* update counters before going down */
 	rtl8169_update_counters(dev);
 
@@ -4844,21 +4878,65 @@ static int rtl8169_suspend(struct device
 	return 0;
 }
 
+static void __rtl8169_resume(struct net_device *dev)
+{
+	netif_device_attach(dev);
+	rtl8169_schedule_work(dev, rtl8169_reset_task);
+}
+
 static int rtl8169_resume(struct device *device)
 {
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct net_device *dev = pci_get_drvdata(pdev);
 
-	if (!netif_running(dev))
-		goto out;
+	if (netif_running(dev))
+		__rtl8169_resume(dev);
 
-	netif_device_attach(dev);
+	return 0;
+}
+
+static int rtl8169_runtime_suspend(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	dev_dbg(&pdev->dev, "suspending\n");
+
+	spin_lock_irq(&tp->lock);
+	tp->saved_wolopts = __rtl8169_get_wol(tp);
+	__rtl8169_set_wol(tp, WAKE_ANY);
+	spin_unlock_irq(&tp->lock);
+
+	rtl8169_net_suspend(dev);
+
+	return 0;
+}
+
+static int rtl8169_runtime_resume(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	dev_dbg(&pdev->dev, "resuming\n");
+
+	spin_lock_irq(&tp->lock);
+	__rtl8169_set_wol(tp, tp->saved_wolopts);
+	tp->saved_wolopts = 0;
+	spin_unlock_irq(&tp->lock);
+
+	if (!tp->exiting)
+		__rtl8169_resume(dev);
 
-	rtl8169_schedule_work(dev, rtl8169_reset_task);
-out:
 	return 0;
 }
 
+static int rtl8169_runtime_idle(struct device *device)
+{
+	return -EBUSY;
+}
+
 static const struct dev_pm_ops rtl8169_pm_ops = {
 	.suspend = rtl8169_suspend,
 	.resume = rtl8169_resume,
@@ -4866,6 +4944,9 @@ static const struct dev_pm_ops rtl8169_p
 	.thaw = rtl8169_resume,
 	.poweroff = rtl8169_suspend,
 	.restore = rtl8169_resume,
+	.runtime_suspend = rtl8169_runtime_suspend,
+	.runtime_resume = rtl8169_runtime_resume,
+	.runtime_idle = rtl8169_runtime_idle,
 };
 
 #define RTL8169_PM_OPS	(&rtl8169_pm_ops)
#58 lore raw

RFC PATCH 1/1 e1000e: Add basic runtime PM support (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

In reply to Rafael J. Wysocki
Quoted 5 lines · Rafael J. Wysocki

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Quoted 7 lines

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all conditions I tested it, so it is appended below.

Quoted 20 lines

[1/12] - Add function for checking PME status of devices

[2/12] - Modify wake-up enable propagation so that it's done for PCIe devices
too (this one is in the Jesse's tree already, but it's reproduced here for completness)

[3/12] - PCIe PME root port service driver

[4/12] - "Don't use MSIs for PME signaling" switch for PCIe

[5/12] - ACPI GPE refcounting, from Matthew

[6/12] - ACPI drivers support for GPE refcounting, from Matthew

[7/12] - ACPI removal of the old GPE API, from Matthew

[8/12] - ACPI add fields for handling run-wake devices

[9/12][New] - ACPI add helper function for enabling/disabling wake-up power

Quoted 8 lines

[10/12] - PCI / ACPI platform support for run-time power management

[11/12] - Runtime PM callbacks for the PCI bus type

[12/12] - Runtime PM support for r8169 (experimental)

If there are no objections, I think the patches [1-11/12] are ready for the upstream.

---
From: Rafael J. Wysocki <[email protected]>
Subject: e1000e: Add basic runtime PM support

Add support for suspending the device on link removal and resuming it on link gain.

Based on a patch from Matthew Garrett.

---
 drivers/net/e1000e/netdev.c |   90 ++++++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 24 deletions(-)
drivers/net/e1000e/netdev.c +66 -24
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
 #include <linux/aer.h>
 
 #include "e1000.h"
@@ -3093,12 +3094,18 @@ static int e1000_open(struct net_device 
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	int err;
 
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->state))
 		return -EBUSY;
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	netif_carrier_off(netdev);
 
 	/* allocate transmit descriptors */
@@ -3190,9 +3197,21 @@ err_setup_tx:
 static int e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct pci_dev *pdev = adapter->pdev;
 
 	WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
-	e1000e_down(adapter);
+
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_get_noresume(&pdev->dev);
+		pm_runtime_resume(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+	}
+
+	if (netif_running(netdev))
+		e1000e_down(adapter);
+
 	e1000_power_down_phy(adapter);
 	e1000_free_irq(adapter);
 
@@ -3571,6 +3590,10 @@ static void e1000_watchdog_task(struct w
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			bool txb2b = 1;
+
+			/* This is to cancel scheduled suspend requests. */
+			pm_runtime_resume(netdev->dev.parent);
+
 			/* update snapshot of PHY registers on LSC */
 			e1000_phy_read_status(adapter);
 			mac->ops.get_link_up_info(&adapter->hw,
@@ -3686,6 +3709,8 @@ static void e1000_watchdog_task(struct w
 
 			if (adapter->flags & FLAG_RX_NEEDS_RESTART)
 				schedule_work(&adapter->reset_task);
+			else
+				pm_schedule_suspend(netdev->dev.parent, 100);
 		}
 	}
 
@@ -4489,13 +4514,15 @@ out:
 	return retval;
 }
 
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+			    bool runtime)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl, ctrl_ext, rctl, status;
-	u32 wufc = adapter->wol;
+	/* Runtime suspend should only enable wakeup for link changes */
+	u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
 	int retval = 0;
 
 	netif_device_detach(netdev);
@@ -4653,41 +4680,43 @@ static void e1000e_disable_l1aspm(struct
 }
 
 #ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+static int e1000_idle(struct device *dev)
 {
+	/* Prevent the driver core from changing our state */
+	return -EBUSY;
+}
+
+static int e1000_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int retval;
 	bool wake;
 
-	retval = __e1000_shutdown(pdev, &wake);
+	retval = __e1000_shutdown(pdev, &wake, false);
 	if (!retval)
 		e1000_complete_shutdown(pdev, true, wake);
 
 	return retval;
 }
 
-static int e1000_resume(struct pci_dev *pdev)
+static int e1000_runtime_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	bool wake;
+
+	return __e1000_shutdown(pdev, &wake, true);
+}
+
+static int e1000_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 err;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
 	e1000e_disable_l1aspm(pdev);
 
-	err = pci_enable_device_mem(pdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Cannot enable PCI device from suspend\n");
-		return err;
-	}
-
-	pci_set_master(pdev);
-
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
-
 	e1000e_set_interrupt_capability(adapter);
 	if (netif_running(netdev)) {
 		err = e1000_request_irq(adapter);
@@ -4751,7 +4780,7 @@ static void e1000_shutdown(struct pci_de
 {
 	bool wake = false;
 
-	__e1000_shutdown(pdev, &wake);
+	__e1000_shutdown(pdev, &wake, false);
 
 	if (system_state == SYSTEM_POWER_OFF)
 		e1000_complete_shutdown(pdev, false, wake);
@@ -5274,6 +5303,9 @@ static void __devexit e1000_remove(struc
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
+	if (pci_dev_run_wake(pdev))
+		pm_runtime_resume(&pdev->dev);
+
 	/*
 	 * flush_scheduled work may reschedule our watchdog task, so
 	 * explicitly disable watchdog tasks from being rescheduled
@@ -5393,6 +5425,18 @@ static struct pci_device_id e1000_pci_tb
 };
 MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
 
+static struct dev_pm_ops e1000_pm_ops = {
+	.suspend  = e1000_suspend,
+	.resume   = e1000_resume,
+	.freeze = e1000_suspend,
+	.thaw = e1000_resume,
+	.poweroff = e1000_suspend,
+	.restore = e1000_resume,
+	.runtime_suspend = e1000_runtime_suspend,
+	.runtime_resume = e1000_resume,
+	.runtime_idle = e1000_idle,
+};
+
 /* PCI Device API Driver */
 static struct pci_driver e1000_driver = {
 	.name     = e1000e_driver_name,
@@ -5400,9 +5444,7 @@ static struct pci_driver e1000_driver = 
 	.probe    = e1000_probe,
 	.remove   = __devexit_p(e1000_remove),
 #ifdef CONFIG_PM
-	/* Power Management Hooks */
-	.suspend  = e1000_suspend,
-	.resume   = e1000_resume,
+	.driver.pm = &e1000_pm_ops,
 #endif
 	.shutdown = e1000_shutdown,
 	.err_handler = &e1000_err_handler
#59 lore raw

RFC PATCH 1/1 e1000e: Add basic runtime PM support (rev. 2) (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

Quoted 7 lines
Quoted 4 lines

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all conditions I tested it, so it is appended below.

This one was actually buggy, as it triggered a netdevice.h BUG_ON() during resume from suspend to RAM with network cable attached.

Appended is a new version that doesn't have this problem and enables runtime PM in _probe(), so the device can be put into the low power state if _open() hasn't been called yet (or after _close()).

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / PM / e1000e: Add basic runtime PM support (rev. 2)

Use the PCI run-time power management framework to add simplified run-time PM support to the e1000e driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

Based on a patch from Matthew Garrett.

---
 drivers/net/e1000e/netdev.c |  130 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 106 insertions(+), 24 deletions(-)
drivers/net/e1000e/netdev.c +106 -24
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
 #include <linux/aer.h>
 
 #include "e1000.h"
@@ -3093,12 +3094,15 @@ static int e1000_open(struct net_device 
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	int err;
 
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->state))
 		return -EBUSY;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	netif_carrier_off(netdev);
 
 	/* allocate transmit descriptors */
@@ -3162,6 +3166,8 @@ static int e1000_open(struct net_device 
 	/* fire a link status change interrupt to start the watchdog */
 	ew32(ICS, E1000_ICS_LSC);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 
 err_req_irq:
@@ -3172,6 +3178,7 @@ err_setup_rx:
 	e1000e_free_tx_resources(adapter);
 err_setup_tx:
 	e1000e_reset(adapter);
+	pm_runtime_put_noidle(&pdev->dev);
 
 	return err;
 }
@@ -3190,9 +3197,15 @@ err_setup_tx:
 static int e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct pci_dev *pdev = adapter->pdev;
 
 	WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
-	e1000e_down(adapter);
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	if (!test_bit(__E1000_DOWN, &adapter->state))
+		e1000e_down(adapter);
+
 	e1000_power_down_phy(adapter);
 	e1000_free_irq(adapter);
 
@@ -3216,6 +3229,8 @@ static int e1000_close(struct net_device
 	if (adapter->flags & FLAG_HAS_AMT)
 		e1000_release_hw_control(adapter);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 }
 /**
@@ -3571,6 +3586,10 @@ static void e1000_watchdog_task(struct w
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			bool txb2b = 1;
+
+			/* This is to cancel scheduled suspend requests. */
+			pm_runtime_resume(netdev->dev.parent);
+
 			/* update snapshot of PHY registers on LSC */
 			e1000_phy_read_status(adapter);
 			mac->ops.get_link_up_info(&adapter->hw,
@@ -3686,6 +3705,8 @@ static void e1000_watchdog_task(struct w
 
 			if (adapter->flags & FLAG_RX_NEEDS_RESTART)
 				schedule_work(&adapter->reset_task);
+			else
+				pm_schedule_suspend(netdev->dev.parent, 100);
 		}
 	}
 
@@ -4489,13 +4510,15 @@ out:
 	return retval;
 }
 
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+			    bool runtime)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl, ctrl_ext, rctl, status;
-	u32 wufc = adapter->wol;
+	/* Runtime suspend should only enable wakeup for link changes */
+	u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
 	int retval = 0;
 
 	netif_device_detach(netdev);
@@ -4653,40 +4676,59 @@ static void e1000e_disable_l1aspm(struct
 }
 
 #ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+static bool e1000e_pm_ready(struct e1000_adapter *adapter)
+{
+	return !!adapter->tx_ring->buffer_info;
+}
+
+static int e1000_idle(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	/*  If we're active, prevent the driver core from changing our state */
+	if (e1000e_pm_ready(adapter))
+		return -EBUSY;
+
+	pm_schedule_suspend(dev, MSEC_PER_SEC);
+	return 0;
+}
+
+static int e1000_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int retval;
 	bool wake;
 
-	retval = __e1000_shutdown(pdev, &wake);
+	retval = __e1000_shutdown(pdev, &wake, false);
 	if (!retval)
 		e1000_complete_shutdown(pdev, true, wake);
 
 	return retval;
 }
 
-static int e1000_resume(struct pci_dev *pdev)
+static int e1000_runtime_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	u32 err;
+	bool wake;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	e1000e_disable_l1aspm(pdev);
+	if (e1000e_pm_ready(adapter))
+		return __e1000_shutdown(pdev, &wake, true);
 
-	err = pci_enable_device_mem(pdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Cannot enable PCI device from suspend\n");
-		return err;
-	}
+	return 0;
+}
 
-	pci_set_master(pdev);
+static int __e1000_resume(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct e1000_hw *hw = &adapter->hw;
+	u32 err;
 
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
+	e1000e_disable_l1aspm(pdev);
 
 	e1000e_set_interrupt_capability(adapter);
 	if (netif_running(netdev)) {
@@ -4745,13 +4787,30 @@ static int e1000_resume(struct pci_dev *
 
 	return 0;
 }
+
+static int e1000_resume(struct device *dev)
+{
+	return __e1000_resume(to_pci_dev(dev));
+}
+
+static int e1000_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	if (e1000e_pm_ready(adapter))
+		return __e1000_resume(pdev);
+
+	return 0;
+}
 #endif
 
 static void e1000_shutdown(struct pci_dev *pdev)
 {
 	bool wake = false;
 
-	__e1000_shutdown(pdev, &wake);
+	__e1000_shutdown(pdev, &wake, false);
 
 	if (system_state == SYSTEM_POWER_OFF)
 		e1000_complete_shutdown(pdev, false, wake);
@@ -5231,6 +5290,11 @@ static int __devinit e1000_probe(struct 
 
 	e1000_print_device_info(adapter);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	return 0;
 
 err_register:
@@ -5274,6 +5338,8 @@ static void __devexit e1000_remove(struc
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	/*
 	 * flush_scheduled work may reschedule our watchdog task, so
 	 * explicitly disable watchdog tasks from being rescheduled
@@ -5294,6 +5360,12 @@ static void __devexit e1000_remove(struc
 
 	unregister_netdev(netdev);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/*
 	 * Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant.
@@ -5393,6 +5465,18 @@ static struct pci_device_id e1000_pci_tb
 };
 MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
 
+static const struct dev_pm_ops e1000_pm_ops = {
+	.suspend  = e1000_suspend,
+	.resume   = e1000_resume,
+	.freeze = e1000_suspend,
+	.thaw = e1000_resume,
+	.poweroff = e1000_suspend,
+	.restore = e1000_resume,
+	.runtime_suspend = e1000_runtime_suspend,
+	.runtime_resume = e1000_runtime_resume,
+	.runtime_idle = e1000_idle,
+};
+
 /* PCI Device API Driver */
 static struct pci_driver e1000_driver = {
 	.name     = e1000e_driver_name,
@@ -5400,9 +5484,7 @@ static struct pci_driver e1000_driver = 
 	.probe    = e1000_probe,
 	.remove   = __devexit_p(e1000_remove),
 #ifdef CONFIG_PM
-	/* Power Management Hooks */
-	.suspend  = e1000_suspend,
-	.resume   = e1000_resume,
+	.driver.pm = &e1000_pm_ops,
 #endif
 	.shutdown = e1000_shutdown,
 	.err_handler = &e1000_err_handler
#60 lore raw

RFC PATCH 1/1 e1000e: Add basic runtime PM support (rev. 3) (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

Quoted 18 lines · Rafael J. Wysocki
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

Quoted 7 lines
Quoted 4 lines

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all conditions I tested it, so it is appended below.

Quoted 6 lines

This one was actually buggy, as it triggered a netdevice.h BUG_ON() during resume from suspend to RAM with network cable attached.

Appended is a new version that doesn't have this problem and enables runtime PM in _probe(), so the device can be put into the low power state if _open() hasn't been called yet (or after _close()).

This one also has a few problems (triggers a warning about an already freed interrupt being freed again during suspend to RAM with the adapter suspended at run time if it is maganed by NetworkManager and doesn't put the adapter into the low power state if the link is not present from the start), so appended is a fixed one (at least it doesn't show any more problems in my testing).

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / PM / e1000e: Add basic runtime PM support (rev. 3)

Use the PCI run-time power management framework to add simplified run-time PM support to the e1000e driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

Based on a patch from Matthew Garrett.

---
 drivers/net/e1000e/netdev.c |  141 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 115 insertions(+), 26 deletions(-)
drivers/net/e1000e/netdev.c +115 -26
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
 #include <linux/aer.h>
 
 #include "e1000.h"
@@ -3093,12 +3094,15 @@ static int e1000_open(struct net_device 
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	int err;
 
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->state))
 		return -EBUSY;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	netif_carrier_off(netdev);
 
 	/* allocate transmit descriptors */
@@ -3159,6 +3163,10 @@ static int e1000_open(struct net_device 
 
 	netif_start_queue(netdev);
 
+	pm_runtime_put_noidle(&pdev->dev);
+	if (!e1000_has_link(adapter))
+		pm_schedule_suspend(&pdev->dev, 100);
+
 	/* fire a link status change interrupt to start the watchdog */
 	ew32(ICS, E1000_ICS_LSC);
 
@@ -3172,6 +3180,7 @@ err_setup_rx:
 	e1000e_free_tx_resources(adapter);
 err_setup_tx:
 	e1000e_reset(adapter);
+	pm_runtime_put_noidle(&pdev->dev);
 
 	return err;
 }
@@ -3190,11 +3199,17 @@ err_setup_tx:
 static int e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct pci_dev *pdev = adapter->pdev;
 
 	WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
-	e1000e_down(adapter);
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	if (!test_bit(__E1000_DOWN, &adapter->state)) {
+		e1000e_down(adapter);
+		e1000_free_irq(adapter);
+	}
 	e1000_power_down_phy(adapter);
-	e1000_free_irq(adapter);
 
 	e1000e_free_tx_resources(adapter);
 	e1000e_free_rx_resources(adapter);
@@ -3216,6 +3231,8 @@ static int e1000_close(struct net_device
 	if (adapter->flags & FLAG_HAS_AMT)
 		e1000_release_hw_control(adapter);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 }
 /**
@@ -3571,6 +3588,10 @@ static void e1000_watchdog_task(struct w
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			bool txb2b = 1;
+
+			/* This is to cancel scheduled suspend requests. */
+			pm_runtime_resume(netdev->dev.parent);
+
 			/* update snapshot of PHY registers on LSC */
 			e1000_phy_read_status(adapter);
 			mac->ops.get_link_up_info(&adapter->hw,
@@ -3686,6 +3707,8 @@ static void e1000_watchdog_task(struct w
 
 			if (adapter->flags & FLAG_RX_NEEDS_RESTART)
 				schedule_work(&adapter->reset_task);
+			else
+				pm_schedule_suspend(netdev->dev.parent, 100);
 		}
 	}
 
@@ -4489,13 +4512,15 @@ out:
 	return retval;
 }
 
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+			    bool runtime)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl, ctrl_ext, rctl, status;
-	u32 wufc = adapter->wol;
+	/* Runtime suspend should only enable wakeup for link changes */
+	u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
 	int retval = 0;
 
 	netif_device_detach(netdev);
@@ -4653,40 +4678,59 @@ static void e1000e_disable_l1aspm(struct
 }
 
 #ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+static bool e1000e_pm_ready(struct e1000_adapter *adapter)
+{
+	return !!adapter->tx_ring->buffer_info;
+}
+
+static int e1000_idle(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	/*  If we're active, prevent the driver core from changing our state */
+	if (e1000e_pm_ready(adapter))
+		return -EBUSY;
+
+	pm_schedule_suspend(dev, MSEC_PER_SEC);
+	return 0;
+}
+
+static int e1000_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int retval;
 	bool wake;
 
-	retval = __e1000_shutdown(pdev, &wake);
+	retval = __e1000_shutdown(pdev, &wake, false);
 	if (!retval)
 		e1000_complete_shutdown(pdev, true, wake);
 
 	return retval;
 }
 
-static int e1000_resume(struct pci_dev *pdev)
+static int e1000_runtime_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	u32 err;
+	bool wake;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	e1000e_disable_l1aspm(pdev);
+	if (e1000e_pm_ready(adapter))
+		return __e1000_shutdown(pdev, &wake, true);
 
-	err = pci_enable_device_mem(pdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Cannot enable PCI device from suspend\n");
-		return err;
-	}
+	return 0;
+}
 
-	pci_set_master(pdev);
+static int __e1000_resume(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct e1000_hw *hw = &adapter->hw;
+	u32 err;
 
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
+	e1000e_disable_l1aspm(pdev);
 
 	e1000e_set_interrupt_capability(adapter);
 	if (netif_running(netdev)) {
@@ -4745,13 +4789,30 @@ static int e1000_resume(struct pci_dev *
 
 	return 0;
 }
+
+static int e1000_resume(struct device *dev)
+{
+	return __e1000_resume(to_pci_dev(dev));
+}
+
+static int e1000_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	if (e1000e_pm_ready(adapter))
+		return __e1000_resume(pdev);
+
+	return 0;
+}
 #endif
 
 static void e1000_shutdown(struct pci_dev *pdev)
 {
 	bool wake = false;
 
-	__e1000_shutdown(pdev, &wake);
+	__e1000_shutdown(pdev, &wake, false);
 
 	if (system_state == SYSTEM_POWER_OFF)
 		e1000_complete_shutdown(pdev, false, wake);
@@ -5231,6 +5292,11 @@ static int __devinit e1000_probe(struct 
 
 	e1000_print_device_info(adapter);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	return 0;
 
 err_register:
@@ -5273,12 +5339,16 @@ static void __devexit e1000_remove(struc
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	bool down = test_bit(__E1000_DOWN, &adapter->state);
+
+	pm_runtime_get_sync(&pdev->dev);
 
 	/*
 	 * flush_scheduled work may reschedule our watchdog task, so
 	 * explicitly disable watchdog tasks from being rescheduled
 	 */
-	set_bit(__E1000_DOWN, &adapter->state);
+	if (!down)
+		set_bit(__E1000_DOWN, &adapter->state);
 	del_timer_sync(&adapter->watchdog_timer);
 	del_timer_sync(&adapter->phy_info_timer);
 
@@ -5292,8 +5362,17 @@ static void __devexit e1000_remove(struc
 	if (!(netdev->flags & IFF_UP))
 		e1000_power_down_phy(adapter);
 
+	/* Don't lie to e1000_close() down the road. */
+	if (!down)
+		clear_bit(__E1000_DOWN, &adapter->state);
 	unregister_netdev(netdev);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/*
 	 * Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant.
@@ -5393,6 +5472,18 @@ static struct pci_device_id e1000_pci_tb
 };
 MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
 
+static const struct dev_pm_ops e1000_pm_ops = {
+	.suspend  = e1000_suspend,
+	.resume   = e1000_resume,
+	.freeze = e1000_suspend,
+	.thaw = e1000_resume,
+	.poweroff = e1000_suspend,
+	.restore = e1000_resume,
+	.runtime_suspend = e1000_runtime_suspend,
+	.runtime_resume = e1000_runtime_resume,
+	.runtime_idle = e1000_idle,
+};
+
 /* PCI Device API Driver */
 static struct pci_driver e1000_driver = {
 	.name     = e1000e_driver_name,
@@ -5400,9 +5491,7 @@ static struct pci_driver e1000_driver = 
 	.probe    = e1000_probe,
 	.remove   = __devexit_p(e1000_remove),
 #ifdef CONFIG_PM
-	/* Power Management Hooks */
-	.suspend  = e1000_suspend,
-	.resume   = e1000_resume,
+	.driver.pm = &e1000_pm_ops,
 #endif
 	.shutdown = e1000_shutdown,
 	.err_handler = &e1000_err_handler
#61 lore raw

RFC PATCH 1/1 e1000e: Add basic runtime PM support (rev. 3) (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

In reply to Rafael J. Wysocki
Quoted 18 lines · Rafael J. Wysocki
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

Quoted 7 lines
Quoted 4 lines

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all conditions I tested it, so it is appended below.

Quoted 6 lines

This one was actually buggy, as it triggered a netdevice.h BUG_ON() during resume from suspend to RAM with network cable attached.

Appended is a new version that doesn't have this problem and enables runtime PM in _probe(), so the device can be put into the low power state if _open() hasn't been called yet (or after _close()).

This one also has a few problems (triggers a warning about an already freed interrupt being freed again during suspend to RAM with the adapter suspended at run time if it is maganed by NetworkManager and doesn't put the adapter into the low power state if the link is not present from the start), so appended is a fixed one (at least it doesn't show any more problems in my testing).

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / PM / e1000e: Add basic runtime PM support (rev. 3)

Use the PCI run-time power management framework to add simplified run-time PM support to the e1000e driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

Based on a patch from Matthew Garrett.

---
 drivers/net/e1000e/netdev.c |  141 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 115 insertions(+), 26 deletions(-)
drivers/net/e1000e/netdev.c +115 -26
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
 #include <linux/aer.h>
 
 #include "e1000.h"
@@ -3093,12 +3094,15 @@ static int e1000_open(struct net_device 
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	int err;
 
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->state))
 		return -EBUSY;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	netif_carrier_off(netdev);
 
 	/* allocate transmit descriptors */
@@ -3159,6 +3163,10 @@ static int e1000_open(struct net_device 
 
 	netif_start_queue(netdev);
 
+	pm_runtime_put_noidle(&pdev->dev);
+	if (!e1000_has_link(adapter))
+		pm_schedule_suspend(&pdev->dev, 100);
+
 	/* fire a link status change interrupt to start the watchdog */
 	ew32(ICS, E1000_ICS_LSC);
 
@@ -3172,6 +3180,7 @@ err_setup_rx:
 	e1000e_free_tx_resources(adapter);
 err_setup_tx:
 	e1000e_reset(adapter);
+	pm_runtime_put_noidle(&pdev->dev);
 
 	return err;
 }
@@ -3190,11 +3199,17 @@ err_setup_tx:
 static int e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct pci_dev *pdev = adapter->pdev;
 
 	WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
-	e1000e_down(adapter);
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	if (!test_bit(__E1000_DOWN, &adapter->state)) {
+		e1000e_down(adapter);
+		e1000_free_irq(adapter);
+	}
 	e1000_power_down_phy(adapter);
-	e1000_free_irq(adapter);
 
 	e1000e_free_tx_resources(adapter);
 	e1000e_free_rx_resources(adapter);
@@ -3216,6 +3231,8 @@ static int e1000_close(struct net_device
 	if (adapter->flags & FLAG_HAS_AMT)
 		e1000_release_hw_control(adapter);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 }
 /**
@@ -3571,6 +3588,10 @@ static void e1000_watchdog_task(struct w
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			bool txb2b = 1;
+
+			/* This is to cancel scheduled suspend requests. */
+			pm_runtime_resume(netdev->dev.parent);
+
 			/* update snapshot of PHY registers on LSC */
 			e1000_phy_read_status(adapter);
 			mac->ops.get_link_up_info(&adapter->hw,
@@ -3686,6 +3707,8 @@ static void e1000_watchdog_task(struct w
 
 			if (adapter->flags & FLAG_RX_NEEDS_RESTART)
 				schedule_work(&adapter->reset_task);
+			else
+				pm_schedule_suspend(netdev->dev.parent, 100);
 		}
 	}
 
@@ -4489,13 +4512,15 @@ out:
 	return retval;
 }
 
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+			    bool runtime)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl, ctrl_ext, rctl, status;
-	u32 wufc = adapter->wol;
+	/* Runtime suspend should only enable wakeup for link changes */
+	u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
 	int retval = 0;
 
 	netif_device_detach(netdev);
@@ -4653,40 +4678,59 @@ static void e1000e_disable_l1aspm(struct
 }
 
 #ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+static bool e1000e_pm_ready(struct e1000_adapter *adapter)
+{
+	return !!adapter->tx_ring->buffer_info;
+}
+
+static int e1000_idle(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	/*  If we're active, prevent the driver core from changing our state */
+	if (e1000e_pm_ready(adapter))
+		return -EBUSY;
+
+	pm_schedule_suspend(dev, MSEC_PER_SEC);
+	return 0;
+}
+
+static int e1000_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int retval;
 	bool wake;
 
-	retval = __e1000_shutdown(pdev, &wake);
+	retval = __e1000_shutdown(pdev, &wake, false);
 	if (!retval)
 		e1000_complete_shutdown(pdev, true, wake);
 
 	return retval;
 }
 
-static int e1000_resume(struct pci_dev *pdev)
+static int e1000_runtime_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	u32 err;
+	bool wake;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	e1000e_disable_l1aspm(pdev);
+	if (e1000e_pm_ready(adapter))
+		return __e1000_shutdown(pdev, &wake, true);
 
-	err = pci_enable_device_mem(pdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Cannot enable PCI device from suspend\n");
-		return err;
-	}
+	return 0;
+}
 
-	pci_set_master(pdev);
+static int __e1000_resume(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct e1000_hw *hw = &adapter->hw;
+	u32 err;
 
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
+	e1000e_disable_l1aspm(pdev);
 
 	e1000e_set_interrupt_capability(adapter);
 	if (netif_running(netdev)) {
@@ -4745,13 +4789,30 @@ static int e1000_resume(struct pci_dev *
 
 	return 0;
 }
+
+static int e1000_resume(struct device *dev)
+{
+	return __e1000_resume(to_pci_dev(dev));
+}
+
+static int e1000_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	if (e1000e_pm_ready(adapter))
+		return __e1000_resume(pdev);
+
+	return 0;
+}
 #endif
 
 static void e1000_shutdown(struct pci_dev *pdev)
 {
 	bool wake = false;
 
-	__e1000_shutdown(pdev, &wake);
+	__e1000_shutdown(pdev, &wake, false);
 
 	if (system_state == SYSTEM_POWER_OFF)
 		e1000_complete_shutdown(pdev, false, wake);
@@ -5231,6 +5292,11 @@ static int __devinit e1000_probe(struct 
 
 	e1000_print_device_info(adapter);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	return 0;
 
 err_register:
@@ -5273,12 +5339,16 @@ static void __devexit e1000_remove(struc
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	bool down = test_bit(__E1000_DOWN, &adapter->state);
+
+	pm_runtime_get_sync(&pdev->dev);
 
 	/*
 	 * flush_scheduled work may reschedule our watchdog task, so
 	 * explicitly disable watchdog tasks from being rescheduled
 	 */
-	set_bit(__E1000_DOWN, &adapter->state);
+	if (!down)
+		set_bit(__E1000_DOWN, &adapter->state);
 	del_timer_sync(&adapter->watchdog_timer);
 	del_timer_sync(&adapter->phy_info_timer);
 
@@ -5292,8 +5362,17 @@ static void __devexit e1000_remove(struc
 	if (!(netdev->flags & IFF_UP))
 		e1000_power_down_phy(adapter);
 
+	/* Don't lie to e1000_close() down the road. */
+	if (!down)
+		clear_bit(__E1000_DOWN, &adapter->state);
 	unregister_netdev(netdev);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/*
 	 * Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant.
@@ -5393,6 +5472,18 @@ static struct pci_device_id e1000_pci_tb
 };
 MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
 
+static const struct dev_pm_ops e1000_pm_ops = {
+	.suspend  = e1000_suspend,
+	.resume   = e1000_resume,
+	.freeze = e1000_suspend,
+	.thaw = e1000_resume,
+	.poweroff = e1000_suspend,
+	.restore = e1000_resume,
+	.runtime_suspend = e1000_runtime_suspend,
+	.runtime_resume = e1000_runtime_resume,
+	.runtime_idle = e1000_idle,
+};
+
 /* PCI Device API Driver */
 static struct pci_driver e1000_driver = {
 	.name     = e1000e_driver_name,
@@ -5400,9 +5491,7 @@ static struct pci_driver e1000_driver = 
 	.probe    = e1000_probe,
 	.remove   = __devexit_p(e1000_remove),
 #ifdef CONFIG_PM
-	/* Power Management Hooks */
-	.suspend  = e1000_suspend,
-	.resume   = e1000_resume,
+	.driver.pm = &e1000_pm_ops,
 #endif
 	.shutdown = e1000_shutdown,
 	.err_handler = &e1000_err_handler
#62 lore raw

RFC PATCH 1/1 e1000e: Add basic runtime PM support (rev. 2) (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

In reply to Rafael J. Wysocki
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · Rafael J. Wysocki

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

Quoted 7 lines
Quoted 4 lines

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all conditions I tested it, so it is appended below.

This one was actually buggy, as it triggered a netdevice.h BUG_ON() during resume from suspend to RAM with network cable attached.

Appended is a new version that doesn't have this problem and enables runtime PM in _probe(), so the device can be put into the low power state if _open() hasn't been called yet (or after _close()).

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PCI / PM / e1000e: Add basic runtime PM support (rev. 2)

Use the PCI run-time power management framework to add simplified run-time PM support to the e1000e driver. Namely, make the driver suspend the device when the link is off and set it up for generating wake-up event after the link has been detected again.

Based on a patch from Matthew Garrett.

---
 drivers/net/e1000e/netdev.c |  130 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 106 insertions(+), 24 deletions(-)
drivers/net/e1000e/netdev.c +106 -24
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
 #include <linux/aer.h>
 
 #include "e1000.h"
@@ -3093,12 +3094,15 @@ static int e1000_open(struct net_device 
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	int err;
 
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->state))
 		return -EBUSY;
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	netif_carrier_off(netdev);
 
 	/* allocate transmit descriptors */
@@ -3162,6 +3166,8 @@ static int e1000_open(struct net_device 
 	/* fire a link status change interrupt to start the watchdog */
 	ew32(ICS, E1000_ICS_LSC);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 
 err_req_irq:
@@ -3172,6 +3178,7 @@ err_setup_rx:
 	e1000e_free_tx_resources(adapter);
 err_setup_tx:
 	e1000e_reset(adapter);
+	pm_runtime_put_noidle(&pdev->dev);
 
 	return err;
 }
@@ -3190,9 +3197,15 @@ err_setup_tx:
 static int e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct pci_dev *pdev = adapter->pdev;
 
 	WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
-	e1000e_down(adapter);
+
+	pm_runtime_get_sync(&pdev->dev);
+
+	if (!test_bit(__E1000_DOWN, &adapter->state))
+		e1000e_down(adapter);
+
 	e1000_power_down_phy(adapter);
 	e1000_free_irq(adapter);
 
@@ -3216,6 +3229,8 @@ static int e1000_close(struct net_device
 	if (adapter->flags & FLAG_HAS_AMT)
 		e1000_release_hw_control(adapter);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return 0;
 }
 /**
@@ -3571,6 +3586,10 @@ static void e1000_watchdog_task(struct w
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			bool txb2b = 1;
+
+			/* This is to cancel scheduled suspend requests. */
+			pm_runtime_resume(netdev->dev.parent);
+
 			/* update snapshot of PHY registers on LSC */
 			e1000_phy_read_status(adapter);
 			mac->ops.get_link_up_info(&adapter->hw,
@@ -3686,6 +3705,8 @@ static void e1000_watchdog_task(struct w
 
 			if (adapter->flags & FLAG_RX_NEEDS_RESTART)
 				schedule_work(&adapter->reset_task);
+			else
+				pm_schedule_suspend(netdev->dev.parent, 100);
 		}
 	}
 
@@ -4489,13 +4510,15 @@ out:
 	return retval;
 }
 
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+			    bool runtime)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl, ctrl_ext, rctl, status;
-	u32 wufc = adapter->wol;
+	/* Runtime suspend should only enable wakeup for link changes */
+	u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
 	int retval = 0;
 
 	netif_device_detach(netdev);
@@ -4653,40 +4676,59 @@ static void e1000e_disable_l1aspm(struct
 }
 
 #ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+static bool e1000e_pm_ready(struct e1000_adapter *adapter)
+{
+	return !!adapter->tx_ring->buffer_info;
+}
+
+static int e1000_idle(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	/*  If we're active, prevent the driver core from changing our state */
+	if (e1000e_pm_ready(adapter))
+		return -EBUSY;
+
+	pm_schedule_suspend(dev, MSEC_PER_SEC);
+	return 0;
+}
+
+static int e1000_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int retval;
 	bool wake;
 
-	retval = __e1000_shutdown(pdev, &wake);
+	retval = __e1000_shutdown(pdev, &wake, false);
 	if (!retval)
 		e1000_complete_shutdown(pdev, true, wake);
 
 	return retval;
 }
 
-static int e1000_resume(struct pci_dev *pdev)
+static int e1000_runtime_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	u32 err;
+	bool wake;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	e1000e_disable_l1aspm(pdev);
+	if (e1000e_pm_ready(adapter))
+		return __e1000_shutdown(pdev, &wake, true);
 
-	err = pci_enable_device_mem(pdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Cannot enable PCI device from suspend\n");
-		return err;
-	}
+	return 0;
+}
 
-	pci_set_master(pdev);
+static int __e1000_resume(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct e1000_hw *hw = &adapter->hw;
+	u32 err;
 
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
+	e1000e_disable_l1aspm(pdev);
 
 	e1000e_set_interrupt_capability(adapter);
 	if (netif_running(netdev)) {
@@ -4745,13 +4787,30 @@ static int e1000_resume(struct pci_dev *
 
 	return 0;
 }
+
+static int e1000_resume(struct device *dev)
+{
+	return __e1000_resume(to_pci_dev(dev));
+}
+
+static int e1000_runtime_resume(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	if (e1000e_pm_ready(adapter))
+		return __e1000_resume(pdev);
+
+	return 0;
+}
 #endif
 
 static void e1000_shutdown(struct pci_dev *pdev)
 {
 	bool wake = false;
 
-	__e1000_shutdown(pdev, &wake);
+	__e1000_shutdown(pdev, &wake, false);
 
 	if (system_state == SYSTEM_POWER_OFF)
 		e1000_complete_shutdown(pdev, false, wake);
@@ -5231,6 +5290,11 @@ static int __devinit e1000_probe(struct 
 
 	e1000_print_device_info(adapter);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	return 0;
 
 err_register:
@@ -5274,6 +5338,8 @@ static void __devexit e1000_remove(struc
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
+	pm_runtime_get_sync(&pdev->dev);
+
 	/*
 	 * flush_scheduled work may reschedule our watchdog task, so
 	 * explicitly disable watchdog tasks from being rescheduled
@@ -5294,6 +5360,12 @@ static void __devexit e1000_remove(struc
 
 	unregister_netdev(netdev);
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
+	pm_runtime_put_noidle(&pdev->dev);
+
 	/*
 	 * Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant.
@@ -5393,6 +5465,18 @@ static struct pci_device_id e1000_pci_tb
 };
 MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
 
+static const struct dev_pm_ops e1000_pm_ops = {
+	.suspend  = e1000_suspend,
+	.resume   = e1000_resume,
+	.freeze = e1000_suspend,
+	.thaw = e1000_resume,
+	.poweroff = e1000_suspend,
+	.restore = e1000_resume,
+	.runtime_suspend = e1000_runtime_suspend,
+	.runtime_resume = e1000_runtime_resume,
+	.runtime_idle = e1000_idle,
+};
+
 /* PCI Device API Driver */
 static struct pci_driver e1000_driver = {
 	.name     = e1000e_driver_name,
@@ -5400,9 +5484,7 @@ static struct pci_driver e1000_driver = 
 	.probe    = e1000_probe,
 	.remove   = __devexit_p(e1000_remove),
 #ifdef CONFIG_PM
-	/* Power Management Hooks */
-	.suspend  = e1000_suspend,
-	.resume   = e1000_resume,
+	.driver.pm = &e1000_pm_ops,
 #endif
 	.shutdown = e1000_shutdown,
 	.err_handler = &e1000_err_handler
#63 lore raw

RFC PATCH 1/1 e1000e: Add basic runtime PM support (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

In reply to Rafael J. Wysocki
Quoted 5 lines · Rafael J. Wysocki

Hi,

The following (updated) series of patches provides preliminary run-time power management support for PCI devices through ACPI and/or the native PCIe PME.

Quoted 7 lines

Some patches have been modified since the previous iteration, one patch has been merged and there's one more.

I've tested this patchset with the native PCIe PME mechanism using the r8169 driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all conditions I tested it, so it is appended below.

Quoted 20 lines

[1/12] - Add function for checking PME status of devices

[2/12] - Modify wake-up enable propagation so that it's done for PCIe devices
too (this one is in the Jesse's tree already, but it's reproduced here for completness)

[3/12] - PCIe PME root port service driver

[4/12] - "Don't use MSIs for PME signaling" switch for PCIe

[5/12] - ACPI GPE refcounting, from Matthew

[6/12] - ACPI drivers support for GPE refcounting, from Matthew

[7/12] - ACPI removal of the old GPE API, from Matthew

[8/12] - ACPI add fields for handling run-wake devices

[9/12][New] - ACPI add helper function for enabling/disabling wake-up power

Quoted 8 lines

[10/12] - PCI / ACPI platform support for run-time power management

[11/12] - Runtime PM callbacks for the PCI bus type

[12/12] - Runtime PM support for r8169 (experimental)

If there are no objections, I think the patches [1-11/12] are ready for the upstream.

---
From: Rafael J. Wysocki <[email protected]>
Subject: e1000e: Add basic runtime PM support

Add support for suspending the device on link removal and resuming it on link gain.

Based on a patch from Matthew Garrett.

---
 drivers/net/e1000e/netdev.c |   90 ++++++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 24 deletions(-)
drivers/net/e1000e/netdev.c +66 -24
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
@@ -44,6 +44,7 @@
 #include <linux/cpu.h>
 #include <linux/smp.h>
 #include <linux/pm_qos_params.h>
+#include <linux/pm_runtime.h>
 #include <linux/aer.h>
 
 #include "e1000.h"
@@ -3093,12 +3094,18 @@ static int e1000_open(struct net_device 
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	int err;
 
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->state))
 		return -EBUSY;
 
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_set_active(&pdev->dev);
+		pm_runtime_enable(&pdev->dev);
+	}
+
 	netif_carrier_off(netdev);
 
 	/* allocate transmit descriptors */
@@ -3190,9 +3197,21 @@ err_setup_tx:
 static int e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
+	struct pci_dev *pdev = adapter->pdev;
 
 	WARN_ON(test_bit(__E1000_RESETTING, &adapter->state));
-	e1000e_down(adapter);
+
+	if (pci_dev_run_wake(pdev)) {
+		pm_runtime_get_noresume(&pdev->dev);
+		pm_runtime_resume(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+	}
+
+	if (netif_running(netdev))
+		e1000e_down(adapter);
+
 	e1000_power_down_phy(adapter);
 	e1000_free_irq(adapter);
 
@@ -3571,6 +3590,10 @@ static void e1000_watchdog_task(struct w
 	if (link) {
 		if (!netif_carrier_ok(netdev)) {
 			bool txb2b = 1;
+
+			/* This is to cancel scheduled suspend requests. */
+			pm_runtime_resume(netdev->dev.parent);
+
 			/* update snapshot of PHY registers on LSC */
 			e1000_phy_read_status(adapter);
 			mac->ops.get_link_up_info(&adapter->hw,
@@ -3686,6 +3709,8 @@ static void e1000_watchdog_task(struct w
 
 			if (adapter->flags & FLAG_RX_NEEDS_RESTART)
 				schedule_work(&adapter->reset_task);
+			else
+				pm_schedule_suspend(netdev->dev.parent, 100);
 		}
 	}
 
@@ -4489,13 +4514,15 @@ out:
 	return retval;
 }
 
-static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
+static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake,
+			    bool runtime)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl, ctrl_ext, rctl, status;
-	u32 wufc = adapter->wol;
+	/* Runtime suspend should only enable wakeup for link changes */
+	u32 wufc = runtime ? E1000_WUFC_LNKC : adapter->wol;
 	int retval = 0;
 
 	netif_device_detach(netdev);
@@ -4653,41 +4680,43 @@ static void e1000e_disable_l1aspm(struct
 }
 
 #ifdef CONFIG_PM
-static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
+static int e1000_idle(struct device *dev)
 {
+	/* Prevent the driver core from changing our state */
+	return -EBUSY;
+}
+
+static int e1000_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int retval;
 	bool wake;
 
-	retval = __e1000_shutdown(pdev, &wake);
+	retval = __e1000_shutdown(pdev, &wake, false);
 	if (!retval)
 		e1000_complete_shutdown(pdev, true, wake);
 
 	return retval;
 }
 
-static int e1000_resume(struct pci_dev *pdev)
+static int e1000_runtime_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	bool wake;
+
+	return __e1000_shutdown(pdev, &wake, true);
+}
+
+static int e1000_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	u32 err;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
 	e1000e_disable_l1aspm(pdev);
 
-	err = pci_enable_device_mem(pdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Cannot enable PCI device from suspend\n");
-		return err;
-	}
-
-	pci_set_master(pdev);
-
-	pci_enable_wake(pdev, PCI_D3hot, 0);
-	pci_enable_wake(pdev, PCI_D3cold, 0);
-
 	e1000e_set_interrupt_capability(adapter);
 	if (netif_running(netdev)) {
 		err = e1000_request_irq(adapter);
@@ -4751,7 +4780,7 @@ static void e1000_shutdown(struct pci_de
 {
 	bool wake = false;
 
-	__e1000_shutdown(pdev, &wake);
+	__e1000_shutdown(pdev, &wake, false);
 
 	if (system_state == SYSTEM_POWER_OFF)
 		e1000_complete_shutdown(pdev, false, wake);
@@ -5274,6 +5303,9 @@ static void __devexit e1000_remove(struc
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
+	if (pci_dev_run_wake(pdev))
+		pm_runtime_resume(&pdev->dev);
+
 	/*
 	 * flush_scheduled work may reschedule our watchdog task, so
 	 * explicitly disable watchdog tasks from being rescheduled
@@ -5393,6 +5425,18 @@ static struct pci_device_id e1000_pci_tb
 };
 MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
 
+static struct dev_pm_ops e1000_pm_ops = {
+	.suspend  = e1000_suspend,
+	.resume   = e1000_resume,
+	.freeze = e1000_suspend,
+	.thaw = e1000_resume,
+	.poweroff = e1000_suspend,
+	.restore = e1000_resume,
+	.runtime_suspend = e1000_runtime_suspend,
+	.runtime_resume = e1000_resume,
+	.runtime_idle = e1000_idle,
+};
+
 /* PCI Device API Driver */
 static struct pci_driver e1000_driver = {
 	.name     = e1000e_driver_name,
@@ -5400,9 +5444,7 @@ static struct pci_driver e1000_driver = 
 	.probe    = e1000_probe,
 	.remove   = __devexit_p(e1000_remove),
 #ifdef CONFIG_PM
-	/* Power Management Hooks */
-	.suspend  = e1000_suspend,
-	.resume   = e1000_resume,
+	.driver.pm = &e1000_pm_ops,
 #endif
 	.shutdown = e1000_shutdown,
 	.err_handler = &e1000_err_handler