2012-10-29 09:10:37

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 0/7] ACPI / PM: ACPI power management callback routines for subsystems

Hi All,

The following series of patches rearranges ACPI device power management
code and introduces universal PM callback routines that may be used by
bus types that don't support power management natively and/or by drivers
of devices with those bus types.

[1/7] Move routines for adding/removing device wakeup notifiers into a separate
file.
[2/7] Move device power state selection routine to the new device PM file.
[3/7] Move runtime remote wakeup setup routine to the new device PM file.
[4/7] Split device wakeup management routines, so that it is possible to
reduce the number of acpi_bus_get_device() calls.
[5/7] Provide low-lever device PM functions operationg on struct acpi_device
objects.
[6/7] Move device PM functions related to sleep states from sleep.c to the
new device PM file.
[7/7] Provide ACPI PM callback routines for subsystems.

The patches are on top of the current Linus' tree with the git branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-qos

merged. The resulting kernel has been run on my test-bed Toshiba Portege R500
and hasn't crashed it, but more testing is still needed, so please use it with
care.

Thanks,
Rafael


--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.


2012-10-29 09:10:24

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

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

If the caller of acpi_bus_set_power() already has a pointer to the
struct acpi_device object corresponding to the device in question, it
doesn't make sense for it to go through acpi_bus_get_device(), which
may be costly, because it involves acquiring the global ACPI
namespace mutex.

For this reason, export the function operating on struct acpi_device
objects used internally by acpi_bus_set_power(), so that it may be
called instead of acpi_bus_set_power() in the above case, and change
its name to acpi_device_set_power().

Additionally, introduce two inline wrappers for checking ACPI PM
capabilities of devices represented by struct acpi_device objects.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 15 ++++++++++++---
include/acpi/acpi_bus.h | 11 +++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)

Index: linux/drivers/acpi/bus.c
===================================================================
--- linux.orig/drivers/acpi/bus.c
+++ linux/drivers/acpi/bus.c
@@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
}


-static int __acpi_bus_set_power(struct acpi_device *device, int state)
+/**
+ * acpi_device_set_power - Set power state of an ACPI device.
+ * @device: Device to set the power state of.
+ * @state: New power state to set.
+ *
+ * Callers must ensure that the device is power manageable before using this
+ * function.
+ */
+int acpi_device_set_power(struct acpi_device *device, int state)
{
int result = 0;
acpi_status status = AE_OK;
@@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a

return result;
}
+EXPORT_SYMBOL(acpi_device_set_power);


int acpi_bus_set_power(acpi_handle handle, int state)
@@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
return -ENODEV;
}

- return __acpi_bus_set_power(device, state);
+ return acpi_device_set_power(device, state);
}
EXPORT_SYMBOL(acpi_bus_set_power);

@@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
if (result)
return result;

- result = __acpi_bus_set_power(device, state);
+ result = acpi_device_set_power(device, state);
if (!result && state_p)
*state_p = state;

Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
unsigned long long *sta);
int acpi_bus_get_status(struct acpi_device *device);
int acpi_bus_set_power(acpi_handle handle, int state);
+int acpi_device_set_power(struct acpi_device *device, int state);
int acpi_bus_update_power(acpi_handle handle, int *state_p);
bool acpi_bus_power_manageable(acpi_handle handle);
bool acpi_bus_can_wakeup(acpi_handle handle);
@@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
}
#endif

+static inline bool acpi_device_power_manageable(struct acpi_device *adev)
+{
+ return adev->flags.power_manageable;
+}
+
+static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
+{
+ return adev->wakeup.flags.valid;
+}
+
#else /* CONFIG_ACPI */

static inline int register_acpi_bus_type(void *bus) { return 0; }

2012-10-29 09:10:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 2/7] ACPI / PM: Move device power state selection routine to device_pm.c

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

The ACPI function for choosing device power state is now located
in drivers/acpi/sleep.c, but drivers/acpi/device_pm.c is a more
logical place for it, so move it there.

However, instead of moving the function entirely, move its core only
under a different name and with a different list of arguments, so
that it is more flexible, and leave a wrapper around it in the
original location.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/device_pm.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/sleep.c | 88 +-------------------------------------
include/acpi/acpi_bus.h | 15 ++++++
3 files changed, 124 insertions(+), 86 deletions(-)

Index: linux/drivers/acpi/device_pm.c
===================================================================
--- linux.orig/drivers/acpi/device_pm.c
+++ linux/drivers/acpi/device_pm.c
@@ -23,7 +23,9 @@
*/

#include <linux/device.h>
+#include <linux/export.h>
#include <linux/mutex.h>
+#include <linux/pm_qos.h>

#include <acpi/acpi.h>
#include <acpi/acpi_bus.h>
@@ -89,3 +91,108 @@ acpi_status acpi_remove_pm_notifier(stru
mutex_unlock(&acpi_pm_notifier_lock);
return status;
}
+
+/**
+ * acpi_device_power_state - Get preferred power state of ACPI device.
+ * @dev: Device whose preferred target power state to return.
+ * @adev: ACPI device node corresponding to @dev.
+ * @target_state: System state to match the resultant device state.
+ * @d_max_in: Deepest low-power state to take into consideration.
+ * @d_min_p: Location to store the upper limit of the allowed states range.
+ * Return value: Preferred power state of the device on success, -ENODEV
+ * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
+ *
+ * Find the lowest power (highest number) ACPI device power state that the
+ * device can be in while the system is in the state represented by
+ * @target_state. If @d_min_p is set, the highest power (lowest number) device
+ * power state that @dev can be in for the given system sleep state is stored
+ * at the location pointed to by it.
+ *
+ * Callers must ensure that @dev and @adev are valid pointers and that @adev
+ * actually corresponds to @dev before using this function.
+ */
+int acpi_device_power_state(struct device *dev, struct acpi_device *adev,
+ u32 target_state, int d_max_in, int *d_min_p)
+{
+ char acpi_method[] = "_SxD";
+ unsigned long long d_min, d_max;
+ bool wakeup = false;
+
+ if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
+ return -EINVAL;
+
+ if (d_max_in > ACPI_STATE_D3_HOT) {
+ enum pm_qos_flags_status stat;
+
+ stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
+ if (stat == PM_QOS_FLAGS_ALL)
+ d_max_in = ACPI_STATE_D3_HOT;
+ }
+
+ acpi_method[2] = '0' + target_state;
+ /*
+ * If the sleep state is S0, the lowest limit from ACPI is D3,
+ * but if the device has _S0W, we will use the value from _S0W
+ * as the lowest limit from ACPI. Finally, we will constrain
+ * the lowest limit with the specified one.
+ */
+ d_min = ACPI_STATE_D0;
+ d_max = ACPI_STATE_D3;
+
+ /*
+ * If present, _SxD methods return the minimum D-state (highest power
+ * state) we can use for the corresponding S-states. Otherwise, the
+ * minimum D-state is D0 (ACPI 3.x).
+ *
+ * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
+ * provided -- that's our fault recovery, we ignore retval.
+ */
+ if (target_state > ACPI_STATE_S0) {
+ acpi_evaluate_integer(adev->handle, acpi_method, NULL, &d_min);
+ wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
+ && adev->wakeup.sleep_state >= target_state;
+ } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
+ PM_QOS_FLAGS_NONE) {
+ wakeup = adev->wakeup.flags.valid;
+ }
+
+ /*
+ * If _PRW says we can wake up the system from the target sleep state,
+ * the D-state returned by _SxD is sufficient for that (we assume a
+ * wakeup-aware driver if wake is set). Still, if _SxW exists
+ * (ACPI 3.x), it should return the maximum (lowest power) D-state that
+ * can wake the system. _S0W may be valid, too.
+ */
+ if (wakeup) {
+ acpi_status status;
+
+ acpi_method[3] = 'W';
+ status = acpi_evaluate_integer(adev->handle, acpi_method, NULL,
+ &d_max);
+ if (ACPI_FAILURE(status)) {
+ if (target_state != ACPI_STATE_S0 ||
+ status != AE_NOT_FOUND)
+ d_max = d_min;
+ } else if (d_max < d_min) {
+ /* Warn the user of the broken DSDT */
+ printk(KERN_WARNING "ACPI: Wrong value from %s\n",
+ acpi_method);
+ /* Sanitize it */
+ d_min = d_max;
+ }
+ }
+
+ if (d_max_in < d_min)
+ return -EINVAL;
+ if (d_min_p)
+ *d_min_p = d_min;
+ /* constrain d_max with specified lowest limit (max number) */
+ if (d_max > d_max_in) {
+ for (d_max = d_max_in; d_max > d_min; d_max--) {
+ if (adev->power.states[d_max].flags.valid)
+ break;
+ }
+ }
+ return d_max;
+}
+EXPORT_SYMBOL_GPL(acpi_device_power_state);
Index: linux/drivers/acpi/sleep.c
===================================================================
--- linux.orig/drivers/acpi/sleep.c
+++ linux/drivers/acpi/sleep.c
@@ -19,7 +19,6 @@
#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
-#include <linux/pm_qos.h>

#include <asm/io.h>

@@ -706,101 +705,20 @@ int acpi_suspend(u32 acpi_state)
* Return value: Preferred power state of the device on success, -ENODEV
* (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
*
- * Find the lowest power (highest number) ACPI device power state that the
- * device can be in while the system is in the sleep state represented
- * by %acpi_target_sleep_state. If @d_min_p is set, the highest power (lowest
- * number) device power state that @dev can be in for the given system sleep
- * state is stored at the location pointed to by it.
- *
* The caller must ensure that @dev is valid before using this function.
*/
int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p, int d_max_in)
{
acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
struct acpi_device *adev;
- char acpi_method[] = "_SxD";
- unsigned long long d_min, d_max;
- bool wakeup = false;

- if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
- return -EINVAL;
if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
- printk(KERN_DEBUG "ACPI handle has no context!\n");
+ dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
return -ENODEV;
}
- if (d_max_in > ACPI_STATE_D3_HOT) {
- enum pm_qos_flags_status stat;
-
- stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
- if (stat == PM_QOS_FLAGS_ALL)
- d_max_in = ACPI_STATE_D3_HOT;
- }
-
- acpi_method[2] = '0' + acpi_target_sleep_state;
- /*
- * If the sleep state is S0, the lowest limit from ACPI is D3,
- * but if the device has _S0W, we will use the value from _S0W
- * as the lowest limit from ACPI. Finally, we will constrain
- * the lowest limit with the specified one.
- */
- d_min = ACPI_STATE_D0;
- d_max = ACPI_STATE_D3;

- /*
- * If present, _SxD methods return the minimum D-state (highest power
- * state) we can use for the corresponding S-states. Otherwise, the
- * minimum D-state is D0 (ACPI 3.x).
- *
- * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
- * provided -- that's our fault recovery, we ignore retval.
- */
- if (acpi_target_sleep_state > ACPI_STATE_S0) {
- acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
- wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
- && adev->wakeup.sleep_state >= acpi_target_sleep_state;
- } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
- PM_QOS_FLAGS_NONE) {
- wakeup = adev->wakeup.flags.valid;
- }
-
- /*
- * If _PRW says we can wake up the system from the target sleep state,
- * the D-state returned by _SxD is sufficient for that (we assume a
- * wakeup-aware driver if wake is set). Still, if _SxW exists
- * (ACPI 3.x), it should return the maximum (lowest power) D-state that
- * can wake the system. _S0W may be valid, too.
- */
- if (wakeup) {
- acpi_status status;
-
- acpi_method[3] = 'W';
- status = acpi_evaluate_integer(handle, acpi_method, NULL,
- &d_max);
- if (ACPI_FAILURE(status)) {
- if (acpi_target_sleep_state != ACPI_STATE_S0 ||
- status != AE_NOT_FOUND)
- d_max = d_min;
- } else if (d_max < d_min) {
- /* Warn the user of the broken DSDT */
- printk(KERN_WARNING "ACPI: Wrong value from %s\n",
- acpi_method);
- /* Sanitize it */
- d_min = d_max;
- }
- }
-
- if (d_max_in < d_min)
- return -EINVAL;
- if (d_min_p)
- *d_min_p = d_min;
- /* constrain d_max with specified lowest limit (max number) */
- if (d_max > d_max_in) {
- for (d_max = d_max_in; d_max > d_min; d_max--) {
- if (adev->power.states[d_max].flags.valid)
- break;
- }
- }
- return d_max;
+ return acpi_device_power_state(dev, adev, acpi_target_sleep_state,
+ d_max_in, d_min_p);
}
EXPORT_SYMBOL(acpi_pm_device_sleep_state);
#endif /* CONFIG_PM */
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -419,6 +419,8 @@ acpi_status acpi_add_pm_notifier(struct
acpi_notify_handler handler, void *context);
acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
acpi_notify_handler handler);
+int acpi_device_power_state(struct device *dev, struct acpi_device *adev,
+ u32 target_state, int d_max_in, int *d_min_p);
int acpi_pm_device_sleep_state(struct device *, int *, int);
#else
static inline acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
@@ -432,12 +434,23 @@ static inline acpi_status acpi_remove_pm
{
return AE_SUPPORT;
}
-static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
+static inline int __acpi_device_power_state(int m, int *p)
{
if (p)
*p = ACPI_STATE_D0;
return (m >= ACPI_STATE_D0 && m <= ACPI_STATE_D3) ? m : ACPI_STATE_D0;
}
+static inline int acpi_device_power_state(struct device *dev,
+ struct acpi_device *adev,
+ u32 target_state, int d_max_in,
+ int *d_min_p)
+{
+ return __acpi_device_power_state(d_max_in, d_min_p);
+}
+static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
+{
+ return __acpi_device_power_state(m, p);
+}
#endif

#ifdef CONFIG_PM_RUNTIME

2012-10-29 09:10:39

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 1/7] ACPI / PM: Move routines for adding/removing device wakeup notifiers

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

ACPI routines for adding and removing device wakeup notifiers are
currently defined in a PCI-specific file, but they will be necessary
for non-PCI devices too, so move them to a separate file under
drivers/acpi and rename them to indicate their ACPI origins.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/Makefile | 3 +
drivers/acpi/device_pm.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/pci-acpi.c | 71 ++----------------------------------
include/acpi/acpi_bus.h | 15 +++++++
4 files changed, 112 insertions(+), 68 deletions(-)

Index: linux/drivers/acpi/device_pm.c
===================================================================
--- /dev/null
+++ linux/drivers/acpi/device_pm.c
@@ -0,0 +1,91 @@
+/*
+ * drivers/acpi/device_pm.c - ACPI device power management routines.
+ *
+ * Copyright (C) 2012, Intel Corp.
+ * Author: Rafael J. Wysocki <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <linux/device.h>
+#include <linux/mutex.h>
+
+#include <acpi/acpi.h>
+#include <acpi/acpi_bus.h>
+
+static DEFINE_MUTEX(acpi_pm_notifier_lock);
+
+/**
+ * acpi_add_pm_notifier - Register PM notifier for given ACPI device.
+ * @adev: ACPI device to add the notifier for.
+ * @context: Context information to pass to the notifier routine.
+ *
+ * NOTE: @adev need not be a run-wake or wakeup device to be a valid source of
+ * PM wakeup events. For example, wakeup events may be generated for bridges
+ * if one of the devices below the bridge is signaling wakeup, even if the
+ * bridge itself doesn't have a wakeup GPE associated with it.
+ */
+acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
+ acpi_notify_handler handler, void *context)
+{
+ acpi_status status = AE_ALREADY_EXISTS;
+
+ mutex_lock(&acpi_pm_notifier_lock);
+
+ if (adev->wakeup.flags.notifier_present)
+ goto out;
+
+ status = acpi_install_notify_handler(adev->handle,
+ ACPI_SYSTEM_NOTIFY,
+ handler, context);
+ if (ACPI_FAILURE(status))
+ goto out;
+
+ adev->wakeup.flags.notifier_present = true;
+
+ out:
+ mutex_unlock(&acpi_pm_notifier_lock);
+ return status;
+}
+
+/**
+ * acpi_remove_pm_notifier - Unregister PM notifier from given ACPI device.
+ * @adev: ACPI device to remove the notifier from.
+ */
+acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
+ acpi_notify_handler handler)
+{
+ acpi_status status = AE_BAD_PARAMETER;
+
+ mutex_lock(&acpi_pm_notifier_lock);
+
+ if (!adev->wakeup.flags.notifier_present)
+ goto out;
+
+ status = acpi_remove_notify_handler(adev->handle,
+ ACPI_SYSTEM_NOTIFY,
+ handler);
+ if (ACPI_FAILURE(status))
+ goto out;
+
+ adev->wakeup.flags.notifier_present = false;
+
+ out:
+ mutex_unlock(&acpi_pm_notifier_lock);
+ return status;
+}
Index: linux/drivers/pci/pci-acpi.c
===================================================================
--- linux.orig/drivers/pci/pci-acpi.c
+++ linux/drivers/pci/pci-acpi.c
@@ -20,8 +20,6 @@
#include <linux/pm_qos.h>
#include "pci.h"

-static DEFINE_MUTEX(pci_acpi_pm_notify_mtx);
-
/**
* pci_acpi_wake_bus - Wake-up notification handler for root buses.
* @handle: ACPI handle of a device the notification is for.
@@ -69,67 +67,6 @@ static void pci_acpi_wake_dev(acpi_handl
}

/**
- * add_pm_notifier - Register PM notifier for given ACPI device.
- * @dev: ACPI device to add the notifier for.
- * @context: PCI device or bus to check for PME status if an event is signaled.
- *
- * NOTE: @dev need not be a run-wake or wake-up device to be a valid source of
- * PM wake-up events. For example, wake-up events may be generated for bridges
- * if one of the devices below the bridge is signaling PME, even if the bridge
- * itself doesn't have a wake-up GPE associated with it.
- */
-static acpi_status add_pm_notifier(struct acpi_device *dev,
- acpi_notify_handler handler,
- void *context)
-{
- acpi_status status = AE_ALREADY_EXISTS;
-
- mutex_lock(&pci_acpi_pm_notify_mtx);
-
- if (dev->wakeup.flags.notifier_present)
- goto out;
-
- status = acpi_install_notify_handler(dev->handle,
- ACPI_SYSTEM_NOTIFY,
- handler, context);
- if (ACPI_FAILURE(status))
- goto out;
-
- dev->wakeup.flags.notifier_present = true;
-
- out:
- mutex_unlock(&pci_acpi_pm_notify_mtx);
- return status;
-}
-
-/**
- * remove_pm_notifier - Unregister PM notifier from given ACPI device.
- * @dev: ACPI device to remove the notifier from.
- */
-static acpi_status remove_pm_notifier(struct acpi_device *dev,
- acpi_notify_handler handler)
-{
- acpi_status status = AE_BAD_PARAMETER;
-
- mutex_lock(&pci_acpi_pm_notify_mtx);
-
- if (!dev->wakeup.flags.notifier_present)
- goto out;
-
- status = acpi_remove_notify_handler(dev->handle,
- ACPI_SYSTEM_NOTIFY,
- handler);
- if (ACPI_FAILURE(status))
- goto out;
-
- dev->wakeup.flags.notifier_present = false;
-
- out:
- mutex_unlock(&pci_acpi_pm_notify_mtx);
- return status;
-}
-
-/**
* pci_acpi_add_bus_pm_notifier - Register PM notifier for given PCI bus.
* @dev: ACPI device to add the notifier for.
* @pci_bus: PCI bus to walk checking for PME status if an event is signaled.
@@ -137,7 +74,7 @@ static acpi_status remove_pm_notifier(st
acpi_status pci_acpi_add_bus_pm_notifier(struct acpi_device *dev,
struct pci_bus *pci_bus)
{
- return add_pm_notifier(dev, pci_acpi_wake_bus, pci_bus);
+ return acpi_add_pm_notifier(dev, pci_acpi_wake_bus, pci_bus);
}

/**
@@ -146,7 +83,7 @@ acpi_status pci_acpi_add_bus_pm_notifier
*/
acpi_status pci_acpi_remove_bus_pm_notifier(struct acpi_device *dev)
{
- return remove_pm_notifier(dev, pci_acpi_wake_bus);
+ return acpi_remove_pm_notifier(dev, pci_acpi_wake_bus);
}

/**
@@ -157,7 +94,7 @@ acpi_status pci_acpi_remove_bus_pm_notif
acpi_status pci_acpi_add_pm_notifier(struct acpi_device *dev,
struct pci_dev *pci_dev)
{
- return add_pm_notifier(dev, pci_acpi_wake_dev, pci_dev);
+ return acpi_add_pm_notifier(dev, pci_acpi_wake_dev, pci_dev);
}

/**
@@ -166,7 +103,7 @@ acpi_status pci_acpi_add_pm_notifier(str
*/
acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev)
{
- return remove_pm_notifier(dev, pci_acpi_wake_dev);
+ return acpi_remove_pm_notifier(dev, pci_acpi_wake_dev);
}

phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
Index: linux/drivers/acpi/Makefile
===================================================================
--- linux.orig/drivers/acpi/Makefile
+++ linux/drivers/acpi/Makefile
@@ -21,9 +21,10 @@ obj-y += acpi.o \
acpi-y += osl.o utils.o reboot.o
acpi-y += nvs.o

-# sleep related files
+# Power management related files
acpi-y += wakeup.o
acpi-y += sleep.o
+acpi-$(CONFIG_PM) += device_pm.o
acpi-$(CONFIG_ACPI_SLEEP) += proc.o


Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -415,8 +415,23 @@ int acpi_enable_wakeup_device_power(stru
int acpi_disable_wakeup_device_power(struct acpi_device *dev);

#ifdef CONFIG_PM
+acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
+ acpi_notify_handler handler, void *context);
+acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
+ acpi_notify_handler handler);
int acpi_pm_device_sleep_state(struct device *, int *, int);
#else
+static inline acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
+ acpi_notify_handler handler,
+ void *context)
+{
+ return AE_SUPPORT;
+}
+static inline acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
+ acpi_notify_handler handler)
+{
+ return AE_SUPPORT;
+}
static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
{
if (p)

2012-10-29 09:10:22

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 6/7] ACPI / PM: Move device PM functions related to sleep states

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

Introduce helper function returning the target sleep state of the
system and use it to move the remaining device power management
functions from sleep.c to device_pm.c.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/device_pm.c | 54 ++++++++++++++++++++++++++++++++++++++++
drivers/acpi/sleep.c | 63 ++++-------------------------------------------
include/acpi/acpi_bus.h | 1
3 files changed, 61 insertions(+), 57 deletions(-)

Index: linux/drivers/acpi/sleep.c
===================================================================
--- linux.orig/drivers/acpi/sleep.c
+++ linux/drivers/acpi/sleep.c
@@ -80,6 +80,12 @@ static int acpi_sleep_prepare(u32 acpi_s

#ifdef CONFIG_ACPI_SLEEP
static u32 acpi_target_sleep_state = ACPI_STATE_S0;
+
+u32 acpi_target_system_state(void)
+{
+ return acpi_target_sleep_state;
+}
+
static bool pwr_btn_event_pending;

/*
@@ -695,63 +701,6 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}

-#ifdef CONFIG_PM
-/**
- * acpi_pm_device_sleep_state - Get preferred power state of ACPI device.
- * @dev: Device whose preferred target power state to return.
- * @d_min_p: Location to store the upper limit of the allowed states range.
- * @d_max_in: Deepest low-power state to take into consideration.
- * Return value: Preferred power state of the device on success, -ENODEV
- * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
- *
- * The caller must ensure that @dev is valid before using this function.
- */
-int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p, int d_max_in)
-{
- acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
- struct acpi_device *adev;
-
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
- dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
- return -ENODEV;
- }
-
- return acpi_device_power_state(dev, adev, acpi_target_sleep_state,
- d_max_in, d_min_p);
-}
-EXPORT_SYMBOL(acpi_pm_device_sleep_state);
-#endif /* CONFIG_PM */
-
-#ifdef CONFIG_PM_SLEEP
-/**
- * acpi_pm_device_sleep_wake - Enable or disable device to wake up the system.
- * @dev: Device to enable/desible to wake up the system from sleep states.
- * @enable: Whether to enable or disable @dev to wake up the system.
- */
-int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
-{
- acpi_handle handle;
- struct acpi_device *adev;
- int error;
-
- if (!device_can_wakeup(dev))
- return -EINVAL;
-
- handle = DEVICE_ACPI_HANDLE(dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
- dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
- return -ENODEV;
- }
-
- error = __acpi_device_sleep_wake(adev, acpi_target_sleep_state, enable);
- if (!error)
- dev_info(dev, "System wakeup %s by ACPI\n",
- enable ? "enabled" : "disabled");
-
- return error;
-}
-#endif /* CONFIG_PM_SLEEP */
-
static void acpi_power_off_prepare(void)
{
/* Prepare to power off the system */
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -416,6 +416,7 @@ int acpi_enable_wakeup_device_power(stru
int acpi_disable_wakeup_device_power(struct acpi_device *dev);

#ifdef CONFIG_PM
+u32 acpi_target_system_state(void);
acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
acpi_notify_handler handler, void *context);
acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
Index: linux/drivers/acpi/device_pm.c
===================================================================
--- linux.orig/drivers/acpi/device_pm.c
+++ linux/drivers/acpi/device_pm.c
@@ -198,6 +198,31 @@ int acpi_device_power_state(struct devic
}
EXPORT_SYMBOL_GPL(acpi_device_power_state);

+/**
+ * acpi_pm_device_sleep_state - Get preferred power state of ACPI device.
+ * @dev: Device whose preferred target power state to return.
+ * @d_min_p: Location to store the upper limit of the allowed states range.
+ * @d_max_in: Deepest low-power state to take into consideration.
+ * Return value: Preferred power state of the device on success, -ENODEV
+ * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
+ *
+ * The caller must ensure that @dev is valid before using this function.
+ */
+int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p, int d_max_in)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+ struct acpi_device *adev;
+
+ if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
+ dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
+ return -ENODEV;
+ }
+
+ return acpi_device_power_state(dev, adev, acpi_target_system_state(),
+ d_max_in, d_min_p);
+}
+EXPORT_SYMBOL(acpi_pm_device_sleep_state);
+
#ifdef CONFIG_PM_RUNTIME
/**
* __acpi_device_run_wake - Enable/disable runtime remote wakeup for device.
@@ -274,4 +299,33 @@ int __acpi_device_sleep_wake(struct acpi
acpi_enable_wakeup_device_power(adev, target_state) :
acpi_disable_wakeup_device_power(adev);
}
+
+/**
+ * acpi_pm_device_sleep_wake - Enable or disable device to wake up the system.
+ * @dev: Device to enable/desible to wake up the system from sleep states.
+ * @enable: Whether to enable or disable @dev to wake up the system.
+ */
+int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
+{
+ acpi_handle handle;
+ struct acpi_device *adev;
+ int error;
+
+ if (!device_can_wakeup(dev))
+ return -EINVAL;
+
+ handle = DEVICE_ACPI_HANDLE(dev);
+ if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
+ dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
+ return -ENODEV;
+ }
+
+ error = __acpi_device_sleep_wake(adev, acpi_target_system_state(),
+ enable);
+ if (!error)
+ dev_info(dev, "System wakeup %s by ACPI\n",
+ enable ? "enabled" : "disabled");
+
+ return error;
+}
#endif /* CONFIG_PM_SLEEP */

2012-10-29 09:10:19

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 7/7] ACPI / PM: Provide ACPI PM callback routines for subsystems

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

Some bus types don't support power management natively, but generally
there may be device nodes in ACPI tables corresponding to the devices
whose bus types they are (under ACPI 5 those bus types may be SPI,
I2C and platform). If that is the case, standard ACPI power
management may be applied to those devices, although currently the
kernel has no means for that.

For this reason, provide a set of routines that may be used as power
management callbacks for such devices. This may be done in three
different ways.

(1) Device drivers handling the devices in question may run
acpi_dev_pm_attach() in their .probe() routines, which (on
success) will cause the devices to be added to the general ACPI
PM domain and ACPI power management will be used for them going
forward. Then, acpi_dev_pm_detach() may be used to remove the
devices from the general ACPI PM domain if ACPI power management
is not necessary for them any more.

(2) The devices' subsystems may use acpi_subsys_runtime_suspend(),
acpi_subsys_runtime_resume(), acpi_subsys_prepare(),
acpi_subsys_suspend_late(), acpi_subsys_resume_early() as their
power management callbacks in the same way as the general ACPI
PM domain does that.

(3) The devices' drivers may execute acpi_dev_suspend_late(),
acpi_dev_resume_early(), acpi_dev_runtime_suspend(),
acpi_dev_runtime_resume() from their power management callbacks
as appropriate, if that's absolutely necessary, but it is not
recommended to do that, because such drivers may not work
without ACPI support as a result.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/device_pm.c | 314 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 34 +++++
2 files changed, 348 insertions(+)

Index: linux/drivers/acpi/device_pm.c
===================================================================
--- linux.orig/drivers/acpi/device_pm.c
+++ linux/drivers/acpi/device_pm.c
@@ -225,6 +225,22 @@ EXPORT_SYMBOL(acpi_pm_device_sleep_state

#ifdef CONFIG_PM_RUNTIME
/**
+ * acpi_wakeup_device - Wakeup notification handler for ACPI devices.
+ * @handle: ACPI handle of the device the notification is for.
+ * @event: Type of the signaled event.
+ * @context: Device corresponding to @handle.
+ */
+static void acpi_wakeup_device(acpi_handle handle, u32 event, void *context)
+{
+ struct device *dev = context;
+
+ if (event == ACPI_NOTIFY_DEVICE_WAKE && dev) {
+ pm_wakeup_event(dev, 0);
+ pm_runtime_resume(dev);
+ }
+}
+
+/**
* __acpi_device_run_wake - Enable/disable runtime remote wakeup for device.
* @adev: ACPI device to enable/disable the remote wakeup for.
* @enable: Whether to enable or disable the wakeup functionality.
@@ -329,3 +345,301 @@ int acpi_pm_device_sleep_wake(struct dev
return error;
}
#endif /* CONFIG_PM_SLEEP */
+
+/**
+ * acpi_dev_pm_get_node - Get ACPI device node for the given physical device.
+ * @dev: Device to get the ACPI node for.
+ */
+static struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
+{
+ acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+ struct acpi_device *adev;
+
+ return handle && ACPI_SUCCESS(acpi_bus_get_device(handle, &adev)) ?
+ adev : NULL;
+}
+
+/**
+ * acpi_dev_pm_low_power - Put ACPI device into a low-power state.
+ * @dev: Device to put into a low-power state.
+ * @adev: ACPI device node corresponding to @dev.
+ * @system_state: System state to choose the device state for.
+ */
+static int acpi_dev_pm_low_power(struct device *dev, struct acpi_device *adev,
+ u32 system_state)
+{
+ int power_state;
+
+ if (!acpi_device_power_manageable(adev))
+ return 0;
+
+ power_state = acpi_device_power_state(dev, adev, system_state,
+ ACPI_STATE_D3, NULL);
+ if (power_state < ACPI_STATE_D0 || power_state > ACPI_STATE_D3)
+ return -EIO;
+
+ return acpi_device_set_power(adev, power_state);
+}
+
+/**
+ * acpi_dev_pm_full_power - Put ACPI device into the full-power state.
+ * @adev: ACPI device node to put into the full-power state.
+ */
+static int acpi_dev_pm_full_power(struct acpi_device *adev)
+{
+ return acpi_device_power_manageable(adev) ?
+ acpi_device_set_power(adev, ACPI_STATE_D0) : 0;
+}
+
+#ifdef CONFIG_PM_RUNTIME
+/**
+ * acpi_dev_runtime_suspend - Put device into a low-power state using ACPI.
+ * @dev: Device to put into a low-power state.
+ *
+ * Put the given device into a runtime low-power state using the standard ACPI
+ * mechanism. Set up remote wakeup if desired, choose the state to put the
+ * device into (this checks if remote wakeup is expected to work too), and set
+ * the power state of the device.
+ */
+int acpi_dev_runtime_suspend(struct device *dev)
+{
+ struct acpi_device *adev = acpi_dev_pm_get_node(dev);
+ bool remote_wakeup;
+ int error;
+
+ if (!adev)
+ return 0;
+
+ remote_wakeup = dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) >
+ PM_QOS_FLAGS_NONE;
+ error = __acpi_device_run_wake(adev, remote_wakeup);
+ if (remote_wakeup && error)
+ return -EAGAIN;
+
+ error = acpi_dev_pm_low_power(dev, adev, ACPI_STATE_S0);
+ if (error)
+ __acpi_device_run_wake(adev, false);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_runtime_suspend);
+
+/**
+ * acpi_dev_runtime_resume - Put device into the full-power state using ACPI.
+ * @dev: Device to put into the full-power state.
+ *
+ * Put the given device into the full-power state using the standard ACPI
+ * mechanism at run time. Set the power state of the device to ACPI D0 and
+ * disable remote wakeup.
+ */
+int acpi_dev_runtime_resume(struct device *dev)
+{
+ struct acpi_device *adev = acpi_dev_pm_get_node(dev);
+ int error;
+
+ if (!adev)
+ return 0;
+
+ error = acpi_dev_pm_full_power(adev);
+ __acpi_device_run_wake(adev, false);
+ return error;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_runtime_resume);
+
+/**
+ * acpi_subsys_runtime_suspend - Suspend device using ACPI.
+ * @dev: Device to suspend.
+ *
+ * Carry out the generic runtime suspend procedure for @dev and use ACPI to put
+ * it into a runtime low-power state.
+ */
+int acpi_subsys_runtime_suspend(struct device *dev)
+{
+ int ret = pm_generic_runtime_suspend(dev);
+ return ret ? ret : acpi_dev_runtime_suspend(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_subsys_runtime_suspend);
+
+/**
+ * acpi_subsys_runtime_resume - Resume device using ACPI.
+ * @dev: Device to Resume.
+ *
+ * Use ACPI to put the given device into the full-power state and carry out the
+ * generic runtime resume procedure for it.
+ */
+int acpi_subsys_runtime_resume(struct device *dev)
+{
+ int ret = acpi_dev_runtime_resume(dev);
+ return ret ? ret : pm_generic_runtime_resume(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_subsys_runtime_resume);
+#endif /* CONFIG_PM_RUNTIME */
+
+#ifdef CONFIG_PM_SLEEP
+/**
+ * acpi_dev_suspend_late - Put device into a low-power state using ACPI.
+ * @dev: Device to put into a low-power state.
+ *
+ * Put the given device into a low-power state during system transition to a
+ * sleep state using the standard ACPI mechanism. Set up system wakeup if
+ * desired, choose the state to put the device into (this checks if system
+ * wakeup is expected to work too), and set the power state of the device.
+ */
+int acpi_dev_suspend_late(struct device *dev)
+{
+ struct acpi_device *adev = acpi_dev_pm_get_node(dev);
+ u32 target_state;
+ bool wakeup;
+ int error;
+
+ if (!adev)
+ return 0;
+
+ target_state = acpi_target_system_state();
+ wakeup = device_may_wakeup(dev);
+ error = __acpi_device_sleep_wake(adev, target_state, wakeup);
+ if (wakeup && error)
+ return error;
+
+ error = acpi_dev_pm_low_power(dev, adev, target_state);
+ if (error)
+ __acpi_device_sleep_wake(adev, ACPI_STATE_UNKNOWN, false);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_suspend_late);
+
+/**
+ * acpi_dev_resume_early - Put device into the full-power state using ACPI.
+ * @dev: Device to put into the full-power state.
+ *
+ * Put the given device into the full-power state using the standard ACPI
+ * mechanism during system transition to the working state. Set the power
+ * state of the device to ACPI D0 and disable remote wakeup.
+ */
+int acpi_dev_resume_early(struct device *dev)
+{
+ struct acpi_device *adev = acpi_dev_pm_get_node(dev);
+ int error;
+
+ if (!adev)
+ return 0;
+
+ error = acpi_dev_pm_full_power(adev);
+ __acpi_device_sleep_wake(adev, ACPI_STATE_UNKNOWN, false);
+ return error;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_resume_early);
+
+/**
+ * acpi_subsys_prepare - Prepare device for system transition to a sleep state.
+ * @dev: Device to prepare.
+ */
+int acpi_subsys_prepare(struct device *dev)
+{
+ /*
+ * Follow PCI and resume devices suspended at run time before running
+ * their system suspend callbacks.
+ */
+ pm_runtime_resume(dev);
+ return pm_generic_prepare(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_subsys_prepare);
+
+/**
+ * acpi_subsys_suspend_late - Suspend device using ACPI.
+ * @dev: Device to suspend.
+ *
+ * Carry out the generic late suspend procedure for @dev and use ACPI to put
+ * it into a low-power state during system transition into a sleep state.
+ */
+int acpi_subsys_suspend_late(struct device *dev)
+{
+ int ret = pm_generic_suspend_late(dev);
+ return ret ? ret : acpi_dev_suspend_late(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_subsys_suspend_late);
+
+/**
+ * acpi_subsys_resume_early - Resume device using ACPI.
+ * @dev: Device to Resume.
+ *
+ * Use ACPI to put the given device into the full-power state and carry out the
+ * generic early resume procedure for it during system transition into the
+ * working state.
+ */
+int acpi_subsys_resume_early(struct device *dev)
+{
+ int ret = acpi_dev_runtime_resume(dev);
+ return ret ? ret : pm_generic_resume_early(dev);
+}
+EXPORT_SYMBOL_GPL(acpi_subsys_resume_early);
+#endif /* CONFIG_PM_SLEEP */
+
+static struct dev_pm_domain acpi_general_pm_domain = {
+ .ops = {
+#ifdef CONFIG_PM_RUNTIME
+ .runtime_suspend = acpi_subsys_runtime_suspend,
+ .runtime_resume = acpi_subsys_runtime_resume,
+ .runtime_idle = pm_generic_runtime_idle,
+#endif
+#ifdef CONFIG_PM_SLEEP
+ .prepare = acpi_subsys_prepare,
+ .suspend_late = acpi_subsys_suspend_late,
+ .resume_early = acpi_subsys_resume_early,
+ .poweroff_late = acpi_subsys_suspend_late,
+ .restore_early = acpi_subsys_resume_early,
+#endif
+ },
+};
+
+/**
+ * acpi_dev_pm_attach - Prepare device for ACPI power management.
+ * @dev: Device to prepare.
+ *
+ * If @dev has a valid ACPI handle that has a valid struct acpi_device object
+ * attached to it, install a wakeup notification handler for the device and
+ * add it to the general ACPI PM domain.
+ *
+ * This assumes that the @dev's bus type uses generic power management callbacks
+ * (or doesn't use any power management callbacks at all).
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ */
+int acpi_dev_pm_attach(struct device *dev)
+{
+ struct acpi_device *adev = acpi_dev_pm_get_node(dev);
+
+ if (!adev)
+ return -ENODEV;
+
+ if (dev->pm_domain)
+ return -EEXIST;
+
+ acpi_add_pm_notifier(adev, acpi_wakeup_device, dev);
+ dev->pm_domain = &acpi_general_pm_domain;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
+
+/**
+ * acpi_dev_pm_detach - Remove ACPI power management from the device.
+ * @dev: Device to take care of.
+ *
+ * Remove the device from the general ACPI PM domain and remove its wakeup
+ * notifier.
+ *
+ * Callers must ensure proper synchronization of this function with power
+ * management callbacks.
+ */
+void acpi_dev_pm_detach(struct device *dev)
+{
+ struct acpi_device *adev = acpi_dev_pm_get_node(dev);
+
+ if (adev && dev->pm_domain == &acpi_general_pm_domain) {
+ dev->pm_domain = NULL;
+ acpi_remove_pm_notifier(adev, acpi_wakeup_device);
+ }
+}
+EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
Index: linux/include/linux/acpi.h
===================================================================
--- linux.orig/include/linux/acpi.h
+++ linux/include/linux/acpi.h
@@ -434,4 +434,38 @@ acpi_status acpi_os_prepare_sleep(u8 sle
#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
#endif

+#if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
+int acpi_dev_runtime_suspend(struct device *dev);
+int acpi_dev_runtime_resume(struct device *dev);
+int acpi_subsys_runtime_suspend(struct device *dev);
+int acpi_subsys_runtime_resume(struct device *dev);
+#else
+static inline int acpi_dev_runtime_suspend(struct device *dev) { return 0; }
+static inline int acpi_dev_runtime_resume(struct device *dev) { return 0; }
+static inline int acpi_subsys_runtime_suspend(struct device *dev) { return 0; }
+static inline int acpi_subsys_runtime_resume(struct device *dev) { return 0; }
+#endif
+
+#ifdef CONFIG_ACPI_SLEEP
+int acpi_dev_suspend_late(struct device *dev);
+int acpi_dev_resume_early(struct device *dev);
+int acpi_subsys_prepare(struct device *dev);
+int acpi_subsys_suspend_late(struct device *dev);
+int acpi_subsys_resume_early(struct device *dev);
+#else
+static inline int acpi_dev_suspend_late(struct device *dev) { return 0; }
+static inline int acpi_dev_resume_early(struct device *dev) { return 0; }
+static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
+static inline int acpi_subsys_suspend_late(struct device *dev) { return 0; }
+static inline int acpi_subsys_resume_early(struct device *dev) { return 0; }
+#endif
+
+#if defined(CONFIG_ACPI) && defined(CONFIG_PM)
+int acpi_dev_pm_attach(struct device *dev);
+int acpi_dev_pm_detach(struct device *dev);
+#else
+static inline int acpi_dev_pm_attach(struct device *dev) { return -ENODEV; }
+static inline void acpi_dev_pm_detach(struct device *dev) {}
+#endif
+
#endif /*_LINUX_ACPI_H*/

2012-10-29 09:11:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 3/7] ACPI / PM: Move runtime remote wakeup setup routine to device_pm.c

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

The ACPI function for setting up devices to do runtime remote
wakeup is now located in drivers/acpi/sleep.c, but
drivers/acpi/device_pm.c is a more logical place for it, so move it
there.

No functional changes should result from this modification.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/device_pm.c | 39 +++++++++++++++++++++++++++++++++++++++
drivers/acpi/sleep.c | 39 ---------------------------------------
2 files changed, 39 insertions(+), 39 deletions(-)

Index: linux/drivers/acpi/device_pm.c
===================================================================
--- linux.orig/drivers/acpi/device_pm.c
+++ linux/drivers/acpi/device_pm.c
@@ -26,6 +26,7 @@
#include <linux/export.h>
#include <linux/mutex.h>
#include <linux/pm_qos.h>
+#include <linux/pm_runtime.h>

#include <acpi/acpi.h>
#include <acpi/acpi_bus.h>
@@ -196,3 +197,41 @@ int acpi_device_power_state(struct devic
return d_max;
}
EXPORT_SYMBOL_GPL(acpi_device_power_state);
+
+#ifdef CONFIG_PM_RUNTIME
+/**
+ * acpi_pm_device_run_wake - Enable/disable remote wakeup for given device.
+ * @phys_dev: Device to enable/disable the platform to wake up.
+ * @enable: Whether to enable or disable the wakeup functionality.
+ *
+ * Find the ACPI device object corresponding to @phys_dev and try to
+ * enable/disable the GPE associated with it, so that it can generate
+ * wakeup signals for the device in response to external (remote) events.
+ */
+int acpi_pm_device_run_wake(struct device *phys_dev, bool enable)
+{
+ struct acpi_device *dev;
+ acpi_handle handle;
+
+ if (!device_run_wake(phys_dev))
+ return -EINVAL;
+
+ handle = DEVICE_ACPI_HANDLE(phys_dev);
+ if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
+ dev_dbg(phys_dev, "ACPI handle has no context in %s!\n",
+ __func__);
+ return -ENODEV;
+ }
+
+ if (enable) {
+ acpi_enable_wakeup_device_power(dev, ACPI_STATE_S0);
+ acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
+ } else {
+ acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
+ acpi_disable_wakeup_device_power(dev);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_pm_device_run_wake);
+#endif /* CONFIG_PM_RUNTIME */
Index: linux/drivers/acpi/sleep.c
===================================================================
--- linux.orig/drivers/acpi/sleep.c
+++ linux/drivers/acpi/sleep.c
@@ -18,7 +18,6 @@
#include <linux/reboot.h>
#include <linux/acpi.h>
#include <linux/module.h>
-#include <linux/pm_runtime.h>

#include <asm/io.h>

@@ -723,44 +722,6 @@ int acpi_pm_device_sleep_state(struct de
EXPORT_SYMBOL(acpi_pm_device_sleep_state);
#endif /* CONFIG_PM */

-#ifdef CONFIG_PM_RUNTIME
-/**
- * acpi_pm_device_run_wake - Enable/disable remote wakeup for given device.
- * @phys_dev: Device to enable/disable the platform to wake up.
- * @enable: Whether to enable or disable the wakeup functionality.
- *
- * Find the ACPI device object corresponding to @phys_dev and try to
- * enable/disable the GPE associated with it, so that it can generate
- * wakeup signals for the device in response to external (remote) events.
- */
-int acpi_pm_device_run_wake(struct device *phys_dev, bool enable)
-{
- struct acpi_device *dev;
- acpi_handle handle;
-
- if (!device_run_wake(phys_dev))
- return -EINVAL;
-
- handle = DEVICE_ACPI_HANDLE(phys_dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
- dev_dbg(phys_dev, "ACPI handle has no context in %s!\n",
- __func__);
- return -ENODEV;
- }
-
- if (enable) {
- acpi_enable_wakeup_device_power(dev, ACPI_STATE_S0);
- acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
- } else {
- acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
- acpi_disable_wakeup_device_power(dev);
- }
-
- return 0;
-}
-EXPORT_SYMBOL(acpi_pm_device_run_wake);
-#endif /* CONFIG_PM_RUNTIME */
-
#ifdef CONFIG_PM_SLEEP
/**
* acpi_pm_device_sleep_wake - Enable or disable device to wake up the system.

2012-10-29 09:12:09

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 4/7] ACPI / PM: Split device wakeup management routines

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

Two device wakeup management routines in device_pm.c and sleep.c,
acpi_pm_device_run_wake() and acpi_pm_device_sleep_wake(), take a
device pointer argument and use it to obtain the ACPI handle of the
corresponding ACPI namespace node. That handle is then used to get
the address of the struct acpi_device object corresponding to the
struct device passed as the argument.

Unfortunately, that last operation may be costly, because it involves
taking the global ACPI namespace mutex, so it shouldn't be carried
out too often. However, the callers of those routines usually call
them in a row with acpi_pm_device_sleep_state() which also takes that
mutex for the same reason, so it would be more efficient if they ran
acpi_bus_get_device() themselves to obtain a pointer to the struct
acpi_device object in question and then passed that pointer to the
appropriate PM routines.

To make that possible, split each of the PM routines mentioned above
in two parts, one taking a struct acpi_device pointer argument and
the other implementing the current interface for compatibility.

Additionally, change acpi_pm_device_run_wake() to actually return
an error code if there is an error while setting up runtime remote
wakeup for the device.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/device_pm.c | 74 ++++++++++++++++++++++++++++++++++++-----------
drivers/acpi/sleep.c | 8 +----
include/acpi/acpi_bus.h | 11 ++++++
3 files changed, 71 insertions(+), 22 deletions(-)

Index: linux/drivers/acpi/device_pm.c
===================================================================
--- linux.orig/drivers/acpi/device_pm.c
+++ linux/drivers/acpi/device_pm.c
@@ -200,38 +200,78 @@ EXPORT_SYMBOL_GPL(acpi_device_power_stat

#ifdef CONFIG_PM_RUNTIME
/**
- * acpi_pm_device_run_wake - Enable/disable remote wakeup for given device.
- * @phys_dev: Device to enable/disable the platform to wake up.
+ * __acpi_device_run_wake - Enable/disable runtime remote wakeup for device.
+ * @adev: ACPI device to enable/disable the remote wakeup for.
* @enable: Whether to enable or disable the wakeup functionality.
*
- * Find the ACPI device object corresponding to @phys_dev and try to
- * enable/disable the GPE associated with it, so that it can generate
- * wakeup signals for the device in response to external (remote) events.
+ * Enable/disable the GPE associated with @adev so that it can generate
+ * wakeup signals for the device in response to external (remote) events and
+ * enable/disable device wakeup power.
+ *
+ * Callers must ensure that @adev is a valid ACPI device node before executing
+ * this function.
+ */
+int __acpi_device_run_wake(struct acpi_device *adev, bool enable)
+{
+ struct acpi_device_wakeup *wakeup = &adev->wakeup;
+
+ if (enable) {
+ acpi_status res;
+ int error;
+
+ error = acpi_enable_wakeup_device_power(adev, ACPI_STATE_S0);
+ if (error)
+ return error;
+
+ res = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+ if (ACPI_FAILURE(res)) {
+ acpi_disable_wakeup_device_power(adev);
+ return -EIO;
+ }
+ } else {
+ acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+ acpi_disable_wakeup_device_power(adev);
+ }
+ return 0;
+}
+
+/**
+ * acpi_pm_device_run_wake - Enable/disable remote wakeup for given device.
+ * @dev: Device to enable/disable the platform to wake up.
+ * @enable: Whether to enable or disable the wakeup functionality.
*/
int acpi_pm_device_run_wake(struct device *phys_dev, bool enable)
{
- struct acpi_device *dev;
+ struct acpi_device *adev;
acpi_handle handle;

if (!device_run_wake(phys_dev))
return -EINVAL;

handle = DEVICE_ACPI_HANDLE(phys_dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
- dev_dbg(phys_dev, "ACPI handle has no context in %s!\n",
+ if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
+ dev_dbg(phys_dev, "ACPI handle without context in %s!\n",
__func__);
return -ENODEV;
}

- if (enable) {
- acpi_enable_wakeup_device_power(dev, ACPI_STATE_S0);
- acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
- } else {
- acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
- acpi_disable_wakeup_device_power(dev);
- }
-
- return 0;
+ return __acpi_device_run_wake(adev, enable);
}
EXPORT_SYMBOL(acpi_pm_device_run_wake);
#endif /* CONFIG_PM_RUNTIME */
+
+ #ifdef CONFIG_PM_SLEEP
+/**
+ * __acpi_device_sleep_wake - Enable or disable device to wake up the system.
+ * @dev: Device to enable/desible to wake up the system.
+ * @target_state: System state the device is supposed to wake up from.
+ * @enable: Whether to enable or disable @dev to wake up the system.
+ */
+int __acpi_device_sleep_wake(struct acpi_device *adev, u32 target_state,
+ bool enable)
+{
+ return enable ?
+ acpi_enable_wakeup_device_power(adev, target_state) :
+ acpi_disable_wakeup_device_power(adev);
+}
+#endif /* CONFIG_PM_SLEEP */
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -454,8 +454,13 @@ static inline int acpi_pm_device_sleep_s
#endif

#ifdef CONFIG_PM_RUNTIME
+int __acpi_device_run_wake(struct acpi_device *, bool);
int acpi_pm_device_run_wake(struct device *, bool);
#else
+static inline int __acpi_device_run_wake(struct acpi_device *adev, bool en)
+{
+ return -ENODEV;
+}
static inline int acpi_pm_device_run_wake(struct device *dev, bool enable)
{
return -ENODEV;
@@ -463,8 +468,14 @@ static inline int acpi_pm_device_run_wak
#endif

#ifdef CONFIG_PM_SLEEP
+int __acpi_device_sleep_wake(struct acpi_device *, u32, bool);
int acpi_pm_device_sleep_wake(struct device *, bool);
#else
+static inline int __acpi_device_sleep_wake(struct acpi_device *adev,
+ u32 target_state, bool enable)
+{
+ return -ENODEV;
+}
static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
{
return -ENODEV;
Index: linux/drivers/acpi/sleep.c
===================================================================
--- linux.orig/drivers/acpi/sleep.c
+++ linux/drivers/acpi/sleep.c
@@ -739,15 +739,13 @@ int acpi_pm_device_sleep_wake(struct dev

handle = DEVICE_ACPI_HANDLE(dev);
if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
- dev_dbg(dev, "ACPI handle has no context in %s!\n", __func__);
+ dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
return -ENODEV;
}

- error = enable ?
- acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) :
- acpi_disable_wakeup_device_power(adev);
+ error = __acpi_device_sleep_wake(adev, acpi_target_sleep_state, enable);
if (!error)
- dev_info(dev, "wake-up capability %s by ACPI\n",
+ dev_info(dev, "System wakeup %s by ACPI\n",
enable ? "enabled" : "disabled");

return error;

2012-10-30 07:28:50

by Aaron Lu

[permalink] [raw]
Subject: Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> If the caller of acpi_bus_set_power() already has a pointer to the
> struct acpi_device object corresponding to the device in question, it
> doesn't make sense for it to go through acpi_bus_get_device(), which
> may be costly, because it involves acquiring the global ACPI
> namespace mutex.
>
> For this reason, export the function operating on struct acpi_device
> objects used internally by acpi_bus_set_power(), so that it may be
> called instead of acpi_bus_set_power() in the above case, and change
> its name to acpi_device_set_power().
>
> Additionally, introduce two inline wrappers for checking ACPI PM
> capabilities of devices represented by struct acpi_device objects.

What about adding yet another wrapper to check power off capability of
the device? If device has _PS3 or _PRx, it means the device can be
powered off from ACPI's perspective. This is useful for ZPODD when
deciding if platform has the required ability to support it.

Thanks,
Aaron

>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/bus.c | 15 ++++++++++++---
> include/acpi/acpi_bus.h | 11 +++++++++++
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> Index: linux/drivers/acpi/bus.c
> ===================================================================
> --- linux.orig/drivers/acpi/bus.c
> +++ linux/drivers/acpi/bus.c
> @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
> }
>
>
> -static int __acpi_bus_set_power(struct acpi_device *device, int state)
> +/**
> + * acpi_device_set_power - Set power state of an ACPI device.
> + * @device: Device to set the power state of.
> + * @state: New power state to set.
> + *
> + * Callers must ensure that the device is power manageable before using this
> + * function.
> + */
> +int acpi_device_set_power(struct acpi_device *device, int state)
> {
> int result = 0;
> acpi_status status = AE_OK;
> @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
>
> return result;
> }
> +EXPORT_SYMBOL(acpi_device_set_power);
>
>
> int acpi_bus_set_power(acpi_handle handle, int state)
> @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
> return -ENODEV;
> }
>
> - return __acpi_bus_set_power(device, state);
> + return acpi_device_set_power(device, state);
> }
> EXPORT_SYMBOL(acpi_bus_set_power);
>
> @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
> if (result)
> return result;
>
> - result = __acpi_bus_set_power(device, state);
> + result = acpi_device_set_power(device, state);
> if (!result && state_p)
> *state_p = state;
>
> Index: linux/include/acpi/acpi_bus.h
> ===================================================================
> --- linux.orig/include/acpi/acpi_bus.h
> +++ linux/include/acpi/acpi_bus.h
> @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
> unsigned long long *sta);
> int acpi_bus_get_status(struct acpi_device *device);
> int acpi_bus_set_power(acpi_handle handle, int state);
> +int acpi_device_set_power(struct acpi_device *device, int state);
> int acpi_bus_update_power(acpi_handle handle, int *state_p);
> bool acpi_bus_power_manageable(acpi_handle handle);
> bool acpi_bus_can_wakeup(acpi_handle handle);
> @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
> }
> #endif
>
> +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
> +{
> + return adev->flags.power_manageable;
> +}
> +
> +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
> +{
> + return adev->wakeup.flags.valid;
> +}
> +
> #else /* CONFIG_ACPI */
>
> static inline int register_acpi_bus_type(void *bus) { return 0; }
>

2012-10-30 15:16:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > If the caller of acpi_bus_set_power() already has a pointer to the
> > struct acpi_device object corresponding to the device in question, it
> > doesn't make sense for it to go through acpi_bus_get_device(), which
> > may be costly, because it involves acquiring the global ACPI
> > namespace mutex.
> >
> > For this reason, export the function operating on struct acpi_device
> > objects used internally by acpi_bus_set_power(), so that it may be
> > called instead of acpi_bus_set_power() in the above case, and change
> > its name to acpi_device_set_power().
> >
> > Additionally, introduce two inline wrappers for checking ACPI PM
> > capabilities of devices represented by struct acpi_device objects.
>
> What about adding yet another wrapper to check power off capability of
> the device? If device has _PS3 or _PRx, it means the device can be
> powered off from ACPI's perspective. This is useful for ZPODD when
> deciding if platform has the required ability to support it.

Sure, no problem with that. Perhaps you can cut a patch for that
on top of this series?

Rafael


> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/acpi/bus.c | 15 ++++++++++++---
> > include/acpi/acpi_bus.h | 11 +++++++++++
> > 2 files changed, 23 insertions(+), 3 deletions(-)
> >
> > Index: linux/drivers/acpi/bus.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/bus.c
> > +++ linux/drivers/acpi/bus.c
> > @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
> > }
> >
> >
> > -static int __acpi_bus_set_power(struct acpi_device *device, int state)
> > +/**
> > + * acpi_device_set_power - Set power state of an ACPI device.
> > + * @device: Device to set the power state of.
> > + * @state: New power state to set.
> > + *
> > + * Callers must ensure that the device is power manageable before using this
> > + * function.
> > + */
> > +int acpi_device_set_power(struct acpi_device *device, int state)
> > {
> > int result = 0;
> > acpi_status status = AE_OK;
> > @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
> >
> > return result;
> > }
> > +EXPORT_SYMBOL(acpi_device_set_power);
> >
> >
> > int acpi_bus_set_power(acpi_handle handle, int state)
> > @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
> > return -ENODEV;
> > }
> >
> > - return __acpi_bus_set_power(device, state);
> > + return acpi_device_set_power(device, state);
> > }
> > EXPORT_SYMBOL(acpi_bus_set_power);
> >
> > @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
> > if (result)
> > return result;
> >
> > - result = __acpi_bus_set_power(device, state);
> > + result = acpi_device_set_power(device, state);
> > if (!result && state_p)
> > *state_p = state;
> >
> > Index: linux/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux.orig/include/acpi/acpi_bus.h
> > +++ linux/include/acpi/acpi_bus.h
> > @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
> > unsigned long long *sta);
> > int acpi_bus_get_status(struct acpi_device *device);
> > int acpi_bus_set_power(acpi_handle handle, int state);
> > +int acpi_device_set_power(struct acpi_device *device, int state);
> > int acpi_bus_update_power(acpi_handle handle, int *state_p);
> > bool acpi_bus_power_manageable(acpi_handle handle);
> > bool acpi_bus_can_wakeup(acpi_handle handle);
> > @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
> > }
> > #endif
> >
> > +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
> > +{
> > + return adev->flags.power_manageable;
> > +}
> > +
> > +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
> > +{
> > + return adev->wakeup.flags.valid;
> > +}
> > +
> > #else /* CONFIG_ACPI */
> >
> > static inline int register_acpi_bus_type(void *bus) { return 0; }
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2012-11-02 05:17:28

by Aaron Lu

[permalink] [raw]
Subject: Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
> On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
>> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <[email protected]>
>>>
>>> If the caller of acpi_bus_set_power() already has a pointer to the
>>> struct acpi_device object corresponding to the device in question, it
>>> doesn't make sense for it to go through acpi_bus_get_device(), which
>>> may be costly, because it involves acquiring the global ACPI
>>> namespace mutex.
>>>
>>> For this reason, export the function operating on struct acpi_device
>>> objects used internally by acpi_bus_set_power(), so that it may be
>>> called instead of acpi_bus_set_power() in the above case, and change
>>> its name to acpi_device_set_power().
>>>
>>> Additionally, introduce two inline wrappers for checking ACPI PM
>>> capabilities of devices represented by struct acpi_device objects.
>>
>> What about adding yet another wrapper to check power off capability of
>> the device? If device has _PS3 or _PRx, it means the device can be
>> powered off from ACPI's perspective. This is useful for ZPODD when
>> deciding if platform has the required ability to support it.
>
> Sure, no problem with that. Perhaps you can cut a patch for that
> on top of this series?

Do you think it is reasonable to add a new field to acpi_state.flags to
represent if we, as OSPM, have a way to put the device into a ACPI
device state? This field can be set once in acpi_bus_get_power_flags and
used afterwards.

The valid field of acpi_state.flags is what we have today, and it means
whether this ACPI device state is valid for the device, but not that if
OSPM can actually put the device into that power state.

Thanks,
Aaron

>
> Rafael
>
>
>>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>>> ---
>>> drivers/acpi/bus.c | 15 ++++++++++++---
>>> include/acpi/acpi_bus.h | 11 +++++++++++
>>> 2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> Index: linux/drivers/acpi/bus.c
>>> ===================================================================
>>> --- linux.orig/drivers/acpi/bus.c
>>> +++ linux/drivers/acpi/bus.c
>>> @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
>>> }
>>>
>>>
>>> -static int __acpi_bus_set_power(struct acpi_device *device, int state)
>>> +/**
>>> + * acpi_device_set_power - Set power state of an ACPI device.
>>> + * @device: Device to set the power state of.
>>> + * @state: New power state to set.
>>> + *
>>> + * Callers must ensure that the device is power manageable before using this
>>> + * function.
>>> + */
>>> +int acpi_device_set_power(struct acpi_device *device, int state)
>>> {
>>> int result = 0;
>>> acpi_status status = AE_OK;
>>> @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
>>>
>>> return result;
>>> }
>>> +EXPORT_SYMBOL(acpi_device_set_power);
>>>
>>>
>>> int acpi_bus_set_power(acpi_handle handle, int state)
>>> @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
>>> return -ENODEV;
>>> }
>>>
>>> - return __acpi_bus_set_power(device, state);
>>> + return acpi_device_set_power(device, state);
>>> }
>>> EXPORT_SYMBOL(acpi_bus_set_power);
>>>
>>> @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
>>> if (result)
>>> return result;
>>>
>>> - result = __acpi_bus_set_power(device, state);
>>> + result = acpi_device_set_power(device, state);
>>> if (!result && state_p)
>>> *state_p = state;
>>>
>>> Index: linux/include/acpi/acpi_bus.h
>>> ===================================================================
>>> --- linux.orig/include/acpi/acpi_bus.h
>>> +++ linux/include/acpi/acpi_bus.h
>>> @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
>>> unsigned long long *sta);
>>> int acpi_bus_get_status(struct acpi_device *device);
>>> int acpi_bus_set_power(acpi_handle handle, int state);
>>> +int acpi_device_set_power(struct acpi_device *device, int state);
>>> int acpi_bus_update_power(acpi_handle handle, int *state_p);
>>> bool acpi_bus_power_manageable(acpi_handle handle);
>>> bool acpi_bus_can_wakeup(acpi_handle handle);
>>> @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
>>> }
>>> #endif
>>>
>>> +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
>>> +{
>>> + return adev->flags.power_manageable;
>>> +}
>>> +
>>> +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
>>> +{
>>> + return adev->wakeup.flags.valid;
>>> +}
>>> +
>>> #else /* CONFIG_ACPI */
>>>
>>> static inline int register_acpi_bus_type(void *bus) { return 0; }
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>

2012-11-02 11:15:38

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device

On Friday, November 02, 2012 01:17:10 PM Aaron Lu wrote:
> On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
> > On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
> >> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <[email protected]>
> >>>
> >>> If the caller of acpi_bus_set_power() already has a pointer to the
> >>> struct acpi_device object corresponding to the device in question, it
> >>> doesn't make sense for it to go through acpi_bus_get_device(), which
> >>> may be costly, because it involves acquiring the global ACPI
> >>> namespace mutex.
> >>>
> >>> For this reason, export the function operating on struct acpi_device
> >>> objects used internally by acpi_bus_set_power(), so that it may be
> >>> called instead of acpi_bus_set_power() in the above case, and change
> >>> its name to acpi_device_set_power().
> >>>
> >>> Additionally, introduce two inline wrappers for checking ACPI PM
> >>> capabilities of devices represented by struct acpi_device objects.
> >>
> >> What about adding yet another wrapper to check power off capability of
> >> the device? If device has _PS3 or _PRx, it means the device can be
> >> powered off from ACPI's perspective. This is useful for ZPODD when
> >> deciding if platform has the required ability to support it.
> >
> > Sure, no problem with that. Perhaps you can cut a patch for that
> > on top of this series?
>
> Do you think it is reasonable to add a new field to acpi_state.flags to
> represent if we, as OSPM, have a way to put the device into a ACPI
> device state? This field can be set once in acpi_bus_get_power_flags and
> used afterwards.
>
> The valid field of acpi_state.flags is what we have today, and it means
> whether this ACPI device state is valid for the device, but not that if
> OSPM can actually put the device into that power state.

Yes, I think that adding such a new flag would make sense.

Thanks,
Rafael


--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2012-11-06 04:56:25

by Aaron Lu

[permalink] [raw]
Subject: Re: [PATCH 2/7] ACPI / PM: Move device power state selection routine to device_pm.c

This patch doesn't apply...

I'm trying on Linus' master branch, HEAD is v3.7-rc4, and I've merged
your pm-qos branch on top of v3.7-rc4.

Thanks,
Aaron

On Mon, Oct 29, 2012 at 10:09:09AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The ACPI function for choosing device power state is now located
> in drivers/acpi/sleep.c, but drivers/acpi/device_pm.c is a more
> logical place for it, so move it there.
>
> However, instead of moving the function entirely, move its core only
> under a different name and with a different list of arguments, so
> that it is more flexible, and leave a wrapper around it in the
> original location.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/device_pm.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/sleep.c | 88 +-------------------------------------
> include/acpi/acpi_bus.h | 15 ++++++
> 3 files changed, 124 insertions(+), 86 deletions(-)
>
> Index: linux/drivers/acpi/device_pm.c
> ===================================================================
> --- linux.orig/drivers/acpi/device_pm.c
> +++ linux/drivers/acpi/device_pm.c
> @@ -23,7 +23,9 @@
> */
>
> #include <linux/device.h>
> +#include <linux/export.h>
> #include <linux/mutex.h>
> +#include <linux/pm_qos.h>
>
> #include <acpi/acpi.h>
> #include <acpi/acpi_bus.h>
> @@ -89,3 +91,108 @@ acpi_status acpi_remove_pm_notifier(stru
> mutex_unlock(&acpi_pm_notifier_lock);
> return status;
> }
> +
> +/**
> + * acpi_device_power_state - Get preferred power state of ACPI device.
> + * @dev: Device whose preferred target power state to return.
> + * @adev: ACPI device node corresponding to @dev.
> + * @target_state: System state to match the resultant device state.
> + * @d_max_in: Deepest low-power state to take into consideration.
> + * @d_min_p: Location to store the upper limit of the allowed states range.
> + * Return value: Preferred power state of the device on success, -ENODEV
> + * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
> + *
> + * Find the lowest power (highest number) ACPI device power state that the
> + * device can be in while the system is in the state represented by
> + * @target_state. If @d_min_p is set, the highest power (lowest number) device
> + * power state that @dev can be in for the given system sleep state is stored
> + * at the location pointed to by it.
> + *
> + * Callers must ensure that @dev and @adev are valid pointers and that @adev
> + * actually corresponds to @dev before using this function.
> + */
> +int acpi_device_power_state(struct device *dev, struct acpi_device *adev,
> + u32 target_state, int d_max_in, int *d_min_p)
> +{
> + char acpi_method[] = "_SxD";
> + unsigned long long d_min, d_max;
> + bool wakeup = false;
> +
> + if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
> + return -EINVAL;
> +
> + if (d_max_in > ACPI_STATE_D3_HOT) {
> + enum pm_qos_flags_status stat;
> +
> + stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
> + if (stat == PM_QOS_FLAGS_ALL)
> + d_max_in = ACPI_STATE_D3_HOT;
> + }
> +
> + acpi_method[2] = '0' + target_state;
> + /*
> + * If the sleep state is S0, the lowest limit from ACPI is D3,
> + * but if the device has _S0W, we will use the value from _S0W
> + * as the lowest limit from ACPI. Finally, we will constrain
> + * the lowest limit with the specified one.
> + */
> + d_min = ACPI_STATE_D0;
> + d_max = ACPI_STATE_D3;
> +
> + /*
> + * If present, _SxD methods return the minimum D-state (highest power
> + * state) we can use for the corresponding S-states. Otherwise, the
> + * minimum D-state is D0 (ACPI 3.x).
> + *
> + * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
> + * provided -- that's our fault recovery, we ignore retval.
> + */
> + if (target_state > ACPI_STATE_S0) {
> + acpi_evaluate_integer(adev->handle, acpi_method, NULL, &d_min);
> + wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
> + && adev->wakeup.sleep_state >= target_state;
> + } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
> + PM_QOS_FLAGS_NONE) {
> + wakeup = adev->wakeup.flags.valid;
> + }
> +
> + /*
> + * If _PRW says we can wake up the system from the target sleep state,
> + * the D-state returned by _SxD is sufficient for that (we assume a
> + * wakeup-aware driver if wake is set). Still, if _SxW exists
> + * (ACPI 3.x), it should return the maximum (lowest power) D-state that
> + * can wake the system. _S0W may be valid, too.
> + */
> + if (wakeup) {
> + acpi_status status;
> +
> + acpi_method[3] = 'W';
> + status = acpi_evaluate_integer(adev->handle, acpi_method, NULL,
> + &d_max);
> + if (ACPI_FAILURE(status)) {
> + if (target_state != ACPI_STATE_S0 ||
> + status != AE_NOT_FOUND)
> + d_max = d_min;
> + } else if (d_max < d_min) {
> + /* Warn the user of the broken DSDT */
> + printk(KERN_WARNING "ACPI: Wrong value from %s\n",
> + acpi_method);
> + /* Sanitize it */
> + d_min = d_max;
> + }
> + }
> +
> + if (d_max_in < d_min)
> + return -EINVAL;
> + if (d_min_p)
> + *d_min_p = d_min;
> + /* constrain d_max with specified lowest limit (max number) */
> + if (d_max > d_max_in) {
> + for (d_max = d_max_in; d_max > d_min; d_max--) {
> + if (adev->power.states[d_max].flags.valid)
> + break;
> + }
> + }
> + return d_max;
> +}
> +EXPORT_SYMBOL_GPL(acpi_device_power_state);
> Index: linux/drivers/acpi/sleep.c
> ===================================================================
> --- linux.orig/drivers/acpi/sleep.c
> +++ linux/drivers/acpi/sleep.c
> @@ -19,7 +19,6 @@
> #include <linux/acpi.h>
> #include <linux/module.h>
> #include <linux/pm_runtime.h>
> -#include <linux/pm_qos.h>
>
> #include <asm/io.h>
>
> @@ -706,101 +705,20 @@ int acpi_suspend(u32 acpi_state)
> * Return value: Preferred power state of the device on success, -ENODEV
> * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
> *
> - * Find the lowest power (highest number) ACPI device power state that the
> - * device can be in while the system is in the sleep state represented
> - * by %acpi_target_sleep_state. If @d_min_p is set, the highest power (lowest
> - * number) device power state that @dev can be in for the given system sleep
> - * state is stored at the location pointed to by it.
> - *
> * The caller must ensure that @dev is valid before using this function.
> */
> int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p, int d_max_in)
> {
> acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
> struct acpi_device *adev;
> - char acpi_method[] = "_SxD";
> - unsigned long long d_min, d_max;
> - bool wakeup = false;
>
> - if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
> - return -EINVAL;
> if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
> - printk(KERN_DEBUG "ACPI handle has no context!\n");
> + dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
> return -ENODEV;
> }
> - if (d_max_in > ACPI_STATE_D3_HOT) {
> - enum pm_qos_flags_status stat;
> -
> - stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
> - if (stat == PM_QOS_FLAGS_ALL)
> - d_max_in = ACPI_STATE_D3_HOT;
> - }
> -
> - acpi_method[2] = '0' + acpi_target_sleep_state;
> - /*
> - * If the sleep state is S0, the lowest limit from ACPI is D3,
> - * but if the device has _S0W, we will use the value from _S0W
> - * as the lowest limit from ACPI. Finally, we will constrain
> - * the lowest limit with the specified one.
> - */
> - d_min = ACPI_STATE_D0;
> - d_max = ACPI_STATE_D3;
>
> - /*
> - * If present, _SxD methods return the minimum D-state (highest power
> - * state) we can use for the corresponding S-states. Otherwise, the
> - * minimum D-state is D0 (ACPI 3.x).
> - *
> - * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
> - * provided -- that's our fault recovery, we ignore retval.
> - */
> - if (acpi_target_sleep_state > ACPI_STATE_S0) {
> - acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
> - wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
> - && adev->wakeup.sleep_state >= acpi_target_sleep_state;
> - } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
> - PM_QOS_FLAGS_NONE) {
> - wakeup = adev->wakeup.flags.valid;
> - }
> -
> - /*
> - * If _PRW says we can wake up the system from the target sleep state,
> - * the D-state returned by _SxD is sufficient for that (we assume a
> - * wakeup-aware driver if wake is set). Still, if _SxW exists
> - * (ACPI 3.x), it should return the maximum (lowest power) D-state that
> - * can wake the system. _S0W may be valid, too.
> - */
> - if (wakeup) {
> - acpi_status status;
> -
> - acpi_method[3] = 'W';
> - status = acpi_evaluate_integer(handle, acpi_method, NULL,
> - &d_max);
> - if (ACPI_FAILURE(status)) {
> - if (acpi_target_sleep_state != ACPI_STATE_S0 ||
> - status != AE_NOT_FOUND)
> - d_max = d_min;
> - } else if (d_max < d_min) {
> - /* Warn the user of the broken DSDT */
> - printk(KERN_WARNING "ACPI: Wrong value from %s\n",
> - acpi_method);
> - /* Sanitize it */
> - d_min = d_max;
> - }
> - }
> -
> - if (d_max_in < d_min)
> - return -EINVAL;
> - if (d_min_p)
> - *d_min_p = d_min;
> - /* constrain d_max with specified lowest limit (max number) */
> - if (d_max > d_max_in) {
> - for (d_max = d_max_in; d_max > d_min; d_max--) {
> - if (adev->power.states[d_max].flags.valid)
> - break;
> - }
> - }
> - return d_max;
> + return acpi_device_power_state(dev, adev, acpi_target_sleep_state,
> + d_max_in, d_min_p);
> }
> EXPORT_SYMBOL(acpi_pm_device_sleep_state);
> #endif /* CONFIG_PM */
> Index: linux/include/acpi/acpi_bus.h
> ===================================================================
> --- linux.orig/include/acpi/acpi_bus.h
> +++ linux/include/acpi/acpi_bus.h
> @@ -419,6 +419,8 @@ acpi_status acpi_add_pm_notifier(struct
> acpi_notify_handler handler, void *context);
> acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
> acpi_notify_handler handler);
> +int acpi_device_power_state(struct device *dev, struct acpi_device *adev,
> + u32 target_state, int d_max_in, int *d_min_p);
> int acpi_pm_device_sleep_state(struct device *, int *, int);
> #else
> static inline acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
> @@ -432,12 +434,23 @@ static inline acpi_status acpi_remove_pm
> {
> return AE_SUPPORT;
> }
> -static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
> +static inline int __acpi_device_power_state(int m, int *p)
> {
> if (p)
> *p = ACPI_STATE_D0;
> return (m >= ACPI_STATE_D0 && m <= ACPI_STATE_D3) ? m : ACPI_STATE_D0;
> }
> +static inline int acpi_device_power_state(struct device *dev,
> + struct acpi_device *adev,
> + u32 target_state, int d_max_in,
> + int *d_min_p)
> +{
> + return __acpi_device_power_state(d_max_in, d_min_p);
> +}
> +static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
> +{
> + return __acpi_device_power_state(m, p);
> +}
> #endif
>
> #ifdef CONFIG_PM_RUNTIME
>

2012-11-06 12:58:54

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 2/7] ACPI / PM: Move device power state selection routine to device_pm.c

On Tuesday, November 06, 2012 12:56:07 PM Aaron Lu wrote:
> This patch doesn't apply...
>
> I'm trying on Linus' master branch, HEAD is v3.7-rc4, and I've merged
> your pm-qos branch on top of v3.7-rc4.

Well, it applies for me.

You can use the linux-next branch of my tree instead. It may contain
some fixes made after this patch had been sent.

Thanks,
Rafael


> On Mon, Oct 29, 2012 at 10:09:09AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > The ACPI function for choosing device power state is now located
> > in drivers/acpi/sleep.c, but drivers/acpi/device_pm.c is a more
> > logical place for it, so move it there.
> >
> > However, instead of moving the function entirely, move its core only
> > under a different name and with a different list of arguments, so
> > that it is more flexible, and leave a wrapper around it in the
> > original location.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/acpi/device_pm.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> > drivers/acpi/sleep.c | 88 +-------------------------------------
> > include/acpi/acpi_bus.h | 15 ++++++
> > 3 files changed, 124 insertions(+), 86 deletions(-)
> >
> > Index: linux/drivers/acpi/device_pm.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/device_pm.c
> > +++ linux/drivers/acpi/device_pm.c
> > @@ -23,7 +23,9 @@
> > */
> >
> > #include <linux/device.h>
> > +#include <linux/export.h>
> > #include <linux/mutex.h>
> > +#include <linux/pm_qos.h>
> >
> > #include <acpi/acpi.h>
> > #include <acpi/acpi_bus.h>
> > @@ -89,3 +91,108 @@ acpi_status acpi_remove_pm_notifier(stru
> > mutex_unlock(&acpi_pm_notifier_lock);
> > return status;
> > }
> > +
> > +/**
> > + * acpi_device_power_state - Get preferred power state of ACPI device.
> > + * @dev: Device whose preferred target power state to return.
> > + * @adev: ACPI device node corresponding to @dev.
> > + * @target_state: System state to match the resultant device state.
> > + * @d_max_in: Deepest low-power state to take into consideration.
> > + * @d_min_p: Location to store the upper limit of the allowed states range.
> > + * Return value: Preferred power state of the device on success, -ENODEV
> > + * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
> > + *
> > + * Find the lowest power (highest number) ACPI device power state that the
> > + * device can be in while the system is in the state represented by
> > + * @target_state. If @d_min_p is set, the highest power (lowest number) device
> > + * power state that @dev can be in for the given system sleep state is stored
> > + * at the location pointed to by it.
> > + *
> > + * Callers must ensure that @dev and @adev are valid pointers and that @adev
> > + * actually corresponds to @dev before using this function.
> > + */
> > +int acpi_device_power_state(struct device *dev, struct acpi_device *adev,
> > + u32 target_state, int d_max_in, int *d_min_p)
> > +{
> > + char acpi_method[] = "_SxD";
> > + unsigned long long d_min, d_max;
> > + bool wakeup = false;
> > +
> > + if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
> > + return -EINVAL;
> > +
> > + if (d_max_in > ACPI_STATE_D3_HOT) {
> > + enum pm_qos_flags_status stat;
> > +
> > + stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
> > + if (stat == PM_QOS_FLAGS_ALL)
> > + d_max_in = ACPI_STATE_D3_HOT;
> > + }
> > +
> > + acpi_method[2] = '0' + target_state;
> > + /*
> > + * If the sleep state is S0, the lowest limit from ACPI is D3,
> > + * but if the device has _S0W, we will use the value from _S0W
> > + * as the lowest limit from ACPI. Finally, we will constrain
> > + * the lowest limit with the specified one.
> > + */
> > + d_min = ACPI_STATE_D0;
> > + d_max = ACPI_STATE_D3;
> > +
> > + /*
> > + * If present, _SxD methods return the minimum D-state (highest power
> > + * state) we can use for the corresponding S-states. Otherwise, the
> > + * minimum D-state is D0 (ACPI 3.x).
> > + *
> > + * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
> > + * provided -- that's our fault recovery, we ignore retval.
> > + */
> > + if (target_state > ACPI_STATE_S0) {
> > + acpi_evaluate_integer(adev->handle, acpi_method, NULL, &d_min);
> > + wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
> > + && adev->wakeup.sleep_state >= target_state;
> > + } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
> > + PM_QOS_FLAGS_NONE) {
> > + wakeup = adev->wakeup.flags.valid;
> > + }
> > +
> > + /*
> > + * If _PRW says we can wake up the system from the target sleep state,
> > + * the D-state returned by _SxD is sufficient for that (we assume a
> > + * wakeup-aware driver if wake is set). Still, if _SxW exists
> > + * (ACPI 3.x), it should return the maximum (lowest power) D-state that
> > + * can wake the system. _S0W may be valid, too.
> > + */
> > + if (wakeup) {
> > + acpi_status status;
> > +
> > + acpi_method[3] = 'W';
> > + status = acpi_evaluate_integer(adev->handle, acpi_method, NULL,
> > + &d_max);
> > + if (ACPI_FAILURE(status)) {
> > + if (target_state != ACPI_STATE_S0 ||
> > + status != AE_NOT_FOUND)
> > + d_max = d_min;
> > + } else if (d_max < d_min) {
> > + /* Warn the user of the broken DSDT */
> > + printk(KERN_WARNING "ACPI: Wrong value from %s\n",
> > + acpi_method);
> > + /* Sanitize it */
> > + d_min = d_max;
> > + }
> > + }
> > +
> > + if (d_max_in < d_min)
> > + return -EINVAL;
> > + if (d_min_p)
> > + *d_min_p = d_min;
> > + /* constrain d_max with specified lowest limit (max number) */
> > + if (d_max > d_max_in) {
> > + for (d_max = d_max_in; d_max > d_min; d_max--) {
> > + if (adev->power.states[d_max].flags.valid)
> > + break;
> > + }
> > + }
> > + return d_max;
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_device_power_state);
> > Index: linux/drivers/acpi/sleep.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/sleep.c
> > +++ linux/drivers/acpi/sleep.c
> > @@ -19,7 +19,6 @@
> > #include <linux/acpi.h>
> > #include <linux/module.h>
> > #include <linux/pm_runtime.h>
> > -#include <linux/pm_qos.h>
> >
> > #include <asm/io.h>
> >
> > @@ -706,101 +705,20 @@ int acpi_suspend(u32 acpi_state)
> > * Return value: Preferred power state of the device on success, -ENODEV
> > * (if there's no 'struct acpi_device' for @dev) or -EINVAL on failure
> > *
> > - * Find the lowest power (highest number) ACPI device power state that the
> > - * device can be in while the system is in the sleep state represented
> > - * by %acpi_target_sleep_state. If @d_min_p is set, the highest power (lowest
> > - * number) device power state that @dev can be in for the given system sleep
> > - * state is stored at the location pointed to by it.
> > - *
> > * The caller must ensure that @dev is valid before using this function.
> > */
> > int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p, int d_max_in)
> > {
> > acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
> > struct acpi_device *adev;
> > - char acpi_method[] = "_SxD";
> > - unsigned long long d_min, d_max;
> > - bool wakeup = false;
> >
> > - if (d_max_in < ACPI_STATE_D0 || d_max_in > ACPI_STATE_D3)
> > - return -EINVAL;
> > if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
> > - printk(KERN_DEBUG "ACPI handle has no context!\n");
> > + dev_dbg(dev, "ACPI handle without context in %s!\n", __func__);
> > return -ENODEV;
> > }
> > - if (d_max_in > ACPI_STATE_D3_HOT) {
> > - enum pm_qos_flags_status stat;
> > -
> > - stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_POWER_OFF);
> > - if (stat == PM_QOS_FLAGS_ALL)
> > - d_max_in = ACPI_STATE_D3_HOT;
> > - }
> > -
> > - acpi_method[2] = '0' + acpi_target_sleep_state;
> > - /*
> > - * If the sleep state is S0, the lowest limit from ACPI is D3,
> > - * but if the device has _S0W, we will use the value from _S0W
> > - * as the lowest limit from ACPI. Finally, we will constrain
> > - * the lowest limit with the specified one.
> > - */
> > - d_min = ACPI_STATE_D0;
> > - d_max = ACPI_STATE_D3;
> >
> > - /*
> > - * If present, _SxD methods return the minimum D-state (highest power
> > - * state) we can use for the corresponding S-states. Otherwise, the
> > - * minimum D-state is D0 (ACPI 3.x).
> > - *
> > - * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
> > - * provided -- that's our fault recovery, we ignore retval.
> > - */
> > - if (acpi_target_sleep_state > ACPI_STATE_S0) {
> > - acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
> > - wakeup = device_may_wakeup(dev) && adev->wakeup.flags.valid
> > - && adev->wakeup.sleep_state >= acpi_target_sleep_state;
> > - } else if (dev_pm_qos_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP) !=
> > - PM_QOS_FLAGS_NONE) {
> > - wakeup = adev->wakeup.flags.valid;
> > - }
> > -
> > - /*
> > - * If _PRW says we can wake up the system from the target sleep state,
> > - * the D-state returned by _SxD is sufficient for that (we assume a
> > - * wakeup-aware driver if wake is set). Still, if _SxW exists
> > - * (ACPI 3.x), it should return the maximum (lowest power) D-state that
> > - * can wake the system. _S0W may be valid, too.
> > - */
> > - if (wakeup) {
> > - acpi_status status;
> > -
> > - acpi_method[3] = 'W';
> > - status = acpi_evaluate_integer(handle, acpi_method, NULL,
> > - &d_max);
> > - if (ACPI_FAILURE(status)) {
> > - if (acpi_target_sleep_state != ACPI_STATE_S0 ||
> > - status != AE_NOT_FOUND)
> > - d_max = d_min;
> > - } else if (d_max < d_min) {
> > - /* Warn the user of the broken DSDT */
> > - printk(KERN_WARNING "ACPI: Wrong value from %s\n",
> > - acpi_method);
> > - /* Sanitize it */
> > - d_min = d_max;
> > - }
> > - }
> > -
> > - if (d_max_in < d_min)
> > - return -EINVAL;
> > - if (d_min_p)
> > - *d_min_p = d_min;
> > - /* constrain d_max with specified lowest limit (max number) */
> > - if (d_max > d_max_in) {
> > - for (d_max = d_max_in; d_max > d_min; d_max--) {
> > - if (adev->power.states[d_max].flags.valid)
> > - break;
> > - }
> > - }
> > - return d_max;
> > + return acpi_device_power_state(dev, adev, acpi_target_sleep_state,
> > + d_max_in, d_min_p);
> > }
> > EXPORT_SYMBOL(acpi_pm_device_sleep_state);
> > #endif /* CONFIG_PM */
> > Index: linux/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux.orig/include/acpi/acpi_bus.h
> > +++ linux/include/acpi/acpi_bus.h
> > @@ -419,6 +419,8 @@ acpi_status acpi_add_pm_notifier(struct
> > acpi_notify_handler handler, void *context);
> > acpi_status acpi_remove_pm_notifier(struct acpi_device *adev,
> > acpi_notify_handler handler);
> > +int acpi_device_power_state(struct device *dev, struct acpi_device *adev,
> > + u32 target_state, int d_max_in, int *d_min_p);
> > int acpi_pm_device_sleep_state(struct device *, int *, int);
> > #else
> > static inline acpi_status acpi_add_pm_notifier(struct acpi_device *adev,
> > @@ -432,12 +434,23 @@ static inline acpi_status acpi_remove_pm
> > {
> > return AE_SUPPORT;
> > }
> > -static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
> > +static inline int __acpi_device_power_state(int m, int *p)
> > {
> > if (p)
> > *p = ACPI_STATE_D0;
> > return (m >= ACPI_STATE_D0 && m <= ACPI_STATE_D3) ? m : ACPI_STATE_D0;
> > }
> > +static inline int acpi_device_power_state(struct device *dev,
> > + struct acpi_device *adev,
> > + u32 target_state, int d_max_in,
> > + int *d_min_p)
> > +{
> > + return __acpi_device_power_state(d_max_in, d_min_p);
> > +}
> > +static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m)
> > +{
> > + return __acpi_device_power_state(m, p);
> > +}
> > #endif
> >
> > #ifdef CONFIG_PM_RUNTIME
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2012-11-06 19:52:54

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH 6/7] ACPI / PM: Move device PM functions related to sleep states

Commit b87b49cd0efd ("ACPI / PM: Move device PM functions related to sleep
states") declared acpi_target_system_state() for CONFIG_PM_SLEEP whereas
it is only defined for CONFIG_ACPI_SLEEP, resulting in the following link
error:

drivers/built-in.o: In function `acpi_pm_device_sleep_wake':
drivers/acpi/device_pm.c:342: undefined reference to `acpi_target_system_state'
drivers/built-in.o: In function `acpi_dev_suspend_late':
drivers/acpi/device_pm.c:501: undefined reference to `acpi_target_system_state'
drivers/built-in.o: In function `acpi_pm_device_sleep_state':
drivers/acpi/device_pm.c:221: undefined reference to `acpi_target_system_state'

Define it only for CONFIG_ACPI_SLEEP and fallback to a dummy definition
for other configs.

Signed-off-by: David Rientjes <[email protected]>
---
include/acpi/acpi_bus.h | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -469,11 +469,9 @@ static inline int acpi_pm_device_run_wake(struct device *dev, bool enable)
#endif

#ifdef CONFIG_PM_SLEEP
-u32 acpi_target_system_state(void);
int __acpi_device_sleep_wake(struct acpi_device *, u32, bool);
int acpi_pm_device_sleep_wake(struct device *, bool);
#else
-static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
static inline int __acpi_device_sleep_wake(struct acpi_device *adev,
u32 target_state, bool enable)
{
@@ -485,6 +483,12 @@ static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
}
#endif

+#ifdef CONFIG_ACPI_SLEEP
+u32 acpi_target_system_state(void);
+#else
+static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
+#endif
+
static inline bool acpi_device_power_manageable(struct acpi_device *adev)
{
return adev->flags.power_manageable;

2012-11-08 19:20:09

by David Rientjes

[permalink] [raw]
Subject: [patch] acpi, pm: fix build breakage

Commit b87b49cd0efd ("ACPI / PM: Move device PM functions related to sleep
states") declared acpi_target_system_state() for CONFIG_PM_SLEEP whereas
it is only defined for CONFIG_ACPI_SLEEP, resulting in the following link
error:

drivers/built-in.o: In function `acpi_pm_device_sleep_wake':
drivers/acpi/device_pm.c:342: undefined reference to `acpi_target_system_state'
drivers/built-in.o: In function `acpi_dev_suspend_late':
drivers/acpi/device_pm.c:501: undefined reference to `acpi_target_system_state'
drivers/built-in.o: In function `acpi_pm_device_sleep_state':
drivers/acpi/device_pm.c:221: undefined reference to `acpi_target_system_state'

Define it only for CONFIG_ACPI_SLEEP and fallback to a dummy definition
for other configs.

Signed-off-by: David Rientjes <[email protected]>
---
include/acpi/acpi_bus.h | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -469,11 +469,9 @@ static inline int acpi_pm_device_run_wake(struct device *dev, bool enable)
#endif

#ifdef CONFIG_PM_SLEEP
-u32 acpi_target_system_state(void);
int __acpi_device_sleep_wake(struct acpi_device *, u32, bool);
int acpi_pm_device_sleep_wake(struct device *, bool);
#else
-static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
static inline int __acpi_device_sleep_wake(struct acpi_device *adev,
u32 target_state, bool enable)
{
@@ -485,6 +483,12 @@ static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
}
#endif

+#ifdef CONFIG_ACPI_SLEEP
+u32 acpi_target_system_state(void);
+#else
+static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
+#endif
+
static inline bool acpi_device_power_manageable(struct acpi_device *adev)
{
return adev->flags.power_manageable;

2012-11-08 21:11:25

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [patch] acpi, pm: fix build breakage

On Thursday, November 08, 2012 11:20:01 AM David Rientjes wrote:
> Commit b87b49cd0efd ("ACPI / PM: Move device PM functions related to sleep
> states") declared acpi_target_system_state() for CONFIG_PM_SLEEP whereas
> it is only defined for CONFIG_ACPI_SLEEP, resulting in the following link
> error:
>
> drivers/built-in.o: In function `acpi_pm_device_sleep_wake':
> drivers/acpi/device_pm.c:342: undefined reference to `acpi_target_system_state'
> drivers/built-in.o: In function `acpi_dev_suspend_late':
> drivers/acpi/device_pm.c:501: undefined reference to `acpi_target_system_state'
> drivers/built-in.o: In function `acpi_pm_device_sleep_state':
> drivers/acpi/device_pm.c:221: undefined reference to `acpi_target_system_state'
>
> Define it only for CONFIG_ACPI_SLEEP and fallback to a dummy definition
> for other configs.
>
> Signed-off-by: David Rientjes <[email protected]>

I've received the patch and will apply it when I get back home from Spain.

Thanks,
Rafael


> ---
> include/acpi/acpi_bus.h | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -469,11 +469,9 @@ static inline int acpi_pm_device_run_wake(struct device *dev, bool enable)
> #endif
>
> #ifdef CONFIG_PM_SLEEP
> -u32 acpi_target_system_state(void);
> int __acpi_device_sleep_wake(struct acpi_device *, u32, bool);
> int acpi_pm_device_sleep_wake(struct device *, bool);
> #else
> -static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
> static inline int __acpi_device_sleep_wake(struct acpi_device *adev,
> u32 target_state, bool enable)
> {
> @@ -485,6 +483,12 @@ static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
> }
> #endif
>
> +#ifdef CONFIG_ACPI_SLEEP
> +u32 acpi_target_system_state(void);
> +#else
> +static inline u32 acpi_target_system_state(void) { return ACPI_STATE_S0; }
> +#endif
> +
> static inline bool acpi_device_power_manageable(struct acpi_device *adev)
> {
> return adev->flags.power_manageable;
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.