linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3)
@ 2008-07-01 21:56 Rafael J. Wysocki
  2008-07-01 22:00 ` [PATCH 1/9] PCI ACPI: Remove acpi_platform_enable_wakeup Rafael J. Wysocki
                   ` (10 more replies)
  0 siblings, 11 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 21:56 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui

Hi,

This is a new (3rd) iteration of the series of patches intended to rework the power
management of PCI devices so that the handling of their wake-up functionality
is consistent with the other PM operations and so that the user space can
manage the wake-up functionality of PCI devices using the
/sys/devices/.../power/wakeup interface regardless of whether the wake-up
function is based on the native PCI mechanism (PME#), or it is handled by the
platform (eg. ACPI).

Apart from the ACPI and PCI changes necessary for this purpose, the series
includes some clean-ups that are not strictly required, but make the code more
straightforward and (IMHO) easier to follow.

The patchset is on top of the linux-next from today.  It contains one
additional patch [1/9] which is a revert of another patch that shouldn't have
been put into linux-next (at least not in the way it was done).

Please review.

Thanks,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/9] PCI ACPI: Remove acpi_platform_enable_wakeup
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
@ 2008-07-01 22:00 ` Rafael J. Wysocki
  2008-07-01 22:03 ` [PATCH 2/9] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:00 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui, Stephen Rothwell, Tobias Diedrich, Jeff Garzik

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI ACPI: Remove acpi_platform_enable_wakeup

This patch removes "Fix forcedeth hibernate/wake-on-lan problems",
commit f5ccbcfacaae57e3312e623432a79d5f1f079cf5 in linux-next,
which is incorrect, because it directly manipulates the
wakeup.state.enabled flags of ACPI devices while these flags are
supposed to be used by the user space to control wake-up.  Moreover,
it affects not only the device it is supposed to fix (forcedeth), but
also every PCI device that happens to call
device_init_wakeup(dev, 1).

Apart from this, it has been pushed as a network driver fix, while it should
have gone through either ACPI or PCI tree.  Also, the other patches in this
series are intended to fix the issue it was supposed to fix, but in a more
complete way.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci-acpi.c |   20 --------------------
 1 file changed, 20 deletions(-)

Index: linux-next/drivers/pci/pci-acpi.c
===================================================================
--- linux-next.orig/drivers/pci/pci-acpi.c
+++ linux-next/drivers/pci/pci-acpi.c
@@ -259,25 +259,6 @@ static pci_power_t acpi_pci_choose_state
 	}
 	return PCI_POWER_ERROR;
 }
-
-static int acpi_platform_enable_wakeup(struct device *dev, int is_on)
-{
-	struct acpi_device	*adev;
-	int			status;
-
-	if (!device_can_wakeup(dev))
-		return -EINVAL;
-
-	if (is_on && !device_may_wakeup(dev))
-		return -EINVAL;
-
-	status = acpi_bus_get_device(DEVICE_ACPI_HANDLE(dev), &adev);
-	if (status < 0)
-		return status;
-
-	adev->wakeup.state.enabled = !!is_on;
-	return 0;
-}
 #endif
 
 static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
@@ -362,7 +343,6 @@ static int __init acpi_pci_init(void)
 		return 0;
 #ifdef	CONFIG_ACPI_SLEEP
 	platform_pci_choose_state = acpi_pci_choose_state;
-	platform_enable_wakeup = acpi_platform_enable_wakeup;
 #endif
 	platform_pci_set_power_state = acpi_pci_set_power_state;
 	return 0;

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 2/9] ACPI: Introduce acpi_bus_power_manageable function
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
  2008-07-01 22:00 ` [PATCH 1/9] PCI ACPI: Remove acpi_platform_enable_wakeup Rafael J. Wysocki
@ 2008-07-01 22:03 ` Rafael J. Wysocki
  2008-07-01 22:04 ` [PATCH 3/9] PCI: Introduce platform_pci_power_manageable function Rafael J. Wysocki
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:03 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

ACPI: Introduce acpi_bus_power_manageable function

Introduce function acpi_bus_power_manageable() allowing other
(dependent) subsystems to check if ACPI is able to power manage given
device.  This may be useful, for example, for PCI device power
management.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
 drivers/acpi/bus.c      |   11 +++++++++++
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 12 insertions(+)

Index: linux-next/drivers/acpi/bus.c
===================================================================
--- linux-next.orig/drivers/acpi/bus.c
+++ linux-next/drivers/acpi/bus.c
@@ -295,6 +295,17 @@ int acpi_bus_set_power(acpi_handle handl
 
 EXPORT_SYMBOL(acpi_bus_set_power);
 
+bool acpi_bus_power_manageable(acpi_handle handle)
+{
+	struct acpi_device *device;
+	int result;
+
+	result = acpi_bus_get_device(handle, &device);
+	return result ? false : device->flags.power_manageable;
+}
+
+EXPORT_SYMBOL(acpi_bus_power_manageable);
+
 /* --------------------------------------------------------------------------
                                 Event Management
    -------------------------------------------------------------------------- */
Index: linux-next/include/acpi/acpi_bus.h
===================================================================
--- linux-next.orig/include/acpi/acpi_bus.h
+++ linux-next/include/acpi/acpi_bus.h
@@ -335,6 +335,7 @@ void acpi_bus_data_handler(acpi_handle h
 int acpi_bus_get_status(struct acpi_device *device);
 int acpi_bus_get_power(acpi_handle handle, int *state);
 int acpi_bus_set_power(acpi_handle handle, int state);
+bool acpi_bus_power_manageable(acpi_handle handle);
 #ifdef CONFIG_ACPI_PROC_EVENT
 int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
 int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 3/9] PCI: Introduce platform_pci_power_manageable function
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
  2008-07-01 22:00 ` [PATCH 1/9] PCI ACPI: Remove acpi_platform_enable_wakeup Rafael J. Wysocki
  2008-07-01 22:03 ` [PATCH 2/9] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
@ 2008-07-01 22:04 ` Rafael J. Wysocki
  2008-07-01 22:05 ` [PATCH 4/9] PCI: Rework pci_set_power_state function (rev. 3) Rafael J. Wysocki
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:04 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI: Introduce platform_pci_power_manageable function

Introduce function pointer platform_pci_power_manageable to be used
by the platform-related code to point to a function allowing us to
check if given device is power manageable by the platform.

Introduce acpi_pci_power_manageable() playing that role for ACPI.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci-acpi.c |   19 +++++++++++++------
 drivers/pci/pci.c      |   40 ++++++++++++++++++++++++++++++----------
 drivers/pci/pci.h      |   26 ++++++++++++++++++++++----
 3 files changed, 65 insertions(+), 20 deletions(-)

Index: linux-next/drivers/pci/pci-acpi.c
===================================================================
--- linux-next.orig/drivers/pci/pci-acpi.c
+++ linux-next/drivers/pci/pci-acpi.c
@@ -215,7 +215,6 @@ acpi_status pci_osc_control_set(acpi_han
 }
 EXPORT_SYMBOL(pci_osc_control_set);
 
-#ifdef CONFIG_ACPI_SLEEP
 /*
  * _SxD returns the D-state with the highest power
  * (lowest D-state number) supported in the S-state "x".
@@ -259,7 +258,13 @@ static pci_power_t acpi_pci_choose_state
 	}
 	return PCI_POWER_ERROR;
 }
-#endif
+
+static bool acpi_pci_power_manageable(struct pci_dev *dev)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	return handle ? acpi_bus_power_manageable(handle) : false;
+}
 
 static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
@@ -290,6 +295,11 @@ static int acpi_pci_set_power_state(stru
 	return -EINVAL;
 }
 
+static struct pci_platform_pm_ops acpi_pci_platform_pm = {
+	.is_manageable = acpi_pci_power_manageable,
+	.set_state = acpi_pci_set_power_state,
+	.choose_state = acpi_pci_choose_state,
+};
 
 /* ACPI bus type */
 static int acpi_pci_find_device(struct device *dev, acpi_handle *handle)
@@ -341,10 +351,7 @@ static int __init acpi_pci_init(void)
 	ret = register_acpi_bus_type(&acpi_pci_bus);
 	if (ret)
 		return 0;
-#ifdef	CONFIG_ACPI_SLEEP
-	platform_pci_choose_state = acpi_pci_choose_state;
-#endif
-	platform_pci_set_power_state = acpi_pci_set_power_state;
+	pci_set_platform_pm(&acpi_pci_platform_pm);
 	return 0;
 }
 arch_initcall(acpi_pci_init);
Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -376,7 +376,32 @@ pci_restore_bars(struct pci_dev *dev)
 		pci_update_resource(dev, &dev->resource[i], i);
 }
 
-int (*platform_pci_set_power_state)(struct pci_dev *dev, pci_power_t t);
+static struct pci_platform_pm_ops *pci_platform_pm;
+
+int pci_set_platform_pm(struct pci_platform_pm_ops *ops)
+{
+	if (!ops->is_manageable || !ops->set_state || !ops->choose_state)
+		return -EINVAL;
+	pci_platform_pm = ops;
+	return 0;
+}
+
+static inline bool platform_pci_power_manageable(struct pci_dev *dev)
+{
+	return pci_platform_pm ? pci_platform_pm->is_manageable(dev) : false;
+}
+
+static inline int platform_pci_set_power_state(struct pci_dev *dev,
+                                                pci_power_t t)
+{
+	return pci_platform_pm ? pci_platform_pm->set_state(dev, t) : -ENOSYS;
+}
+
+static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev)
+{
+	return pci_platform_pm ?
+			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
+}
 
 /**
  * pci_set_power_state - Set the power state of a PCI device
@@ -479,8 +504,7 @@ pci_set_power_state(struct pci_dev *dev,
 	 * Give firmware a chance to be called, such as ACPI _PRx, _PSx
 	 * Firmware method after native method ?
 	 */
-	if (platform_pci_set_power_state)
-		platform_pci_set_power_state(dev, state);
+	platform_pci_set_power_state(dev, state);
 
 	dev->current_state = state;
 
@@ -505,8 +529,6 @@ pci_set_power_state(struct pci_dev *dev,
 	return 0;
 }
 
-pci_power_t (*platform_pci_choose_state)(struct pci_dev *dev);
- 
 /**
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
@@ -524,11 +546,9 @@ pci_power_t pci_choose_state(struct pci_
 	if (!pci_find_capability(dev, PCI_CAP_ID_PM))
 		return PCI_D0;
 
-	if (platform_pci_choose_state) {
-		ret = platform_pci_choose_state(dev);
-		if (ret != PCI_POWER_ERROR)
-			return ret;
-	}
+	ret = platform_pci_choose_state(dev);
+	if (ret != PCI_POWER_ERROR)
+		return ret;
 
 	switch (state.event) {
 	case PM_EVENT_ON:
Index: linux-next/drivers/pci/pci.h
===================================================================
--- linux-next.orig/drivers/pci/pci.h
+++ linux-next/drivers/pci/pci.h
@@ -5,10 +5,28 @@ extern int pci_create_sysfs_dev_files(st
 extern void pci_remove_sysfs_dev_files(struct pci_dev *pdev);
 extern void pci_cleanup_rom(struct pci_dev *dev);
 
-/* Firmware callbacks */
-extern pci_power_t (*platform_pci_choose_state)(struct pci_dev *dev);
-extern int (*platform_pci_set_power_state)(struct pci_dev *dev,
-						pci_power_t state);
+/**
+ * Firmware PM callbacks
+ *
+ * @is_manageable - returns 'true' if given device is power manageable by the
+ *                  platform firmware
+ *
+ * @set_state - invokes the platform firmware to set the device's power state
+ *
+ * @choose_state - returns PCI power state of given device preferred by the
+ *                 platform; to be used during system-wide transitions from a
+ *                 sleeping state to the working state and vice versa
+ *
+ * If given platform is generally capable of power managing PCI devices, all of
+ * these callbacks are mandatory.
+ */
+struct pci_platform_pm_ops {
+	bool (*is_manageable)(struct pci_dev *dev);
+	int (*set_state)(struct pci_dev *dev, pci_power_t state);
+	pci_power_t (*choose_state)(struct pci_dev *dev);
+};
+
+extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 
 extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
 extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 4/9] PCI: Rework pci_set_power_state function (rev. 3)
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2008-07-01 22:04 ` [PATCH 3/9] PCI: Introduce platform_pci_power_manageable function Rafael J. Wysocki
@ 2008-07-01 22:05 ` Rafael J. Wysocki
  2008-07-01 22:06 ` [PATCH 5/9] ACPI: Introduce acpi_device_sleep_wake function Rafael J. Wysocki
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:05 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI: Rework pci_set_power_state function (rev. 3)

Rework pci_set_power_state() so that the platform callback is
invoked before the native mechanism, if necessary.  Also, make
the function check if the device is power manageable by the
platform before invoking the platform callback.

This may matter if the device dependent on additional power
resources controlled by the platform is being put into D0, in which
case those power resources must be turned on before we attempt to
handle the device itself. 

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
 drivers/pci/pci-acpi.c |   16 +++--
 drivers/pci/pci.c      |  152 +++++++++++++++++++++++++++++++++----------------
 2 files changed, 115 insertions(+), 53 deletions(-)

Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -404,67 +404,56 @@ static inline pci_power_t platform_pci_c
 }
 
 /**
- * pci_set_power_state - Set the power state of a PCI device
- * @dev: PCI device to be suspended
- * @state: PCI power state (D0, D1, D2, D3hot, D3cold) we're entering
- *
- * Transition a device to a new power state, using the Power Management 
- * Capabilities in the device's config space.
- *
- * RETURN VALUE: 
- * -EINVAL if trying to enter a lower state than we're already in.
- * 0 if we're already in the requested state.
- * -EIO if device does not support PCI PM.
- * 0 if we can successfully change the power state.
+ * pci_raw_set_power_state - Use PCI PM registers to set the power state of
+ *                           given PCI device
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
+ *
+ * RETURN VALUE:
+ * -EINVAL if the requested state is invalid.
+ * -EIO if device does not support PCI PM or its PM capabilities register has a
+ * wrong version, or device doesn't support the requested state.
+ * 0 if device already is in the requested state.
+ * 0 if device's power state has been successfully changed.
  */
-int
-pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+static int
+pci_raw_set_power_state(struct pci_dev *dev, int pm, pci_power_t state)
 {
-	int pm, need_restore = 0;
 	u16 pmcsr, pmc;
+	bool need_restore = false;
 
-	/* bound the state we're entering */
-	if (state > PCI_D3hot)
-		state = PCI_D3hot;
-
-	/*
-	 * If the device or the parent bridge can't support PCI PM, ignore
-	 * the request if we're doing anything besides putting it into D0
-	 * (which would only happen on boot).
-	 */
-	if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
-		return 0;
-
-	/* find PCI PM capability in list */
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-
-	/* abort if the device doesn't support PM capabilities */
 	if (!pm)
 		return -EIO;
 
+	if (state < PCI_D0 || state > PCI_D3hot)
+		return -EINVAL;
+
 	/* Validate current state:
 	 * Can enter D0 from any state, but if we can only go deeper 
 	 * to sleep if we're already in a low power state
 	 */
-	if (state != PCI_D0 && dev->current_state > state) {
+	if (dev->current_state == state) {
+		/* we're already there */
+		return 0;
+	} else if (state != PCI_D0 && dev->current_state <= PCI_D3cold
+	    && dev->current_state > state) {
 		dev_err(&dev->dev, "invalid power transition "
 			"(from state %d to %d)\n", dev->current_state, state);
 		return -EINVAL;
-	} else if (dev->current_state == state)
-		return 0;        /* we're already there */
+	}
 
+	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
 
-	pci_read_config_word(dev,pm + PCI_PM_PMC,&pmc);
 	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
-		dev_printk(KERN_DEBUG, &dev->dev, "unsupported PM cap regs "
-			   "version (%u)\n", pmc & PCI_PM_CAP_VER_MASK);
+		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
+			pmc & PCI_PM_CAP_VER_MASK);
 		return -EIO;
 	}
 
 	/* check if this device supports the desired state */
-	if (state == PCI_D1 && !(pmc & PCI_PM_CAP_D1))
-		return -EIO;
-	else if (state == PCI_D2 && !(pmc & PCI_PM_CAP_D2))
+	if ((state == PCI_D1 && !(pmc & PCI_PM_CAP_D1))
+	   || (state == PCI_D2 && !(pmc & PCI_PM_CAP_D2)))
 		return -EIO;
 
 	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
@@ -483,7 +472,7 @@ pci_set_power_state(struct pci_dev *dev,
 	case PCI_UNKNOWN: /* Boot-up */
 		if ((pmcsr & PCI_PM_CTRL_STATE_MASK) == PCI_D3hot
 		 && !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET))
-			need_restore = 1;
+			need_restore = true;
 		/* Fall-through: force to D0 */
 	default:
 		pmcsr = 0;
@@ -500,12 +489,6 @@ pci_set_power_state(struct pci_dev *dev,
 	else if (state == PCI_D2 || dev->current_state == PCI_D2)
 		udelay(200);
 
-	/*
-	 * Give firmware a chance to be called, such as ACPI _PRx, _PSx
-	 * Firmware method after native method ?
-	 */
-	platform_pci_set_power_state(dev, state);
-
 	dev->current_state = state;
 
 	/* According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT
@@ -530,6 +513,81 @@ pci_set_power_state(struct pci_dev *dev,
 }
 
 /**
+ * pci_update_current_state - Read PCI power state of given device from its
+ *                            PCI PM registers and cache it
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ */
+static void pci_update_current_state(struct pci_dev *dev, int pm)
+{
+	if (pm) {
+		u16 pmcsr;
+
+		pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+		dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK);
+	}
+}
+
+/**
+ * pci_set_power_state - Set the power state of a PCI device
+ * @dev: PCI device to handle.
+ * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
+ *
+ * Transition a device to a new power state, using the platform formware and/or
+ * the device's PCI PM registers.
+ *
+ * RETURN VALUE:
+ * -EINVAL if the requested state is invalid.
+ * -EIO if device does not support PCI PM or its PM capabilities register has a
+ * wrong version, or device doesn't support the requested state.
+ * 0 if device already is in the requested state.
+ * 0 if device's power state has been successfully changed.
+ */
+int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+	int pm, error;
+
+	/* bound the state we're entering */
+	if (state > PCI_D3hot)
+		state = PCI_D3hot;
+	else if (state < PCI_D0)
+		state = PCI_D0;
+	else if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
+		/*
+		 * If the device or the parent bridge do not support PCI PM,
+		 * ignore the request if we're doing anything other than putting
+		 * it into D0 (which would only happen on boot).
+		 */
+		return 0;
+
+	/* Find PCI PM capability in the list */
+	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+
+	if (state == PCI_D0 && platform_pci_power_manageable(dev)) {
+		/*
+		 * Allow the platform to change the state, for example via ACPI
+		 * _PR0, _PS0 and some such, but do not trust it.
+		 */
+		int ret = platform_pci_set_power_state(dev, PCI_D0);
+		if (!ret)
+			pci_update_current_state(dev, pm);
+	}
+
+	error = pci_raw_set_power_state(dev, pm, state);
+
+	if (state > PCI_D0 && platform_pci_power_manageable(dev)) {
+		/* Allow the platform to finalize the transition */
+		int ret = platform_pci_set_power_state(dev, state);
+		if (!ret) {
+			pci_update_current_state(dev, pm);
+			error = 0;
+		}
+	}
+
+	return error;
+}
+
+/**
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
  * @state: target sleep state for the whole system. This is the value
Index: linux-next/drivers/pci/pci-acpi.c
===================================================================
--- linux-next.orig/drivers/pci/pci-acpi.c
+++ linux-next/drivers/pci/pci-acpi.c
@@ -277,12 +277,11 @@ static int acpi_pci_set_power_state(stru
 		[PCI_D3hot] = ACPI_STATE_D3,
 		[PCI_D3cold] = ACPI_STATE_D3
 	};
+	int error = -EINVAL;
 
-	if (!handle)
-		return -ENODEV;
 	/* If the ACPI device has _EJ0, ignore the device */
-	if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
-		return 0;
+	if (!handle || ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
+		return -ENODEV;
 
 	switch (state) {
 	case PCI_D0:
@@ -290,9 +289,14 @@ static int acpi_pci_set_power_state(stru
 	case PCI_D2:
 	case PCI_D3hot:
 	case PCI_D3cold:
-		return acpi_bus_set_power(handle, state_conv[state]);
+		error = acpi_bus_set_power(handle, state_conv[state]);
 	}
-	return -EINVAL;
+
+	if (!error)
+		printk(KERN_INFO "PCI: Power state of %s changed by ACPI\n",
+			pci_name(dev));
+
+	return error;
 }
 
 static struct pci_platform_pm_ops acpi_pci_platform_pm = {

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 5/9] ACPI: Introduce acpi_device_sleep_wake function
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2008-07-01 22:05 ` [PATCH 4/9] PCI: Rework pci_set_power_state function (rev. 3) Rafael J. Wysocki
@ 2008-07-01 22:06 ` Rafael J. Wysocki
  2008-07-01 22:07 ` [PATCH 6/9] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:06 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

ACPI: Introduce acpi_device_sleep_wake function

The currect ACPI code attempts to execute _PSW at three different
places and in one of them only it tries to execute _DSW before _PSW,
which is inconsistent with the other two cases.

Move the execution of _DSW and _PSW into a separate function called
acpi_device_sleep_wake() and call it wherever appropriate instead of
executing _DSW and/or _PSW directly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
 drivers/acpi/power.c        |  118 ++++++++++++++++++++++++++++++--------------
 drivers/acpi/scan.c         |   42 ++-------------
 drivers/acpi/sleep/wakeup.c |    2 
 include/acpi/acpi_drivers.h |    4 +
 4 files changed, 92 insertions(+), 74 deletions(-)

Index: linux-next/drivers/acpi/power.c
===================================================================
--- linux-next.orig/drivers/acpi/power.c
+++ linux-next/drivers/acpi/power.c
@@ -292,69 +292,115 @@ static int acpi_power_off_device(acpi_ha
 	return 0;
 }
 
+/**
+ * acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
+ *                          ACPI 3.0) _PSW (Power State Wake)
+ * @dev: Device to handle.
+ * @enable: 0 - disable, 1 - enable the wake capabilities of the device.
+ * @sleep_state: Target sleep state of the system.
+ * @dev_state: Target power state of the device.
+ *
+ * Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
+ * State Wake) for the device, if present.  On failure reset the device's
+ * wakeup.flags.valid flag.
+ *
+ * RETURN VALUE:
+ * 0 if either _DSW or _PSW has been successfully executed
+ * 0 if neither _DSW nor _PSW has been found
+ * -ENODEV if the execution of either _DSW or _PSW has failed
+ */
+int acpi_device_sleep_wake(struct acpi_device *dev,
+                           int enable, int sleep_state, int dev_state)
+{
+	union acpi_object in_arg[3];
+	struct acpi_object_list arg_list = { 3, in_arg };
+	acpi_status status = AE_OK;
+
+	/*
+	 * Try to execute _DSW first.
+	 *
+	 * Three agruments are needed for the _DSW object:
+	 * Argument 0: enable/disable the wake capabilities
+	 * Argument 1: target system state
+	 * Argument 2: target device state
+	 * When _DSW object is called to disable the wake capabilities, maybe
+	 * the first argument is filled. The values of the other two agruments
+	 * are meaningless.
+	 */
+	in_arg[0].type = ACPI_TYPE_INTEGER;
+	in_arg[0].integer.value = enable;
+	in_arg[1].type = ACPI_TYPE_INTEGER;
+	in_arg[1].integer.value = sleep_state;
+	in_arg[2].type = ACPI_TYPE_INTEGER;
+	in_arg[2].integer.value = dev_state;
+	status = acpi_evaluate_object(dev->handle, "_DSW", &arg_list, NULL);
+	if (ACPI_SUCCESS(status)) {
+		return 0;
+	} else if (status != AE_NOT_FOUND) {
+		printk(KERN_ERR PREFIX "_DSW execution failed\n");
+		dev->wakeup.flags.valid = 0;
+		return -ENODEV;
+	}
+
+	/* Execute _PSW */
+	arg_list.count = 1;
+	in_arg[0].integer.value = enable;
+	status = acpi_evaluate_object(dev->handle, "_PSW", &arg_list, NULL);
+	if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
+		printk(KERN_ERR PREFIX "_PSW execution failed\n");
+		dev->wakeup.flags.valid = 0;
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 /*
  * Prepare a wakeup device, two steps (Ref ACPI 2.0:P229):
  * 1. Power on the power resources required for the wakeup device 
- * 2. Enable _PSW (power state wake) for the device if present
+ * 2. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
+ *    State Wake) for the device, if present
  */
-int acpi_enable_wakeup_device_power(struct acpi_device *dev)
+int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
 {
-	union acpi_object arg = { ACPI_TYPE_INTEGER };
-	struct acpi_object_list arg_list = { 1, &arg };
-	acpi_status status = AE_OK;
 	int i;
-	int ret = 0;
 
 	if (!dev || !dev->wakeup.flags.valid)
-		return -1;
+		return -EINVAL;
 
-	arg.integer.value = 1;
 	/* Open power resource */
 	for (i = 0; i < dev->wakeup.resources.count; i++) {
-		ret = acpi_power_on(dev->wakeup.resources.handles[i], dev);
+		int ret = acpi_power_on(dev->wakeup.resources.handles[i], dev);
 		if (ret) {
 			printk(KERN_ERR PREFIX "Transition power state\n");
 			dev->wakeup.flags.valid = 0;
-			return -1;
+			return -ENODEV;
 		}
 	}
 
-	/* Execute PSW */
-	status = acpi_evaluate_object(dev->handle, "_PSW", &arg_list, NULL);
-	if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
-		printk(KERN_ERR PREFIX "Evaluate _PSW\n");
-		dev->wakeup.flags.valid = 0;
-		ret = -1;
-	}
-
-	return ret;
+	/*
+	 * Passing 3 as the third argument below means the device may be placed
+	 * in arbitrary power state afterwards.
+	 */
+	return acpi_device_sleep_wake(dev, 1, sleep_state, 3);
 }
 
 /*
  * Shutdown a wakeup device, counterpart of above method
- * 1. Disable _PSW (power state wake)
+ * 1. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
+ *    State Wake) for the device, if present
  * 2. Shutdown down the power resources
  */
 int acpi_disable_wakeup_device_power(struct acpi_device *dev)
 {
-	union acpi_object arg = { ACPI_TYPE_INTEGER };
-	struct acpi_object_list arg_list = { 1, &arg };
-	acpi_status status = AE_OK;
-	int i;
-	int ret = 0;
-
+	int i, ret;
 
 	if (!dev || !dev->wakeup.flags.valid)
-		return -1;
+		return -EINVAL;
 
-	arg.integer.value = 0;
-	/* Execute PSW */
-	status = acpi_evaluate_object(dev->handle, "_PSW", &arg_list, NULL);
-	if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
-		printk(KERN_ERR PREFIX "Evaluate _PSW\n");
-		dev->wakeup.flags.valid = 0;
-		return -1;
-	}
+	ret = acpi_device_sleep_wake(dev, 0, 0, 0);
+	if (ret)
+		return ret;
 
 	/* Close power resource */
 	for (i = 0; i < dev->wakeup.resources.count; i++) {
@@ -362,7 +408,7 @@ int acpi_disable_wakeup_device_power(str
 		if (ret) {
 			printk(KERN_ERR PREFIX "Transition power state\n");
 			dev->wakeup.flags.valid = 0;
-			return -1;
+			return -ENODEV;
 		}
 	}
 
Index: linux-next/drivers/acpi/sleep/wakeup.c
===================================================================
--- linux-next.orig/drivers/acpi/sleep/wakeup.c
+++ linux-next/drivers/acpi/sleep/wakeup.c
@@ -42,7 +42,7 @@ void acpi_enable_wakeup_device_prep(u8 s
 			continue;
 
 		spin_unlock(&acpi_device_lock);
-		acpi_enable_wakeup_device_power(dev);
+		acpi_enable_wakeup_device_power(dev, sleep_state);
 		spin_lock(&acpi_device_lock);
 	}
 	spin_unlock(&acpi_device_lock);
Index: linux-next/include/acpi/acpi_drivers.h
===================================================================
--- linux-next.orig/include/acpi/acpi_drivers.h
+++ linux-next/include/acpi/acpi_drivers.h
@@ -87,7 +87,9 @@ struct pci_bus *pci_acpi_scan_root(struc
    -------------------------------------------------------------------------- */
 
 #ifdef CONFIG_ACPI_POWER
-int acpi_enable_wakeup_device_power(struct acpi_device *dev);
+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);
Index: linux-next/drivers/acpi/scan.c
===================================================================
--- linux-next.orig/drivers/acpi/scan.c
+++ linux-next/drivers/acpi/scan.c
@@ -703,9 +703,7 @@ static int acpi_bus_get_wakeup_device_fl
 	acpi_status status = 0;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *package = NULL;
-	union acpi_object in_arg[3];
-	struct acpi_object_list arg_list = { 3, in_arg };
-	acpi_status psw_status = AE_OK;
+	int psw_error;
 
 	struct acpi_device_id button_device_ids[] = {
 		{"PNP0C0D", 0},
@@ -737,39 +735,11 @@ static int acpi_bus_get_wakeup_device_fl
 	 * So it is necessary to call _DSW object first. Only when it is not
 	 * present will the _PSW object used.
 	 */
-	/*
-	 * Three agruments are needed for the _DSW object.
-	 * Argument 0: enable/disable the wake capabilities
-	 * When _DSW object is called to disable the wake capabilities, maybe
-	 * the first argument is filled. The value of the other two agruments
-	 * is meaningless.
-	 */
-	in_arg[0].type = ACPI_TYPE_INTEGER;
-	in_arg[0].integer.value = 0;
-	in_arg[1].type = ACPI_TYPE_INTEGER;
-	in_arg[1].integer.value = 0;
-	in_arg[2].type = ACPI_TYPE_INTEGER;
-	in_arg[2].integer.value = 0;
-	psw_status = acpi_evaluate_object(device->handle, "_DSW",
-						&arg_list, NULL);
-	if (ACPI_FAILURE(psw_status) && (psw_status != AE_NOT_FOUND))
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "error in evaluate _DSW\n"));
-	/*
-	 * When the _DSW object is not present, OSPM will call _PSW object.
-	 */
-	if (psw_status == AE_NOT_FOUND) {
-		/*
-		 * Only one agruments is required for the _PSW object.
-		 * agrument 0: enable/disable the wake capabilities
-		 */
-		arg_list.count = 1;
-		in_arg[0].integer.value = 0;
-		psw_status = acpi_evaluate_object(device->handle, "_PSW",
-						&arg_list, NULL);
-		if (ACPI_FAILURE(psw_status) && (psw_status != AE_NOT_FOUND))
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO, "error in "
-						"evaluate _PSW\n"));
-	}
+	psw_error = acpi_device_sleep_wake(device, 0, 0, 0);
+	if (psw_error)
+		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;

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 6/9] ACPI: Introduce new device wakeup flag 'prepared'
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2008-07-01 22:06 ` [PATCH 5/9] ACPI: Introduce acpi_device_sleep_wake function Rafael J. Wysocki
@ 2008-07-01 22:07 ` Rafael J. Wysocki
  2008-07-01 22:08 ` [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4) Rafael J. Wysocki
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:07 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui

From: Rafael J. Wysocki <rjw@sisk.pl>

ACPI: Introduce new device wakeup flag 'prepared'

Introduce additional flag 'prepared' in
struct acpi_device_wakeup_flags and use it to prevent devices from
being enable/disabled do wake up the system multiple times in a row
(this does not happen currently, but will be possible after some of
the following patches).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/power.c    |   24 ++++++++++++++++++++++--
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

Index: linux-next/drivers/acpi/power.c
===================================================================
--- linux-next.orig/drivers/acpi/power.c
+++ linux-next/drivers/acpi/power.c
@@ -363,11 +363,18 @@ int acpi_device_sleep_wake(struct acpi_d
  */
 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
 {
-	int i;
+	int i, err;
 
 	if (!dev || !dev->wakeup.flags.valid)
 		return -EINVAL;
 
+	/*
+	 * Do not execute the code below twice in a row without calling
+	 * acpi_disable_wakeup_device_power() in between for the same device
+	 */
+	if (dev->wakeup.flags.prepared)
+		return 0;
+
 	/* Open power resource */
 	for (i = 0; i < dev->wakeup.resources.count; i++) {
 		int ret = acpi_power_on(dev->wakeup.resources.handles[i], dev);
@@ -382,7 +389,11 @@ int acpi_enable_wakeup_device_power(stru
 	 * Passing 3 as the third argument below means the device may be placed
 	 * in arbitrary power state afterwards.
 	 */
-	return acpi_device_sleep_wake(dev, 1, sleep_state, 3);
+	err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
+	if (!err)
+		dev->wakeup.flags.prepared = 1;
+
+	return err;
 }
 
 /*
@@ -398,6 +409,15 @@ int acpi_disable_wakeup_device_power(str
 	if (!dev || !dev->wakeup.flags.valid)
 		return -EINVAL;
 
+	/*
+	 * Do not execute the code below twice in a row without calling
+	 * acpi_enable_wakeup_device_power() in between for the same device
+	 */
+	if (!dev->wakeup.flags.prepared)
+		return 0;
+
+	dev->wakeup.flags.prepared = 0;
+
 	ret = acpi_device_sleep_wake(dev, 0, 0, 0);
 	if (ret)
 		return ret;
Index: linux-next/include/acpi/acpi_bus.h
===================================================================
--- linux-next.orig/include/acpi/acpi_bus.h
+++ linux-next/include/acpi/acpi_bus.h
@@ -259,6 +259,7 @@ struct acpi_device_perf {
 /* Wakeup Management */
 struct acpi_device_wakeup_flags {
 	u8 valid:1;		/* Can successfully enable wakeup? */
+	u8 prepared:1;		/* Has the wake-up capability been enabled? */
 	u8 run_wake:1;		/* Run-Wake GPE devices */
 };
 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4)
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2008-07-01 22:07 ` [PATCH 6/9] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
@ 2008-07-01 22:08 ` Rafael J. Wysocki
  2008-07-02  7:30   ` Zhao Yakui
  2008-07-01 22:09 ` [PATCH 8/9] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:08 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI ACPI: Rework PCI handling of wake-up (rev. 4)

* Introduce function acpi_pm_device_sleep_wake() for enabling and
  disabling the system wake-up capability of devices that are power
  manageable by ACPI.

* Introduce function acpi_bus_can_wakeup() allowing other (dependent)
  subsystems to check if ACPI is able to enable the system wake-up
  capability of given device.

* Introduce callback .sleep_wake() in struct pci_platform_pm_ops and
  for the ACPI PCI 'driver' make it use acpi_pm_device_sleep_wake().

* Introduce callback .can_wakeup() in struct pci_platform_pm_ops and
  for the ACPI 'driver' make it use acpi_bus_can_wakeup().

* Move the PME# handlig code out of pci_enable_wake() and split it
  into two functions, pci_pme_capable() and pci_pme_active(),
  allowing the caller to check if given device is capable of
  generating PME# from given power state and to enable/disable the
  device's PME# functionality, respectively.

* Modify pci_enable_wake() to use the new ACPI callbacks and the new
  PME#-related functions.

* Drop the generic .platform_enable_wakeup() callback that is not
  used any more.

* Introduce device_set_wakeup_capable() that will set the
  power.can_wakeup flag of given device.

* Rework PCI device PM initialization so that, if given device is
  capable of generating wake-up events, either natively through the
  PME# mechanism, or with the help of the platform, its
  power.can_wakeup flag is set and its power.should_wakeup flag is
  unset as appropriate.

* Make ACPI set the power.can_wakeup flag for devices found to be
  wake-up capable by it.

* Make the ACPI wake-up code enable/disable GPEs for devices that
  have the wakeup.flags.prepared flag set (which means that their
  wake-up power has been enabled).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/bus.c          |   11 ++
 drivers/acpi/glue.c         |    2 
 drivers/acpi/sleep/main.c   |   25 ++++++
 drivers/acpi/sleep/wakeup.c |   11 +-
 drivers/base/power/sysfs.c  |    3 
 drivers/pci/pci-acpi.c      |   19 ++++
 drivers/pci/pci.c           |  169 ++++++++++++++++++++++++++++++++------------
 drivers/pci/pci.h           |    8 ++
 drivers/pci/probe.c         |   47 ------------
 include/acpi/acpi_bus.h     |    2 
 include/linux/pm_wakeup.h   |   26 +-----
 11 files changed, 207 insertions(+), 116 deletions(-)

Index: linux-next/drivers/acpi/sleep/main.c
===================================================================
--- linux-next.orig/drivers/acpi/sleep/main.c
+++ linux-next/drivers/acpi/sleep/main.c
@@ -467,6 +467,31 @@ int acpi_pm_device_sleep_state(struct de
 		*d_min_p = d_min;
 	return d_max;
 }
+
+/**
+ *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
+ *                                  capability of given device
+ *	@dev: device to handle
+ *	@enable: 'true' - enable, 'false' - disable the wake-up capability
+ */
+int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
+{
+	acpi_handle handle;
+	struct acpi_device *adev;
+
+	if (!device_may_wakeup(dev))
+		return -EINVAL;
+
+	handle = DEVICE_ACPI_HANDLE(dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
+		printk(KERN_DEBUG "ACPI handle has no context!\n");
+		return -ENODEV;
+	}
+
+	return enable ?
+		acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
+		acpi_disable_wakeup_device_power(adev);
+}
 #endif
 
 static void acpi_power_off_prepare(void)
Index: linux-next/drivers/pci/pci-acpi.c
===================================================================
--- linux-next.orig/drivers/pci/pci-acpi.c
+++ linux-next/drivers/pci/pci-acpi.c
@@ -299,10 +299,29 @@ static int acpi_pci_set_power_state(stru
 	return error;
 }
 
+static bool acpi_pci_can_wakeup(struct pci_dev *dev)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	return handle ? acpi_bus_can_wakeup(handle) : false;
+}
+
+static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
+{
+	int error = acpi_pm_device_sleep_wake(&dev->dev, enable);
+
+	if (!error)
+		printk(KERN_INFO "PCI: Wake-up capability of %s %s by ACPI\n",
+			pci_name(dev), enable ? "enabled" : "disabled");
+	return error;
+}
+
 static struct pci_platform_pm_ops acpi_pci_platform_pm = {
 	.is_manageable = acpi_pci_power_manageable,
 	.set_state = acpi_pci_set_power_state,
 	.choose_state = acpi_pci_choose_state,
+	.can_wakeup = acpi_pci_can_wakeup,
+	.sleep_wake = acpi_pci_sleep_wake,
 };
 
 /* ACPI bus type */
Index: linux-next/drivers/pci/pci.h
===================================================================
--- linux-next.orig/drivers/pci/pci.h
+++ linux-next/drivers/pci/pci.h
@@ -17,6 +17,11 @@ extern void pci_cleanup_rom(struct pci_d
  *                 platform; to be used during system-wide transitions from a
  *                 sleeping state to the working state and vice versa
  *
+ * @can_wakeup - returns 'true' if given device is capable of waking up the
+ *               system from a sleeping state
+ *
+ * @sleep_wake - enables/disables the system wake up capability of given device
+ *
  * If given platform is generally capable of power managing PCI devices, all of
  * these callbacks are mandatory.
  */
@@ -24,9 +29,12 @@ struct pci_platform_pm_ops {
 	bool (*is_manageable)(struct pci_dev *dev);
 	int (*set_state)(struct pci_dev *dev, pci_power_t state);
 	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);
 };
 
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
+extern void pci_pm_init(struct pci_dev *dev);
 
 extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
 extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
Index: linux-next/include/acpi/acpi_bus.h
===================================================================
--- linux-next.orig/include/acpi/acpi_bus.h
+++ linux-next/include/acpi/acpi_bus.h
@@ -337,6 +337,7 @@ int acpi_bus_get_status(struct acpi_devi
 int acpi_bus_get_power(acpi_handle handle, int *state);
 int acpi_bus_set_power(acpi_handle handle, int state);
 bool acpi_bus_power_manageable(acpi_handle handle);
+bool acpi_bus_can_wakeup(acpi_handle handle);
 #ifdef CONFIG_ACPI_PROC_EVENT
 int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
 int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
@@ -379,6 +380,7 @@ acpi_handle acpi_get_pci_rootbridge_hand
 
 #ifdef CONFIG_PM_SLEEP
 int acpi_pm_device_sleep_state(struct device *, int *);
+int acpi_pm_device_sleep_wake(struct device *, bool);
 #else /* !CONFIG_PM_SLEEP */
 static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
 {
Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -380,7 +380,8 @@ static struct pci_platform_pm_ops *pci_p
 
 int pci_set_platform_pm(struct pci_platform_pm_ops *ops)
 {
-	if (!ops->is_manageable || !ops->set_state || !ops->choose_state)
+	if (!ops->is_manageable || !ops->set_state || !ops->choose_state
+	    || !ops->sleep_wake || !ops->can_wakeup)
 		return -EINVAL;
 	pci_platform_pm = ops;
 	return 0;
@@ -403,6 +404,17 @@ static inline pci_power_t platform_pci_c
 			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
 }
 
+static inline bool platform_pci_can_wakeup(struct pci_dev *dev)
+{
+	return pci_platform_pm ? pci_platform_pm->can_wakeup(dev) : false;
+}
+
+static inline int platform_pci_sleep_wake(struct pci_dev *dev, bool enable)
+{
+	return pci_platform_pm ?
+			pci_platform_pm->sleep_wake(dev, enable) : -ENODEV;
+}
+
 /**
  * pci_raw_set_power_state - Use PCI PM registers to set the power state of
  *                           given PCI device
@@ -1036,6 +1048,56 @@ int pci_set_pcie_reset_state(struct pci_
 }
 
 /**
+ * pci_pme_capable - check the capability of PCI device to generate PME#
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ * @state: PCI state from which device will issue PME#.
+ */
+static bool pci_pme_capable(struct pci_dev *dev, int pm, pci_power_t state)
+{
+	u16 pmc;
+
+	if (!pm)
+		return false;
+
+	/* Check device's ability to generate PME# from given state */
+	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
+
+	pmc &= PCI_PM_CAP_PME_MASK;
+	pmc >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
+
+	return !!(pmc & (1 << state));
+}
+
+/**
+ * pci_pme_active - enable or disable PCI device's PME# function
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ * @enable: 'true' to enable PME# generation; 'false' to disable it.
+ *
+ * The caller must verify that the device is capable of generating PME# before
+ * calling this function with @enable equal to 'true'.
+ */
+static void pci_pme_active(struct pci_dev *dev, int pm, bool enable)
+{
+	u16 pmcsr;
+
+	if (!pm)
+		return;
+
+	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+	/* Clear PME_Status by writing 1 to it and enable PME# */
+	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
+	if (!enable)
+		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
+
+	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
+
+	printk(KERN_INFO "PCI: PME# from device %s %s\n", pci_name(dev),
+		enable ? "enabled" : "disabled");
+}
+
+/**
  * pci_enable_wake - enable PCI device as wakeup event source
  * @dev: PCI device affected
  * @state: PCI state from which device will issue wakeup events
@@ -1046,66 +1108,83 @@ int pci_set_pcie_reset_state(struct pci_
  * called automatically by this routine.
  *
  * Devices with legacy power management (no standard PCI PM capabilities)
- * always require such platform hooks.  Depending on the platform, devices
- * supporting the standard PCI PME# signal may require such platform hooks;
- * they always update bits in config space to allow PME# generation.
- *
- * -EIO is returned if the device can't ever be a wakeup event source.
- * -EINVAL is returned if the device can't generate wakeup events from
- * the specified PCI state.  Returns zero if the operation is successful.
+ * always require such platform hooks.
+ *
+ * RETURN VALUE:
+ * 0 is returned on success
+ * -EINVAL is returned if device is not supposed to wake up the system
+ * Error code depending on the platform is returned if both the platform and
+ * the native mechanism fail to enable the generation of wake-up events
  */
 int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable)
 {
 	int pm;
-	int status;
-	u16 value;
+	int error = 0;
+	bool pme_done = false;
+
+	if (!device_may_wakeup(&dev->dev))
+		return -EINVAL;
 
-	/* Note that drivers should verify device_may_wakeup(&dev->dev)
-	 * before calling this function.  Platform code should report
-	 * errors when drivers try to enable wakeup on devices that
-	 * can't issue wakeups, or on which wakeups were disabled by
-	 * userspace updating the /sys/devices.../power/wakeup file.
+	/*
+	 * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don
+	 * Anderson we should be doing PME# wake enable followed by ACPI wake
+	 * enable.  To disable wake-up we call the platform first, for symmetry.
 	 */
 
-	status = call_platform_enable_wakeup(&dev->dev, enable);
+	if (!enable && platform_pci_can_wakeup(dev))
+		error = platform_pci_sleep_wake(dev, false);
 
-	/* find PCI PM capability in list */
 	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	if (!enable || pci_pme_capable(dev, pm, state)) {
+		pci_pme_active(dev, pm, enable);
+		pme_done = true;
+	}
 
-	/* If device doesn't support PM Capabilities, but caller wants to
-	 * disable wake events, it's a NOP.  Otherwise fail unless the
-	 * platform hooks handled this legacy device already.
-	 */
-	if (!pm)
-		return enable ? status : 0;
+	if (enable && platform_pci_can_wakeup(dev))
+		error = platform_pci_sleep_wake(dev, true);
 
+	return pme_done ? 0 : error;
+}
+
+/**
+ * pci_pm_init - Initialize PM functions of given PCI device
+ * @dev: PCI device to handle.
+ */
+void pci_pm_init(struct pci_dev *dev)
+{
+	int pm;
+	u16 pmc;
+
+	/* find PCI PM capability in list */
+	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	if (!pm)
+		return;
 	/* Check device's ability to generate PME# */
-	pci_read_config_word(dev,pm+PCI_PM_PMC,&value);
+	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
 
-	value &= PCI_PM_CAP_PME_MASK;
-	value >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
+	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
+		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
+			pmc & PCI_PM_CAP_VER_MASK);
+		return;
+	}
 
-	/* Check if it can generate PME# from requested state. */
-	if (!value || !(value & (1 << state))) {
-		/* if it can't, revert what the platform hook changed,
-		 * always reporting the base "EINVAL, can't PME#" error
+	if (pmc & PCI_PM_CAP_PME_MASK) {
+		dev_printk(KERN_INFO, &dev->dev,
+			"PME# supported from%s%s%s%s%s\n",
+			(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
+			(pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
+			(pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
+			(pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
+			(pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
+		/*
+		 * Make device's PM flags reflect the wake-up capability, but
+		 * let the user space enable it to wake up the system as needed.
 		 */
-		if (enable)
-			call_platform_enable_wakeup(&dev->dev, 0);
-		return enable ? -EINVAL : 0;
+		device_set_wakeup_capable(&dev->dev, true);
+		device_set_wakeup_enable(&dev->dev, false);
+		/* Disable the PME# generation functionality */
+		pci_pme_active(dev, pm, false);
 	}
-
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
-
-	/* Clear PME_Status by writing 1 to it and enable PME# */
-	value |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
-
-	if (!enable)
-		value &= ~PCI_PM_CTRL_PME_ENABLE;
-
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
-
-	return 0;
 }
 
 int
Index: linux-next/include/linux/pm_wakeup.h
===================================================================
--- linux-next.orig/include/linux/pm_wakeup.h
+++ linux-next/include/linux/pm_wakeup.h
@@ -35,6 +35,11 @@ static inline void device_init_wakeup(st
 	dev->power.can_wakeup = dev->power.should_wakeup = !!val;
 }
 
+static inline void device_set_wakeup_capable(struct device *dev, int val)
+{
+	dev->power.can_wakeup = !!val;
+}
+
 static inline int device_can_wakeup(struct device *dev)
 {
 	return dev->power.can_wakeup;
@@ -47,21 +52,7 @@ static inline void device_set_wakeup_ena
 
 static inline int device_may_wakeup(struct device *dev)
 {
-	return dev->power.can_wakeup & dev->power.should_wakeup;
-}
-
-/*
- * Platform hook to activate device wakeup capability, if that's not already
- * handled by enable_irq_wake() etc.
- * Returns zero on success, else negative errno
- */
-extern int (*platform_enable_wakeup)(struct device *dev, int is_on);
-
-static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
-{
-	if (platform_enable_wakeup)
-		return (*platform_enable_wakeup)(dev, is_on);
-	return 0;
+	return dev->power.can_wakeup && dev->power.should_wakeup;
 }
 
 #else /* !CONFIG_PM */
@@ -80,11 +71,6 @@ static inline int device_can_wakeup(stru
 #define device_set_wakeup_enable(dev, val)	do {} while (0)
 #define device_may_wakeup(dev)			0
 
-static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
-{
-	return 0;
-}
-
 #endif /* !CONFIG_PM */
 
 #endif /* _LINUX_PM_WAKEUP_H */
Index: linux-next/drivers/base/power/sysfs.c
===================================================================
--- linux-next.orig/drivers/base/power/sysfs.c
+++ linux-next/drivers/base/power/sysfs.c
@@ -6,9 +6,6 @@
 #include <linux/string.h>
 #include "power.h"
 
-int (*platform_enable_wakeup)(struct device *dev, int is_on);
-
-
 /*
  *	wakeup - Report/change current wakeup option for device
  *
Index: linux-next/drivers/acpi/bus.c
===================================================================
--- linux-next.orig/drivers/acpi/bus.c
+++ linux-next/drivers/acpi/bus.c
@@ -306,6 +306,17 @@ bool acpi_bus_power_manageable(acpi_hand
 
 EXPORT_SYMBOL(acpi_bus_power_manageable);
 
+bool acpi_bus_can_wakeup(acpi_handle handle)
+{
+	struct acpi_device *device;
+	int result;
+
+	result = acpi_bus_get_device(handle, &device);
+	return result ? false : device->wakeup.flags.valid;
+}
+
+EXPORT_SYMBOL(acpi_bus_can_wakeup);
+
 /* --------------------------------------------------------------------------
                                 Event Management
    -------------------------------------------------------------------------- */
Index: linux-next/drivers/pci/probe.c
===================================================================
--- linux-next.orig/drivers/pci/probe.c
+++ linux-next/drivers/pci/probe.c
@@ -860,49 +860,6 @@ int pci_cfg_space_size_ext(struct pci_de
 	return PCI_CFG_SPACE_SIZE;
 }
 
-/**
- * pci_disable_pme - Disable the PME function of PCI device
- * @dev: PCI device affected
- * -EINVAL is returned if PCI device doesn't support PME.
- * Zero is returned if the PME is supported and can be disabled.
- */
-static int pci_disable_pme(struct pci_dev *dev)
-{
-	int pm;
-	u16 value;
-
-	/* find PCI PM capability in list */
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-
-	/* If device doesn't support PM Capabilities, it means that PME is
-	 * not supported.
-	 */
-	if (!pm)
-		return -EINVAL;
-	/* Check device's ability to generate PME# */
-	pci_read_config_word(dev, pm + PCI_PM_PMC, &value);
-
-	value &= PCI_PM_CAP_PME_MASK;
-	/* Check if it can generate PME# */
-	if (!value) {
-		/*
-		 * If it is zero, it means that PME is still unsupported
-		 * although there exists the PM capability.
-		 */
-		return -EINVAL;
-	}
-
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
-
-	/* Clear PME_Status by writing 1 to it */
-	value |= PCI_PM_CTRL_PME_STATUS ;
-	/* Disable PME enable bit */
-	value &= ~PCI_PM_CTRL_PME_ENABLE;
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
-
-	return 0;
-}
-
 int pci_cfg_space_size(struct pci_dev *dev)
 {
 	int pos;
@@ -1010,7 +967,6 @@ static struct pci_dev *pci_scan_device(s
 	}
 
 	pci_vpd_pci22_init(dev);
-	pci_disable_pme(dev);
 
 	return dev;
 }
@@ -1031,6 +987,9 @@ void pci_device_add(struct pci_dev *dev,
 	/* Fix up broken headers */
 	pci_fixup_device(pci_fixup_header, dev);
 
+	/* Initialize power management of the device */
+	pci_pm_init(dev);
+
 	/*
 	 * Add the device to our list of discovered devices
 	 * and the bus list for fixup functions, etc.
Index: linux-next/drivers/acpi/glue.c
===================================================================
--- linux-next.orig/drivers/acpi/glue.c
+++ linux-next/drivers/acpi/glue.c
@@ -165,6 +165,8 @@ static int acpi_bind_one(struct device *
 				"firmware_node");
 		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
 				"physical_node");
+		if (acpi_dev->wakeup.flags.valid)
+			device_set_wakeup_capable(dev, true);
 	}
 
 	return 0;
Index: linux-next/drivers/acpi/sleep/wakeup.c
===================================================================
--- linux-next.orig/drivers/acpi/sleep/wakeup.c
+++ linux-next/drivers/acpi/sleep/wakeup.c
@@ -66,13 +66,15 @@ void acpi_enable_wakeup_device(u8 sleep_
 	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
 		struct acpi_device *dev =
 			container_of(node, struct acpi_device, wakeup_list);
+
 		if (!dev->wakeup.flags.valid)
 			continue;
+
 		/* If users want to disable run-wake GPE,
 		 * we only disable it for wake and leave it for runtime
 		 */
-		if (!dev->wakeup.state.enabled ||
-		    sleep_state > (u32) dev->wakeup.sleep_state) {
+		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
+		    || sleep_state > (u32) dev->wakeup.sleep_state) {
 			if (dev->wakeup.flags.run_wake) {
 				spin_unlock(&acpi_device_lock);
 				/* set_gpe_type will disable GPE, leave it like that */
@@ -110,8 +112,9 @@ void acpi_disable_wakeup_device(u8 sleep
 
 		if (!dev->wakeup.flags.valid)
 			continue;
-		if (!dev->wakeup.state.enabled ||
-		    sleep_state > (u32) dev->wakeup.sleep_state) {
+
+		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
+		    || sleep_state > (u32) dev->wakeup.sleep_state) {
 			if (dev->wakeup.flags.run_wake) {
 				spin_unlock(&acpi_device_lock);
 				acpi_set_gpe_type(dev->wakeup.gpe_device,

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 8/9] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2008-07-01 22:08 ` [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4) Rafael J. Wysocki
@ 2008-07-01 22:09 ` Rafael J. Wysocki
  2008-07-01 22:10 ` [PATCH 9/9] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:09 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep

Introduce functions pci_prepare_to_sleep() and pci_back_from_sleep(),
to be used by the PCI drivers that want to place their devices into
the lowest power state appropiate for them (PCI_D3hot, if the device
is not supposed to wake up the system, or the deepest state from
which the wake-up is possible, otherwise) while the system is being
prepared to go into a sleeping state and to put them back into D0
during the subsequent transition to the working state.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c   |   83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    2 +
 2 files changed, 85 insertions(+)

Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -1147,6 +1147,87 @@ int pci_enable_wake(struct pci_dev *dev,
 }
 
 /**
+ * pci_prepare_to_sleep - prepare PCI device for system-wide transition into
+ *                        a sleep state
+ * @dev: Device to handle.
+ *
+ * Choose the power state appropriate for the device depending on whether
+ * it can wake up the system and/or is power manageable by the platform
+ * (PCI_D3hot is the default) and put the device into that state.
+ */
+int pci_prepare_to_sleep(struct pci_dev *dev)
+{
+	pci_power_t target_state = PCI_D3hot;
+	int pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	int error;
+
+	if (platform_pci_power_manageable(dev)) {
+		/*
+		 * Call the platform to choose the target state of the device
+		 * and enable wake-up from this state if supported.
+		 */
+		pci_power_t state = platform_pci_choose_state(dev);
+
+		switch (state) {
+		case PCI_POWER_ERROR:
+		case PCI_UNKNOWN:
+			break;
+		case PCI_D1:
+		case PCI_D2:
+			if (pci_no_d1d2(dev))
+				break;
+		default:
+			target_state = state;
+			pci_enable_wake(dev, target_state, true);
+		}
+	} else if (device_may_wakeup(&dev->dev)) {
+		/*
+		 * Find the deepest state from which the device can generate
+		 * wake-up events, make it the target state and enable device
+		 * to generate PME#.
+		 */
+		u16 pmc;
+
+		if (!pm)
+			return -EIO;
+
+		pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
+		if (pmc & PCI_PM_CAP_PME_MASK) {
+			if (!(pmc & PCI_PM_CAP_PME_D3)) {
+				/* Device cannot generate PME# from D3_hot */
+				if (pmc & PCI_PM_CAP_PME_D2)
+					target_state = PCI_D2;
+				else if (pmc & PCI_PM_CAP_PME_D1)
+					target_state = PCI_D1;
+				else
+					target_state = PCI_D0;
+			}
+			pci_pme_active(dev, pm, true);
+		}
+	}
+
+	error = pci_set_power_state(dev, target_state);
+
+	if (error)
+		pci_enable_wake(dev, target_state, false);
+
+	return error;
+}
+
+/**
+ * pci_back_from_sleep - turn PCI device on during system-wide transition into
+ *                       the working state a sleep state
+ * @dev: Device to handle.
+ *
+ * Disable device's sytem wake-up capability and put it into D0.
+ */
+int pci_back_from_sleep(struct pci_dev *dev)
+{
+	pci_enable_wake(dev, PCI_D0, false);
+	return pci_set_power_state(dev, PCI_D0);
+}
+
+/**
  * pci_pm_init - Initialize PM functions of given PCI device
  * @dev: PCI device to handle.
  */
@@ -1853,5 +1934,7 @@ EXPORT_SYMBOL(pci_set_power_state);
 EXPORT_SYMBOL(pci_save_state);
 EXPORT_SYMBOL(pci_restore_state);
 EXPORT_SYMBOL(pci_enable_wake);
+EXPORT_SYMBOL(pci_prepare_to_sleep);
+EXPORT_SYMBOL(pci_back_from_sleep);
 EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
 
Index: linux-next/include/linux/pci.h
===================================================================
--- linux-next.orig/include/linux/pci.h
+++ linux-next/include/linux/pci.h
@@ -633,6 +633,8 @@ int pci_restore_state(struct pci_dev *de
 int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
 pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
 int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable);
+int pci_prepare_to_sleep(struct pci_dev *dev);
+int pci_back_from_sleep(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);

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 9/9] PCI: Simplify PCI device PM code (rev. 4)
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (7 preceding siblings ...)
  2008-07-01 22:09 ` [PATCH 8/9] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
@ 2008-07-01 22:10 ` Rafael J. Wysocki
  2008-07-03 12:04 ` [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-01 22:10 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Alan Stern, Andi Kleen, Jesse Barnes, pm list, Zhang Rui,
	Zhao Yakui

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI: Simplify PCI device PM code (rev. 4)

If the offset of PCI device's PM capability in its configuration
space, the mask of states that the device supports PME# from and the
D1 and D2 support bits are cached in the corresponding
struct pci_dev, the PCI device PM code can be simplified quite a bit.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c        |  118 ++++++++++++++++++++---------------------------
 include/linux/pci.h      |    8 ++-
 include/linux/pci_regs.h |    1 
 3 files changed, 60 insertions(+), 67 deletions(-)

Index: linux-next/include/linux/pci.h
===================================================================
--- linux-next.orig/include/linux/pci.h
+++ linux-next/include/linux/pci.h
@@ -177,6 +177,13 @@ struct pci_dev {
 	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
 					   this is D0-D3, D0 being fully functional,
 					   and D3 being off. */
+	int		pm_cap;		/* PM capability offset in the
+					   configuration space */
+	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
+					   can be generated */
+	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 */
 
 #ifdef CONFIG_PCIEASPM
 	struct pcie_link_state	*link_state;	/* ASPM link state. */
@@ -201,7 +208,6 @@ struct pci_dev {
 	unsigned int	is_added:1;
 	unsigned int	is_busmaster:1; /* device is busmaster */
 	unsigned int	no_msi:1;	/* device may not use msi */
-	unsigned int	no_d1d2:1;   /* only allow d0 or d3 */
 	unsigned int	block_ucfg_access:1;	/* userspace config space access is blocked */
 	unsigned int	broken_parity_status:1;	/* Device generates false positive parity */
 	unsigned int 	msi_enabled:1;
Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -419,7 +419,6 @@ static inline int platform_pci_sleep_wak
  * pci_raw_set_power_state - Use PCI PM registers to set the power state of
  *                           given PCI device
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
  *
  * RETURN VALUE:
@@ -430,12 +429,12 @@ static inline int platform_pci_sleep_wak
  * 0 if device's power state has been successfully changed.
  */
 static int
-pci_raw_set_power_state(struct pci_dev *dev, int pm, pci_power_t state)
+pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
-	u16 pmcsr, pmc;
+	u16 pmcsr;
 	bool need_restore = false;
 
-	if (!pm)
+	if (!dev->pm_cap)
 		return -EIO;
 
 	if (state < PCI_D0 || state > PCI_D3hot)
@@ -455,20 +454,12 @@ pci_raw_set_power_state(struct pci_dev *
 		return -EINVAL;
 	}
 
-	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
-
-	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
-		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
-			pmc & PCI_PM_CAP_VER_MASK);
-		return -EIO;
-	}
-
 	/* check if this device supports the desired state */
-	if ((state == PCI_D1 && !(pmc & PCI_PM_CAP_D1))
-	   || (state == PCI_D2 && !(pmc & PCI_PM_CAP_D2)))
+	if ((state == PCI_D1 && !dev->d1_support)
+	   || (state == PCI_D2 && !dev->d2_support))
 		return -EIO;
 
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 
 	/* If we're (effectively) in D3, force entire word to 0.
 	 * This doesn't affect PME_Status, disables PME_En, and
@@ -492,7 +483,7 @@ pci_raw_set_power_state(struct pci_dev *
 	}
 
 	/* enter specified state */
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
+	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 
 	/* Mandatory power management transition delays */
 	/* see PCI PM 1.1 5.6.1 table 18 */
@@ -528,14 +519,13 @@ pci_raw_set_power_state(struct pci_dev *
  * pci_update_current_state - Read PCI power state of given device from its
  *                            PCI PM registers and cache it
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  */
-static void pci_update_current_state(struct pci_dev *dev, int pm)
+static void pci_update_current_state(struct pci_dev *dev)
 {
-	if (pm) {
+	if (dev->pm_cap) {
 		u16 pmcsr;
 
-		pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+		pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 		dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK);
 	}
 }
@@ -557,7 +547,7 @@ static void pci_update_current_state(str
  */
 int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
-	int pm, error;
+	int error;
 
 	/* bound the state we're entering */
 	if (state > PCI_D3hot)
@@ -572,9 +562,6 @@ int pci_set_power_state(struct pci_dev *
 		 */
 		return 0;
 
-	/* Find PCI PM capability in the list */
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-
 	if (state == PCI_D0 && platform_pci_power_manageable(dev)) {
 		/*
 		 * Allow the platform to change the state, for example via ACPI
@@ -582,16 +569,16 @@ int pci_set_power_state(struct pci_dev *
 		 */
 		int ret = platform_pci_set_power_state(dev, PCI_D0);
 		if (!ret)
-			pci_update_current_state(dev, pm);
+			pci_update_current_state(dev);
 	}
 
-	error = pci_raw_set_power_state(dev, pm, state);
+	error = pci_raw_set_power_state(dev, state);
 
 	if (state > PCI_D0 && platform_pci_power_manageable(dev)) {
 		/* Allow the platform to finalize the transition */
 		int ret = platform_pci_set_power_state(dev, state);
 		if (!ret) {
-			pci_update_current_state(dev, pm);
+			pci_update_current_state(dev);
 			error = 0;
 		}
 	}
@@ -1050,48 +1037,38 @@ int pci_set_pcie_reset_state(struct pci_
 /**
  * pci_pme_capable - check the capability of PCI device to generate PME#
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  * @state: PCI state from which device will issue PME#.
  */
-static bool pci_pme_capable(struct pci_dev *dev, int pm, pci_power_t state)
+static bool pci_pme_capable(struct pci_dev *dev, pci_power_t state)
 {
-	u16 pmc;
-
-	if (!pm)
+	if (!dev->pm_cap)
 		return false;
 
-	/* Check device's ability to generate PME# from given state */
-	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
-
-	pmc &= PCI_PM_CAP_PME_MASK;
-	pmc >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
-
-	return !!(pmc & (1 << state));
+	return !!(dev->pme_support & (1 << state));
 }
 
 /**
  * pci_pme_active - enable or disable PCI device's PME# function
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  * @enable: 'true' to enable PME# generation; 'false' to disable it.
  *
  * The caller must verify that the device is capable of generating PME# before
  * calling this function with @enable equal to 'true'.
  */
-static void pci_pme_active(struct pci_dev *dev, int pm, bool enable)
+static void pci_pme_active(struct pci_dev *dev, bool enable)
 {
 	u16 pmcsr;
 
-	if (!pm)
+	if (!dev->pm_cap)
 		return;
 
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	/* Clear PME_Status by writing 1 to it and enable PME# */
 	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
 	if (!enable)
 		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
 
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
+	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 
 	printk(KERN_INFO "PCI: PME# from device %s %s\n", pci_name(dev),
 		enable ? "enabled" : "disabled");
@@ -1118,7 +1095,6 @@ static void pci_pme_active(struct pci_de
  */
 int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable)
 {
-	int pm;
 	int error = 0;
 	bool pme_done = false;
 
@@ -1134,9 +1110,8 @@ int pci_enable_wake(struct pci_dev *dev,
 	if (!enable && platform_pci_can_wakeup(dev))
 		error = platform_pci_sleep_wake(dev, false);
 
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (!enable || pci_pme_capable(dev, pm, state)) {
-		pci_pme_active(dev, pm, enable);
+	if (!enable || pci_pme_capable(dev, state)) {
+		pci_pme_active(dev, enable);
 		pme_done = true;
 	}
 
@@ -1158,7 +1133,6 @@ int pci_enable_wake(struct pci_dev *dev,
 int pci_prepare_to_sleep(struct pci_dev *dev)
 {
 	pci_power_t target_state = PCI_D3hot;
-	int pm = pci_find_capability(dev, PCI_CAP_ID_PM);
 	int error;
 
 	if (platform_pci_power_manageable(dev)) {
@@ -1186,23 +1160,14 @@ int pci_prepare_to_sleep(struct pci_dev 
 		 * wake-up events, make it the target state and enable device
 		 * to generate PME#.
 		 */
-		u16 pmc;
-
-		if (!pm)
+		if (!dev->pm_cap)
 			return -EIO;
 
-		pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
-		if (pmc & PCI_PM_CAP_PME_MASK) {
-			if (!(pmc & PCI_PM_CAP_PME_D3)) {
-				/* Device cannot generate PME# from D3_hot */
-				if (pmc & PCI_PM_CAP_PME_D2)
-					target_state = PCI_D2;
-				else if (pmc & PCI_PM_CAP_PME_D1)
-					target_state = PCI_D1;
-				else
-					target_state = PCI_D0;
-			}
-			pci_pme_active(dev, pm, true);
+		if (dev->pme_support) {
+			while (target_state
+			      && !(dev->pme_support & (1 << target_state)))
+				target_state--;
+			pci_pme_active(dev, true);
 		}
 	}
 
@@ -1236,6 +1201,8 @@ void pci_pm_init(struct pci_dev *dev)
 	int pm;
 	u16 pmc;
 
+	dev->pm_cap = 0;
+
 	/* find PCI PM capability in list */
 	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
 	if (!pm)
@@ -1249,7 +1216,23 @@ void pci_pm_init(struct pci_dev *dev)
 		return;
 	}
 
-	if (pmc & PCI_PM_CAP_PME_MASK) {
+	dev->pm_cap = pm;
+
+	dev->d1_support = false;
+	dev->d2_support = false;
+	if (!pci_no_d1d2(dev)) {
+		if (pmc & PCI_PM_CAP_D1) {
+			dev_printk(KERN_DEBUG, &dev->dev, "supports D1\n");
+			dev->d1_support = true;
+		}
+		if (pmc & PCI_PM_CAP_D2) {
+			dev_printk(KERN_DEBUG, &dev->dev, "supports D2\n");
+			dev->d2_support = true;
+		}
+	}
+
+	pmc &= PCI_PM_CAP_PME_MASK;
+	if (pmc) {
 		dev_printk(KERN_INFO, &dev->dev,
 			"PME# supported from%s%s%s%s%s\n",
 			(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
@@ -1257,6 +1240,7 @@ void pci_pm_init(struct pci_dev *dev)
 			(pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
 			(pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
 			(pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
+		dev->pme_support = pmc >> PCI_PM_CAP_PME_SHIFT;
 		/*
 		 * Make device's PM flags reflect the wake-up capability, but
 		 * let the user space enable it to wake up the system as needed.
@@ -1264,7 +1248,9 @@ void pci_pm_init(struct pci_dev *dev)
 		device_set_wakeup_capable(&dev->dev, true);
 		device_set_wakeup_enable(&dev->dev, false);
 		/* Disable the PME# generation functionality */
-		pci_pme_active(dev, pm, false);
+		pci_pme_active(dev, false);
+	} else {
+		dev->pme_support = 0;
 	}
 }
 
Index: linux-next/include/linux/pci_regs.h
===================================================================
--- linux-next.orig/include/linux/pci_regs.h
+++ linux-next/include/linux/pci_regs.h
@@ -231,6 +231,7 @@
 #define  PCI_PM_CAP_PME_D2	0x2000	/* PME# from D2 */
 #define  PCI_PM_CAP_PME_D3	0x4000	/* PME# from D3 (hot) */
 #define  PCI_PM_CAP_PME_D3cold	0x8000	/* PME# from D3 (cold) */
+#define  PCI_PM_CAP_PME_SHIFT	11	/* Start of the PME Mask in PMC */
 #define PCI_PM_CTRL		4	/* PM control and status register */
 #define  PCI_PM_CTRL_STATE_MASK	0x0003	/* Current power state (D0 to D3) */
 #define  PCI_PM_CTRL_NO_SOFT_RESET	0x0004	/* No reset for D3hot->D0 */

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4)
  2008-07-01 22:08 ` [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4) Rafael J. Wysocki
@ 2008-07-02  7:30   ` Zhao Yakui
  2008-07-02 10:16     ` Rafael J. Wysocki
  0 siblings, 1 reply; 33+ messages in thread
From: Zhao Yakui @ 2008-07-02  7:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, Jesse Barnes,
	pm list, Zhang Rui

On Wed, 2008-07-02 at 00:08 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> PCI ACPI: Rework PCI handling of wake-up (rev. 4)
> 
> * Introduce function acpi_pm_device_sleep_wake() for enabling and
>   disabling the system wake-up capability of devices that are power
>   manageable by ACPI.
> 
> * Introduce function acpi_bus_can_wakeup() allowing other (dependent)
>   subsystems to check if ACPI is able to enable the system wake-up
>   capability of given device.
> 
> * Introduce callback .sleep_wake() in struct pci_platform_pm_ops and
>   for the ACPI PCI 'driver' make it use acpi_pm_device_sleep_wake().
> 
> * Introduce callback .can_wakeup() in struct pci_platform_pm_ops and
>   for the ACPI 'driver' make it use acpi_bus_can_wakeup().
> 
> * Move the PME# handlig code out of pci_enable_wake() and split it
>   into two functions, pci_pme_capable() and pci_pme_active(),
>   allowing the caller to check if given device is capable of
>   generating PME# from given power state and to enable/disable the
>   device's PME# functionality, respectively.
> 
> * Modify pci_enable_wake() to use the new ACPI callbacks and the new
>   PME#-related functions.
> 
> * Drop the generic .platform_enable_wakeup() callback that is not
>   used any more.
> 
> * Introduce device_set_wakeup_capable() that will set the
>   power.can_wakeup flag of given device.
> 
> * Rework PCI device PM initialization so that, if given device is
>   capable of generating wake-up events, either natively through the
>   PME# mechanism, or with the help of the platform, its
>   power.can_wakeup flag is set and its power.should_wakeup flag is
>   unset as appropriate.
> 
> * Make ACPI set the power.can_wakeup flag for devices found to be
>   wake-up capable by it.
> 
> * Make the ACPI wake-up code enable/disable GPEs for devices that
>   have the wakeup.flags.prepared flag set (which means that their
>   wake-up power has been enabled).
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/bus.c          |   11 ++
>  drivers/acpi/glue.c         |    2 
>  drivers/acpi/sleep/main.c   |   25 ++++++
>  drivers/acpi/sleep/wakeup.c |   11 +-
>  drivers/base/power/sysfs.c  |    3 
>  drivers/pci/pci-acpi.c      |   19 ++++
>  drivers/pci/pci.c           |  169 ++++++++++++++++++++++++++++++++------------
>  drivers/pci/pci.h           |    8 ++
>  drivers/pci/probe.c         |   47 ------------
>  include/acpi/acpi_bus.h     |    2 
>  include/linux/pm_wakeup.h   |   26 +-----
>  11 files changed, 207 insertions(+), 116 deletions(-)
> 
> Index: linux-next/drivers/acpi/sleep/main.c
> ===================================================================
> --- linux-next.orig/drivers/acpi/sleep/main.c
> +++ linux-next/drivers/acpi/sleep/main.c
> @@ -467,6 +467,31 @@ int acpi_pm_device_sleep_state(struct de
>  		*d_min_p = d_min;
>  	return d_max;
>  }
> +
> +/**
> + *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
> + *                                  capability of given device
> + *	@dev: device to handle
> + *	@enable: 'true' - enable, 'false' - disable the wake-up capability
> + */
> +int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
> +{
> +	acpi_handle handle;
> +	struct acpi_device *adev;
> +
> +	if (!device_may_wakeup(dev))
> +		return -EINVAL;
> +
> +	handle = DEVICE_ACPI_HANDLE(dev);
> +	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
> +		printk(KERN_DEBUG "ACPI handle has no context!\n");
> +		return -ENODEV;
> +	}
> +
> +	return enable ?
> +		acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
> +		acpi_disable_wakeup_device_power(adev);
> +}
>  #endif
>  
>  static void acpi_power_off_prepare(void)
> Index: linux-next/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-next.orig/drivers/pci/pci-acpi.c
> +++ linux-next/drivers/pci/pci-acpi.c
> @@ -299,10 +299,29 @@ static int acpi_pci_set_power_state(stru
>  	return error;
>  }
>  
> +static bool acpi_pci_can_wakeup(struct pci_dev *dev)
> +{
> +	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
> +
> +	return handle ? acpi_bus_can_wakeup(handle) : false;
> +}
> +
> +static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
> +{
> +	int error = acpi_pm_device_sleep_wake(&dev->dev, enable);
> +
> +	if (!error)
> +		printk(KERN_INFO "PCI: Wake-up capability of %s %s by ACPI\n",
> +			pci_name(dev), enable ? "enabled" : "disabled");
> +	return error;
> +}
> +
>  static struct pci_platform_pm_ops acpi_pci_platform_pm = {
>  	.is_manageable = acpi_pci_power_manageable,
>  	.set_state = acpi_pci_set_power_state,
>  	.choose_state = acpi_pci_choose_state,
> +	.can_wakeup = acpi_pci_can_wakeup,
> +	.sleep_wake = acpi_pci_sleep_wake,
>  };
>  
>  /* ACPI bus type */
> Index: linux-next/drivers/pci/pci.h
> ===================================================================
> --- linux-next.orig/drivers/pci/pci.h
> +++ linux-next/drivers/pci/pci.h
> @@ -17,6 +17,11 @@ extern void pci_cleanup_rom(struct pci_d
>   *                 platform; to be used during system-wide transitions from a
>   *                 sleeping state to the working state and vice versa
>   *
> + * @can_wakeup - returns 'true' if given device is capable of waking up the
> + *               system from a sleeping state
> + *
> + * @sleep_wake - enables/disables the system wake up capability of given device
> + *
>   * If given platform is generally capable of power managing PCI devices, all of
>   * these callbacks are mandatory.
>   */
> @@ -24,9 +29,12 @@ struct pci_platform_pm_ops {
>  	bool (*is_manageable)(struct pci_dev *dev);
>  	int (*set_state)(struct pci_dev *dev, pci_power_t state);
>  	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);
>  };
>  
>  extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
> +extern void pci_pm_init(struct pci_dev *dev);
>  
>  extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
>  extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
> Index: linux-next/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-next.orig/include/acpi/acpi_bus.h
> +++ linux-next/include/acpi/acpi_bus.h
> @@ -337,6 +337,7 @@ int acpi_bus_get_status(struct acpi_devi
>  int acpi_bus_get_power(acpi_handle handle, int *state);
>  int acpi_bus_set_power(acpi_handle handle, int state);
>  bool acpi_bus_power_manageable(acpi_handle handle);
> +bool acpi_bus_can_wakeup(acpi_handle handle);
>  #ifdef CONFIG_ACPI_PROC_EVENT
>  int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
>  int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
> @@ -379,6 +380,7 @@ acpi_handle acpi_get_pci_rootbridge_hand
>  
>  #ifdef CONFIG_PM_SLEEP
>  int acpi_pm_device_sleep_state(struct device *, int *);
> +int acpi_pm_device_sleep_wake(struct device *, bool);
>  #else /* !CONFIG_PM_SLEEP */
>  static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
>  {
> Index: linux-next/drivers/pci/pci.c
> ===================================================================
> --- linux-next.orig/drivers/pci/pci.c
> +++ linux-next/drivers/pci/pci.c
> @@ -380,7 +380,8 @@ static struct pci_platform_pm_ops *pci_p
>  
>  int pci_set_platform_pm(struct pci_platform_pm_ops *ops)
>  {
> -	if (!ops->is_manageable || !ops->set_state || !ops->choose_state)
> +	if (!ops->is_manageable || !ops->set_state || !ops->choose_state
> +	    || !ops->sleep_wake || !ops->can_wakeup)
>  		return -EINVAL;
>  	pci_platform_pm = ops;
>  	return 0;
> @@ -403,6 +404,17 @@ static inline pci_power_t platform_pci_c
>  			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
>  }
>  
> +static inline bool platform_pci_can_wakeup(struct pci_dev *dev)
> +{
> +	return pci_platform_pm ? pci_platform_pm->can_wakeup(dev) : false;
> +}
> +
> +static inline int platform_pci_sleep_wake(struct pci_dev *dev, bool enable)
> +{
> +	return pci_platform_pm ?
> +			pci_platform_pm->sleep_wake(dev, enable) : -ENODEV;
> +}
> +
>  /**
>   * pci_raw_set_power_state - Use PCI PM registers to set the power state of
>   *                           given PCI device
> @@ -1036,6 +1048,56 @@ int pci_set_pcie_reset_state(struct pci_
>  }
>  
>  /**
> + * pci_pme_capable - check the capability of PCI device to generate PME#
> + * @dev: PCI device to handle.
> + * @pm: PCI PM capability offset of the device.
> + * @state: PCI state from which device will issue PME#.
> + */
> +static bool pci_pme_capable(struct pci_dev *dev, int pm, pci_power_t state)
> +{
> +	u16 pmc;
> +
> +	if (!pm)
> +		return false;
> +
> +	/* Check device's ability to generate PME# from given state */
> +	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
> +
> +	pmc &= PCI_PM_CAP_PME_MASK;
> +	pmc >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
> +
> +	return !!(pmc & (1 << state));
> +}
> +
> +/**
> + * pci_pme_active - enable or disable PCI device's PME# function
> + * @dev: PCI device to handle.
> + * @pm: PCI PM capability offset of the device.
> + * @enable: 'true' to enable PME# generation; 'false' to disable it.
> + *
> + * The caller must verify that the device is capable of generating PME# before
> + * calling this function with @enable equal to 'true'.
> + */
> +static void pci_pme_active(struct pci_dev *dev, int pm, bool enable)
> +{
> +	u16 pmcsr;
> +
> +	if (!pm)
> +		return;
> +
> +	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
> +	/* Clear PME_Status by writing 1 to it and enable PME# */
> +	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
> +	if (!enable)
> +		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
> +
> +	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
> +
> +	printk(KERN_INFO "PCI: PME# from device %s %s\n", pci_name(dev),
> +		enable ? "enabled" : "disabled");
> +}
> +
> +/**
>   * pci_enable_wake - enable PCI device as wakeup event source
>   * @dev: PCI device affected
>   * @state: PCI state from which device will issue wakeup events
> @@ -1046,66 +1108,83 @@ int pci_set_pcie_reset_state(struct pci_
>   * called automatically by this routine.
>   *
>   * Devices with legacy power management (no standard PCI PM capabilities)
> - * always require such platform hooks.  Depending on the platform, devices
> - * supporting the standard PCI PME# signal may require such platform hooks;
> - * they always update bits in config space to allow PME# generation.
> - *
> - * -EIO is returned if the device can't ever be a wakeup event source.
> - * -EINVAL is returned if the device can't generate wakeup events from
> - * the specified PCI state.  Returns zero if the operation is successful.
> + * always require such platform hooks.
> + *
> + * RETURN VALUE:
> + * 0 is returned on success
> + * -EINVAL is returned if device is not supposed to wake up the system
> + * Error code depending on the platform is returned if both the platform and
> + * the native mechanism fail to enable the generation of wake-up events
>   */
>  int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable)
>  {
>  	int pm;
> -	int status;
> -	u16 value;
> +	int error = 0;
> +	bool pme_done = false;
> +
> +	if (!device_may_wakeup(&dev->dev))
> +		return -EINVAL;
>  
> -	/* Note that drivers should verify device_may_wakeup(&dev->dev)
> -	 * before calling this function.  Platform code should report
> -	 * errors when drivers try to enable wakeup on devices that
> -	 * can't issue wakeups, or on which wakeups were disabled by
> -	 * userspace updating the /sys/devices.../power/wakeup file.
> +	/*
> +	 * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don
> +	 * Anderson we should be doing PME# wake enable followed by ACPI wake
> +	 * enable.  To disable wake-up we call the platform first, for symmetry.
>  	 */
>  
> -	status = call_platform_enable_wakeup(&dev->dev, enable);
> +	if (!enable && platform_pci_can_wakeup(dev))
> +		error = platform_pci_sleep_wake(dev, false);
>  
> -	/* find PCI PM capability in list */
>  	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> +	if (!enable || pci_pme_capable(dev, pm, state)) {
> +		pci_pme_active(dev, pm, enable);
> +		pme_done = true;
> +	}
>  
> -	/* If device doesn't support PM Capabilities, but caller wants to
> -	 * disable wake events, it's a NOP.  Otherwise fail unless the
> -	 * platform hooks handled this legacy device already.
> -	 */
> -	if (!pm)
> -		return enable ? status : 0;
> +	if (enable && platform_pci_can_wakeup(dev))
> +		error = platform_pci_sleep_wake(dev, true);
>  
> +	return pme_done ? 0 : error;
> +}
> +
> +/**
> + * pci_pm_init - Initialize PM functions of given PCI device
> + * @dev: PCI device to handle.
> + */
> +void pci_pm_init(struct pci_dev *dev)
> +{
> +	int pm;
> +	u16 pmc;
> +
> +	/* find PCI PM capability in list */
> +	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> +	if (!pm)
> +		return;
>  	/* Check device's ability to generate PME# */
> -	pci_read_config_word(dev,pm+PCI_PM_PMC,&value);
> +	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
>  
> -	value &= PCI_PM_CAP_PME_MASK;
> -	value >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
> +	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
> +		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
> +			pmc & PCI_PM_CAP_VER_MASK);
> +		return;
> +	}
>  
> -	/* Check if it can generate PME# from requested state. */
> -	if (!value || !(value & (1 << state))) {
> -		/* if it can't, revert what the platform hook changed,
> -		 * always reporting the base "EINVAL, can't PME#" error
> +	if (pmc & PCI_PM_CAP_PME_MASK) {
> +		dev_printk(KERN_INFO, &dev->dev,
> +			"PME# supported from%s%s%s%s%s\n",
> +			(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> +			(pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> +			(pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
> +			(pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
> +			(pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
> +		/*
> +		 * Make device's PM flags reflect the wake-up capability, but
> +		 * let the user space enable it to wake up the system as needed.
>  		 */
> -		if (enable)
> -			call_platform_enable_wakeup(&dev->dev, 0);
> -		return enable ? -EINVAL : 0;
> +		device_set_wakeup_capable(&dev->dev, true);
> +		device_set_wakeup_enable(&dev->dev, false);
> +		/* Disable the PME# generation functionality */
> +		pci_pme_active(dev, pm, false);
Agree. The dev.power.should_wakeup is unset for the PCI device that
supports PME. When it is required to wakeup the sleeping system, user
space can set the flag of dev->power.should_wakeup again. 
      But the device_init_wakeup(dev, 1) is still called in some
drivers, which means that dev->power.should_wakeup is set. For example:
USB driver for USB host controllers.
>  	}
> -
> -	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
> -
> -	/* Clear PME_Status by writing 1 to it and enable PME# */
> -	value |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
> -
> -	if (!enable)
> -		value &= ~PCI_PM_CTRL_PME_ENABLE;
> -
> -	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
> -
> -	return 0;
>  }
>  
>  int
> Index: linux-next/include/linux/pm_wakeup.h
> ===================================================================
> --- linux-next.orig/include/linux/pm_wakeup.h
> +++ linux-next/include/linux/pm_wakeup.h
> @@ -35,6 +35,11 @@ static inline void device_init_wakeup(st
>  	dev->power.can_wakeup = dev->power.should_wakeup = !!val;
>  }
>  
> +static inline void device_set_wakeup_capable(struct device *dev, int val)
> +{
> +	dev->power.can_wakeup = !!val;
> +}
> +
>  static inline int device_can_wakeup(struct device *dev)
>  {
>  	return dev->power.can_wakeup;
> @@ -47,21 +52,7 @@ static inline void device_set_wakeup_ena
>  
>  static inline int device_may_wakeup(struct device *dev)
>  {
> -	return dev->power.can_wakeup & dev->power.should_wakeup;
> -}
> -
> -/*
> - * Platform hook to activate device wakeup capability, if that's not already
> - * handled by enable_irq_wake() etc.
> - * Returns zero on success, else negative errno
> - */
> -extern int (*platform_enable_wakeup)(struct device *dev, int is_on);
> -
> -static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
> -{
> -	if (platform_enable_wakeup)
> -		return (*platform_enable_wakeup)(dev, is_on);
> -	return 0;
> +	return dev->power.can_wakeup && dev->power.should_wakeup;
>  }
>  
>  #else /* !CONFIG_PM */
> @@ -80,11 +71,6 @@ static inline int device_can_wakeup(stru
>  #define device_set_wakeup_enable(dev, val)	do {} while (0)
>  #define device_may_wakeup(dev)			0
>  
> -static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
> -{
> -	return 0;
> -}
> -
>  #endif /* !CONFIG_PM */
>  
>  #endif /* _LINUX_PM_WAKEUP_H */
> Index: linux-next/drivers/base/power/sysfs.c
> ===================================================================
> --- linux-next.orig/drivers/base/power/sysfs.c
> +++ linux-next/drivers/base/power/sysfs.c
> @@ -6,9 +6,6 @@
>  #include <linux/string.h>
>  #include "power.h"
>  
> -int (*platform_enable_wakeup)(struct device *dev, int is_on);
> -
> -
>  /*
>   *	wakeup - Report/change current wakeup option for device
>   *
> Index: linux-next/drivers/acpi/bus.c
> ===================================================================
> --- linux-next.orig/drivers/acpi/bus.c
> +++ linux-next/drivers/acpi/bus.c
> @@ -306,6 +306,17 @@ bool acpi_bus_power_manageable(acpi_hand
>  
>  EXPORT_SYMBOL(acpi_bus_power_manageable);
>  
> +bool acpi_bus_can_wakeup(acpi_handle handle)
> +{
> +	struct acpi_device *device;
> +	int result;
> +
> +	result = acpi_bus_get_device(handle, &device);
> +	return result ? false : device->wakeup.flags.valid;
> +}
> +
> +EXPORT_SYMBOL(acpi_bus_can_wakeup);
> +
>  /* --------------------------------------------------------------------------
>                                  Event Management
>     -------------------------------------------------------------------------- */
> Index: linux-next/drivers/pci/probe.c
> ===================================================================
> --- linux-next.orig/drivers/pci/probe.c
> +++ linux-next/drivers/pci/probe.c
> @@ -860,49 +860,6 @@ int pci_cfg_space_size_ext(struct pci_de
>  	return PCI_CFG_SPACE_SIZE;
>  }
>  
> -/**
> - * pci_disable_pme - Disable the PME function of PCI device
> - * @dev: PCI device affected
> - * -EINVAL is returned if PCI device doesn't support PME.
> - * Zero is returned if the PME is supported and can be disabled.
> - */
> -static int pci_disable_pme(struct pci_dev *dev)
> -{
> -	int pm;
> -	u16 value;
> -
> -	/* find PCI PM capability in list */
> -	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> -
> -	/* If device doesn't support PM Capabilities, it means that PME is
> -	 * not supported.
> -	 */
> -	if (!pm)
> -		return -EINVAL;
> -	/* Check device's ability to generate PME# */
> -	pci_read_config_word(dev, pm + PCI_PM_PMC, &value);
> -
> -	value &= PCI_PM_CAP_PME_MASK;
> -	/* Check if it can generate PME# */
> -	if (!value) {
> -		/*
> -		 * If it is zero, it means that PME is still unsupported
> -		 * although there exists the PM capability.
> -		 */
> -		return -EINVAL;
> -	}
> -
> -	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
> -
> -	/* Clear PME_Status by writing 1 to it */
> -	value |= PCI_PM_CTRL_PME_STATUS ;
> -	/* Disable PME enable bit */
> -	value &= ~PCI_PM_CTRL_PME_ENABLE;
> -	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
> -
> -	return 0;
> -}
> -
>  int pci_cfg_space_size(struct pci_dev *dev)
>  {
>  	int pos;
> @@ -1010,7 +967,6 @@ static struct pci_dev *pci_scan_device(s
>  	}
>  
>  	pci_vpd_pci22_init(dev);
> -	pci_disable_pme(dev);
>  
>  	return dev;
>  }
> @@ -1031,6 +987,9 @@ void pci_device_add(struct pci_dev *dev,
>  	/* Fix up broken headers */
>  	pci_fixup_device(pci_fixup_header, dev);
>  
> +	/* Initialize power management of the device */
> +	pci_pm_init(dev);
> +
>  	/*
>  	 * Add the device to our list of discovered devices
>  	 * and the bus list for fixup functions, etc.
> Index: linux-next/drivers/acpi/glue.c
> ===================================================================
> --- linux-next.orig/drivers/acpi/glue.c
> +++ linux-next/drivers/acpi/glue.c
> @@ -165,6 +165,8 @@ static int acpi_bind_one(struct device *
>  				"firmware_node");
>  		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
>  				"physical_node");
> +		if (acpi_dev->wakeup.flags.valid)
> +			device_set_wakeup_capable(dev, true);
>  	}
>  
>  	return 0;
> Index: linux-next/drivers/acpi/sleep/wakeup.c
> ===================================================================
> --- linux-next.orig/drivers/acpi/sleep/wakeup.c
> +++ linux-next/drivers/acpi/sleep/wakeup.c
> @@ -66,13 +66,15 @@ void acpi_enable_wakeup_device(u8 sleep_
>  	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
>  		struct acpi_device *dev =
>  			container_of(node, struct acpi_device, wakeup_list);
> +
>  		if (!dev->wakeup.flags.valid)
>  			continue;
> +
>  		/* If users want to disable run-wake GPE,
>  		 * we only disable it for wake and leave it for runtime
>  		 */
> -		if (!dev->wakeup.state.enabled ||
> -		    sleep_state > (u32) dev->wakeup.sleep_state) {
> +		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> +		    || sleep_state > (u32) dev->wakeup.sleep_state) {
>  			if (dev->wakeup.flags.run_wake) {
>  				spin_unlock(&acpi_device_lock);
>  				/* set_gpe_type will disable GPE, leave it like that */
If the dev->wakeup.flags.preprared is introduced, it seems that the
problem becomes more complex. Whether acpi_enable_wakeup_device_power is
called is related with the flags of dev->wakeup.state.enabled. 
   If the dev->wakeup.state.enabled is set, the
acpi_enable_wakeup_device_power will be called, in which the
dev->wakeup.flags.prepared is set. Otherwise the
dev->wakeup.flags.prepared is still unset.
   In such case it seems that whether the GPE is enabled is related with
the dev->wakeup.state.enabled.
   Of course it is also OK.

Thanks.
   Yakui
> @@ -110,8 +112,9 @@ void acpi_disable_wakeup_device(u8 sleep
>  
>  		if (!dev->wakeup.flags.valid)
>  			continue;
> -		if (!dev->wakeup.state.enabled ||
> -		    sleep_state > (u32) dev->wakeup.sleep_state) {
> +
> +		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> +		    || sleep_state > (u32) dev->wakeup.sleep_state) {
>  			if (dev->wakeup.flags.run_wake) {
>  				spin_unlock(&acpi_device_lock);
>  				acpi_set_gpe_type(dev->wakeup.gpe_device,


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4)
  2008-07-02  7:30   ` Zhao Yakui
@ 2008-07-02 10:16     ` Rafael J. Wysocki
  0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-02 10:16 UTC (permalink / raw)
  To: Zhao Yakui
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, Jesse Barnes,
	pm list, Zhang Rui

On Wednesday, 2 of July 2008, Zhao Yakui wrote:
> On Wed, 2008-07-02 at 00:08 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > PCI ACPI: Rework PCI handling of wake-up (rev. 4)
> > 
> > * Introduce function acpi_pm_device_sleep_wake() for enabling and
> >   disabling the system wake-up capability of devices that are power
> >   manageable by ACPI.
> > 
> > * Introduce function acpi_bus_can_wakeup() allowing other (dependent)
> >   subsystems to check if ACPI is able to enable the system wake-up
> >   capability of given device.
> > 
> > * Introduce callback .sleep_wake() in struct pci_platform_pm_ops and
> >   for the ACPI PCI 'driver' make it use acpi_pm_device_sleep_wake().
> > 
> > * Introduce callback .can_wakeup() in struct pci_platform_pm_ops and
> >   for the ACPI 'driver' make it use acpi_bus_can_wakeup().
> > 
> > * Move the PME# handlig code out of pci_enable_wake() and split it
> >   into two functions, pci_pme_capable() and pci_pme_active(),
> >   allowing the caller to check if given device is capable of
> >   generating PME# from given power state and to enable/disable the
> >   device's PME# functionality, respectively.
> > 
> > * Modify pci_enable_wake() to use the new ACPI callbacks and the new
> >   PME#-related functions.
> > 
> > * Drop the generic .platform_enable_wakeup() callback that is not
> >   used any more.
> > 
> > * Introduce device_set_wakeup_capable() that will set the
> >   power.can_wakeup flag of given device.
> > 
> > * Rework PCI device PM initialization so that, if given device is
> >   capable of generating wake-up events, either natively through the
> >   PME# mechanism, or with the help of the platform, its
> >   power.can_wakeup flag is set and its power.should_wakeup flag is
> >   unset as appropriate.
> > 
> > * Make ACPI set the power.can_wakeup flag for devices found to be
> >   wake-up capable by it.
> > 
> > * Make the ACPI wake-up code enable/disable GPEs for devices that
> >   have the wakeup.flags.prepared flag set (which means that their
> >   wake-up power has been enabled).
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/acpi/bus.c          |   11 ++
> >  drivers/acpi/glue.c         |    2 
> >  drivers/acpi/sleep/main.c   |   25 ++++++
> >  drivers/acpi/sleep/wakeup.c |   11 +-
> >  drivers/base/power/sysfs.c  |    3 
> >  drivers/pci/pci-acpi.c      |   19 ++++
> >  drivers/pci/pci.c           |  169 ++++++++++++++++++++++++++++++++------------
> >  drivers/pci/pci.h           |    8 ++
> >  drivers/pci/probe.c         |   47 ------------
> >  include/acpi/acpi_bus.h     |    2 
> >  include/linux/pm_wakeup.h   |   26 +-----
> >  11 files changed, 207 insertions(+), 116 deletions(-)
> > 
> > Index: linux-next/drivers/acpi/sleep/main.c
> > ===================================================================
> > --- linux-next.orig/drivers/acpi/sleep/main.c
> > +++ linux-next/drivers/acpi/sleep/main.c
> > @@ -467,6 +467,31 @@ int acpi_pm_device_sleep_state(struct de
> >  		*d_min_p = d_min;
> >  	return d_max;
> >  }
> > +
> > +/**
> > + *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
> > + *                                  capability of given device
> > + *	@dev: device to handle
> > + *	@enable: 'true' - enable, 'false' - disable the wake-up capability
> > + */
> > +int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
> > +{
> > +	acpi_handle handle;
> > +	struct acpi_device *adev;
> > +
> > +	if (!device_may_wakeup(dev))
> > +		return -EINVAL;
> > +
> > +	handle = DEVICE_ACPI_HANDLE(dev);
> > +	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
> > +		printk(KERN_DEBUG "ACPI handle has no context!\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	return enable ?
> > +		acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
> > +		acpi_disable_wakeup_device_power(adev);
> > +}
> >  #endif
> >  
> >  static void acpi_power_off_prepare(void)
> > Index: linux-next/drivers/pci/pci-acpi.c
> > ===================================================================
> > --- linux-next.orig/drivers/pci/pci-acpi.c
> > +++ linux-next/drivers/pci/pci-acpi.c
> > @@ -299,10 +299,29 @@ static int acpi_pci_set_power_state(stru
> >  	return error;
> >  }
> >  
> > +static bool acpi_pci_can_wakeup(struct pci_dev *dev)
> > +{
> > +	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
> > +
> > +	return handle ? acpi_bus_can_wakeup(handle) : false;
> > +}
> > +
> > +static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
> > +{
> > +	int error = acpi_pm_device_sleep_wake(&dev->dev, enable);
> > +
> > +	if (!error)
> > +		printk(KERN_INFO "PCI: Wake-up capability of %s %s by ACPI\n",
> > +			pci_name(dev), enable ? "enabled" : "disabled");
> > +	return error;
> > +}
> > +
> >  static struct pci_platform_pm_ops acpi_pci_platform_pm = {
> >  	.is_manageable = acpi_pci_power_manageable,
> >  	.set_state = acpi_pci_set_power_state,
> >  	.choose_state = acpi_pci_choose_state,
> > +	.can_wakeup = acpi_pci_can_wakeup,
> > +	.sleep_wake = acpi_pci_sleep_wake,
> >  };
> >  
> >  /* ACPI bus type */
> > Index: linux-next/drivers/pci/pci.h
> > ===================================================================
> > --- linux-next.orig/drivers/pci/pci.h
> > +++ linux-next/drivers/pci/pci.h
> > @@ -17,6 +17,11 @@ extern void pci_cleanup_rom(struct pci_d
> >   *                 platform; to be used during system-wide transitions from a
> >   *                 sleeping state to the working state and vice versa
> >   *
> > + * @can_wakeup - returns 'true' if given device is capable of waking up the
> > + *               system from a sleeping state
> > + *
> > + * @sleep_wake - enables/disables the system wake up capability of given device
> > + *
> >   * If given platform is generally capable of power managing PCI devices, all of
> >   * these callbacks are mandatory.
> >   */
> > @@ -24,9 +29,12 @@ struct pci_platform_pm_ops {
> >  	bool (*is_manageable)(struct pci_dev *dev);
> >  	int (*set_state)(struct pci_dev *dev, pci_power_t state);
> >  	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);
> >  };
> >  
> >  extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
> > +extern void pci_pm_init(struct pci_dev *dev);
> >  
> >  extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
> >  extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
> > Index: linux-next/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux-next.orig/include/acpi/acpi_bus.h
> > +++ linux-next/include/acpi/acpi_bus.h
> > @@ -337,6 +337,7 @@ int acpi_bus_get_status(struct acpi_devi
> >  int acpi_bus_get_power(acpi_handle handle, int *state);
> >  int acpi_bus_set_power(acpi_handle handle, int state);
> >  bool acpi_bus_power_manageable(acpi_handle handle);
> > +bool acpi_bus_can_wakeup(acpi_handle handle);
> >  #ifdef CONFIG_ACPI_PROC_EVENT
> >  int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
> >  int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
> > @@ -379,6 +380,7 @@ acpi_handle acpi_get_pci_rootbridge_hand
> >  
> >  #ifdef CONFIG_PM_SLEEP
> >  int acpi_pm_device_sleep_state(struct device *, int *);
> > +int acpi_pm_device_sleep_wake(struct device *, bool);
> >  #else /* !CONFIG_PM_SLEEP */
> >  static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
> >  {
> > Index: linux-next/drivers/pci/pci.c
> > ===================================================================
> > --- linux-next.orig/drivers/pci/pci.c
> > +++ linux-next/drivers/pci/pci.c
> > @@ -380,7 +380,8 @@ static struct pci_platform_pm_ops *pci_p
> >  
> >  int pci_set_platform_pm(struct pci_platform_pm_ops *ops)
> >  {
> > -	if (!ops->is_manageable || !ops->set_state || !ops->choose_state)
> > +	if (!ops->is_manageable || !ops->set_state || !ops->choose_state
> > +	    || !ops->sleep_wake || !ops->can_wakeup)
> >  		return -EINVAL;
> >  	pci_platform_pm = ops;
> >  	return 0;
> > @@ -403,6 +404,17 @@ static inline pci_power_t platform_pci_c
> >  			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
> >  }
> >  
> > +static inline bool platform_pci_can_wakeup(struct pci_dev *dev)
> > +{
> > +	return pci_platform_pm ? pci_platform_pm->can_wakeup(dev) : false;
> > +}
> > +
> > +static inline int platform_pci_sleep_wake(struct pci_dev *dev, bool enable)
> > +{
> > +	return pci_platform_pm ?
> > +			pci_platform_pm->sleep_wake(dev, enable) : -ENODEV;
> > +}
> > +
> >  /**
> >   * pci_raw_set_power_state - Use PCI PM registers to set the power state of
> >   *                           given PCI device
> > @@ -1036,6 +1048,56 @@ int pci_set_pcie_reset_state(struct pci_
> >  }
> >  
> >  /**
> > + * pci_pme_capable - check the capability of PCI device to generate PME#
> > + * @dev: PCI device to handle.
> > + * @pm: PCI PM capability offset of the device.
> > + * @state: PCI state from which device will issue PME#.
> > + */
> > +static bool pci_pme_capable(struct pci_dev *dev, int pm, pci_power_t state)
> > +{
> > +	u16 pmc;
> > +
> > +	if (!pm)
> > +		return false;
> > +
> > +	/* Check device's ability to generate PME# from given state */
> > +	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
> > +
> > +	pmc &= PCI_PM_CAP_PME_MASK;
> > +	pmc >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
> > +
> > +	return !!(pmc & (1 << state));
> > +}
> > +
> > +/**
> > + * pci_pme_active - enable or disable PCI device's PME# function
> > + * @dev: PCI device to handle.
> > + * @pm: PCI PM capability offset of the device.
> > + * @enable: 'true' to enable PME# generation; 'false' to disable it.
> > + *
> > + * The caller must verify that the device is capable of generating PME# before
> > + * calling this function with @enable equal to 'true'.
> > + */
> > +static void pci_pme_active(struct pci_dev *dev, int pm, bool enable)
> > +{
> > +	u16 pmcsr;
> > +
> > +	if (!pm)
> > +		return;
> > +
> > +	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
> > +	/* Clear PME_Status by writing 1 to it and enable PME# */
> > +	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
> > +	if (!enable)
> > +		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
> > +
> > +	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
> > +
> > +	printk(KERN_INFO "PCI: PME# from device %s %s\n", pci_name(dev),
> > +		enable ? "enabled" : "disabled");
> > +}
> > +
> > +/**
> >   * pci_enable_wake - enable PCI device as wakeup event source
> >   * @dev: PCI device affected
> >   * @state: PCI state from which device will issue wakeup events
> > @@ -1046,66 +1108,83 @@ int pci_set_pcie_reset_state(struct pci_
> >   * called automatically by this routine.
> >   *
> >   * Devices with legacy power management (no standard PCI PM capabilities)
> > - * always require such platform hooks.  Depending on the platform, devices
> > - * supporting the standard PCI PME# signal may require such platform hooks;
> > - * they always update bits in config space to allow PME# generation.
> > - *
> > - * -EIO is returned if the device can't ever be a wakeup event source.
> > - * -EINVAL is returned if the device can't generate wakeup events from
> > - * the specified PCI state.  Returns zero if the operation is successful.
> > + * always require such platform hooks.
> > + *
> > + * RETURN VALUE:
> > + * 0 is returned on success
> > + * -EINVAL is returned if device is not supposed to wake up the system
> > + * Error code depending on the platform is returned if both the platform and
> > + * the native mechanism fail to enable the generation of wake-up events
> >   */
> >  int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable)
> >  {
> >  	int pm;
> > -	int status;
> > -	u16 value;
> > +	int error = 0;
> > +	bool pme_done = false;
> > +
> > +	if (!device_may_wakeup(&dev->dev))
> > +		return -EINVAL;
> >  
> > -	/* Note that drivers should verify device_may_wakeup(&dev->dev)
> > -	 * before calling this function.  Platform code should report
> > -	 * errors when drivers try to enable wakeup on devices that
> > -	 * can't issue wakeups, or on which wakeups were disabled by
> > -	 * userspace updating the /sys/devices.../power/wakeup file.
> > +	/*
> > +	 * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don
> > +	 * Anderson we should be doing PME# wake enable followed by ACPI wake
> > +	 * enable.  To disable wake-up we call the platform first, for symmetry.
> >  	 */
> >  
> > -	status = call_platform_enable_wakeup(&dev->dev, enable);
> > +	if (!enable && platform_pci_can_wakeup(dev))
> > +		error = platform_pci_sleep_wake(dev, false);
> >  
> > -	/* find PCI PM capability in list */
> >  	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> > +	if (!enable || pci_pme_capable(dev, pm, state)) {
> > +		pci_pme_active(dev, pm, enable);
> > +		pme_done = true;
> > +	}
> >  
> > -	/* If device doesn't support PM Capabilities, but caller wants to
> > -	 * disable wake events, it's a NOP.  Otherwise fail unless the
> > -	 * platform hooks handled this legacy device already.
> > -	 */
> > -	if (!pm)
> > -		return enable ? status : 0;
> > +	if (enable && platform_pci_can_wakeup(dev))
> > +		error = platform_pci_sleep_wake(dev, true);
> >  
> > +	return pme_done ? 0 : error;
> > +}
> > +
> > +/**
> > + * pci_pm_init - Initialize PM functions of given PCI device
> > + * @dev: PCI device to handle.
> > + */
> > +void pci_pm_init(struct pci_dev *dev)
> > +{
> > +	int pm;
> > +	u16 pmc;
> > +
> > +	/* find PCI PM capability in list */
> > +	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> > +	if (!pm)
> > +		return;
> >  	/* Check device's ability to generate PME# */
> > -	pci_read_config_word(dev,pm+PCI_PM_PMC,&value);
> > +	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
> >  
> > -	value &= PCI_PM_CAP_PME_MASK;
> > -	value >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
> > +	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
> > +		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
> > +			pmc & PCI_PM_CAP_VER_MASK);
> > +		return;
> > +	}
> >  
> > -	/* Check if it can generate PME# from requested state. */
> > -	if (!value || !(value & (1 << state))) {
> > -		/* if it can't, revert what the platform hook changed,
> > -		 * always reporting the base "EINVAL, can't PME#" error
> > +	if (pmc & PCI_PM_CAP_PME_MASK) {
> > +		dev_printk(KERN_INFO, &dev->dev,
> > +			"PME# supported from%s%s%s%s%s\n",
> > +			(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
> > +			(pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
> > +			(pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
> > +			(pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
> > +			(pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
> > +		/*
> > +		 * Make device's PM flags reflect the wake-up capability, but
> > +		 * let the user space enable it to wake up the system as needed.
> >  		 */
> > -		if (enable)
> > -			call_platform_enable_wakeup(&dev->dev, 0);
> > -		return enable ? -EINVAL : 0;
> > +		device_set_wakeup_capable(&dev->dev, true);
> > +		device_set_wakeup_enable(&dev->dev, false);
> > +		/* Disable the PME# generation functionality */
> > +		pci_pme_active(dev, pm, false);
> Agree. The dev.power.should_wakeup is unset for the PCI device that
> supports PME. When it is required to wakeup the sleeping system, user
> space can set the flag of dev->power.should_wakeup again. 
>       But the device_init_wakeup(dev, 1) is still called in some
> drivers, which means that dev->power.should_wakeup is set. For example:
> USB driver for USB host controllers.
> >  	}
> > -
> > -	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
> > -
> > -	/* Clear PME_Status by writing 1 to it and enable PME# */
> > -	value |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
> > -
> > -	if (!enable)
> > -		value &= ~PCI_PM_CTRL_PME_ENABLE;
> > -
> > -	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
> > -
> > -	return 0;
> >  }
> >  
> >  int
> > Index: linux-next/include/linux/pm_wakeup.h
> > ===================================================================
> > --- linux-next.orig/include/linux/pm_wakeup.h
> > +++ linux-next/include/linux/pm_wakeup.h
> > @@ -35,6 +35,11 @@ static inline void device_init_wakeup(st
> >  	dev->power.can_wakeup = dev->power.should_wakeup = !!val;
> >  }
> >  
> > +static inline void device_set_wakeup_capable(struct device *dev, int val)
> > +{
> > +	dev->power.can_wakeup = !!val;
> > +}
> > +
> >  static inline int device_can_wakeup(struct device *dev)
> >  {
> >  	return dev->power.can_wakeup;
> > @@ -47,21 +52,7 @@ static inline void device_set_wakeup_ena
> >  
> >  static inline int device_may_wakeup(struct device *dev)
> >  {
> > -	return dev->power.can_wakeup & dev->power.should_wakeup;
> > -}
> > -
> > -/*
> > - * Platform hook to activate device wakeup capability, if that's not already
> > - * handled by enable_irq_wake() etc.
> > - * Returns zero on success, else negative errno
> > - */
> > -extern int (*platform_enable_wakeup)(struct device *dev, int is_on);
> > -
> > -static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
> > -{
> > -	if (platform_enable_wakeup)
> > -		return (*platform_enable_wakeup)(dev, is_on);
> > -	return 0;
> > +	return dev->power.can_wakeup && dev->power.should_wakeup;
> >  }
> >  
> >  #else /* !CONFIG_PM */
> > @@ -80,11 +71,6 @@ static inline int device_can_wakeup(stru
> >  #define device_set_wakeup_enable(dev, val)	do {} while (0)
> >  #define device_may_wakeup(dev)			0
> >  
> > -static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
> > -{
> > -	return 0;
> > -}
> > -
> >  #endif /* !CONFIG_PM */
> >  
> >  #endif /* _LINUX_PM_WAKEUP_H */
> > Index: linux-next/drivers/base/power/sysfs.c
> > ===================================================================
> > --- linux-next.orig/drivers/base/power/sysfs.c
> > +++ linux-next/drivers/base/power/sysfs.c
> > @@ -6,9 +6,6 @@
> >  #include <linux/string.h>
> >  #include "power.h"
> >  
> > -int (*platform_enable_wakeup)(struct device *dev, int is_on);
> > -
> > -
> >  /*
> >   *	wakeup - Report/change current wakeup option for device
> >   *
> > Index: linux-next/drivers/acpi/bus.c
> > ===================================================================
> > --- linux-next.orig/drivers/acpi/bus.c
> > +++ linux-next/drivers/acpi/bus.c
> > @@ -306,6 +306,17 @@ bool acpi_bus_power_manageable(acpi_hand
> >  
> >  EXPORT_SYMBOL(acpi_bus_power_manageable);
> >  
> > +bool acpi_bus_can_wakeup(acpi_handle handle)
> > +{
> > +	struct acpi_device *device;
> > +	int result;
> > +
> > +	result = acpi_bus_get_device(handle, &device);
> > +	return result ? false : device->wakeup.flags.valid;
> > +}
> > +
> > +EXPORT_SYMBOL(acpi_bus_can_wakeup);
> > +
> >  /* --------------------------------------------------------------------------
> >                                  Event Management
> >     -------------------------------------------------------------------------- */
> > Index: linux-next/drivers/pci/probe.c
> > ===================================================================
> > --- linux-next.orig/drivers/pci/probe.c
> > +++ linux-next/drivers/pci/probe.c
> > @@ -860,49 +860,6 @@ int pci_cfg_space_size_ext(struct pci_de
> >  	return PCI_CFG_SPACE_SIZE;
> >  }
> >  
> > -/**
> > - * pci_disable_pme - Disable the PME function of PCI device
> > - * @dev: PCI device affected
> > - * -EINVAL is returned if PCI device doesn't support PME.
> > - * Zero is returned if the PME is supported and can be disabled.
> > - */
> > -static int pci_disable_pme(struct pci_dev *dev)
> > -{
> > -	int pm;
> > -	u16 value;
> > -
> > -	/* find PCI PM capability in list */
> > -	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
> > -
> > -	/* If device doesn't support PM Capabilities, it means that PME is
> > -	 * not supported.
> > -	 */
> > -	if (!pm)
> > -		return -EINVAL;
> > -	/* Check device's ability to generate PME# */
> > -	pci_read_config_word(dev, pm + PCI_PM_PMC, &value);
> > -
> > -	value &= PCI_PM_CAP_PME_MASK;
> > -	/* Check if it can generate PME# */
> > -	if (!value) {
> > -		/*
> > -		 * If it is zero, it means that PME is still unsupported
> > -		 * although there exists the PM capability.
> > -		 */
> > -		return -EINVAL;
> > -	}
> > -
> > -	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
> > -
> > -	/* Clear PME_Status by writing 1 to it */
> > -	value |= PCI_PM_CTRL_PME_STATUS ;
> > -	/* Disable PME enable bit */
> > -	value &= ~PCI_PM_CTRL_PME_ENABLE;
> > -	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
> > -
> > -	return 0;
> > -}
> > -
> >  int pci_cfg_space_size(struct pci_dev *dev)
> >  {
> >  	int pos;
> > @@ -1010,7 +967,6 @@ static struct pci_dev *pci_scan_device(s
> >  	}
> >  
> >  	pci_vpd_pci22_init(dev);
> > -	pci_disable_pme(dev);
> >  
> >  	return dev;
> >  }
> > @@ -1031,6 +987,9 @@ void pci_device_add(struct pci_dev *dev,
> >  	/* Fix up broken headers */
> >  	pci_fixup_device(pci_fixup_header, dev);
> >  
> > +	/* Initialize power management of the device */
> > +	pci_pm_init(dev);
> > +
> >  	/*
> >  	 * Add the device to our list of discovered devices
> >  	 * and the bus list for fixup functions, etc.
> > Index: linux-next/drivers/acpi/glue.c
> > ===================================================================
> > --- linux-next.orig/drivers/acpi/glue.c
> > +++ linux-next/drivers/acpi/glue.c
> > @@ -165,6 +165,8 @@ static int acpi_bind_one(struct device *
> >  				"firmware_node");
> >  		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
> >  				"physical_node");
> > +		if (acpi_dev->wakeup.flags.valid)
> > +			device_set_wakeup_capable(dev, true);
> >  	}
> >  
> >  	return 0;
> > Index: linux-next/drivers/acpi/sleep/wakeup.c
> > ===================================================================
> > --- linux-next.orig/drivers/acpi/sleep/wakeup.c
> > +++ linux-next/drivers/acpi/sleep/wakeup.c
> > @@ -66,13 +66,15 @@ void acpi_enable_wakeup_device(u8 sleep_
> >  	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> >  		struct acpi_device *dev =
> >  			container_of(node, struct acpi_device, wakeup_list);
> > +
> >  		if (!dev->wakeup.flags.valid)
> >  			continue;
> > +
> >  		/* If users want to disable run-wake GPE,
> >  		 * we only disable it for wake and leave it for runtime
> >  		 */
> > -		if (!dev->wakeup.state.enabled ||
> > -		    sleep_state > (u32) dev->wakeup.sleep_state) {
> > +		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> > +		    || sleep_state > (u32) dev->wakeup.sleep_state) {
> >  			if (dev->wakeup.flags.run_wake) {
> >  				spin_unlock(&acpi_device_lock);
> >  				/* set_gpe_type will disable GPE, leave it like that */
> If the dev->wakeup.flags.preprared is introduced, it seems that the
> problem becomes more complex. Whether acpi_enable_wakeup_device_power is
> called is related with the flags of dev->wakeup.state.enabled. 
>    If the dev->wakeup.state.enabled is set, the
> acpi_enable_wakeup_device_power will be called, in which the
> dev->wakeup.flags.prepared is set. Otherwise the
> dev->wakeup.flags.prepared is still unset.
>    In such case it seems that whether the GPE is enabled is related with
> the dev->wakeup.state.enabled.
>    Of course it is also OK.

Please note, however, that acpi_enable_wakeup_device_power() will also be
called for PCI devices that have both dev->dev.power.can_wakeup and
dev->dev.power.should_wakeup set, through acpi_pm_device_sleep_wake().

Those devices will have dev->wakeup.flags.prepared set and that will result in
the GPE being enabled for them, regardless of the status of their
dev->wakeup.state.enabled flags.  Thus the user space can control the wake-up
capability of such devices without using the /proc/acpi/wakeup interface, but
this has no effect on (a) non-PCI devices and (b) PCI devices the drivers of
which don't invoke pci_enable_wake(dev, state, true).

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3)
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (8 preceding siblings ...)
  2008-07-01 22:10 ` [PATCH 9/9] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
@ 2008-07-03 12:04 ` Rafael J. Wysocki
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
  10 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-03 12:04 UTC (permalink / raw)
  To: ACPI Devel Maling List
  Cc: Tobias Diedrich, Jesse Barnes, Zhao Yakui, Andi Kleen, pm list

On Tuesday, 1 of July 2008, Rafael J. Wysocki wrote:
> Hi,
> 
> This is a new (3rd) iteration of the series of patches intended to rework the power
> management of PCI devices so that the handling of their wake-up functionality
> is consistent with the other PM operations and so that the user space can
> manage the wake-up functionality of PCI devices using the
> /sys/devices/.../power/wakeup interface regardless of whether the wake-up
> function is based on the native PCI mechanism (PME#), or it is handled by the
> platform (eg. ACPI).
> 
> Apart from the ACPI and PCI changes necessary for this purpose, the series
> includes some clean-ups that are not strictly required, but make the code more
> straightforward and (IMHO) easier to follow.
> 
> The patchset is on top of the linux-next from today.  It contains one
> additional patch [1/9] which is a revert of another patch that shouldn't have
> been put into linux-next (at least not in the way it was done).
> 
> Please review.

FWIW, I have tested this patchset on top of the yesterday's linux-next on a box
with the Asus A8N SLI mainboard and the WOL using Magic Packet works after
hibernation (forcedeth driver).

Full dmesg including boot and one hibernation/resume cycle is at:
http://www.sisk.pl/kernel/debug/20080702/dmesg-A8N-SLI.log

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4)
  2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
                   ` (9 preceding siblings ...)
  2008-07-03 12:04 ` [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
@ 2008-07-07  1:30 ` Rafael J. Wysocki
  2008-07-07  1:30   ` [PATCH 1/8] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
                     ` (9 more replies)
  10 siblings, 10 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:30 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

Hi Jesse,
 
This is a new (4th) iteration of the series of patches intended to rework the power
management of PCI devices so that the handling of their wake-up functionality
is consistent with the other PM operations and so that the user space can
manage the wake-up functionality of PCI devices using the
/sys/devices/.../power/wakeup interface regardless of whether the wake-up
function is based on the native PCI mechanism (PME#), or it is handled by the
platform (eg. ACPI).
 
Apart from the ACPI and PCI changes necessary for this purpose, the series
includes some clean-ups that are not strictly required, but make the code more
straightforward and (IMHO) easier to follow.

The patchset is on top of the linux-next branch of the PCI tree and is regarded
as 2.6.27 material.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/8] ACPI: Introduce acpi_bus_power_manageable function
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
@ 2008-07-07  1:30   ` Rafael J. Wysocki
  2008-07-07  1:32   ` [PATCH 2/8] PCI: Introduce platform_pci_power_manageable function Rafael J. Wysocki
                     ` (8 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:30 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

ACPI: Introduce acpi_bus_power_manageable function

Introduce function acpi_bus_power_manageable() allowing other
(dependent) subsystems to check if ACPI is able to power manage given
device.  This may be useful, for example, for PCI device power
management.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
 drivers/acpi/bus.c      |   11 +++++++++++
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 12 insertions(+)

Index: linux-next/drivers/acpi/bus.c
===================================================================
--- linux-next.orig/drivers/acpi/bus.c
+++ linux-next/drivers/acpi/bus.c
@@ -295,6 +295,17 @@ int acpi_bus_set_power(acpi_handle handl
 
 EXPORT_SYMBOL(acpi_bus_set_power);
 
+bool acpi_bus_power_manageable(acpi_handle handle)
+{
+	struct acpi_device *device;
+	int result;
+
+	result = acpi_bus_get_device(handle, &device);
+	return result ? false : device->flags.power_manageable;
+}
+
+EXPORT_SYMBOL(acpi_bus_power_manageable);
+
 /* --------------------------------------------------------------------------
                                 Event Management
    -------------------------------------------------------------------------- */
Index: linux-next/include/acpi/acpi_bus.h
===================================================================
--- linux-next.orig/include/acpi/acpi_bus.h
+++ linux-next/include/acpi/acpi_bus.h
@@ -335,6 +335,7 @@ void acpi_bus_data_handler(acpi_handle h
 int acpi_bus_get_status(struct acpi_device *device);
 int acpi_bus_get_power(acpi_handle handle, int *state);
 int acpi_bus_set_power(acpi_handle handle, int state);
+bool acpi_bus_power_manageable(acpi_handle handle);
 #ifdef CONFIG_ACPI_PROC_EVENT
 int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
 int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 2/8] PCI: Introduce platform_pci_power_manageable function
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
  2008-07-07  1:30   ` [PATCH 1/8] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
@ 2008-07-07  1:32   ` Rafael J. Wysocki
  2008-07-07  1:32   ` [PATCH 3/8] PCI: Rework pci_set_power_state function (rev. 4) Rafael J. Wysocki
                     ` (7 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:32 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI: Introduce platform_pci_power_manageable function

Introduce function pointer platform_pci_power_manageable to be used
by the platform-related code to point to a function allowing us to
check if given device is power manageable by the platform.

Introduce acpi_pci_power_manageable() playing that role for ACPI.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci-acpi.c |   19 +++++++++++++------
 drivers/pci/pci.c      |   40 ++++++++++++++++++++++++++++++----------
 drivers/pci/pci.h      |   26 ++++++++++++++++++++++----
 3 files changed, 65 insertions(+), 20 deletions(-)

Index: linux-next/drivers/pci/pci-acpi.c
===================================================================
--- linux-next.orig/drivers/pci/pci-acpi.c
+++ linux-next/drivers/pci/pci-acpi.c
@@ -215,7 +215,6 @@ acpi_status pci_osc_control_set(acpi_han
 }
 EXPORT_SYMBOL(pci_osc_control_set);
 
-#ifdef CONFIG_ACPI_SLEEP
 /*
  * _SxD returns the D-state with the highest power
  * (lowest D-state number) supported in the S-state "x".
@@ -259,7 +258,13 @@ static pci_power_t acpi_pci_choose_state
 	}
 	return PCI_POWER_ERROR;
 }
-#endif
+
+static bool acpi_pci_power_manageable(struct pci_dev *dev)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	return handle ? acpi_bus_power_manageable(handle) : false;
+}
 
 static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
@@ -290,6 +295,11 @@ static int acpi_pci_set_power_state(stru
 	return -EINVAL;
 }
 
+static struct pci_platform_pm_ops acpi_pci_platform_pm = {
+	.is_manageable = acpi_pci_power_manageable,
+	.set_state = acpi_pci_set_power_state,
+	.choose_state = acpi_pci_choose_state,
+};
 
 /* ACPI bus type */
 static int acpi_pci_find_device(struct device *dev, acpi_handle *handle)
@@ -341,10 +351,7 @@ static int __init acpi_pci_init(void)
 	ret = register_acpi_bus_type(&acpi_pci_bus);
 	if (ret)
 		return 0;
-#ifdef	CONFIG_ACPI_SLEEP
-	platform_pci_choose_state = acpi_pci_choose_state;
-#endif
-	platform_pci_set_power_state = acpi_pci_set_power_state;
+	pci_set_platform_pm(&acpi_pci_platform_pm);
 	return 0;
 }
 arch_initcall(acpi_pci_init);
Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -376,7 +376,32 @@ pci_restore_bars(struct pci_dev *dev)
 		pci_update_resource(dev, &dev->resource[i], i);
 }
 
-int (*platform_pci_set_power_state)(struct pci_dev *dev, pci_power_t t);
+static struct pci_platform_pm_ops *pci_platform_pm;
+
+int pci_set_platform_pm(struct pci_platform_pm_ops *ops)
+{
+	if (!ops->is_manageable || !ops->set_state || !ops->choose_state)
+		return -EINVAL;
+	pci_platform_pm = ops;
+	return 0;
+}
+
+static inline bool platform_pci_power_manageable(struct pci_dev *dev)
+{
+	return pci_platform_pm ? pci_platform_pm->is_manageable(dev) : false;
+}
+
+static inline int platform_pci_set_power_state(struct pci_dev *dev,
+                                                pci_power_t t)
+{
+	return pci_platform_pm ? pci_platform_pm->set_state(dev, t) : -ENOSYS;
+}
+
+static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev)
+{
+	return pci_platform_pm ?
+			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
+}
 
 /**
  * pci_set_power_state - Set the power state of a PCI device
@@ -479,8 +504,7 @@ pci_set_power_state(struct pci_dev *dev,
 	 * Give firmware a chance to be called, such as ACPI _PRx, _PSx
 	 * Firmware method after native method ?
 	 */
-	if (platform_pci_set_power_state)
-		platform_pci_set_power_state(dev, state);
+	platform_pci_set_power_state(dev, state);
 
 	dev->current_state = state;
 
@@ -505,8 +529,6 @@ pci_set_power_state(struct pci_dev *dev,
 	return 0;
 }
 
-pci_power_t (*platform_pci_choose_state)(struct pci_dev *dev);
- 
 /**
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
@@ -524,11 +546,9 @@ pci_power_t pci_choose_state(struct pci_
 	if (!pci_find_capability(dev, PCI_CAP_ID_PM))
 		return PCI_D0;
 
-	if (platform_pci_choose_state) {
-		ret = platform_pci_choose_state(dev);
-		if (ret != PCI_POWER_ERROR)
-			return ret;
-	}
+	ret = platform_pci_choose_state(dev);
+	if (ret != PCI_POWER_ERROR)
+		return ret;
 
 	switch (state.event) {
 	case PM_EVENT_ON:
Index: linux-next/drivers/pci/pci.h
===================================================================
--- linux-next.orig/drivers/pci/pci.h
+++ linux-next/drivers/pci/pci.h
@@ -5,10 +5,28 @@ extern int pci_create_sysfs_dev_files(st
 extern void pci_remove_sysfs_dev_files(struct pci_dev *pdev);
 extern void pci_cleanup_rom(struct pci_dev *dev);
 
-/* Firmware callbacks */
-extern pci_power_t (*platform_pci_choose_state)(struct pci_dev *dev);
-extern int (*platform_pci_set_power_state)(struct pci_dev *dev,
-						pci_power_t state);
+/**
+ * Firmware PM callbacks
+ *
+ * @is_manageable - returns 'true' if given device is power manageable by the
+ *                  platform firmware
+ *
+ * @set_state - invokes the platform firmware to set the device's power state
+ *
+ * @choose_state - returns PCI power state of given device preferred by the
+ *                 platform; to be used during system-wide transitions from a
+ *                 sleeping state to the working state and vice versa
+ *
+ * If given platform is generally capable of power managing PCI devices, all of
+ * these callbacks are mandatory.
+ */
+struct pci_platform_pm_ops {
+	bool (*is_manageable)(struct pci_dev *dev);
+	int (*set_state)(struct pci_dev *dev, pci_power_t state);
+	pci_power_t (*choose_state)(struct pci_dev *dev);
+};
+
+extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
 
 extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
 extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 3/8] PCI: Rework pci_set_power_state function (rev. 4)
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
  2008-07-07  1:30   ` [PATCH 1/8] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
  2008-07-07  1:32   ` [PATCH 2/8] PCI: Introduce platform_pci_power_manageable function Rafael J. Wysocki
@ 2008-07-07  1:32   ` Rafael J. Wysocki
  2008-07-07  1:33   ` [PATCH 4/8] ACPI: Introduce acpi_device_sleep_wake function Rafael J. Wysocki
                     ` (6 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:32 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI: Rework pci_set_power_state function (rev. 4)

Rework pci_set_power_state() so that the platform callback is
invoked before the native mechanism, if necessary.  Also, make
the function check if the device is power manageable by the
platform before invoking the platform callback.

This may matter if the device dependent on additional power
resources controlled by the platform is being put into D0, in which
case those power resources must be turned on before we attempt to
handle the device itself. 

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
 drivers/pci/pci-acpi.c |   16 +++--
 drivers/pci/pci.c      |  152 +++++++++++++++++++++++++++++++++----------------
 2 files changed, 115 insertions(+), 53 deletions(-)

Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -404,67 +404,56 @@ static inline pci_power_t platform_pci_c
 }
 
 /**
- * pci_set_power_state - Set the power state of a PCI device
- * @dev: PCI device to be suspended
- * @state: PCI power state (D0, D1, D2, D3hot, D3cold) we're entering
- *
- * Transition a device to a new power state, using the Power Management 
- * Capabilities in the device's config space.
- *
- * RETURN VALUE: 
- * -EINVAL if trying to enter a lower state than we're already in.
- * 0 if we're already in the requested state.
- * -EIO if device does not support PCI PM.
- * 0 if we can successfully change the power state.
+ * pci_raw_set_power_state - Use PCI PM registers to set the power state of
+ *                           given PCI device
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
+ *
+ * RETURN VALUE:
+ * -EINVAL if the requested state is invalid.
+ * -EIO if device does not support PCI PM or its PM capabilities register has a
+ * wrong version, or device doesn't support the requested state.
+ * 0 if device already is in the requested state.
+ * 0 if device's power state has been successfully changed.
  */
-int
-pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+static int
+pci_raw_set_power_state(struct pci_dev *dev, int pm, pci_power_t state)
 {
-	int pm, need_restore = 0;
 	u16 pmcsr, pmc;
+	bool need_restore = false;
 
-	/* bound the state we're entering */
-	if (state > PCI_D3hot)
-		state = PCI_D3hot;
-
-	/*
-	 * If the device or the parent bridge can't support PCI PM, ignore
-	 * the request if we're doing anything besides putting it into D0
-	 * (which would only happen on boot).
-	 */
-	if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
-		return 0;
-
-	/* find PCI PM capability in list */
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-
-	/* abort if the device doesn't support PM capabilities */
 	if (!pm)
 		return -EIO;
 
+	if (state < PCI_D0 || state > PCI_D3hot)
+		return -EINVAL;
+
 	/* Validate current state:
 	 * Can enter D0 from any state, but if we can only go deeper 
 	 * to sleep if we're already in a low power state
 	 */
-	if (state != PCI_D0 && dev->current_state > state) {
+	if (dev->current_state == state) {
+		/* we're already there */
+		return 0;
+	} else if (state != PCI_D0 && dev->current_state <= PCI_D3cold
+	    && dev->current_state > state) {
 		dev_err(&dev->dev, "invalid power transition "
 			"(from state %d to %d)\n", dev->current_state, state);
 		return -EINVAL;
-	} else if (dev->current_state == state)
-		return 0;        /* we're already there */
+	}
 
+	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
 
-	pci_read_config_word(dev,pm + PCI_PM_PMC,&pmc);
 	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
-		dev_printk(KERN_DEBUG, &dev->dev, "unsupported PM cap regs "
-			   "version (%u)\n", pmc & PCI_PM_CAP_VER_MASK);
+		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
+			pmc & PCI_PM_CAP_VER_MASK);
 		return -EIO;
 	}
 
 	/* check if this device supports the desired state */
-	if (state == PCI_D1 && !(pmc & PCI_PM_CAP_D1))
-		return -EIO;
-	else if (state == PCI_D2 && !(pmc & PCI_PM_CAP_D2))
+	if ((state == PCI_D1 && !(pmc & PCI_PM_CAP_D1))
+	   || (state == PCI_D2 && !(pmc & PCI_PM_CAP_D2)))
 		return -EIO;
 
 	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
@@ -483,7 +472,7 @@ pci_set_power_state(struct pci_dev *dev,
 	case PCI_UNKNOWN: /* Boot-up */
 		if ((pmcsr & PCI_PM_CTRL_STATE_MASK) == PCI_D3hot
 		 && !(pmcsr & PCI_PM_CTRL_NO_SOFT_RESET))
-			need_restore = 1;
+			need_restore = true;
 		/* Fall-through: force to D0 */
 	default:
 		pmcsr = 0;
@@ -500,12 +489,6 @@ pci_set_power_state(struct pci_dev *dev,
 	else if (state == PCI_D2 || dev->current_state == PCI_D2)
 		udelay(200);
 
-	/*
-	 * Give firmware a chance to be called, such as ACPI _PRx, _PSx
-	 * Firmware method after native method ?
-	 */
-	platform_pci_set_power_state(dev, state);
-
 	dev->current_state = state;
 
 	/* According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT
@@ -530,6 +513,81 @@ pci_set_power_state(struct pci_dev *dev,
 }
 
 /**
+ * pci_update_current_state - Read PCI power state of given device from its
+ *                            PCI PM registers and cache it
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ */
+static void pci_update_current_state(struct pci_dev *dev, int pm)
+{
+	if (pm) {
+		u16 pmcsr;
+
+		pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+		dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK);
+	}
+}
+
+/**
+ * pci_set_power_state - Set the power state of a PCI device
+ * @dev: PCI device to handle.
+ * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
+ *
+ * Transition a device to a new power state, using the platform formware and/or
+ * the device's PCI PM registers.
+ *
+ * RETURN VALUE:
+ * -EINVAL if the requested state is invalid.
+ * -EIO if device does not support PCI PM or its PM capabilities register has a
+ * wrong version, or device doesn't support the requested state.
+ * 0 if device already is in the requested state.
+ * 0 if device's power state has been successfully changed.
+ */
+int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+	int pm, error;
+
+	/* bound the state we're entering */
+	if (state > PCI_D3hot)
+		state = PCI_D3hot;
+	else if (state < PCI_D0)
+		state = PCI_D0;
+	else if ((state == PCI_D1 || state == PCI_D2) && pci_no_d1d2(dev))
+		/*
+		 * If the device or the parent bridge do not support PCI PM,
+		 * ignore the request if we're doing anything other than putting
+		 * it into D0 (which would only happen on boot).
+		 */
+		return 0;
+
+	/* Find PCI PM capability in the list */
+	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+
+	if (state == PCI_D0 && platform_pci_power_manageable(dev)) {
+		/*
+		 * Allow the platform to change the state, for example via ACPI
+		 * _PR0, _PS0 and some such, but do not trust it.
+		 */
+		int ret = platform_pci_set_power_state(dev, PCI_D0);
+		if (!ret)
+			pci_update_current_state(dev, pm);
+	}
+
+	error = pci_raw_set_power_state(dev, pm, state);
+
+	if (state > PCI_D0 && platform_pci_power_manageable(dev)) {
+		/* Allow the platform to finalize the transition */
+		int ret = platform_pci_set_power_state(dev, state);
+		if (!ret) {
+			pci_update_current_state(dev, pm);
+			error = 0;
+		}
+	}
+
+	return error;
+}
+
+/**
  * pci_choose_state - Choose the power state of a PCI device
  * @dev: PCI device to be suspended
  * @state: target sleep state for the whole system. This is the value
Index: linux-next/drivers/pci/pci-acpi.c
===================================================================
--- linux-next.orig/drivers/pci/pci-acpi.c
+++ linux-next/drivers/pci/pci-acpi.c
@@ -277,12 +277,11 @@ static int acpi_pci_set_power_state(stru
 		[PCI_D3hot] = ACPI_STATE_D3,
 		[PCI_D3cold] = ACPI_STATE_D3
 	};
+	int error = -EINVAL;
 
-	if (!handle)
-		return -ENODEV;
 	/* If the ACPI device has _EJ0, ignore the device */
-	if (ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
-		return 0;
+	if (!handle || ACPI_SUCCESS(acpi_get_handle(handle, "_EJ0", &tmp)))
+		return -ENODEV;
 
 	switch (state) {
 	case PCI_D0:
@@ -290,9 +289,14 @@ static int acpi_pci_set_power_state(stru
 	case PCI_D2:
 	case PCI_D3hot:
 	case PCI_D3cold:
-		return acpi_bus_set_power(handle, state_conv[state]);
+		error = acpi_bus_set_power(handle, state_conv[state]);
 	}
-	return -EINVAL;
+
+	if (!error)
+		dev_printk(KERN_INFO, &dev->dev,
+				"power state changed by ACPI to D%d\n", state);
+
+	return error;
 }
 
 static struct pci_platform_pm_ops acpi_pci_platform_pm = {


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 4/8] ACPI: Introduce acpi_device_sleep_wake function
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (2 preceding siblings ...)
  2008-07-07  1:32   ` [PATCH 3/8] PCI: Rework pci_set_power_state function (rev. 4) Rafael J. Wysocki
@ 2008-07-07  1:33   ` Rafael J. Wysocki
  2008-07-07  1:34   ` [PATCH 5/8] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
                     ` (5 subsequent siblings)
  9 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:33 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

ACPI: Introduce acpi_device_sleep_wake function

The currect ACPI code attempts to execute _PSW at three different
places and in one of them only it tries to execute _DSW before _PSW,
which is inconsistent with the other two cases.

Move the execution of _DSW and _PSW into a separate function called
acpi_device_sleep_wake() and call it wherever appropriate instead of
executing _DSW and/or _PSW directly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@suse.cz>
---
 drivers/acpi/power.c        |  118 ++++++++++++++++++++++++++++++--------------
 drivers/acpi/scan.c         |   42 ++-------------
 drivers/acpi/sleep/wakeup.c |    2 
 include/acpi/acpi_drivers.h |    4 +
 4 files changed, 92 insertions(+), 74 deletions(-)

Index: linux-pci/drivers/acpi/power.c
===================================================================
--- linux-pci.orig/drivers/acpi/power.c
+++ linux-pci/drivers/acpi/power.c
@@ -292,69 +292,115 @@ static int acpi_power_off_device(acpi_ha
 	return 0;
 }
 
+/**
+ * acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
+ *                          ACPI 3.0) _PSW (Power State Wake)
+ * @dev: Device to handle.
+ * @enable: 0 - disable, 1 - enable the wake capabilities of the device.
+ * @sleep_state: Target sleep state of the system.
+ * @dev_state: Target power state of the device.
+ *
+ * Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
+ * State Wake) for the device, if present.  On failure reset the device's
+ * wakeup.flags.valid flag.
+ *
+ * RETURN VALUE:
+ * 0 if either _DSW or _PSW has been successfully executed
+ * 0 if neither _DSW nor _PSW has been found
+ * -ENODEV if the execution of either _DSW or _PSW has failed
+ */
+int acpi_device_sleep_wake(struct acpi_device *dev,
+                           int enable, int sleep_state, int dev_state)
+{
+	union acpi_object in_arg[3];
+	struct acpi_object_list arg_list = { 3, in_arg };
+	acpi_status status = AE_OK;
+
+	/*
+	 * Try to execute _DSW first.
+	 *
+	 * Three agruments are needed for the _DSW object:
+	 * Argument 0: enable/disable the wake capabilities
+	 * Argument 1: target system state
+	 * Argument 2: target device state
+	 * When _DSW object is called to disable the wake capabilities, maybe
+	 * the first argument is filled. The values of the other two agruments
+	 * are meaningless.
+	 */
+	in_arg[0].type = ACPI_TYPE_INTEGER;
+	in_arg[0].integer.value = enable;
+	in_arg[1].type = ACPI_TYPE_INTEGER;
+	in_arg[1].integer.value = sleep_state;
+	in_arg[2].type = ACPI_TYPE_INTEGER;
+	in_arg[2].integer.value = dev_state;
+	status = acpi_evaluate_object(dev->handle, "_DSW", &arg_list, NULL);
+	if (ACPI_SUCCESS(status)) {
+		return 0;
+	} else if (status != AE_NOT_FOUND) {
+		printk(KERN_ERR PREFIX "_DSW execution failed\n");
+		dev->wakeup.flags.valid = 0;
+		return -ENODEV;
+	}
+
+	/* Execute _PSW */
+	arg_list.count = 1;
+	in_arg[0].integer.value = enable;
+	status = acpi_evaluate_object(dev->handle, "_PSW", &arg_list, NULL);
+	if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
+		printk(KERN_ERR PREFIX "_PSW execution failed\n");
+		dev->wakeup.flags.valid = 0;
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 /*
  * Prepare a wakeup device, two steps (Ref ACPI 2.0:P229):
  * 1. Power on the power resources required for the wakeup device 
- * 2. Enable _PSW (power state wake) for the device if present
+ * 2. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
+ *    State Wake) for the device, if present
  */
-int acpi_enable_wakeup_device_power(struct acpi_device *dev)
+int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
 {
-	union acpi_object arg = { ACPI_TYPE_INTEGER };
-	struct acpi_object_list arg_list = { 1, &arg };
-	acpi_status status = AE_OK;
 	int i;
-	int ret = 0;
 
 	if (!dev || !dev->wakeup.flags.valid)
-		return -1;
+		return -EINVAL;
 
-	arg.integer.value = 1;
 	/* Open power resource */
 	for (i = 0; i < dev->wakeup.resources.count; i++) {
-		ret = acpi_power_on(dev->wakeup.resources.handles[i], dev);
+		int ret = acpi_power_on(dev->wakeup.resources.handles[i], dev);
 		if (ret) {
 			printk(KERN_ERR PREFIX "Transition power state\n");
 			dev->wakeup.flags.valid = 0;
-			return -1;
+			return -ENODEV;
 		}
 	}
 
-	/* Execute PSW */
-	status = acpi_evaluate_object(dev->handle, "_PSW", &arg_list, NULL);
-	if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
-		printk(KERN_ERR PREFIX "Evaluate _PSW\n");
-		dev->wakeup.flags.valid = 0;
-		ret = -1;
-	}
-
-	return ret;
+	/*
+	 * Passing 3 as the third argument below means the device may be placed
+	 * in arbitrary power state afterwards.
+	 */
+	return acpi_device_sleep_wake(dev, 1, sleep_state, 3);
 }
 
 /*
  * Shutdown a wakeup device, counterpart of above method
- * 1. Disable _PSW (power state wake)
+ * 1. Execute _DSW (Device Sleep Wake) or (deprecated in ACPI 3.0) _PSW (Power
+ *    State Wake) for the device, if present
  * 2. Shutdown down the power resources
  */
 int acpi_disable_wakeup_device_power(struct acpi_device *dev)
 {
-	union acpi_object arg = { ACPI_TYPE_INTEGER };
-	struct acpi_object_list arg_list = { 1, &arg };
-	acpi_status status = AE_OK;
-	int i;
-	int ret = 0;
-
+	int i, ret;
 
 	if (!dev || !dev->wakeup.flags.valid)
-		return -1;
+		return -EINVAL;
 
-	arg.integer.value = 0;
-	/* Execute PSW */
-	status = acpi_evaluate_object(dev->handle, "_PSW", &arg_list, NULL);
-	if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
-		printk(KERN_ERR PREFIX "Evaluate _PSW\n");
-		dev->wakeup.flags.valid = 0;
-		return -1;
-	}
+	ret = acpi_device_sleep_wake(dev, 0, 0, 0);
+	if (ret)
+		return ret;
 
 	/* Close power resource */
 	for (i = 0; i < dev->wakeup.resources.count; i++) {
@@ -362,7 +408,7 @@ int acpi_disable_wakeup_device_power(str
 		if (ret) {
 			printk(KERN_ERR PREFIX "Transition power state\n");
 			dev->wakeup.flags.valid = 0;
-			return -1;
+			return -ENODEV;
 		}
 	}
 
Index: linux-pci/drivers/acpi/sleep/wakeup.c
===================================================================
--- linux-pci.orig/drivers/acpi/sleep/wakeup.c
+++ linux-pci/drivers/acpi/sleep/wakeup.c
@@ -42,7 +42,7 @@ void acpi_enable_wakeup_device_prep(u8 s
 			continue;
 
 		spin_unlock(&acpi_device_lock);
-		acpi_enable_wakeup_device_power(dev);
+		acpi_enable_wakeup_device_power(dev, sleep_state);
 		spin_lock(&acpi_device_lock);
 	}
 	spin_unlock(&acpi_device_lock);
Index: linux-pci/include/acpi/acpi_drivers.h
===================================================================
--- linux-pci.orig/include/acpi/acpi_drivers.h
+++ linux-pci/include/acpi/acpi_drivers.h
@@ -87,7 +87,9 @@ struct pci_bus *pci_acpi_scan_root(struc
    -------------------------------------------------------------------------- */
 
 #ifdef CONFIG_ACPI_POWER
-int acpi_enable_wakeup_device_power(struct acpi_device *dev);
+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);
Index: linux-pci/drivers/acpi/scan.c
===================================================================
--- linux-pci.orig/drivers/acpi/scan.c
+++ linux-pci/drivers/acpi/scan.c
@@ -691,9 +691,7 @@ static int acpi_bus_get_wakeup_device_fl
 	acpi_status status = 0;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *package = NULL;
-	union acpi_object in_arg[3];
-	struct acpi_object_list arg_list = { 3, in_arg };
-	acpi_status psw_status = AE_OK;
+	int psw_error;
 
 	struct acpi_device_id button_device_ids[] = {
 		{"PNP0C0D", 0},
@@ -725,39 +723,11 @@ static int acpi_bus_get_wakeup_device_fl
 	 * So it is necessary to call _DSW object first. Only when it is not
 	 * present will the _PSW object used.
 	 */
-	/*
-	 * Three agruments are needed for the _DSW object.
-	 * Argument 0: enable/disable the wake capabilities
-	 * When _DSW object is called to disable the wake capabilities, maybe
-	 * the first argument is filled. The value of the other two agruments
-	 * is meaningless.
-	 */
-	in_arg[0].type = ACPI_TYPE_INTEGER;
-	in_arg[0].integer.value = 0;
-	in_arg[1].type = ACPI_TYPE_INTEGER;
-	in_arg[1].integer.value = 0;
-	in_arg[2].type = ACPI_TYPE_INTEGER;
-	in_arg[2].integer.value = 0;
-	psw_status = acpi_evaluate_object(device->handle, "_DSW",
-						&arg_list, NULL);
-	if (ACPI_FAILURE(psw_status) && (psw_status != AE_NOT_FOUND))
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "error in evaluate _DSW\n"));
-	/*
-	 * When the _DSW object is not present, OSPM will call _PSW object.
-	 */
-	if (psw_status == AE_NOT_FOUND) {
-		/*
-		 * Only one agruments is required for the _PSW object.
-		 * agrument 0: enable/disable the wake capabilities
-		 */
-		arg_list.count = 1;
-		in_arg[0].integer.value = 0;
-		psw_status = acpi_evaluate_object(device->handle, "_PSW",
-						&arg_list, NULL);
-		if (ACPI_FAILURE(psw_status) && (psw_status != AE_NOT_FOUND))
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO, "error in "
-						"evaluate _PSW\n"));
-	}
+	psw_error = acpi_device_sleep_wake(device, 0, 0, 0);
+	if (psw_error)
+		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;


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 5/8] ACPI: Introduce new device wakeup flag 'prepared'
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (3 preceding siblings ...)
  2008-07-07  1:33   ` [PATCH 4/8] ACPI: Introduce acpi_device_sleep_wake function Rafael J. Wysocki
@ 2008-07-07  1:34   ` Rafael J. Wysocki
  2008-07-07 14:08     ` Pavel Machek
  2008-07-07  1:34   ` [PATCH 6/8] PCI ACPI: Rework PCI handling of wake-up (rev. 5) Rafael J. Wysocki
                     ` (4 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:34 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

ACPI: Introduce new device wakeup flag 'prepared'

Introduce additional flag 'prepared' in
struct acpi_device_wakeup_flags and use it to prevent devices from
being enable/disabled do wake up the system multiple times in a row
(this does not happen currently, but will be possible after some of
the following patches).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/power.c    |   24 ++++++++++++++++++++++--
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

Index: linux-next/drivers/acpi/power.c
===================================================================
--- linux-next.orig/drivers/acpi/power.c
+++ linux-next/drivers/acpi/power.c
@@ -363,11 +363,18 @@ int acpi_device_sleep_wake(struct acpi_d
  */
 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
 {
-	int i;
+	int i, err;
 
 	if (!dev || !dev->wakeup.flags.valid)
 		return -EINVAL;
 
+	/*
+	 * Do not execute the code below twice in a row without calling
+	 * acpi_disable_wakeup_device_power() in between for the same device
+	 */
+	if (dev->wakeup.flags.prepared)
+		return 0;
+
 	/* Open power resource */
 	for (i = 0; i < dev->wakeup.resources.count; i++) {
 		int ret = acpi_power_on(dev->wakeup.resources.handles[i], dev);
@@ -382,7 +389,11 @@ int acpi_enable_wakeup_device_power(stru
 	 * Passing 3 as the third argument below means the device may be placed
 	 * in arbitrary power state afterwards.
 	 */
-	return acpi_device_sleep_wake(dev, 1, sleep_state, 3);
+	err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
+	if (!err)
+		dev->wakeup.flags.prepared = 1;
+
+	return err;
 }
 
 /*
@@ -398,6 +409,15 @@ int acpi_disable_wakeup_device_power(str
 	if (!dev || !dev->wakeup.flags.valid)
 		return -EINVAL;
 
+	/*
+	 * Do not execute the code below twice in a row without calling
+	 * acpi_enable_wakeup_device_power() in between for the same device
+	 */
+	if (!dev->wakeup.flags.prepared)
+		return 0;
+
+	dev->wakeup.flags.prepared = 0;
+
 	ret = acpi_device_sleep_wake(dev, 0, 0, 0);
 	if (ret)
 		return ret;
Index: linux-next/include/acpi/acpi_bus.h
===================================================================
--- linux-next.orig/include/acpi/acpi_bus.h
+++ linux-next/include/acpi/acpi_bus.h
@@ -259,6 +259,7 @@ struct acpi_device_perf {
 /* Wakeup Management */
 struct acpi_device_wakeup_flags {
 	u8 valid:1;		/* Can successfully enable wakeup? */
+	u8 prepared:1;		/* Has the wake-up capability been enabled? */
 	u8 run_wake:1;		/* Run-Wake GPE devices */
 };
 


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 6/8] PCI ACPI: Rework PCI handling of wake-up (rev. 5)
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (4 preceding siblings ...)
  2008-07-07  1:34   ` [PATCH 5/8] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
@ 2008-07-07  1:34   ` Rafael J. Wysocki
  2008-07-11 20:30     ` Pavel Machek
  2008-07-07  1:35   ` [PATCH 7/8] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
                     ` (3 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:34 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI ACPI: Rework PCI handling of wake-up (rev. 5)

* Introduce function acpi_pm_device_sleep_wake() for enabling and
  disabling the system wake-up capability of devices that are power
  manageable by ACPI.

* Introduce function acpi_bus_can_wakeup() allowing other (dependent)
  subsystems to check if ACPI is able to enable the system wake-up
  capability of given device.

* Introduce callback .sleep_wake() in struct pci_platform_pm_ops and
  for the ACPI PCI 'driver' make it use acpi_pm_device_sleep_wake().

* Introduce callback .can_wakeup() in struct pci_platform_pm_ops and
  for the ACPI 'driver' make it use acpi_bus_can_wakeup().

* Move the PME# handlig code out of pci_enable_wake() and split it
  into two functions, pci_pme_capable() and pci_pme_active(),
  allowing the caller to check if given device is capable of
  generating PME# from given power state and to enable/disable the
  device's PME# functionality, respectively.

* Modify pci_enable_wake() to use the new ACPI callbacks and the new
  PME#-related functions.

* Drop the generic .platform_enable_wakeup() callback that is not
  used any more.

* Introduce device_set_wakeup_capable() that will set the
  power.can_wakeup flag of given device.

* Rework PCI device PM initialization so that, if given device is
  capable of generating wake-up events, either natively through the
  PME# mechanism, or with the help of the platform, its
  power.can_wakeup flag is set and its power.should_wakeup flag is
  unset as appropriate.

* Make ACPI set the power.can_wakeup flag for devices found to be
  wake-up capable by it.

* Make the ACPI wake-up code enable/disable GPEs for devices that
  have the wakeup.flags.prepared flag set (which means that their
  wake-up power has been enabled).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/bus.c          |   11 ++
 drivers/acpi/glue.c         |    2 
 drivers/acpi/sleep/main.c   |   25 ++++++
 drivers/acpi/sleep/wakeup.c |   11 +-
 drivers/base/power/sysfs.c  |    3 
 drivers/pci/pci-acpi.c      |   20 +++++
 drivers/pci/pci.c           |  169 ++++++++++++++++++++++++++++++++------------
 drivers/pci/pci.h           |    8 ++
 drivers/pci/probe.c         |   47 ------------
 include/acpi/acpi_bus.h     |    2 
 include/linux/pm_wakeup.h   |   26 +-----
 11 files changed, 208 insertions(+), 116 deletions(-)

Index: linux-pci/drivers/acpi/sleep/main.c
===================================================================
--- linux-pci.orig/drivers/acpi/sleep/main.c
+++ linux-pci/drivers/acpi/sleep/main.c
@@ -468,6 +468,31 @@ int acpi_pm_device_sleep_state(struct de
 		*d_min_p = d_min;
 	return d_max;
 }
+
+/**
+ *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
+ *                                  capability of given device
+ *	@dev: device to handle
+ *	@enable: 'true' - enable, 'false' - disable the wake-up capability
+ */
+int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
+{
+	acpi_handle handle;
+	struct acpi_device *adev;
+
+	if (!device_may_wakeup(dev))
+		return -EINVAL;
+
+	handle = DEVICE_ACPI_HANDLE(dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
+		printk(KERN_DEBUG "ACPI handle has no context!\n");
+		return -ENODEV;
+	}
+
+	return enable ?
+		acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
+		acpi_disable_wakeup_device_power(adev);
+}
 #endif
 
 static void acpi_power_off_prepare(void)
Index: linux-pci/drivers/pci/pci-acpi.c
===================================================================
--- linux-pci.orig/drivers/pci/pci-acpi.c
+++ linux-pci/drivers/pci/pci-acpi.c
@@ -299,10 +299,30 @@ static int acpi_pci_set_power_state(stru
 	return error;
 }
 
+static bool acpi_pci_can_wakeup(struct pci_dev *dev)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	return handle ? acpi_bus_can_wakeup(handle) : false;
+}
+
+static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
+{
+	int error = acpi_pm_device_sleep_wake(&dev->dev, enable);
+
+	if (!error)
+		dev_printk(KERN_INFO, &dev->dev,
+				"wake-up capability %s by ACPI\n",
+				enable ? "enabled" : "disabled");
+	return error;
+}
+
 static struct pci_platform_pm_ops acpi_pci_platform_pm = {
 	.is_manageable = acpi_pci_power_manageable,
 	.set_state = acpi_pci_set_power_state,
 	.choose_state = acpi_pci_choose_state,
+	.can_wakeup = acpi_pci_can_wakeup,
+	.sleep_wake = acpi_pci_sleep_wake,
 };
 
 /* ACPI bus type */
Index: linux-pci/drivers/pci/pci.h
===================================================================
--- linux-pci.orig/drivers/pci/pci.h
+++ linux-pci/drivers/pci/pci.h
@@ -17,6 +17,11 @@ extern void pci_cleanup_rom(struct pci_d
  *                 platform; to be used during system-wide transitions from a
  *                 sleeping state to the working state and vice versa
  *
+ * @can_wakeup - returns 'true' if given device is capable of waking up the
+ *               system from a sleeping state
+ *
+ * @sleep_wake - enables/disables the system wake up capability of given device
+ *
  * If given platform is generally capable of power managing PCI devices, all of
  * these callbacks are mandatory.
  */
@@ -24,9 +29,12 @@ struct pci_platform_pm_ops {
 	bool (*is_manageable)(struct pci_dev *dev);
 	int (*set_state)(struct pci_dev *dev, pci_power_t state);
 	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);
 };
 
 extern int pci_set_platform_pm(struct pci_platform_pm_ops *ops);
+extern void pci_pm_init(struct pci_dev *dev);
 
 extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
 extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
Index: linux-pci/include/acpi/acpi_bus.h
===================================================================
--- linux-pci.orig/include/acpi/acpi_bus.h
+++ linux-pci/include/acpi/acpi_bus.h
@@ -337,6 +337,7 @@ int acpi_bus_get_status(struct acpi_devi
 int acpi_bus_get_power(acpi_handle handle, int *state);
 int acpi_bus_set_power(acpi_handle handle, int state);
 bool acpi_bus_power_manageable(acpi_handle handle);
+bool acpi_bus_can_wakeup(acpi_handle handle);
 #ifdef CONFIG_ACPI_PROC_EVENT
 int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
 int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
@@ -379,6 +380,7 @@ acpi_handle acpi_get_pci_rootbridge_hand
 
 #ifdef CONFIG_PM_SLEEP
 int acpi_pm_device_sleep_state(struct device *, int *);
+int acpi_pm_device_sleep_wake(struct device *, bool);
 #else /* !CONFIG_PM_SLEEP */
 static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
 {
Index: linux-pci/drivers/pci/pci.c
===================================================================
--- linux-pci.orig/drivers/pci/pci.c
+++ linux-pci/drivers/pci/pci.c
@@ -380,7 +380,8 @@ static struct pci_platform_pm_ops *pci_p
 
 int pci_set_platform_pm(struct pci_platform_pm_ops *ops)
 {
-	if (!ops->is_manageable || !ops->set_state || !ops->choose_state)
+	if (!ops->is_manageable || !ops->set_state || !ops->choose_state
+	    || !ops->sleep_wake || !ops->can_wakeup)
 		return -EINVAL;
 	pci_platform_pm = ops;
 	return 0;
@@ -403,6 +404,17 @@ static inline pci_power_t platform_pci_c
 			pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
 }
 
+static inline bool platform_pci_can_wakeup(struct pci_dev *dev)
+{
+	return pci_platform_pm ? pci_platform_pm->can_wakeup(dev) : false;
+}
+
+static inline int platform_pci_sleep_wake(struct pci_dev *dev, bool enable)
+{
+	return pci_platform_pm ?
+			pci_platform_pm->sleep_wake(dev, enable) : -ENODEV;
+}
+
 /**
  * pci_raw_set_power_state - Use PCI PM registers to set the power state of
  *                           given PCI device
@@ -1036,6 +1048,56 @@ int pci_set_pcie_reset_state(struct pci_
 }
 
 /**
+ * pci_pme_capable - check the capability of PCI device to generate PME#
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ * @state: PCI state from which device will issue PME#.
+ */
+static bool pci_pme_capable(struct pci_dev *dev, int pm, pci_power_t state)
+{
+	u16 pmc;
+
+	if (!pm)
+		return false;
+
+	/* Check device's ability to generate PME# from given state */
+	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
+
+	pmc &= PCI_PM_CAP_PME_MASK;
+	pmc >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
+
+	return !!(pmc & (1 << state));
+}
+
+/**
+ * pci_pme_active - enable or disable PCI device's PME# function
+ * @dev: PCI device to handle.
+ * @pm: PCI PM capability offset of the device.
+ * @enable: 'true' to enable PME# generation; 'false' to disable it.
+ *
+ * The caller must verify that the device is capable of generating PME# before
+ * calling this function with @enable equal to 'true'.
+ */
+static void pci_pme_active(struct pci_dev *dev, int pm, bool enable)
+{
+	u16 pmcsr;
+
+	if (!pm)
+		return;
+
+	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+	/* Clear PME_Status by writing 1 to it and enable PME# */
+	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
+	if (!enable)
+		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
+
+	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
+
+	dev_printk(KERN_INFO, &dev->dev, "PME# %s\n",
+			enable ? "enabled" : "disabled");
+}
+
+/**
  * pci_enable_wake - enable PCI device as wakeup event source
  * @dev: PCI device affected
  * @state: PCI state from which device will issue wakeup events
@@ -1046,66 +1108,83 @@ int pci_set_pcie_reset_state(struct pci_
  * called automatically by this routine.
  *
  * Devices with legacy power management (no standard PCI PM capabilities)
- * always require such platform hooks.  Depending on the platform, devices
- * supporting the standard PCI PME# signal may require such platform hooks;
- * they always update bits in config space to allow PME# generation.
- *
- * -EIO is returned if the device can't ever be a wakeup event source.
- * -EINVAL is returned if the device can't generate wakeup events from
- * the specified PCI state.  Returns zero if the operation is successful.
+ * always require such platform hooks.
+ *
+ * RETURN VALUE:
+ * 0 is returned on success
+ * -EINVAL is returned if device is not supposed to wake up the system
+ * Error code depending on the platform is returned if both the platform and
+ * the native mechanism fail to enable the generation of wake-up events
  */
 int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable)
 {
 	int pm;
-	int status;
-	u16 value;
+	int error = 0;
+	bool pme_done = false;
+
+	if (!device_may_wakeup(&dev->dev))
+		return -EINVAL;
 
-	/* Note that drivers should verify device_may_wakeup(&dev->dev)
-	 * before calling this function.  Platform code should report
-	 * errors when drivers try to enable wakeup on devices that
-	 * can't issue wakeups, or on which wakeups were disabled by
-	 * userspace updating the /sys/devices.../power/wakeup file.
+	/*
+	 * According to "PCI System Architecture" 4th ed. by Tom Shanley & Don
+	 * Anderson we should be doing PME# wake enable followed by ACPI wake
+	 * enable.  To disable wake-up we call the platform first, for symmetry.
 	 */
 
-	status = call_platform_enable_wakeup(&dev->dev, enable);
+	if (!enable && platform_pci_can_wakeup(dev))
+		error = platform_pci_sleep_wake(dev, false);
 
-	/* find PCI PM capability in list */
 	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	if (!enable || pci_pme_capable(dev, pm, state)) {
+		pci_pme_active(dev, pm, enable);
+		pme_done = true;
+	}
 
-	/* If device doesn't support PM Capabilities, but caller wants to
-	 * disable wake events, it's a NOP.  Otherwise fail unless the
-	 * platform hooks handled this legacy device already.
-	 */
-	if (!pm)
-		return enable ? status : 0;
+	if (enable && platform_pci_can_wakeup(dev))
+		error = platform_pci_sleep_wake(dev, true);
 
+	return pme_done ? 0 : error;
+}
+
+/**
+ * pci_pm_init - Initialize PM functions of given PCI device
+ * @dev: PCI device to handle.
+ */
+void pci_pm_init(struct pci_dev *dev)
+{
+	int pm;
+	u16 pmc;
+
+	/* find PCI PM capability in list */
+	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	if (!pm)
+		return;
 	/* Check device's ability to generate PME# */
-	pci_read_config_word(dev,pm+PCI_PM_PMC,&value);
+	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
 
-	value &= PCI_PM_CAP_PME_MASK;
-	value >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
+	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
+		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
+			pmc & PCI_PM_CAP_VER_MASK);
+		return;
+	}
 
-	/* Check if it can generate PME# from requested state. */
-	if (!value || !(value & (1 << state))) {
-		/* if it can't, revert what the platform hook changed,
-		 * always reporting the base "EINVAL, can't PME#" error
+	if (pmc & PCI_PM_CAP_PME_MASK) {
+		dev_printk(KERN_INFO, &dev->dev,
+			"PME# supported from%s%s%s%s%s\n",
+			(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
+			(pmc & PCI_PM_CAP_PME_D1) ? " D1" : "",
+			(pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
+			(pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
+			(pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
+		/*
+		 * Make device's PM flags reflect the wake-up capability, but
+		 * let the user space enable it to wake up the system as needed.
 		 */
-		if (enable)
-			call_platform_enable_wakeup(&dev->dev, 0);
-		return enable ? -EINVAL : 0;
+		device_set_wakeup_capable(&dev->dev, true);
+		device_set_wakeup_enable(&dev->dev, false);
+		/* Disable the PME# generation functionality */
+		pci_pme_active(dev, pm, false);
 	}
-
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
-
-	/* Clear PME_Status by writing 1 to it and enable PME# */
-	value |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
-
-	if (!enable)
-		value &= ~PCI_PM_CTRL_PME_ENABLE;
-
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
-
-	return 0;
 }
 
 int
Index: linux-pci/include/linux/pm_wakeup.h
===================================================================
--- linux-pci.orig/include/linux/pm_wakeup.h
+++ linux-pci/include/linux/pm_wakeup.h
@@ -35,6 +35,11 @@ static inline void device_init_wakeup(st
 	dev->power.can_wakeup = dev->power.should_wakeup = !!val;
 }
 
+static inline void device_set_wakeup_capable(struct device *dev, int val)
+{
+	dev->power.can_wakeup = !!val;
+}
+
 static inline int device_can_wakeup(struct device *dev)
 {
 	return dev->power.can_wakeup;
@@ -47,21 +52,7 @@ static inline void device_set_wakeup_ena
 
 static inline int device_may_wakeup(struct device *dev)
 {
-	return dev->power.can_wakeup & dev->power.should_wakeup;
-}
-
-/*
- * Platform hook to activate device wakeup capability, if that's not already
- * handled by enable_irq_wake() etc.
- * Returns zero on success, else negative errno
- */
-extern int (*platform_enable_wakeup)(struct device *dev, int is_on);
-
-static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
-{
-	if (platform_enable_wakeup)
-		return (*platform_enable_wakeup)(dev, is_on);
-	return 0;
+	return dev->power.can_wakeup && dev->power.should_wakeup;
 }
 
 #else /* !CONFIG_PM */
@@ -80,11 +71,6 @@ static inline int device_can_wakeup(stru
 #define device_set_wakeup_enable(dev, val)	do {} while (0)
 #define device_may_wakeup(dev)			0
 
-static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
-{
-	return 0;
-}
-
 #endif /* !CONFIG_PM */
 
 #endif /* _LINUX_PM_WAKEUP_H */
Index: linux-pci/drivers/base/power/sysfs.c
===================================================================
--- linux-pci.orig/drivers/base/power/sysfs.c
+++ linux-pci/drivers/base/power/sysfs.c
@@ -6,9 +6,6 @@
 #include <linux/string.h>
 #include "power.h"
 
-int (*platform_enable_wakeup)(struct device *dev, int is_on);
-
-
 /*
  *	wakeup - Report/change current wakeup option for device
  *
Index: linux-pci/drivers/acpi/bus.c
===================================================================
--- linux-pci.orig/drivers/acpi/bus.c
+++ linux-pci/drivers/acpi/bus.c
@@ -306,6 +306,17 @@ bool acpi_bus_power_manageable(acpi_hand
 
 EXPORT_SYMBOL(acpi_bus_power_manageable);
 
+bool acpi_bus_can_wakeup(acpi_handle handle)
+{
+	struct acpi_device *device;
+	int result;
+
+	result = acpi_bus_get_device(handle, &device);
+	return result ? false : device->wakeup.flags.valid;
+}
+
+EXPORT_SYMBOL(acpi_bus_can_wakeup);
+
 /* --------------------------------------------------------------------------
                                 Event Management
    -------------------------------------------------------------------------- */
Index: linux-pci/drivers/pci/probe.c
===================================================================
--- linux-pci.orig/drivers/pci/probe.c
+++ linux-pci/drivers/pci/probe.c
@@ -860,49 +860,6 @@ int pci_cfg_space_size_ext(struct pci_de
 	return PCI_CFG_SPACE_SIZE;
 }
 
-/**
- * pci_disable_pme - Disable the PME function of PCI device
- * @dev: PCI device affected
- * -EINVAL is returned if PCI device doesn't support PME.
- * Zero is returned if the PME is supported and can be disabled.
- */
-static int pci_disable_pme(struct pci_dev *dev)
-{
-	int pm;
-	u16 value;
-
-	/* find PCI PM capability in list */
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-
-	/* If device doesn't support PM Capabilities, it means that PME is
-	 * not supported.
-	 */
-	if (!pm)
-		return -EINVAL;
-	/* Check device's ability to generate PME# */
-	pci_read_config_word(dev, pm + PCI_PM_PMC, &value);
-
-	value &= PCI_PM_CAP_PME_MASK;
-	/* Check if it can generate PME# */
-	if (!value) {
-		/*
-		 * If it is zero, it means that PME is still unsupported
-		 * although there exists the PM capability.
-		 */
-		return -EINVAL;
-	}
-
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &value);
-
-	/* Clear PME_Status by writing 1 to it */
-	value |= PCI_PM_CTRL_PME_STATUS ;
-	/* Disable PME enable bit */
-	value &= ~PCI_PM_CTRL_PME_ENABLE;
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, value);
-
-	return 0;
-}
-
 int pci_cfg_space_size(struct pci_dev *dev)
 {
 	int pos;
@@ -1010,7 +967,6 @@ static struct pci_dev *pci_scan_device(s
 	}
 
 	pci_vpd_pci22_init(dev);
-	pci_disable_pme(dev);
 
 	return dev;
 }
@@ -1031,6 +987,9 @@ void pci_device_add(struct pci_dev *dev,
 	/* Fix up broken headers */
 	pci_fixup_device(pci_fixup_header, dev);
 
+	/* Initialize power management of the device */
+	pci_pm_init(dev);
+
 	/*
 	 * Add the device to our list of discovered devices
 	 * and the bus list for fixup functions, etc.
Index: linux-pci/drivers/acpi/glue.c
===================================================================
--- linux-pci.orig/drivers/acpi/glue.c
+++ linux-pci/drivers/acpi/glue.c
@@ -166,6 +166,8 @@ static int acpi_bind_one(struct device *
 				"firmware_node");
 		ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
 				"physical_node");
+		if (acpi_dev->wakeup.flags.valid)
+			device_set_wakeup_capable(dev, true);
 	}
 
 	return 0;
Index: linux-pci/drivers/acpi/sleep/wakeup.c
===================================================================
--- linux-pci.orig/drivers/acpi/sleep/wakeup.c
+++ linux-pci/drivers/acpi/sleep/wakeup.c
@@ -66,13 +66,15 @@ void acpi_enable_wakeup_device(u8 sleep_
 	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
 		struct acpi_device *dev =
 			container_of(node, struct acpi_device, wakeup_list);
+
 		if (!dev->wakeup.flags.valid)
 			continue;
+
 		/* If users want to disable run-wake GPE,
 		 * we only disable it for wake and leave it for runtime
 		 */
-		if (!dev->wakeup.state.enabled ||
-		    sleep_state > (u32) dev->wakeup.sleep_state) {
+		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
+		    || sleep_state > (u32) dev->wakeup.sleep_state) {
 			if (dev->wakeup.flags.run_wake) {
 				spin_unlock(&acpi_device_lock);
 				/* set_gpe_type will disable GPE, leave it like that */
@@ -110,8 +112,9 @@ void acpi_disable_wakeup_device(u8 sleep
 
 		if (!dev->wakeup.flags.valid)
 			continue;
-		if (!dev->wakeup.state.enabled ||
-		    sleep_state > (u32) dev->wakeup.sleep_state) {
+
+		if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
+		    || sleep_state > (u32) dev->wakeup.sleep_state) {
 			if (dev->wakeup.flags.run_wake) {
 				spin_unlock(&acpi_device_lock);
 				acpi_set_gpe_type(dev->wakeup.gpe_device,


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 7/8] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (5 preceding siblings ...)
  2008-07-07  1:34   ` [PATCH 6/8] PCI ACPI: Rework PCI handling of wake-up (rev. 5) Rafael J. Wysocki
@ 2008-07-07  1:35   ` Rafael J. Wysocki
  2008-07-11 20:31     ` Pavel Machek
  2008-07-07  1:36   ` [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
                     ` (2 subsequent siblings)
  9 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:35 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep

Introduce functions pci_prepare_to_sleep() and pci_back_from_sleep(),
to be used by the PCI drivers that want to place their devices into
the lowest power state appropiate for them (PCI_D3hot, if the device
is not supposed to wake up the system, or the deepest state from
which the wake-up is possible, otherwise) while the system is being
prepared to go into a sleeping state and to put them back into D0
during the subsequent transition to the working state.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c   |   83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    2 +
 2 files changed, 85 insertions(+)

Index: linux-pci/drivers/pci/pci.c
===================================================================
--- linux-pci.orig/drivers/pci/pci.c
+++ linux-pci/drivers/pci/pci.c
@@ -1147,6 +1147,87 @@ int pci_enable_wake(struct pci_dev *dev,
 }
 
 /**
+ * pci_prepare_to_sleep - prepare PCI device for system-wide transition into
+ *                        a sleep state
+ * @dev: Device to handle.
+ *
+ * Choose the power state appropriate for the device depending on whether
+ * it can wake up the system and/or is power manageable by the platform
+ * (PCI_D3hot is the default) and put the device into that state.
+ */
+int pci_prepare_to_sleep(struct pci_dev *dev)
+{
+	pci_power_t target_state = PCI_D3hot;
+	int pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	int error;
+
+	if (platform_pci_power_manageable(dev)) {
+		/*
+		 * Call the platform to choose the target state of the device
+		 * and enable wake-up from this state if supported.
+		 */
+		pci_power_t state = platform_pci_choose_state(dev);
+
+		switch (state) {
+		case PCI_POWER_ERROR:
+		case PCI_UNKNOWN:
+			break;
+		case PCI_D1:
+		case PCI_D2:
+			if (pci_no_d1d2(dev))
+				break;
+		default:
+			target_state = state;
+			pci_enable_wake(dev, target_state, true);
+		}
+	} else if (device_may_wakeup(&dev->dev)) {
+		/*
+		 * Find the deepest state from which the device can generate
+		 * wake-up events, make it the target state and enable device
+		 * to generate PME#.
+		 */
+		u16 pmc;
+
+		if (!pm)
+			return -EIO;
+
+		pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
+		if (pmc & PCI_PM_CAP_PME_MASK) {
+			if (!(pmc & PCI_PM_CAP_PME_D3)) {
+				/* Device cannot generate PME# from D3_hot */
+				if (pmc & PCI_PM_CAP_PME_D2)
+					target_state = PCI_D2;
+				else if (pmc & PCI_PM_CAP_PME_D1)
+					target_state = PCI_D1;
+				else
+					target_state = PCI_D0;
+			}
+			pci_pme_active(dev, pm, true);
+		}
+	}
+
+	error = pci_set_power_state(dev, target_state);
+
+	if (error)
+		pci_enable_wake(dev, target_state, false);
+
+	return error;
+}
+
+/**
+ * pci_back_from_sleep - turn PCI device on during system-wide transition into
+ *                       the working state a sleep state
+ * @dev: Device to handle.
+ *
+ * Disable device's sytem wake-up capability and put it into D0.
+ */
+int pci_back_from_sleep(struct pci_dev *dev)
+{
+	pci_enable_wake(dev, PCI_D0, false);
+	return pci_set_power_state(dev, PCI_D0);
+}
+
+/**
  * pci_pm_init - Initialize PM functions of given PCI device
  * @dev: PCI device to handle.
  */
@@ -1853,5 +1934,7 @@ EXPORT_SYMBOL(pci_set_power_state);
 EXPORT_SYMBOL(pci_save_state);
 EXPORT_SYMBOL(pci_restore_state);
 EXPORT_SYMBOL(pci_enable_wake);
+EXPORT_SYMBOL(pci_prepare_to_sleep);
+EXPORT_SYMBOL(pci_back_from_sleep);
 EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
 
Index: linux-pci/include/linux/pci.h
===================================================================
--- linux-pci.orig/include/linux/pci.h
+++ linux-pci/include/linux/pci.h
@@ -632,6 +632,8 @@ int pci_restore_state(struct pci_dev *de
 int pci_set_power_state(struct pci_dev *dev, pci_power_t state);
 pci_power_t pci_choose_state(struct pci_dev *dev, pm_message_t state);
 int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable);
+int pci_prepare_to_sleep(struct pci_dev *dev);
+int pci_back_from_sleep(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);


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4)
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (6 preceding siblings ...)
  2008-07-07  1:35   ` [PATCH 7/8] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
@ 2008-07-07  1:36   ` Rafael J. Wysocki
  2008-07-11 20:37     ` Pavel Machek
  2008-07-08  0:49   ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality " Jesse Barnes
  2008-07-13 20:45   ` PCI PM: Fix pci_prepare_to_sleep Rafael J. Wysocki
  9 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-07  1:36 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

From: Rafael J. Wysocki <rjw@sisk.pl>

PCI: Simplify PCI device PM code (rev. 4)

If the offset of PCI device's PM capability in its configuration
space, the mask of states that the device supports PME# from and the
D1 and D2 support bits are cached in the corresponding
struct pci_dev, the PCI device PM code can be simplified quite a bit.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c        |  118 ++++++++++++++++++++---------------------------
 include/linux/pci.h      |    8 ++-
 include/linux/pci_regs.h |    1 
 3 files changed, 60 insertions(+), 67 deletions(-)

Index: linux-pci/include/linux/pci.h
===================================================================
--- linux-pci.orig/include/linux/pci.h
+++ linux-pci/include/linux/pci.h
@@ -177,6 +177,13 @@ struct pci_dev {
 	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
 					   this is D0-D3, D0 being fully functional,
 					   and D3 being off. */
+	int		pm_cap;		/* PM capability offset in the
+					   configuration space */
+	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
+					   can be generated */
+	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 */
 
 #ifdef CONFIG_PCIEASPM
 	struct pcie_link_state	*link_state;	/* ASPM link state. */
@@ -201,7 +208,6 @@ struct pci_dev {
 	unsigned int	is_added:1;
 	unsigned int	is_busmaster:1; /* device is busmaster */
 	unsigned int	no_msi:1;	/* device may not use msi */
-	unsigned int	no_d1d2:1;   /* only allow d0 or d3 */
 	unsigned int	block_ucfg_access:1;	/* userspace config space access is blocked */
 	unsigned int	broken_parity_status:1;	/* Device generates false positive parity */
 	unsigned int 	msi_enabled:1;
Index: linux-pci/drivers/pci/pci.c
===================================================================
--- linux-pci.orig/drivers/pci/pci.c
+++ linux-pci/drivers/pci/pci.c
@@ -419,7 +419,6 @@ static inline int platform_pci_sleep_wak
  * pci_raw_set_power_state - Use PCI PM registers to set the power state of
  *                           given PCI device
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  * @state: PCI power state (D0, D1, D2, D3hot) to put the device into.
  *
  * RETURN VALUE:
@@ -430,12 +429,12 @@ static inline int platform_pci_sleep_wak
  * 0 if device's power state has been successfully changed.
  */
 static int
-pci_raw_set_power_state(struct pci_dev *dev, int pm, pci_power_t state)
+pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
-	u16 pmcsr, pmc;
+	u16 pmcsr;
 	bool need_restore = false;
 
-	if (!pm)
+	if (!dev->pm_cap)
 		return -EIO;
 
 	if (state < PCI_D0 || state > PCI_D3hot)
@@ -455,20 +454,12 @@ pci_raw_set_power_state(struct pci_dev *
 		return -EINVAL;
 	}
 
-	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
-
-	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
-		dev_err(&dev->dev, "unsupported PM cap regs version (%u)\n",
-			pmc & PCI_PM_CAP_VER_MASK);
-		return -EIO;
-	}
-
 	/* check if this device supports the desired state */
-	if ((state == PCI_D1 && !(pmc & PCI_PM_CAP_D1))
-	   || (state == PCI_D2 && !(pmc & PCI_PM_CAP_D2)))
+	if ((state == PCI_D1 && !dev->d1_support)
+	   || (state == PCI_D2 && !dev->d2_support))
 		return -EIO;
 
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 
 	/* If we're (effectively) in D3, force entire word to 0.
 	 * This doesn't affect PME_Status, disables PME_En, and
@@ -492,7 +483,7 @@ pci_raw_set_power_state(struct pci_dev *
 	}
 
 	/* enter specified state */
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
+	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 
 	/* Mandatory power management transition delays */
 	/* see PCI PM 1.1 5.6.1 table 18 */
@@ -528,14 +519,13 @@ pci_raw_set_power_state(struct pci_dev *
  * pci_update_current_state - Read PCI power state of given device from its
  *                            PCI PM registers and cache it
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  */
-static void pci_update_current_state(struct pci_dev *dev, int pm)
+static void pci_update_current_state(struct pci_dev *dev)
 {
-	if (pm) {
+	if (dev->pm_cap) {
 		u16 pmcsr;
 
-		pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+		pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 		dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK);
 	}
 }
@@ -557,7 +547,7 @@ static void pci_update_current_state(str
  */
 int pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
-	int pm, error;
+	int error;
 
 	/* bound the state we're entering */
 	if (state > PCI_D3hot)
@@ -572,9 +562,6 @@ int pci_set_power_state(struct pci_dev *
 		 */
 		return 0;
 
-	/* Find PCI PM capability in the list */
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-
 	if (state == PCI_D0 && platform_pci_power_manageable(dev)) {
 		/*
 		 * Allow the platform to change the state, for example via ACPI
@@ -582,16 +569,16 @@ int pci_set_power_state(struct pci_dev *
 		 */
 		int ret = platform_pci_set_power_state(dev, PCI_D0);
 		if (!ret)
-			pci_update_current_state(dev, pm);
+			pci_update_current_state(dev);
 	}
 
-	error = pci_raw_set_power_state(dev, pm, state);
+	error = pci_raw_set_power_state(dev, state);
 
 	if (state > PCI_D0 && platform_pci_power_manageable(dev)) {
 		/* Allow the platform to finalize the transition */
 		int ret = platform_pci_set_power_state(dev, state);
 		if (!ret) {
-			pci_update_current_state(dev, pm);
+			pci_update_current_state(dev);
 			error = 0;
 		}
 	}
@@ -1050,48 +1037,38 @@ int pci_set_pcie_reset_state(struct pci_
 /**
  * pci_pme_capable - check the capability of PCI device to generate PME#
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  * @state: PCI state from which device will issue PME#.
  */
-static bool pci_pme_capable(struct pci_dev *dev, int pm, pci_power_t state)
+static bool pci_pme_capable(struct pci_dev *dev, pci_power_t state)
 {
-	u16 pmc;
-
-	if (!pm)
+	if (!dev->pm_cap)
 		return false;
 
-	/* Check device's ability to generate PME# from given state */
-	pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
-
-	pmc &= PCI_PM_CAP_PME_MASK;
-	pmc >>= ffs(PCI_PM_CAP_PME_MASK) - 1;   /* First bit of mask */
-
-	return !!(pmc & (1 << state));
+	return !!(dev->pme_support & (1 << state));
 }
 
 /**
  * pci_pme_active - enable or disable PCI device's PME# function
  * @dev: PCI device to handle.
- * @pm: PCI PM capability offset of the device.
  * @enable: 'true' to enable PME# generation; 'false' to disable it.
  *
  * The caller must verify that the device is capable of generating PME# before
  * calling this function with @enable equal to 'true'.
  */
-static void pci_pme_active(struct pci_dev *dev, int pm, bool enable)
+static void pci_pme_active(struct pci_dev *dev, bool enable)
 {
 	u16 pmcsr;
 
-	if (!pm)
+	if (!dev->pm_cap)
 		return;
 
-	pci_read_config_word(dev, pm + PCI_PM_CTRL, &pmcsr);
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	/* Clear PME_Status by writing 1 to it and enable PME# */
 	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
 	if (!enable)
 		pmcsr &= ~PCI_PM_CTRL_PME_ENABLE;
 
-	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
+	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
 
 	dev_printk(KERN_INFO, &dev->dev, "PME# %s\n",
 			enable ? "enabled" : "disabled");
@@ -1118,7 +1095,6 @@ static void pci_pme_active(struct pci_de
  */
 int pci_enable_wake(struct pci_dev *dev, pci_power_t state, int enable)
 {
-	int pm;
 	int error = 0;
 	bool pme_done = false;
 
@@ -1134,9 +1110,8 @@ int pci_enable_wake(struct pci_dev *dev,
 	if (!enable && platform_pci_can_wakeup(dev))
 		error = platform_pci_sleep_wake(dev, false);
 
-	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
-	if (!enable || pci_pme_capable(dev, pm, state)) {
-		pci_pme_active(dev, pm, enable);
+	if (!enable || pci_pme_capable(dev, state)) {
+		pci_pme_active(dev, enable);
 		pme_done = true;
 	}
 
@@ -1158,7 +1133,6 @@ int pci_enable_wake(struct pci_dev *dev,
 int pci_prepare_to_sleep(struct pci_dev *dev)
 {
 	pci_power_t target_state = PCI_D3hot;
-	int pm = pci_find_capability(dev, PCI_CAP_ID_PM);
 	int error;
 
 	if (platform_pci_power_manageable(dev)) {
@@ -1186,23 +1160,14 @@ int pci_prepare_to_sleep(struct pci_dev 
 		 * wake-up events, make it the target state and enable device
 		 * to generate PME#.
 		 */
-		u16 pmc;
-
-		if (!pm)
+		if (!dev->pm_cap)
 			return -EIO;
 
-		pci_read_config_word(dev, pm + PCI_PM_PMC, &pmc);
-		if (pmc & PCI_PM_CAP_PME_MASK) {
-			if (!(pmc & PCI_PM_CAP_PME_D3)) {
-				/* Device cannot generate PME# from D3_hot */
-				if (pmc & PCI_PM_CAP_PME_D2)
-					target_state = PCI_D2;
-				else if (pmc & PCI_PM_CAP_PME_D1)
-					target_state = PCI_D1;
-				else
-					target_state = PCI_D0;
-			}
-			pci_pme_active(dev, pm, true);
+		if (dev->pme_support) {
+			while (target_state
+			      && !(dev->pme_support & (1 << target_state)))
+				target_state--;
+			pci_pme_active(dev, true);
 		}
 	}
 
@@ -1236,6 +1201,8 @@ void pci_pm_init(struct pci_dev *dev)
 	int pm;
 	u16 pmc;
 
+	dev->pm_cap = 0;
+
 	/* find PCI PM capability in list */
 	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
 	if (!pm)
@@ -1249,7 +1216,23 @@ void pci_pm_init(struct pci_dev *dev)
 		return;
 	}
 
-	if (pmc & PCI_PM_CAP_PME_MASK) {
+	dev->pm_cap = pm;
+
+	dev->d1_support = false;
+	dev->d2_support = false;
+	if (!pci_no_d1d2(dev)) {
+		if (pmc & PCI_PM_CAP_D1) {
+			dev_printk(KERN_DEBUG, &dev->dev, "supports D1\n");
+			dev->d1_support = true;
+		}
+		if (pmc & PCI_PM_CAP_D2) {
+			dev_printk(KERN_DEBUG, &dev->dev, "supports D2\n");
+			dev->d2_support = true;
+		}
+	}
+
+	pmc &= PCI_PM_CAP_PME_MASK;
+	if (pmc) {
 		dev_printk(KERN_INFO, &dev->dev,
 			"PME# supported from%s%s%s%s%s\n",
 			(pmc & PCI_PM_CAP_PME_D0) ? " D0" : "",
@@ -1257,6 +1240,7 @@ void pci_pm_init(struct pci_dev *dev)
 			(pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
 			(pmc & PCI_PM_CAP_PME_D3) ? " D3hot" : "",
 			(pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
+		dev->pme_support = pmc >> PCI_PM_CAP_PME_SHIFT;
 		/*
 		 * Make device's PM flags reflect the wake-up capability, but
 		 * let the user space enable it to wake up the system as needed.
@@ -1264,7 +1248,9 @@ void pci_pm_init(struct pci_dev *dev)
 		device_set_wakeup_capable(&dev->dev, true);
 		device_set_wakeup_enable(&dev->dev, false);
 		/* Disable the PME# generation functionality */
-		pci_pme_active(dev, pm, false);
+		pci_pme_active(dev, false);
+	} else {
+		dev->pme_support = 0;
 	}
 }
 
Index: linux-pci/include/linux/pci_regs.h
===================================================================
--- linux-pci.orig/include/linux/pci_regs.h
+++ linux-pci/include/linux/pci_regs.h
@@ -231,6 +231,7 @@
 #define  PCI_PM_CAP_PME_D2	0x2000	/* PME# from D2 */
 #define  PCI_PM_CAP_PME_D3	0x4000	/* PME# from D3 (hot) */
 #define  PCI_PM_CAP_PME_D3cold	0x8000	/* PME# from D3 (cold) */
+#define  PCI_PM_CAP_PME_SHIFT	11	/* Start of the PME Mask in PMC */
 #define PCI_PM_CTRL		4	/* PM control and status register */
 #define  PCI_PM_CTRL_STATE_MASK	0x0003	/* Current power state (D0 to D3) */
 #define  PCI_PM_CTRL_NO_SOFT_RESET	0x0004	/* No reset for D3hot->D0 */


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 5/8] ACPI: Introduce new device wakeup flag 'prepared'
  2008-07-07  1:34   ` [PATCH 5/8] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
@ 2008-07-07 14:08     ` Pavel Machek
  0 siblings, 0 replies; 33+ messages in thread
From: Pavel Machek @ 2008-07-07 14:08 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jesse Barnes, ACPI Devel Maling List, Alan Stern, Andi Kleen,
	pm list, Zhang Rui, Zhao Yakui

On Mon 2008-07-07 03:34:11, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> ACPI: Introduce new device wakeup flag 'prepared'
> 
> Introduce additional flag 'prepared' in
> struct acpi_device_wakeup_flags and use it to prevent devices from
> being enable/disabled do wake up the system multiple times in a row
> (this does not happen currently, but will be possible after some of
> the following patches).
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4)
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (7 preceding siblings ...)
  2008-07-07  1:36   ` [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
@ 2008-07-08  0:49   ` Jesse Barnes
  2008-07-08 14:42     ` Rafael J. Wysocki
  2008-07-13 20:45   ` PCI PM: Fix pci_prepare_to_sleep Rafael J. Wysocki
  9 siblings, 1 reply; 33+ messages in thread
From: Jesse Barnes @ 2008-07-08  0:49 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Zhao Yakui, ACPI Devel Maling List, Pavel Machek, Andi Kleen,
	pm list

On Sunday, July 06, 2008 6:30 pm Rafael J. Wysocki wrote:
> Hi Jesse,
>
> This is a new (4th) iteration of the series of patches intended to rework
> the power management of PCI devices so that the handling of their wake-up
> functionality is consistent with the other PM operations and so that the
> user space can manage the wake-up functionality of PCI devices using the
> /sys/devices/.../power/wakeup interface regardless of whether the wake-up
> function is based on the native PCI mechanism (PME#), or it is handled by
> the platform (eg. ACPI).
>
> Apart from the ACPI and PCI changes necessary for this purpose, the series
> includes some clean-ups that are not strictly required, but make the code
> more straightforward and (IMHO) easier to follow.
>
> The patchset is on top of the linux-next branch of the PCI tree and is
> regarded as 2.6.27 material.

I just tested on one of my 965 based platforms.  Suspend/resume works as well 
as I'd expect normally, but if I use wake-on-lan (using the wakeup code from 
this patchset) the machine wakes up but never seems to finish resuming; it 
stays off the network with video off and the console is unresponsive.  I'll 
try some more tests (including older kernels to see if this is actually 
regression or not), but it would be good if other people could give this 
patch set some attention too.

Thanks,
Jesse

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4)
  2008-07-08  0:49   ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality " Jesse Barnes
@ 2008-07-08 14:42     ` Rafael J. Wysocki
  0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-08 14:42 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

On Tuesday, 8 of July 2008, Jesse Barnes wrote:
> On Sunday, July 06, 2008 6:30 pm Rafael J. Wysocki wrote:
> > Hi Jesse,
> >
> > This is a new (4th) iteration of the series of patches intended to rework
> > the power management of PCI devices so that the handling of their wake-up
> > functionality is consistent with the other PM operations and so that the
> > user space can manage the wake-up functionality of PCI devices using the
> > /sys/devices/.../power/wakeup interface regardless of whether the wake-up
> > function is based on the native PCI mechanism (PME#), or it is handled by
> > the platform (eg. ACPI).
> >
> > Apart from the ACPI and PCI changes necessary for this purpose, the series
> > includes some clean-ups that are not strictly required, but make the code
> > more straightforward and (IMHO) easier to follow.
> >
> > The patchset is on top of the linux-next branch of the PCI tree and is
> > regarded as 2.6.27 material.
> 
> I just tested on one of my 965 based platforms.  Suspend/resume works as well 
> as I'd expect normally, but if I use wake-on-lan (using the wakeup code from 
> this patchset) the machine wakes up but never seems to finish resuming; it 
> stays off the network with video off and the console is unresponsive.

What network adapter is there in the box with this symptoms and which driver
is it handled by?

> I'll try some more tests (including older kernels to see if this is actually 
> regression or not), but it would be good if other people could give this 
> patch set some attention too.

Certainly.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 6/8] PCI ACPI: Rework PCI handling of wake-up (rev. 5)
  2008-07-07  1:34   ` [PATCH 6/8] PCI ACPI: Rework PCI handling of wake-up (rev. 5) Rafael J. Wysocki
@ 2008-07-11 20:30     ` Pavel Machek
  0 siblings, 0 replies; 33+ messages in thread
From: Pavel Machek @ 2008-07-11 20:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jesse Barnes, ACPI Devel Maling List, Alan Stern, Andi Kleen,
	pm list, Zhang Rui, Zhao Yakui

On Mon 2008-07-07 03:34:48, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> PCI ACPI: Rework PCI handling of wake-up (rev. 5)
> 
> * Introduce function acpi_pm_device_sleep_wake() for enabling and
>   disabling the system wake-up capability of devices that are power
>   manageable by ACPI.
> 
> * Introduce function acpi_bus_can_wakeup() allowing other (dependent)
>   subsystems to check if ACPI is able to enable the system wake-up
>   capability of given device.
> 
> * Introduce callback .sleep_wake() in struct pci_platform_pm_ops and
>   for the ACPI PCI 'driver' make it use acpi_pm_device_sleep_wake().
> 
> * Introduce callback .can_wakeup() in struct pci_platform_pm_ops and
>   for the ACPI 'driver' make it use acpi_bus_can_wakeup().
> 
> * Move the PME# handlig code out of pci_enable_wake() and split it
>   into two functions, pci_pme_capable() and pci_pme_active(),
>   allowing the caller to check if given device is capable of
>   generating PME# from given power state and to enable/disable the
>   device's PME# functionality, respectively.
> 
> * Modify pci_enable_wake() to use the new ACPI callbacks and the new
>   PME#-related functions.
> 
> * Drop the generic .platform_enable_wakeup() callback that is not
>   used any more.
> 
> * Introduce device_set_wakeup_capable() that will set the
>   power.can_wakeup flag of given device.
> 
> * Rework PCI device PM initialization so that, if given device is
>   capable of generating wake-up events, either natively through the
>   PME# mechanism, or with the help of the platform, its
>   power.can_wakeup flag is set and its power.should_wakeup flag is
>   unset as appropriate.
> 
> * Make ACPI set the power.can_wakeup flag for devices found to be
>   wake-up capable by it.
> 
> * Make the ACPI wake-up code enable/disable GPEs for devices that
>   have the wakeup.flags.prepared flag set (which means that their
>   wake-up power has been enabled).
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Looks ok to me.

ACK.
							Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 7/8] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep
  2008-07-07  1:35   ` [PATCH 7/8] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
@ 2008-07-11 20:31     ` Pavel Machek
  0 siblings, 0 replies; 33+ messages in thread
From: Pavel Machek @ 2008-07-11 20:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jesse Barnes, ACPI Devel Maling List, Alan Stern, Andi Kleen,
	pm list, Zhang Rui, Zhao Yakui

On Mon 2008-07-07 03:35:26, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep
> 
> Introduce functions pci_prepare_to_sleep() and pci_back_from_sleep(),
> to be used by the PCI drivers that want to place their devices into
> the lowest power state appropiate for them (PCI_D3hot, if the device
> is not supposed to wake up the system, or the deepest state from
> which the wake-up is possible, otherwise) while the system is being
> prepared to go into a sleeping state and to put them back into D0
> during the subsequent transition to the working state.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4)
  2008-07-07  1:36   ` [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
@ 2008-07-11 20:37     ` Pavel Machek
  2008-07-11 20:45       ` Rafael J. Wysocki
  0 siblings, 1 reply; 33+ messages in thread
From: Pavel Machek @ 2008-07-11 20:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jesse Barnes, ACPI Devel Maling List, Alan Stern, Andi Kleen,
	pm list, Zhang Rui, Zhao Yakui

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> PCI: Simplify PCI device PM code (rev. 4)
> 
> If the offset of PCI device's PM capability in its configuration
> space, the mask of states that the device supports PME# from and the
> D1 and D2 support bits are cached in the corresponding
> struct pci_dev, the PCI device PM code can be simplified quite a bit.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/pci/pci.c        |  118 ++++++++++++++++++++---------------------------
>  include/linux/pci.h      |    8 ++-
>  include/linux/pci_regs.h |    1 
>  3 files changed, 60 insertions(+), 67 deletions(-)
> 
> Index: linux-pci/include/linux/pci.h
> ===================================================================
> --- linux-pci.orig/include/linux/pci.h
> +++ linux-pci/include/linux/pci.h
> @@ -177,6 +177,13 @@ struct pci_dev {
>  	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
>  					   this is D0-D3, D0 being fully functional,
>  					   and D3 being off. */
> +	int		pm_cap;		/* PM capability offset in the
> +					   configuration space */
> +	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
> +					   can be generated */
> +	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 */

Uh, "D1", "D2", then "no D1D2" ?

>  	}
>  
> -	if (pmc & PCI_PM_CAP_PME_MASK) {
> +	dev->pm_cap = pm;
> +
> +	dev->d1_support = false;
> +	dev->d2_support = false;

I thought d1_support is unsigned int, not bool?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4)
  2008-07-11 20:37     ` Pavel Machek
@ 2008-07-11 20:45       ` Rafael J. Wysocki
  2008-07-11 20:49         ` Pavel Machek
  0 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-11 20:45 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jesse Barnes, ACPI Devel Maling List, Alan Stern, Andi Kleen,
	pm list, Zhang Rui, Zhao Yakui

On Friday, 11 of July 2008, Pavel Machek wrote:
> Hi!
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > PCI: Simplify PCI device PM code (rev. 4)
> > 
> > If the offset of PCI device's PM capability in its configuration
> > space, the mask of states that the device supports PME# from and the
> > D1 and D2 support bits are cached in the corresponding
> > struct pci_dev, the PCI device PM code can be simplified quite a bit.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/pci/pci.c        |  118 ++++++++++++++++++++---------------------------
> >  include/linux/pci.h      |    8 ++-
> >  include/linux/pci_regs.h |    1 
> >  3 files changed, 60 insertions(+), 67 deletions(-)
> > 
> > Index: linux-pci/include/linux/pci.h
> > ===================================================================
> > --- linux-pci.orig/include/linux/pci.h
> > +++ linux-pci/include/linux/pci.h
> > @@ -177,6 +177,13 @@ struct pci_dev {
> >  	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
> >  					   this is D0-D3, D0 being fully functional,
> >  					   and D3 being off. */
> > +	int		pm_cap;		/* PM capability offset in the
> > +					   configuration space */
> > +	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
> > +					   can be generated */
> > +	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 */
> 
> Uh, "D1", "D2", then "no D1D2" ?

Yes.  They are used for different things.

> >  	}
> >  
> > -	if (pmc & PCI_PM_CAP_PME_MASK) {
> > +	dev->pm_cap = pm;
> > +
> > +	dev->d1_support = false;
> > +	dev->d2_support = false;
> 
> I thought d1_support is unsigned int, not bool?

They are always used as bools, though.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4)
  2008-07-11 20:45       ` Rafael J. Wysocki
@ 2008-07-11 20:49         ` Pavel Machek
  0 siblings, 0 replies; 33+ messages in thread
From: Pavel Machek @ 2008-07-11 20:49 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jesse Barnes, ACPI Devel Maling List, Alan Stern, Andi Kleen,
	pm list, Zhang Rui, Zhao Yakui

On Fri 2008-07-11 22:45:28, Rafael J. Wysocki wrote:
> On Friday, 11 of July 2008, Pavel Machek wrote:
> > Hi!
> > 
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > PCI: Simplify PCI device PM code (rev. 4)
> > > 
> > > If the offset of PCI device's PM capability in its configuration
> > > space, the mask of states that the device supports PME# from and the
> > > D1 and D2 support bits are cached in the corresponding
> > > struct pci_dev, the PCI device PM code can be simplified quite a bit.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > ---
> > >  drivers/pci/pci.c        |  118 ++++++++++++++++++++---------------------------
> > >  include/linux/pci.h      |    8 ++-
> > >  include/linux/pci_regs.h |    1 
> > >  3 files changed, 60 insertions(+), 67 deletions(-)
> > > 
> > > Index: linux-pci/include/linux/pci.h
> > > ===================================================================
> > > --- linux-pci.orig/include/linux/pci.h
> > > +++ linux-pci/include/linux/pci.h
> > > @@ -177,6 +177,13 @@ struct pci_dev {
> > >  	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
> > >  					   this is D0-D3, D0 being fully functional,
> > >  					   and D3 being off. */
> > > +	int		pm_cap;		/* PM capability offset in the
> > > +					   configuration space */
> > > +	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
> > > +					   can be generated */
> > > +	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 */
> > 
> > Uh, "D1", "D2", then "no D1D2" ?
> 
> Yes.  They are used for different things.

Maybe add one line of explanation?

> > >  	}
> > >  
> > > -	if (pmc & PCI_PM_CAP_PME_MASK) {
> > > +	dev->pm_cap = pm;
> > > +
> > > +	dev->d1_support = false;
> > > +	dev->d2_support = false;
> > 
> > I thought d1_support is unsigned int, not bool?
> 
> They are always used as bools, though.

So make them bools? I'm not sure if this is correct C... or were you
trying to save some bits?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 33+ messages in thread

* PCI PM: Fix pci_prepare_to_sleep
  2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
                     ` (8 preceding siblings ...)
  2008-07-08  0:49   ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality " Jesse Barnes
@ 2008-07-13 20:45   ` Rafael J. Wysocki
  2008-07-14 21:27     ` Jesse Barnes
  9 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-13 20:45 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

Hi Jesse,

The recently introduced pci_prepare_to_sleep() needs the following fix,
because there are systems which are not power manageable by ACPI (ie. ACPI
doesn't provide methods to put the device into low power states and back), but
require ACPI hooks to be executed for wake-up to work.

Please apply.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>

Fix pci_prepare_to_sleep() to work on systems that are not power
manageable by ACPI (ie. ACPI doesn't provide methods to put the
device into low power states and back into the full power state), but
require ACPI hooks to be executed for wake-up to work.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/pci/pci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-next/drivers/pci/pci.c
===================================================================
--- linux-next.orig/drivers/pci/pci.c
+++ linux-next/drivers/pci/pci.c
@@ -1153,7 +1153,6 @@ int pci_prepare_to_sleep(struct pci_dev 
 				break;
 		default:
 			target_state = state;
-			pci_enable_wake(dev, target_state, true);
 		}
 	} else if (device_may_wakeup(&dev->dev)) {
 		/*
@@ -1168,10 +1167,11 @@ int pci_prepare_to_sleep(struct pci_dev 
 			while (target_state
 			      && !(dev->pme_support & (1 << target_state)))
 				target_state--;
-			pci_pme_active(dev, true);
 		}
 	}
 
+	pci_enable_wake(dev, target_state, true);
+
 	error = pci_set_power_state(dev, target_state);
 
 	if (error)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PCI PM: Fix pci_prepare_to_sleep
  2008-07-13 20:45   ` PCI PM: Fix pci_prepare_to_sleep Rafael J. Wysocki
@ 2008-07-14 21:27     ` Jesse Barnes
  2008-07-14 21:40       ` Rafael J. Wysocki
  0 siblings, 1 reply; 33+ messages in thread
From: Jesse Barnes @ 2008-07-14 21:27 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Zhao Yakui, ACPI Devel Maling List, Pavel Machek, Andi Kleen,
	pm list

On Sunday, July 13, 2008 1:45 pm Rafael J. Wysocki wrote:
> Hi Jesse,
>
> The recently introduced pci_prepare_to_sleep() needs the following fix,
> because there are systems which are not power manageable by ACPI (ie. ACPI
> doesn't provide methods to put the device into low power states and back),
> but require ACPI hooks to be executed for wake-up to work.
>
> Please apply.
>
> Thanks,
> Rafael
>
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Fix pci_prepare_to_sleep() to work on systems that are not power
> manageable by ACPI (ie. ACPI doesn't provide methods to put the
> device into low power states and back into the full power state), but
> require ACPI hooks to be executed for wake-up to work.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Based on what you've told me so far about the number of ACPI vs. native wakeup 
methods & problems, I'm starting to get a little worried that making this 
stuff work right will require lots of platform specific quirks.  I guess 
that's just par for the course though, and this patch looks fine, so I just 
applied it to my linux-next branch.

Thanks,
Jesse

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: PCI PM: Fix pci_prepare_to_sleep
  2008-07-14 21:27     ` Jesse Barnes
@ 2008-07-14 21:40       ` Rafael J. Wysocki
  0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2008-07-14 21:40 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: ACPI Devel Maling List, Alan Stern, Andi Kleen, pm list,
	Zhang Rui, Zhao Yakui, Pavel Machek

On Monday, 14 of July 2008, Jesse Barnes wrote:
> On Sunday, July 13, 2008 1:45 pm Rafael J. Wysocki wrote:
> > Hi Jesse,
> >
> > The recently introduced pci_prepare_to_sleep() needs the following fix,
> > because there are systems which are not power manageable by ACPI (ie. ACPI
> > doesn't provide methods to put the device into low power states and back),
> > but require ACPI hooks to be executed for wake-up to work.
> >
> > Please apply.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Fix pci_prepare_to_sleep() to work on systems that are not power
> > manageable by ACPI (ie. ACPI doesn't provide methods to put the
> > device into low power states and back into the full power state), but
> > require ACPI hooks to be executed for wake-up to work.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Based on what you've told me so far about the number of ACPI vs. native wakeup 
> methods & problems, I'm starting to get a little worried that making this 
> stuff work right will require lots of platform specific quirks.

Yes, this is likely.

Also, having looked at several network adapters' suspend and resume routines,
I think we'll need to do some fine tuning here and there.

> I guess that's just par for the course though, and this patch looks fine, so
> I just applied it to my linux-next branch.

Thanks!

Rafael

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2008-07-14 21:38 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-01 21:56 [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
2008-07-01 22:00 ` [PATCH 1/9] PCI ACPI: Remove acpi_platform_enable_wakeup Rafael J. Wysocki
2008-07-01 22:03 ` [PATCH 2/9] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
2008-07-01 22:04 ` [PATCH 3/9] PCI: Introduce platform_pci_power_manageable function Rafael J. Wysocki
2008-07-01 22:05 ` [PATCH 4/9] PCI: Rework pci_set_power_state function (rev. 3) Rafael J. Wysocki
2008-07-01 22:06 ` [PATCH 5/9] ACPI: Introduce acpi_device_sleep_wake function Rafael J. Wysocki
2008-07-01 22:07 ` [PATCH 6/9] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
2008-07-01 22:08 ` [PATCH 7/9] PCI ACPI: Rework PCI handling of wake-up (rev. 4) Rafael J. Wysocki
2008-07-02  7:30   ` Zhao Yakui
2008-07-02 10:16     ` Rafael J. Wysocki
2008-07-01 22:09 ` [PATCH 8/9] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
2008-07-01 22:10 ` [PATCH 9/9] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
2008-07-03 12:04 ` [PATCH 0/9] PCI PM: Handling of PCI devices wake-up functionality (rev. 3) Rafael J. Wysocki
2008-07-07  1:30 ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality (rev. 4) Rafael J. Wysocki
2008-07-07  1:30   ` [PATCH 1/8] ACPI: Introduce acpi_bus_power_manageable function Rafael J. Wysocki
2008-07-07  1:32   ` [PATCH 2/8] PCI: Introduce platform_pci_power_manageable function Rafael J. Wysocki
2008-07-07  1:32   ` [PATCH 3/8] PCI: Rework pci_set_power_state function (rev. 4) Rafael J. Wysocki
2008-07-07  1:33   ` [PATCH 4/8] ACPI: Introduce acpi_device_sleep_wake function Rafael J. Wysocki
2008-07-07  1:34   ` [PATCH 5/8] ACPI: Introduce new device wakeup flag 'prepared' Rafael J. Wysocki
2008-07-07 14:08     ` Pavel Machek
2008-07-07  1:34   ` [PATCH 6/8] PCI ACPI: Rework PCI handling of wake-up (rev. 5) Rafael J. Wysocki
2008-07-11 20:30     ` Pavel Machek
2008-07-07  1:35   ` [PATCH 7/8] PCI PM: Introduce pci_prepare_to_sleep and pci_back_from_sleep Rafael J. Wysocki
2008-07-11 20:31     ` Pavel Machek
2008-07-07  1:36   ` [PATCH 8/8] PCI: Simplify PCI device PM code (rev. 4) Rafael J. Wysocki
2008-07-11 20:37     ` Pavel Machek
2008-07-11 20:45       ` Rafael J. Wysocki
2008-07-11 20:49         ` Pavel Machek
2008-07-08  0:49   ` [PATCH 0/8] PCI PM: Handling of PCI devices wake-up functionality " Jesse Barnes
2008-07-08 14:42     ` Rafael J. Wysocki
2008-07-13 20:45   ` PCI PM: Fix pci_prepare_to_sleep Rafael J. Wysocki
2008-07-14 21:27     ` Jesse Barnes
2008-07-14 21:40       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).