* [PATCH 1/12] ACPI / PM: Rework the handling of devices depending on power resources
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
@ 2013-01-04 0:01 ` Rafael J. Wysocki
2013-01-04 0:02 ` [PATCH 2/12] ACPI / scan: More straightforward preparation of ACPI device objects Rafael J. Wysocki
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:01 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Commit 0090def6 (ACPI: Add interface to register/unregister device
to/from power resources) made it possible to indicate to the ACPI
core that if the given device depends on any power resources, then
it should be resumed as soon as all of the power resources required
by it to transition to the D0 power state have been turned on.
Unfortunately, however, this was a mistake, because all devices
depending on power resources should be treated this way (i.e. they
should be resumed when all power resources required by their D0
state have been turned on) and for the majority of those devices
the ACPI core can figure out by itself which (physical) devices
depend on what power resources.
For this reason, replace the code added by commit 0090def6 with a
new, much more straightforward, mechanism that will be used
internally by the ACPI core and remove all references to that code
from kernel subsystems using ACPI.
For the cases when there are (physical) devices that should be
resumed whenever a not directly related ACPI device node goes into
D0 as a result of power resources configuration changes, like in
the SATA case, add two new routines, acpi_dev_pm_add_dependent()
and acpi_dev_pm_remove_dependent(), allowing subsystems to manage
such dependencies. Convert the SATA subsystem to use the new
functions accordingly.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
This patch supersedes https://patchwork.kernel.org/patch/1920551/ and all
previous versions of it.
Thanks,
Rafael
---
drivers/acpi/device_pm.c | 56 +++++++++++
drivers/acpi/internal.h | 1
drivers/acpi/power.c | 219 +++++++++++++++++-----------------------------
drivers/acpi/scan.c | 8 +
drivers/ata/libata-acpi.c | 18 ---
drivers/pci/pci-acpi.c | 2
include/acpi/acpi_bus.h | 3
include/linux/acpi.h | 6 +
8 files changed, 158 insertions(+), 155 deletions(-)
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -639,6 +639,7 @@ static int acpi_device_register(struct a
INIT_LIST_HEAD(&device->wakeup_list);
INIT_LIST_HEAD(&device->physical_node_list);
mutex_init(&device->physical_node_lock);
+ INIT_LIST_HEAD(&device->power_dependent);
new_bus_id = kzalloc(sizeof(struct acpi_device_bus_id), GFP_KERNEL);
if (!new_bus_id) {
@@ -712,8 +713,14 @@ static void acpi_device_unregister(struc
acpi_detach_data(device->handle, acpi_bus_data_handler);
+ acpi_power_add_remove_device(device, false);
acpi_device_remove_files(device);
device_unregister(&device->dev);
+ /*
+ * Drop the reference counts of all power resources the device depends
+ * on and turn off the ones that have no more references.
+ */
+ acpi_power_transition(device, ACPI_STATE_D3_COLD);
}
/* --------------------------------------------------------------------------
@@ -1463,6 +1470,7 @@ static int acpi_add_single_object(struct
end:
if (!result) {
+ acpi_power_add_remove_device(device, true);
acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Adding %s [%s] parent %s\n", dev_name(&device->dev),
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -83,31 +83,20 @@ static struct acpi_driver acpi_power_dri
.drv.pm = &acpi_power_pm,
};
-/*
- * A power managed device
- * A device may rely on multiple power resources.
- * */
-struct acpi_power_managed_device {
- struct device *dev; /* The physical device */
- acpi_handle *handle;
-};
-
-struct acpi_power_resource_device {
- struct acpi_power_managed_device *device;
- struct acpi_power_resource_device *next;
+struct acpi_power_dependent_device {
+ struct list_head node;
+ struct acpi_device *adev;
+ struct work_struct work;
};
struct acpi_power_resource {
- struct acpi_device * device;
+ struct acpi_device *device;
+ struct list_head dependent;
acpi_bus_id name;
u32 system_level;
u32 order;
unsigned int ref_count;
struct mutex resource_lock;
-
- /* List of devices relying on this power resource */
- struct acpi_power_resource_device *devices;
- struct mutex devices_lock;
};
static struct list_head acpi_power_resource_list;
@@ -207,21 +196,30 @@ static int acpi_power_get_list_state(str
return 0;
}
-/* Resume the device when all power resources in _PR0 are on */
-static void acpi_power_on_device(struct acpi_power_managed_device *device)
+static void acpi_power_resume_dependent(struct work_struct *work)
{
- struct acpi_device *acpi_dev;
- acpi_handle handle = device->handle;
+ struct acpi_power_dependent_device *dep;
+ struct acpi_device_physical_node *pn;
+ struct acpi_device *adev;
int state;
- if (acpi_bus_get_device(handle, &acpi_dev))
+ dep = container_of(work, struct acpi_power_dependent_device, work);
+ adev = dep->adev;
+ if (acpi_power_get_inferred_state(adev, &state))
return;
- if(acpi_power_get_inferred_state(acpi_dev, &state))
+ if (state > ACPI_STATE_D0)
return;
- if (state == ACPI_STATE_D0 && pm_runtime_suspended(device->dev))
- pm_request_resume(device->dev);
+ mutex_lock(&adev->physical_node_lock);
+
+ list_for_each_entry(pn, &adev->physical_node_list, node)
+ pm_request_resume(pn->dev);
+
+ list_for_each_entry(pn, &adev->power_dependent, node)
+ pm_request_resume(pn->dev);
+
+ mutex_unlock(&adev->physical_node_lock);
}
static int __acpi_power_on(struct acpi_power_resource *resource)
@@ -244,9 +242,7 @@ static int __acpi_power_on(struct acpi_p
static int acpi_power_on(acpi_handle handle)
{
int result = 0;
- bool resume_device = false;
struct acpi_power_resource *resource = NULL;
- struct acpi_power_resource_device *device_list;
result = acpi_power_get_context(handle, &resource);
if (result)
@@ -260,26 +256,17 @@ static int acpi_power_on(acpi_handle han
resource->name));
} else {
result = __acpi_power_on(resource);
- if (result)
+ if (result) {
resource->ref_count--;
- else
- resume_device = true;
- }
-
- mutex_unlock(&resource->resource_lock);
-
- if (!resume_device)
- return result;
-
- mutex_lock(&resource->devices_lock);
+ } else {
+ struct acpi_power_dependent_device *dep;
- device_list = resource->devices;
- while (device_list) {
- acpi_power_on_device(device_list->device);
- device_list = device_list->next;
+ list_for_each_entry(dep, &resource->dependent, node)
+ schedule_work(&dep->work);
+ }
}
- mutex_unlock(&resource->devices_lock);
+ mutex_unlock(&resource->resource_lock);
return result;
}
@@ -357,119 +344,81 @@ static int acpi_power_on_list(struct acp
return result;
}
-static void __acpi_power_resource_unregister_device(struct device *dev,
- acpi_handle res_handle)
+static void acpi_power_add_dependent(acpi_handle rhandle,
+ struct acpi_device *adev)
{
- struct acpi_power_resource *resource = NULL;
- struct acpi_power_resource_device *prev, *curr;
+ struct acpi_power_dependent_device *dep;
+ struct acpi_power_resource *resource;
- if (acpi_power_get_context(res_handle, &resource))
+ if (!rhandle || !adev || acpi_power_get_context(rhandle, &resource))
return;
- mutex_lock(&resource->devices_lock);
- prev = NULL;
- curr = resource->devices;
- while (curr) {
- if (curr->device->dev == dev) {
- if (!prev)
- resource->devices = curr->next;
- else
- prev->next = curr->next;
-
- kfree(curr);
- break;
- }
-
- prev = curr;
- curr = curr->next;
- }
- mutex_unlock(&resource->devices_lock);
-}
-
-/* Unlink dev from all power resources in _PR0 */
-void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle)
-{
- struct acpi_device *acpi_dev;
- struct acpi_handle_list *list;
- int i;
+ mutex_lock(&resource->resource_lock);
- if (!dev || !handle)
- return;
+ list_for_each_entry(dep, &resource->dependent, node)
+ if (dep->adev == adev)
+ goto out;
- if (acpi_bus_get_device(handle, &acpi_dev))
- return;
+ dep = kzalloc(sizeof(*dep), GFP_KERNEL);
+ if (!dep)
+ goto out;
- list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
+ dep->adev = adev;
+ INIT_WORK(&dep->work, acpi_power_resume_dependent);
+ list_add_tail(&dep->node, &resource->dependent);
- for (i = 0; i < list->count; i++)
- __acpi_power_resource_unregister_device(dev,
- list->handles[i]);
+ out:
+ mutex_unlock(&resource->resource_lock);
}
-EXPORT_SYMBOL_GPL(acpi_power_resource_unregister_device);
-static int __acpi_power_resource_register_device(
- struct acpi_power_managed_device *powered_device, acpi_handle handle)
+static void acpi_power_remove_dependent(acpi_handle rhandle,
+ struct acpi_device *adev)
{
- struct acpi_power_resource *resource = NULL;
- struct acpi_power_resource_device *power_resource_device;
- int result;
+ struct acpi_power_dependent_device *dep;
+ struct acpi_power_resource *resource;
+ struct work_struct *work = NULL;
- result = acpi_power_get_context(handle, &resource);
- if (result)
- return result;
+ if (!rhandle || !adev || acpi_power_get_context(rhandle, &resource))
+ return;
- power_resource_device = kzalloc(
- sizeof(*power_resource_device), GFP_KERNEL);
- if (!power_resource_device)
- return -ENOMEM;
+ mutex_lock(&resource->resource_lock);
- power_resource_device->device = powered_device;
+ list_for_each_entry(dep, &resource->dependent, node)
+ if (dep->adev == adev) {
+ list_del(&dep->node);
+ work = &dep->work;
+ break;
+ }
- mutex_lock(&resource->devices_lock);
- power_resource_device->next = resource->devices;
- resource->devices = power_resource_device;
- mutex_unlock(&resource->devices_lock);
+ mutex_unlock(&resource->resource_lock);
- return 0;
+ if (work) {
+ cancel_work_sync(work);
+ kfree(dep);
+ }
}
-/* Link dev to all power resources in _PR0 */
-int acpi_power_resource_register_device(struct device *dev, acpi_handle handle)
+void acpi_power_add_remove_device(struct acpi_device *adev, bool add)
{
- struct acpi_device *acpi_dev;
- struct acpi_handle_list *list;
- struct acpi_power_managed_device *powered_device;
- int i, ret;
-
- if (!dev || !handle)
- return -ENODEV;
-
- ret = acpi_bus_get_device(handle, &acpi_dev);
- if (ret || !acpi_dev->power.flags.power_resources)
- return -ENODEV;
+ if (adev->power.flags.power_resources) {
+ struct acpi_device_power_state *ps;
+ int j;
- powered_device = kzalloc(sizeof(*powered_device), GFP_KERNEL);
- if (!powered_device)
- return -ENOMEM;
-
- powered_device->dev = dev;
- powered_device->handle = handle;
-
- list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
-
- for (i = 0; i < list->count; i++) {
- ret = __acpi_power_resource_register_device(powered_device,
- list->handles[i]);
+ ps = &adev->power.states[ACPI_STATE_D0];
+ for (j = 0; j < ps->resources.count; j++) {
+ acpi_handle rhandle = ps->resources.handles[j];
- if (ret) {
- acpi_power_resource_unregister_device(dev, handle);
- break;
+ if (add)
+ acpi_power_add_dependent(rhandle, adev);
+ else
+ acpi_power_remove_dependent(rhandle, adev);
}
}
-
- return ret;
}
-EXPORT_SYMBOL_GPL(acpi_power_resource_register_device);
+
+/* --------------------------------------------------------------------------
+ Device Power Management
+ -------------------------------------------------------------------------- */
/**
* acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
@@ -623,10 +572,6 @@ int acpi_disable_wakeup_device_power(str
return err;
}
-/* --------------------------------------------------------------------------
- Device Power Management
- -------------------------------------------------------------------------- */
-
int acpi_power_get_inferred_state(struct acpi_device *device, int *state)
{
int result = 0;
@@ -725,7 +670,7 @@ static int acpi_power_add(struct acpi_de
resource->device = device;
mutex_init(&resource->resource_lock);
- mutex_init(&resource->devices_lock);
+ INIT_LIST_HEAD(&resource->dependent);
strcpy(resource->name, device->pnp.bus_id);
strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_POWER_CLASS);
Index: linux/drivers/acpi/internal.h
===================================================================
--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h
@@ -38,6 +38,7 @@ static inline void acpi_debugfs_init(voi
Power Resource
-------------------------------------------------------------------------- */
int acpi_power_init(void);
+void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
int acpi_device_sleep_wake(struct acpi_device *dev,
int enable, int sleep_state, int dev_state);
int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
Index: linux/drivers/pci/pci-acpi.c
===================================================================
--- linux.orig/drivers/pci/pci-acpi.c
+++ linux/drivers/pci/pci-acpi.c
@@ -327,7 +327,6 @@ static void pci_acpi_setup(struct device
acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
}
- acpi_power_resource_register_device(dev, handle);
if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
return;
@@ -350,7 +349,6 @@ static void pci_acpi_cleanup(struct devi
device_set_run_wake(dev, false);
pci_acpi_remove_pm_notifier(adev);
}
- acpi_power_resource_unregister_device(dev, handle);
if (pci_dev->subordinate)
acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -282,6 +282,7 @@ struct acpi_device {
struct list_head physical_node_list;
struct mutex physical_node_lock;
DECLARE_BITMAP(physical_node_id_bitmap, ACPI_MAX_PHYSICAL_NODE);
+ struct list_head power_dependent;
};
static inline void *acpi_driver_data(struct acpi_device *d)
@@ -337,8 +338,6 @@ int acpi_device_set_power(struct acpi_de
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);
-int acpi_power_resource_register_device(struct device *dev, acpi_handle handle);
-void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle);
#ifdef CONFIG_ACPI_PROC_EVENT
int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
Index: linux/drivers/ata/libata-acpi.c
===================================================================
--- linux.orig/drivers/ata/libata-acpi.c
+++ linux/drivers/ata/libata-acpi.c
@@ -1029,30 +1029,20 @@ static void ata_acpi_register_power_reso
{
struct scsi_device *sdev = dev->sdev;
acpi_handle handle;
- struct device *device;
handle = ata_dev_acpi_handle(dev);
- if (!handle)
- return;
-
- device = &sdev->sdev_gendev;
-
- acpi_power_resource_register_device(device, handle);
+ if (handle)
+ acpi_dev_pm_remove_dependent(handle, &sdev->sdev_gendev);
}
static void ata_acpi_unregister_power_resource(struct ata_device *dev)
{
struct scsi_device *sdev = dev->sdev;
acpi_handle handle;
- struct device *device;
handle = ata_dev_acpi_handle(dev);
- if (!handle)
- return;
-
- device = &sdev->sdev_gendev;
-
- acpi_power_resource_unregister_device(device, handle);
+ if (handle)
+ acpi_dev_pm_remove_dependent(handle, &sdev->sdev_gendev);
}
void ata_acpi_bind(struct ata_device *dev)
Index: linux/drivers/acpi/device_pm.c
===================================================================
--- linux.orig/drivers/acpi/device_pm.c
+++ linux/drivers/acpi/device_pm.c
@@ -665,3 +665,59 @@ void acpi_dev_pm_detach(struct device *d
}
}
EXPORT_SYMBOL_GPL(acpi_dev_pm_detach);
+
+/**
+ * acpi_dev_pm_add_dependent - Add physical device depending for PM.
+ * @handle: Handle of ACPI device node.
+ * @depdev: Device depending on that node for PM.
+ */
+void acpi_dev_pm_add_dependent(acpi_handle handle, struct device *depdev)
+{
+ struct acpi_device_physical_node *dep;
+ struct acpi_device *adev;
+
+ if (!depdev || acpi_bus_get_device(handle, &adev))
+ return;
+
+ mutex_lock(&adev->physical_node_lock);
+
+ list_for_each_entry(dep, &adev->power_dependent, node)
+ if (dep->dev == depdev)
+ goto out;
+
+ dep = kzalloc(sizeof(*dep), GFP_KERNEL);
+ if (dep) {
+ dep->dev = depdev;
+ list_add_tail(&dep->node, &adev->power_dependent);
+ }
+
+ out:
+ mutex_unlock(&adev->physical_node_lock);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_pm_add_dependent);
+
+/**
+ * acpi_dev_pm_remove_dependent - Remove physical device depending for PM.
+ * @handle: Handle of ACPI device node.
+ * @depdev: Device depending on that node for PM.
+ */
+void acpi_dev_pm_remove_dependent(acpi_handle handle, struct device *depdev)
+{
+ struct acpi_device_physical_node *dep;
+ struct acpi_device *adev;
+
+ if (!depdev || acpi_bus_get_device(handle, &adev))
+ return;
+
+ mutex_lock(&adev->physical_node_lock);
+
+ list_for_each_entry(dep, &adev->power_dependent, node)
+ if (dep->dev == depdev) {
+ list_del(&dep->node);
+ kfree(dep);
+ break;
+ }
+
+ mutex_unlock(&adev->physical_node_lock);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_pm_remove_dependent);
Index: linux/include/linux/acpi.h
===================================================================
--- linux.orig/include/linux/acpi.h
+++ linux/include/linux/acpi.h
@@ -529,6 +529,8 @@ static inline int acpi_subsys_resume_ear
struct acpi_device *acpi_dev_pm_get_node(struct device *dev);
int acpi_dev_pm_attach(struct device *dev, bool power_on);
void acpi_dev_pm_detach(struct device *dev, bool power_off);
+void acpi_dev_pm_add_dependent(acpi_handle handle, struct device *depdev);
+void acpi_dev_pm_remove_dependent(acpi_handle handle, struct device *depdev);
#else
static inline struct acpi_device *acpi_dev_pm_get_node(struct device *dev)
{
@@ -539,6 +541,10 @@ static inline int acpi_dev_pm_attach(str
return -ENODEV;
}
static inline void acpi_dev_pm_detach(struct device *dev, bool power_off) {}
+static inline void acpi_dev_pm_add_dependent(acpi_handle handle,
+ struct device *depdev) {}
+static inline void acpi_dev_pm_remove_dependent(acpi_handle handle,
+ struct device *depdev) {}
#endif
#ifdef CONFIG_ACPI
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 2/12] ACPI / scan: More straightforward preparation of ACPI device objects
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
2013-01-04 0:01 ` [PATCH 1/12] ACPI / PM: Rework the handling of devices depending on " Rafael J. Wysocki
@ 2013-01-04 0:02 ` Rafael J. Wysocki
2013-01-04 0:03 ` [PATCH 3/12] ACPI / scan: Treat power resources in a special way Rafael J. Wysocki
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:02 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Simplify the code preparing struct acpi_device objects for
registration by removing useless code, moving different pieces of
code into the functions they belong to and making a couple of int
functions always returning 0 void.
This also fixes a possible memory leak in ACPI device registration
error code path by making acpi_device_register() detach data from
device->handle if device_register() fails and prepares the scanning
code for special-casing ACPI power resources (next patch).
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 184 ++++++++++++++++++----------------------------------
1 file changed, 64 insertions(+), 120 deletions(-)
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -478,6 +478,7 @@ static void acpi_free_ids(struct acpi_de
kfree(id->id);
kfree(id);
}
+ kfree(device->pnp.unique_id);
}
static void acpi_device_release(struct device *dev)
@@ -485,7 +486,6 @@ static void acpi_device_release(struct d
struct acpi_device *acpi_dev = to_acpi_device(dev);
acpi_free_ids(acpi_dev);
- kfree(acpi_dev->pnp.unique_id);
kfree(acpi_dev);
}
@@ -629,6 +629,18 @@ static int acpi_device_register(struct a
struct acpi_device_bus_id *acpi_device_bus_id, *new_bus_id;
int found = 0;
+ if (device->handle) {
+ acpi_status status;
+
+ status = acpi_attach_data(device->handle, acpi_bus_data_handler,
+ device);
+ if (ACPI_FAILURE(status)) {
+ acpi_handle_err(device->handle,
+ "Unable to attach device data\n");
+ return -ENODEV;
+ }
+ }
+
/*
* Linkage
* -------
@@ -643,8 +655,9 @@ static int acpi_device_register(struct a
new_bus_id = kzalloc(sizeof(struct acpi_device_bus_id), GFP_KERNEL);
if (!new_bus_id) {
- printk(KERN_ERR PREFIX "Memory allocation error\n");
- return -ENOMEM;
+ pr_err(PREFIX "Memory allocation error\n");
+ result = -ENOMEM;
+ goto err_detach;
}
mutex_lock(&acpi_device_lock);
@@ -683,7 +696,7 @@ static int acpi_device_register(struct a
result = device_register(&device->dev);
if (result) {
dev_err(&device->dev, "Error registering device\n");
- goto end;
+ goto err;
}
result = acpi_device_setup_files(device);
@@ -693,12 +706,16 @@ static int acpi_device_register(struct a
device->removal_type = ACPI_BUS_REMOVAL_NORMAL;
return 0;
-end:
+
+ err:
mutex_lock(&acpi_device_lock);
if (device->parent)
list_del(&device->node);
list_del(&device->wakeup_list);
mutex_unlock(&acpi_device_lock);
+
+ err_detach:
+ acpi_detach_data(device->handle, acpi_bus_data_handler);
return result;
}
@@ -863,12 +880,6 @@ void acpi_bus_data_handler(acpi_handle h
return;
}
-static int acpi_bus_get_perf_flags(struct acpi_device *device)
-{
- device->performance.state = ACPI_STATE_UNKNOWN;
- return 0;
-}
-
static acpi_status
acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
struct acpi_device_wakeup *wakeup)
@@ -1019,12 +1030,25 @@ static void acpi_bus_get_wakeup_device_f
static void acpi_bus_add_power_resource(acpi_handle handle);
-static int acpi_bus_get_power_flags(struct acpi_device *device)
+static void acpi_bus_get_power_flags(struct acpi_device *device)
{
acpi_status status = 0;
acpi_handle handle = NULL;
u32 i = 0;
+ /* Power resources cannot be power manageable. */
+ if (device->device_type == ACPI_BUS_TYPE_POWER)
+ return;
+
+ /* Presence of _PS0|_PR0 indicates 'power manageable' */
+ status = acpi_get_handle(device->handle, "_PS0", &handle);
+ if (ACPI_FAILURE(status)) {
+ status = acpi_get_handle(device->handle, "_PR0", &handle);
+ if (ACPI_FAILURE(status))
+ return;
+ }
+
+ device->flags.power_manageable = 1;
/*
* Power Management Flags
@@ -1090,16 +1114,13 @@ static int acpi_bus_get_power_flags(stru
device->power.states[ACPI_STATE_D3_COLD].flags.os_accessible = 1;
acpi_bus_init_power(device);
-
- return 0;
}
-static int acpi_bus_get_flags(struct acpi_device *device)
+static void acpi_bus_get_flags(struct acpi_device *device)
{
acpi_status status = AE_OK;
acpi_handle temp = NULL;
-
/* Presence of _STA indicates 'dynamic_status' */
status = acpi_get_handle(device->handle, "_STA", &temp);
if (ACPI_SUCCESS(status))
@@ -1119,21 +1140,6 @@ static int acpi_bus_get_flags(struct acp
if (ACPI_SUCCESS(status))
device->flags.ejectable = 1;
}
-
- /* Power resources cannot be power manageable. */
- if (device->device_type == ACPI_BUS_TYPE_POWER)
- return 0;
-
- /* Presence of _PS0|_PR0 indicates 'power manageable' */
- status = acpi_get_handle(device->handle, "_PS0", &temp);
- if (ACPI_FAILURE(status))
- status = acpi_get_handle(device->handle, "_PR0", &temp);
- if (ACPI_SUCCESS(status))
- device->flags.power_manageable = 1;
-
- /* TBD: Performance management */
-
- return 0;
}
static void acpi_device_get_busid(struct acpi_device *device)
@@ -1358,29 +1364,6 @@ static void acpi_device_set_id(struct ac
}
}
-static int acpi_device_set_context(struct acpi_device *device)
-{
- acpi_status status;
-
- /*
- * Context
- * -------
- * Attach this 'struct acpi_device' to the ACPI object. This makes
- * resolutions from handle->device very efficient. Fixed hardware
- * devices have no handles, so we skip them.
- */
- if (!device->handle)
- return 0;
-
- status = acpi_attach_data(device->handle,
- acpi_bus_data_handler, device);
- if (ACPI_SUCCESS(status))
- return 0;
-
- printk(KERN_ERR PREFIX "Error attaching device data\n");
- return -ENODEV;
-}
-
static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
{
if (!dev)
@@ -1397,6 +1380,20 @@ static int acpi_bus_remove(struct acpi_d
return 0;
}
+static void acpi_init_device_object(struct acpi_device *device,
+ acpi_handle handle,
+ int type, unsigned long long sta)
+{
+ INIT_LIST_HEAD(&device->pnp.ids);
+ device->device_type = type;
+ device->handle = handle;
+ device->parent = acpi_bus_get_parent(handle);
+ STRUCT_TO_INT(device->status) = sta;
+ acpi_device_get_busid(device);
+ acpi_device_set_id(device);
+ acpi_bus_get_flags(device);
+}
+
static int acpi_add_single_object(struct acpi_device **child,
acpi_handle handle, int type,
unsigned long long sta, bool match_driver)
@@ -1411,78 +1408,25 @@ static int acpi_add_single_object(struct
return -ENOMEM;
}
- INIT_LIST_HEAD(&device->pnp.ids);
- device->device_type = type;
- device->handle = handle;
- device->parent = acpi_bus_get_parent(handle);
- STRUCT_TO_INT(device->status) = sta;
-
- acpi_device_get_busid(device);
-
- /*
- * Flags
- * -----
- * Note that we only look for object handles -- cannot evaluate objects
- * until we know the device is present and properly initialized.
- */
- result = acpi_bus_get_flags(device);
- if (result)
- goto end;
-
- /*
- * Initialize Device
- * -----------------
- * TBD: Synch with Core's enumeration/initialization process.
- */
- acpi_device_set_id(device);
-
- /*
- * Power Management
- * ----------------
- */
- if (device->flags.power_manageable) {
- result = acpi_bus_get_power_flags(device);
- if (result)
- goto end;
- }
-
- /*
- * Wakeup device management
- *-----------------------
- */
+ acpi_init_device_object(device, handle, type, sta);
+ acpi_bus_get_power_flags(device);
acpi_bus_get_wakeup_device_flags(device);
- /*
- * Performance Management
- * ----------------------
- */
- if (device->flags.performance_manageable) {
- result = acpi_bus_get_perf_flags(device);
- if (result)
- goto end;
- }
-
- if ((result = acpi_device_set_context(device)))
- goto end;
-
device->flags.match_driver = match_driver;
result = acpi_device_register(device);
-
-end:
- if (!result) {
- acpi_power_add_remove_device(device, true);
- acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- "Adding %s [%s] parent %s\n", dev_name(&device->dev),
- (char *) buffer.pointer,
- device->parent ? dev_name(&device->parent->dev) :
- "(null)"));
- kfree(buffer.pointer);
- *child = device;
- } else
+ if (result) {
acpi_device_release(&device->dev);
+ return result;
+ }
- return result;
+ acpi_power_add_remove_device(device, true);
+ acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Added %s [%s] parent %s\n",
+ dev_name(&device->dev), (char *) buffer.pointer,
+ device->parent ? dev_name(&device->parent->dev) : "(null)"));
+ kfree(buffer.pointer);
+ *child = device;
+ return 0;
}
#define ACPI_STA_DEFAULT (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED | \
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 3/12] ACPI / scan: Treat power resources in a special way
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
2013-01-04 0:01 ` [PATCH 1/12] ACPI / PM: Rework the handling of devices depending on " Rafael J. Wysocki
2013-01-04 0:02 ` [PATCH 2/12] ACPI / scan: More straightforward preparation of ACPI device objects Rafael J. Wysocki
@ 2013-01-04 0:03 ` Rafael J. Wysocki
2013-01-04 0:03 ` [PATCH 4/12] ACPI: Drop power resources driver Rafael J. Wysocki
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:03 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
ACPI power resources need to be treated in a special way by the
namespace scanning code, because they need to be ready to use as
soon as they have been discovered (even before registering ACPI
device nodes using them for power management).
For this reason, it doesn't make sense to separate the preparation
of struct acpi_device objects representing them in the device
hierarchy from the creation of struct acpi_power_resource objects
actually used for power resource manipulation. Accordingly, it
doesn't make sense to define non-empty .add() and .remove() callbacks
in the power resources "driver" (in fact, it is questionable whether
or not it is useful to register such a "driver" at all).
Rearrange the code in scan.c and power.c so that power resources are
initialized entirely by one routine, acpi_add_power_resource(), that
also prepares their struct acpi_device objects and registers them
with the driver core, telling it to use a special release routine,
acpi_release_power_resource(), for removing objects that represent
power resources from memory. Make the ACPI namespace scanning code
in scan.c always use acpi_add_power_resource() for preparing and
registering objects that represent power resources.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/internal.h | 13 +++
drivers/acpi/power.c | 157 ++++++++++++++++++++++--------------------------
drivers/acpi/scan.c | 47 +++++---------
3 files changed, 102 insertions(+), 115 deletions(-)
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -470,7 +470,7 @@ int acpi_match_device_ids(struct acpi_de
}
EXPORT_SYMBOL(acpi_match_device_ids);
-static void acpi_free_ids(struct acpi_device *device)
+void acpi_free_ids(struct acpi_device *device)
{
struct acpi_hardware_id *id, *tmp;
@@ -623,7 +623,8 @@ struct bus_type acpi_bus_type = {
.uevent = acpi_device_uevent,
};
-static int acpi_device_register(struct acpi_device *device)
+int acpi_device_register(struct acpi_device *device,
+ void (*release)(struct device *))
{
int result;
struct acpi_device_bus_id *acpi_device_bus_id, *new_bus_id;
@@ -692,7 +693,7 @@ static int acpi_device_register(struct a
if (device->parent)
device->dev.parent = &device->parent->dev;
device->dev.bus = &acpi_bus_type;
- device->dev.release = &acpi_device_release;
+ device->dev.release = release;
result = device_register(&device->dev);
if (result) {
dev_err(&device->dev, "Error registering device\n");
@@ -1028,18 +1029,12 @@ static void acpi_bus_get_wakeup_device_f
"error in _DSW or _PSW evaluation\n"));
}
-static void acpi_bus_add_power_resource(acpi_handle handle);
-
static void acpi_bus_get_power_flags(struct acpi_device *device)
{
acpi_status status = 0;
acpi_handle handle = NULL;
u32 i = 0;
- /* Power resources cannot be power manageable. */
- if (device->device_type == ACPI_BUS_TYPE_POWER)
- return;
-
/* Presence of _PS0|_PR0 indicates 'power manageable' */
status = acpi_get_handle(device->handle, "_PS0", &handle);
if (ACPI_FAILURE(status)) {
@@ -1074,8 +1069,10 @@ static void acpi_bus_get_power_flags(str
int j;
device->power.flags.power_resources = 1;
- for (j = 0; j < ps->resources.count; j++)
- acpi_bus_add_power_resource(ps->resources.handles[j]);
+ for (j = 0; j < ps->resources.count; j++) {
+ acpi_handle rhandle = ps->resources.handles[j];
+ acpi_add_power_resource(rhandle);
+ }
}
/* Evaluate "_PSx" to see if we can do explicit sets */
@@ -1380,9 +1377,8 @@ static int acpi_bus_remove(struct acpi_d
return 0;
}
-static void acpi_init_device_object(struct acpi_device *device,
- acpi_handle handle,
- int type, unsigned long long sta)
+void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
+ int type, unsigned long long sta)
{
INIT_LIST_HEAD(&device->pnp.ids);
device->device_type = type;
@@ -1413,7 +1409,7 @@ static int acpi_add_single_object(struct
acpi_bus_get_wakeup_device_flags(device);
device->flags.match_driver = match_driver;
- result = acpi_device_register(device);
+ result = acpi_device_register(device, acpi_device_release);
if (result) {
acpi_device_release(&device->dev);
return result;
@@ -1429,19 +1425,6 @@ static int acpi_add_single_object(struct
return 0;
}
-#define ACPI_STA_DEFAULT (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED | \
- ACPI_STA_DEVICE_UI | ACPI_STA_DEVICE_FUNCTIONING)
-
-static void acpi_bus_add_power_resource(acpi_handle handle)
-{
- struct acpi_device *device = NULL;
-
- acpi_bus_get_device(handle, &device);
- if (!device)
- acpi_add_single_object(&device, handle, ACPI_BUS_TYPE_POWER,
- ACPI_STA_DEFAULT, true);
-}
-
static int acpi_bus_type_and_status(acpi_handle handle, int *type,
unsigned long long *sta)
{
@@ -1498,6 +1481,11 @@ static acpi_status acpi_bus_check_add(ac
if (result)
return AE_OK;
+ if (type == ACPI_BUS_TYPE_POWER) {
+ acpi_add_power_resource(handle);
+ return AE_OK;
+ }
+
if (!(sta & ACPI_STA_DEVICE_PRESENT) &&
!(sta & ACPI_STA_DEVICE_FUNCTIONING)) {
struct acpi_device_wakeup wakeup;
@@ -1510,8 +1498,7 @@ static acpi_status acpi_bus_check_add(ac
return AE_CTRL_DEPTH;
}
- acpi_add_single_object(&device, handle, type, sta,
- type == ACPI_BUS_TYPE_POWER);
+ acpi_add_single_object(&device, handle, type, sta, false);
if (!device)
return AE_CTRL_DEPTH;
Index: linux/drivers/acpi/internal.h
===================================================================
--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h
@@ -35,9 +35,22 @@ static inline void acpi_debugfs_init(voi
#endif
/* --------------------------------------------------------------------------
+ Device Node Initialization / Removal
+ -------------------------------------------------------------------------- */
+#define ACPI_STA_DEFAULT (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED | \
+ ACPI_STA_DEVICE_UI | ACPI_STA_DEVICE_FUNCTIONING)
+
+int acpi_device_register(struct acpi_device *device,
+ void (*release)(struct device *));
+void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
+ int type, unsigned long long sta);
+void acpi_free_ids(struct acpi_device *device);
+
+/* --------------------------------------------------------------------------
Power Resource
-------------------------------------------------------------------------- */
int acpi_power_init(void);
+void acpi_add_power_resource(acpi_handle handle);
void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
int acpi_device_sleep_wake(struct acpi_device *dev,
int enable, int sleep_state, int dev_state);
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -58,8 +58,7 @@ ACPI_MODULE_NAME("power");
#define ACPI_POWER_RESOURCE_STATE_ON 0x01
#define ACPI_POWER_RESOURCE_STATE_UNKNOWN 0xFF
-static int acpi_power_add(struct acpi_device *device);
-static int acpi_power_remove(struct acpi_device *device, int type);
+static inline int acpi_power_add(struct acpi_device *device) { return 0; }
static const struct acpi_device_id power_device_ids[] = {
{ACPI_POWER_HID, 0},
@@ -76,10 +75,7 @@ static struct acpi_driver acpi_power_dri
.name = "power",
.class = ACPI_POWER_CLASS,
.ids = power_device_ids,
- .ops = {
- .add = acpi_power_add,
- .remove = acpi_power_remove,
- },
+ .ops.add = acpi_power_add,
.drv.pm = &acpi_power_pm,
};
@@ -90,9 +86,9 @@ struct acpi_power_dependent_device {
};
struct acpi_power_resource {
- struct acpi_device *device;
+ struct acpi_device device;
struct list_head dependent;
- acpi_bus_id name;
+ char *name;
u32 system_level;
u32 order;
unsigned int ref_count;
@@ -105,28 +101,14 @@ static struct list_head acpi_power_resou
Power Resource Management
-------------------------------------------------------------------------- */
-static int
-acpi_power_get_context(acpi_handle handle,
- struct acpi_power_resource **resource)
+struct acpi_power_resource *acpi_power_get_context(acpi_handle handle)
{
- int result = 0;
- struct acpi_device *device = NULL;
-
-
- if (!resource)
- return -ENODEV;
-
- result = acpi_bus_get_device(handle, &device);
- if (result) {
- printk(KERN_WARNING PREFIX "Getting context [%p]\n", handle);
- return result;
- }
+ struct acpi_device *device;
- *resource = acpi_driver_data(device);
- if (!*resource)
- return -ENODEV;
+ if (acpi_bus_get_device(handle, &device))
+ return NULL;
- return 0;
+ return container_of(device, struct acpi_power_resource, device);
}
static int acpi_power_get_state(acpi_handle handle, int *state)
@@ -171,9 +153,9 @@ static int acpi_power_get_list_state(str
acpi_handle handle = list->handles[i];
int result;
- result = acpi_power_get_context(handle, &resource);
- if (result)
- return result;
+ resource = acpi_power_get_context(handle);
+ if (!resource)
+ return -ENODEV;
mutex_lock(&resource->resource_lock);
@@ -226,12 +208,12 @@ static int __acpi_power_on(struct acpi_p
{
acpi_status status = AE_OK;
- status = acpi_evaluate_object(resource->device->handle, "_ON", NULL, NULL);
+ status = acpi_evaluate_object(resource->device.handle, "_ON", NULL, NULL);
if (ACPI_FAILURE(status))
return -ENODEV;
/* Update the power resource's _device_ power state */
- resource->device->power.state = ACPI_STATE_D0;
+ resource->device.power.state = ACPI_STATE_D0;
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Power resource [%s] turned on\n",
resource->name));
@@ -242,11 +224,11 @@ static int __acpi_power_on(struct acpi_p
static int acpi_power_on(acpi_handle handle)
{
int result = 0;
- struct acpi_power_resource *resource = NULL;
+ struct acpi_power_resource *resource;
- result = acpi_power_get_context(handle, &resource);
- if (result)
- return result;
+ resource = acpi_power_get_context(handle);
+ if (!resource)
+ return -ENODEV;
mutex_lock(&resource->resource_lock);
@@ -275,11 +257,11 @@ static int acpi_power_off(acpi_handle ha
{
int result = 0;
acpi_status status = AE_OK;
- struct acpi_power_resource *resource = NULL;
+ struct acpi_power_resource *resource;
- result = acpi_power_get_context(handle, &resource);
- if (result)
- return result;
+ resource = acpi_power_get_context(handle);
+ if (!resource)
+ return -ENODEV;
mutex_lock(&resource->resource_lock);
@@ -297,12 +279,12 @@ static int acpi_power_off(acpi_handle ha
goto unlock;
}
- status = acpi_evaluate_object(resource->device->handle, "_OFF", NULL, NULL);
+ status = acpi_evaluate_object(resource->device.handle, "_OFF", NULL, NULL);
if (ACPI_FAILURE(status)) {
result = -ENODEV;
} else {
/* Update the power resource's _device_ power state */
- resource->device->power.state = ACPI_STATE_D3;
+ resource->device.power.state = ACPI_STATE_D3;
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Power resource [%s] turned off\n",
@@ -350,7 +332,11 @@ static void acpi_power_add_dependent(acp
struct acpi_power_dependent_device *dep;
struct acpi_power_resource *resource;
- if (!rhandle || !adev || acpi_power_get_context(rhandle, &resource))
+ if (!rhandle || !adev)
+ return;
+
+ resource = acpi_power_get_context(rhandle);
+ if (!resource)
return;
mutex_lock(&resource->resource_lock);
@@ -378,7 +364,11 @@ static void acpi_power_remove_dependent(
struct acpi_power_resource *resource;
struct work_struct *work = NULL;
- if (!rhandle || !adev || acpi_power_get_context(rhandle, &resource))
+ if (!rhandle || !adev)
+ return;
+
+ resource = acpi_power_get_context(rhandle);
+ if (!resource)
return;
mutex_lock(&resource->resource_lock);
@@ -648,46 +638,53 @@ int acpi_power_transition(struct acpi_de
return result;
}
-/* --------------------------------------------------------------------------
- Driver Interface
- -------------------------------------------------------------------------- */
+static void acpi_release_power_resource(struct device *dev)
+{
+ struct acpi_device *device = to_acpi_device(dev);
+ struct acpi_power_resource *resource;
-static int acpi_power_add(struct acpi_device *device)
+ acpi_free_ids(device);
+ resource = container_of(device, struct acpi_power_resource, device);
+ kfree(resource);
+}
+
+void acpi_add_power_resource(acpi_handle handle)
{
- int result = 0, state;
- acpi_status status = AE_OK;
- struct acpi_power_resource *resource = NULL;
+ struct acpi_power_resource *resource;
+ struct acpi_device *device = NULL;
union acpi_object acpi_object;
struct acpi_buffer buffer = { sizeof(acpi_object), &acpi_object };
+ acpi_status status;
+ int state, result = -ENODEV;
+ acpi_bus_get_device(handle, &device);
+ if (device)
+ return;
- if (!device)
- return -EINVAL;
-
- resource = kzalloc(sizeof(struct acpi_power_resource), GFP_KERNEL);
+ resource = kzalloc(sizeof(*resource), GFP_KERNEL);
if (!resource)
- return -ENOMEM;
+ return;
- resource->device = device;
+ device = &resource->device;
+ acpi_init_device_object(device, handle, ACPI_BUS_TYPE_POWER,
+ ACPI_STA_DEFAULT);
mutex_init(&resource->resource_lock);
INIT_LIST_HEAD(&resource->dependent);
- strcpy(resource->name, device->pnp.bus_id);
+ resource->name = device->pnp.bus_id;
strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_POWER_CLASS);
- device->driver_data = resource;
/* Evalute the object to get the system level and resource order. */
- status = acpi_evaluate_object(device->handle, NULL, NULL, &buffer);
- if (ACPI_FAILURE(status)) {
- result = -ENODEV;
- goto end;
- }
+ status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ goto out;
+
resource->system_level = acpi_object.power_resource.system_level;
resource->order = acpi_object.power_resource.resource_order;
- result = acpi_power_get_state(device->handle, &state);
+ result = acpi_power_get_state(handle, &state);
if (result)
- goto end;
+ goto out;
switch (state) {
case ACPI_POWER_RESOURCE_STATE_ON:
@@ -698,34 +695,24 @@ static int acpi_power_add(struct acpi_de
break;
default:
device->power.state = ACPI_STATE_UNKNOWN;
- break;
}
printk(KERN_INFO PREFIX "%s [%s] (%s)\n", acpi_device_name(device),
acpi_device_bid(device), state ? "on" : "off");
- end:
+ device->flags.match_driver = true;
+ result = acpi_device_register(device, acpi_release_power_resource);
+
+ out:
if (result)
- kfree(resource);
+ acpi_release_power_resource(&device->dev);
- return result;
+ return;
}
-static int acpi_power_remove(struct acpi_device *device, int type)
-{
- struct acpi_power_resource *resource;
-
- if (!device)
- return -EINVAL;
-
- resource = acpi_driver_data(device);
- if (!resource)
- return -EINVAL;
-
- kfree(resource);
-
- return 0;
-}
+/* --------------------------------------------------------------------------
+ Driver Interface
+ -------------------------------------------------------------------------- */
#ifdef CONFIG_PM_SLEEP
static int acpi_power_resume(struct device *dev)
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 4/12] ACPI: Drop power resources driver
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (2 preceding siblings ...)
2013-01-04 0:03 ` [PATCH 3/12] ACPI / scan: Treat power resources in a special way Rafael J. Wysocki
@ 2013-01-04 0:03 ` Rafael J. Wysocki
2013-01-04 0:04 ` [PATCH 5/12] ACPI: Do not use device power states of power resources Rafael J. Wysocki
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:03 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The ACPI power resources driver is not very useful, because the only
thing it really does is to restore the state of the power resources
that were "on" before system suspend or hibernation, but that may be
achieved in a different way.
Drop the ACPI power resources driver entirely and add
acpi_resume_power_resources() that will walk the list of all
registered power resources during system resume and turn on the ones
that were "on" before the preceding system suspend or hibernation.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/power.c | 91 ++++++++++++++++++---------------------------------
drivers/acpi/scan.c | 1
drivers/acpi/sleep.c | 2 +
drivers/acpi/sleep.h | 2 +
4 files changed, 37 insertions(+), 59 deletions(-)
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -58,27 +58,6 @@ ACPI_MODULE_NAME("power");
#define ACPI_POWER_RESOURCE_STATE_ON 0x01
#define ACPI_POWER_RESOURCE_STATE_UNKNOWN 0xFF
-static inline int acpi_power_add(struct acpi_device *device) { return 0; }
-
-static const struct acpi_device_id power_device_ids[] = {
- {ACPI_POWER_HID, 0},
- {"", 0},
-};
-MODULE_DEVICE_TABLE(acpi, power_device_ids);
-
-#ifdef CONFIG_PM_SLEEP
-static int acpi_power_resume(struct device *dev);
-#endif
-static SIMPLE_DEV_PM_OPS(acpi_power_pm, NULL, acpi_power_resume);
-
-static struct acpi_driver acpi_power_driver = {
- .name = "power",
- .class = ACPI_POWER_CLASS,
- .ids = power_device_ids,
- .ops.add = acpi_power_add,
- .drv.pm = &acpi_power_pm,
-};
-
struct acpi_power_dependent_device {
struct list_head node;
struct acpi_device *adev;
@@ -87,6 +66,7 @@ struct acpi_power_dependent_device {
struct acpi_power_resource {
struct acpi_device device;
+ struct list_head list_node;
struct list_head dependent;
char *name;
u32 system_level;
@@ -95,7 +75,8 @@ struct acpi_power_resource {
struct mutex resource_lock;
};
-static struct list_head acpi_power_resource_list;
+static LIST_HEAD(acpi_power_resource_list);
+static DEFINE_MUTEX(power_resource_list_lock);
/* --------------------------------------------------------------------------
Power Resource Management
@@ -643,8 +624,13 @@ static void acpi_release_power_resource(
struct acpi_device *device = to_acpi_device(dev);
struct acpi_power_resource *resource;
- acpi_free_ids(device);
resource = container_of(device, struct acpi_power_resource, device);
+
+ mutex_lock(&power_resource_list_lock);
+ list_del(&resource->list_node);
+ mutex_unlock(&power_resource_list_lock);
+
+ acpi_free_ids(device);
kfree(resource);
}
@@ -677,14 +663,14 @@ void acpi_add_power_resource(acpi_handle
/* Evalute the object to get the system level and resource order. */
status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
if (ACPI_FAILURE(status))
- goto out;
+ goto err;
resource->system_level = acpi_object.power_resource.system_level;
resource->order = acpi_object.power_resource.resource_order;
result = acpi_power_get_state(handle, &state);
if (result)
- goto out;
+ goto err;
switch (state) {
case ACPI_POWER_RESOURCE_STATE_ON:
@@ -702,51 +688,40 @@ void acpi_add_power_resource(acpi_handle
device->flags.match_driver = true;
result = acpi_device_register(device, acpi_release_power_resource);
-
- out:
if (result)
- acpi_release_power_resource(&device->dev);
+ goto err;
+ mutex_lock(&power_resource_list_lock);
+ list_add(&resource->list_node, &acpi_power_resource_list);
+ mutex_unlock(&power_resource_list_lock);
return;
-}
-/* --------------------------------------------------------------------------
- Driver Interface
- -------------------------------------------------------------------------- */
+ err:
+ acpi_release_power_resource(&device->dev);
+}
-#ifdef CONFIG_PM_SLEEP
-static int acpi_power_resume(struct device *dev)
+#ifdef CONFIG_ACPI_SLEEP
+void acpi_resume_power_resources(void)
{
- int result = 0, state;
- struct acpi_device *device;
struct acpi_power_resource *resource;
- if (!dev)
- return -EINVAL;
-
- device = to_acpi_device(dev);
- resource = acpi_driver_data(device);
- if (!resource)
- return -EINVAL;
+ mutex_lock(&power_resource_list_lock);
- mutex_lock(&resource->resource_lock);
+ list_for_each_entry(resource, &acpi_power_resource_list, list_node) {
+ int result, state;
- result = acpi_power_get_state(device->handle, &state);
- if (result)
- goto unlock;
+ mutex_lock(&resource->resource_lock);
- if (state == ACPI_POWER_RESOURCE_STATE_OFF && resource->ref_count)
- result = __acpi_power_on(resource);
+ result = acpi_power_get_state(resource->device.handle, &state);
+ if (!result && state == ACPI_POWER_RESOURCE_STATE_OFF
+ && resource->ref_count) {
+ dev_info(&resource->device.dev, "Turning ON\n");
+ __acpi_power_on(resource);
+ }
- unlock:
- mutex_unlock(&resource->resource_lock);
+ mutex_unlock(&resource->resource_lock);
+ }
- return result;
+ mutex_unlock(&power_resource_list_lock);
}
#endif
-
-int __init acpi_power_init(void)
-{
- INIT_LIST_HEAD(&acpi_power_resource_list);
- return acpi_bus_register_driver(&acpi_power_driver);
-}
Index: linux/drivers/acpi/sleep.c
===================================================================
--- linux.orig/drivers/acpi/sleep.c
+++ linux/drivers/acpi/sleep.c
@@ -386,6 +386,8 @@ static void acpi_pm_finish(void)
acpi_target_sleep_state = ACPI_STATE_S0;
+ acpi_resume_power_resources();
+
/* If we were woken with the fixed power button, provide a small
* hint to userspace in the form of a wakeup event on the fixed power
* button device (if it can be found).
Index: linux/drivers/acpi/sleep.h
===================================================================
--- linux.orig/drivers/acpi/sleep.h
+++ linux/drivers/acpi/sleep.h
@@ -6,3 +6,5 @@ extern void acpi_disable_wakeup_devices(
extern struct list_head acpi_wakeup_device_list;
extern struct mutex acpi_device_lock;
+
+extern void acpi_resume_power_resources(void);
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -1670,7 +1670,6 @@ int __init acpi_scan_init(void)
printk(KERN_ERR PREFIX "Could not register bus type\n");
}
- acpi_power_init();
acpi_pci_root_init();
/*
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 5/12] ACPI: Do not use device power states of power resources
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (3 preceding siblings ...)
2013-01-04 0:03 ` [PATCH 4/12] ACPI: Drop power resources driver Rafael J. Wysocki
@ 2013-01-04 0:04 ` Rafael J. Wysocki
2013-01-04 0:05 ` [PATCH 6/12] ACPI / PM: Take order attribute of power resources into account Rafael J. Wysocki
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:04 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
ACPI power resource objects have struct acpi_device components, but
they are only used for registering those resources in the device
hierarchy. In particular, power state information stored in them is
completely useless (amnong other things, because the power resources
"devices" are not power manageable), so there is no reason for the
power resources management code to keep it up to date.
Remove the code updating device power states of power resources from
drivers/acpi/power.c.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/power.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -193,9 +193,6 @@ static int __acpi_power_on(struct acpi_p
if (ACPI_FAILURE(status))
return -ENODEV;
- /* Update the power resource's _device_ power state */
- resource->device.power.state = ACPI_STATE_D0;
-
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Power resource [%s] turned on\n",
resource->name));
@@ -261,16 +258,12 @@ static int acpi_power_off(acpi_handle ha
}
status = acpi_evaluate_object(resource->device.handle, "_OFF", NULL, NULL);
- if (ACPI_FAILURE(status)) {
+ if (ACPI_FAILURE(status))
result = -ENODEV;
- } else {
- /* Update the power resource's _device_ power state */
- resource->device.power.state = ACPI_STATE_D3;
-
+ else
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Power resource [%s] turned off\n",
resource->name));
- }
unlock:
mutex_unlock(&resource->resource_lock);
@@ -659,6 +652,7 @@ void acpi_add_power_resource(acpi_handle
resource->name = device->pnp.bus_id;
strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_POWER_CLASS);
+ device->power.state = ACPI_STATE_UNKNOWN;
/* Evalute the object to get the system level and resource order. */
status = acpi_evaluate_object(handle, NULL, NULL, &buffer);
@@ -672,17 +666,6 @@ void acpi_add_power_resource(acpi_handle
if (result)
goto err;
- switch (state) {
- case ACPI_POWER_RESOURCE_STATE_ON:
- device->power.state = ACPI_STATE_D0;
- break;
- case ACPI_POWER_RESOURCE_STATE_OFF:
- device->power.state = ACPI_STATE_D3;
- break;
- default:
- device->power.state = ACPI_STATE_UNKNOWN;
- }
-
printk(KERN_INFO PREFIX "%s [%s] (%s)\n", acpi_device_name(device),
acpi_device_bid(device), state ? "on" : "off");
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 6/12] ACPI / PM: Take order attribute of power resources into account
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (4 preceding siblings ...)
2013-01-04 0:04 ` [PATCH 5/12] ACPI: Do not use device power states of power resources Rafael J. Wysocki
@ 2013-01-04 0:05 ` Rafael J. Wysocki
2013-01-04 0:06 ` [PATCH 7/12] ACPI / PM: Take order attribute of wakeup " Rafael J. Wysocki
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:05 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
ACPI power resources have an order attribute that should be taken
into account when turning them on and off, but it is not used now.
Modify the power resources management code to preserve the
spec-compliant ordering of power resources that power states of
devices depend on (analogous changes will be done separately for
power resources used for wakeup).
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/internal.h | 2
drivers/acpi/power.c | 159 ++++++++++++++++++++++++++----------------------
drivers/acpi/scan.c | 31 +++++++--
include/acpi/acpi_bus.h | 2
4 files changed, 117 insertions(+), 77 deletions(-)
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -201,7 +201,7 @@ struct acpi_device_power_state {
} flags;
int power; /* % Power (compared to D0) */
int latency; /* Dx->D0 time (microseconds) */
- struct acpi_handle_list resources; /* Power resources referenced */
+ struct list_head resources; /* Power resources referenced */
};
struct acpi_device_power {
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -481,11 +481,25 @@ void acpi_free_ids(struct acpi_device *d
kfree(device->pnp.unique_id);
}
+static void acpi_free_power_resources_lists(struct acpi_device *device)
+{
+ int i;
+
+ if (!device->flags.power_manageable)
+ return;
+
+ for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
+ struct acpi_device_power_state *ps = &device->power.states[i];
+ acpi_power_resources_list_free(&ps->resources);
+ }
+}
+
static void acpi_device_release(struct device *dev)
{
struct acpi_device *acpi_dev = to_acpi_device(dev);
acpi_free_ids(acpi_dev);
+ acpi_free_power_resources_lists(acpi_dev);
kfree(acpi_dev);
}
@@ -1061,17 +1075,22 @@ static void acpi_bus_get_power_flags(str
for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
struct acpi_device_power_state *ps = &device->power.states[i];
char object_name[5] = { '_', 'P', 'R', '0' + i, '\0' };
+ struct acpi_handle_list resources;
+ INIT_LIST_HEAD(&ps->resources);
/* Evaluate "_PRx" to se if power resources are referenced */
acpi_evaluate_reference(device->handle, object_name, NULL,
- &ps->resources);
- if (ps->resources.count) {
+ &resources);
+ if (resources.count) {
int j;
device->power.flags.power_resources = 1;
- for (j = 0; j < ps->resources.count; j++) {
- acpi_handle rhandle = ps->resources.handles[j];
+ for (j = 0; j < resources.count; j++) {
+ acpi_handle rhandle = resources.handles[j];
+
acpi_add_power_resource(rhandle);
+ acpi_power_resources_list_add(rhandle,
+ &ps->resources);
}
}
@@ -1085,7 +1104,7 @@ static void acpi_bus_get_power_flags(str
* State is valid if there are means to put the device into it.
* D3hot is only valid if _PR3 present.
*/
- if (ps->resources.count ||
+ if (resources.count ||
(ps->flags.explicit_set && i < ACPI_STATE_D3_HOT)) {
ps->flags.valid = 1;
ps->flags.os_accessible = 1;
@@ -1095,6 +1114,8 @@ static void acpi_bus_get_power_flags(str
ps->latency = -1; /* Unknown - driver assigned */
}
+ INIT_LIST_HEAD(&device->power.states[ACPI_STATE_D3_COLD].resources);
+
/* Set defaults for D0 and D3 states (always valid) */
device->power.states[ACPI_STATE_D0].flags.valid = 1;
device->power.states[ACPI_STATE_D0].power = 100;
Index: linux/drivers/acpi/internal.h
===================================================================
--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h
@@ -50,6 +50,8 @@ void acpi_free_ids(struct acpi_device *d
Power Resource
-------------------------------------------------------------------------- */
int acpi_power_init(void);
+void acpi_power_resources_list_add(acpi_handle handle, struct list_head *list);
+void acpi_power_resources_list_free(struct list_head *list);
void acpi_add_power_resource(acpi_handle handle);
void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
int acpi_device_sleep_wake(struct acpi_device *dev,
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -75,6 +75,11 @@ struct acpi_power_resource {
struct mutex resource_lock;
};
+struct acpi_power_resource_entry {
+ struct list_head node;
+ struct acpi_power_resource *resource;
+};
+
static LIST_HEAD(acpi_power_resource_list);
static DEFINE_MUTEX(power_resource_list_lock);
@@ -92,6 +97,41 @@ struct acpi_power_resource *acpi_power_g
return container_of(device, struct acpi_power_resource, device);
}
+void acpi_power_resources_list_add(acpi_handle handle, struct list_head *list)
+{
+ struct acpi_power_resource *resource = acpi_power_get_context(handle);
+ struct acpi_power_resource_entry *entry;
+
+ if (!resource || !list)
+ return;
+
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return;
+
+ entry->resource = resource;
+ if (!list_empty(list)) {
+ struct acpi_power_resource_entry *e;
+
+ list_for_each_entry(e, list, node)
+ if (e->resource->order > resource->order) {
+ list_add_tail(&entry->node, &e->node);
+ return;
+ }
+ }
+ list_add_tail(&entry->node, list);
+}
+
+void acpi_power_resources_list_free(struct list_head *list)
+{
+ struct acpi_power_resource_entry *entry, *e;
+
+ list_for_each_entry_safe(entry, e, list, node) {
+ list_del(&entry->node);
+ kfree(entry);
+ }
+}
+
static int acpi_power_get_state(acpi_handle handle, int *state)
{
acpi_status status = AE_OK;
@@ -119,31 +159,23 @@ static int acpi_power_get_state(acpi_han
return 0;
}
-static int acpi_power_get_list_state(struct acpi_handle_list *list, int *state)
+static int acpi_power_get_list_state(struct list_head *list, int *state)
{
+ struct acpi_power_resource_entry *entry;
int cur_state;
- int i = 0;
if (!list || !state)
return -EINVAL;
/* The state of the list is 'on' IFF all resources are 'on'. */
-
- for (i = 0; i < list->count; i++) {
- struct acpi_power_resource *resource;
- acpi_handle handle = list->handles[i];
+ list_for_each_entry(entry, list, node) {
+ struct acpi_power_resource *resource = entry->resource;
+ acpi_handle handle = resource->device.handle;
int result;
- resource = acpi_power_get_context(handle);
- if (!resource)
- return -ENODEV;
-
mutex_lock(&resource->resource_lock);
-
result = acpi_power_get_state(handle, &cur_state);
-
mutex_unlock(&resource->resource_lock);
-
if (result)
return result;
@@ -155,7 +187,6 @@ static int acpi_power_get_list_state(str
cur_state ? "on" : "off"));
*state = cur_state;
-
return 0;
}
@@ -199,14 +230,9 @@ static int __acpi_power_on(struct acpi_p
return 0;
}
-static int acpi_power_on(acpi_handle handle)
+static int acpi_power_on(struct acpi_power_resource *resource)
{
- int result = 0;
- struct acpi_power_resource *resource;
-
- resource = acpi_power_get_context(handle);
- if (!resource)
- return -ENODEV;
+ int result = 0;;
mutex_lock(&resource->resource_lock);
@@ -231,15 +257,10 @@ static int acpi_power_on(acpi_handle han
return result;
}
-static int acpi_power_off(acpi_handle handle)
+static int acpi_power_off(struct acpi_power_resource *resource)
{
- int result = 0;
acpi_status status = AE_OK;
- struct acpi_power_resource *resource;
-
- resource = acpi_power_get_context(handle);
- if (!resource)
- return -ENODEV;
+ int result = 0;
mutex_lock(&resource->resource_lock);
@@ -271,47 +292,48 @@ static int acpi_power_off(acpi_handle ha
return result;
}
-static void __acpi_power_off_list(struct acpi_handle_list *list, int num_res)
+static int acpi_power_off_list(struct list_head *list)
{
- int i;
+ struct acpi_power_resource_entry *entry;
+ int result = 0;
- for (i = num_res - 1; i >= 0 ; i--)
- acpi_power_off(list->handles[i]);
-}
+ list_for_each_entry_reverse(entry, list, node) {
+ result = acpi_power_off(entry->resource);
+ if (result)
+ goto err;
+ }
+ return 0;
-static void acpi_power_off_list(struct acpi_handle_list *list)
-{
- __acpi_power_off_list(list, list->count);
+ err:
+ list_for_each_entry_continue(entry, list, node)
+ acpi_power_on(entry->resource);
+
+ return result;
}
-static int acpi_power_on_list(struct acpi_handle_list *list)
+static int acpi_power_on_list(struct list_head *list)
{
+ struct acpi_power_resource_entry *entry;
int result = 0;
- int i;
- for (i = 0; i < list->count; i++) {
- result = acpi_power_on(list->handles[i]);
- if (result) {
- __acpi_power_off_list(list, i);
- break;
- }
+ list_for_each_entry(entry, list, node) {
+ result = acpi_power_on(entry->resource);
+ if (result)
+ goto err;
}
+ return 0;
+
+ err:
+ list_for_each_entry_continue_reverse(entry, list, node)
+ acpi_power_off(entry->resource);
return result;
}
-static void acpi_power_add_dependent(acpi_handle rhandle,
+static void acpi_power_add_dependent(struct acpi_power_resource *resource,
struct acpi_device *adev)
{
struct acpi_power_dependent_device *dep;
- struct acpi_power_resource *resource;
-
- if (!rhandle || !adev)
- return;
-
- resource = acpi_power_get_context(rhandle);
- if (!resource)
- return;
mutex_lock(&resource->resource_lock);
@@ -331,20 +353,12 @@ static void acpi_power_add_dependent(acp
mutex_unlock(&resource->resource_lock);
}
-static void acpi_power_remove_dependent(acpi_handle rhandle,
+static void acpi_power_remove_dependent(struct acpi_power_resource *resource,
struct acpi_device *adev)
{
struct acpi_power_dependent_device *dep;
- struct acpi_power_resource *resource;
struct work_struct *work = NULL;
- if (!rhandle || !adev)
- return;
-
- resource = acpi_power_get_context(rhandle);
- if (!resource)
- return;
-
mutex_lock(&resource->resource_lock);
list_for_each_entry(dep, &resource->dependent, node)
@@ -366,16 +380,16 @@ void acpi_power_add_remove_device(struct
{
if (adev->power.flags.power_resources) {
struct acpi_device_power_state *ps;
- int j;
+ struct acpi_power_resource_entry *entry;
ps = &adev->power.states[ACPI_STATE_D0];
- for (j = 0; j < ps->resources.count; j++) {
- acpi_handle rhandle = ps->resources.handles[j];
+ list_for_each_entry(entry, &ps->resources, node) {
+ struct acpi_power_resource *resource = entry->resource;
if (add)
- acpi_power_add_dependent(rhandle, adev);
+ acpi_power_add_dependent(resource, adev);
else
- acpi_power_remove_dependent(rhandle, adev);
+ acpi_power_remove_dependent(resource, adev);
}
}
}
@@ -539,7 +553,6 @@ int acpi_disable_wakeup_device_power(str
int acpi_power_get_inferred_state(struct acpi_device *device, int *state)
{
int result = 0;
- struct acpi_handle_list *list = NULL;
int list_state = 0;
int i = 0;
@@ -551,8 +564,9 @@ int acpi_power_get_inferred_state(struct
* required for a given D-state are 'on'.
*/
for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
- list = &device->power.states[i].resources;
- if (list->count < 1)
+ struct list_head *list = &device->power.states[i].resources;
+
+ if (list_empty(list))
continue;
result = acpi_power_get_list_state(list, &list_state);
@@ -571,9 +585,12 @@ int acpi_power_get_inferred_state(struct
int acpi_power_on_resources(struct acpi_device *device, int state)
{
- if (!device || state < ACPI_STATE_D0 || state > ACPI_STATE_D3)
+ if (!device || state < ACPI_STATE_D0 || state > ACPI_STATE_D3_COLD)
return -EINVAL;
+ if (state == ACPI_STATE_D3_COLD)
+ return 0;
+
return acpi_power_on_list(&device->power.states[state].resources);
}
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 7/12] ACPI / PM: Take order attribute of wakeup power resources into account
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (5 preceding siblings ...)
2013-01-04 0:05 ` [PATCH 6/12] ACPI / PM: Take order attribute of power resources into account Rafael J. Wysocki
@ 2013-01-04 0:06 ` Rafael J. Wysocki
2013-01-04 0:07 ` [PATCH 8/12] ACPI / scan: Move power state initialization to a separate routine Rafael J. Wysocki
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:06 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
ACPI power resources have an order attribute that should be taken
into account when turning them on and off, but it is not used now.
Modify the power resources management code to preserve the
spec-compliant ordering of wakeup power resources.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/power.c | 44 ++++++++++++++++----------------------------
drivers/acpi/scan.c | 24 ++++++++++++++----------
include/acpi/acpi_bus.h | 2 +-
3 files changed, 31 insertions(+), 39 deletions(-)
Index: linux/include/acpi/acpi_bus.h
===================================================================
--- linux.orig/include/acpi/acpi_bus.h
+++ linux/include/acpi/acpi_bus.h
@@ -244,7 +244,7 @@ struct acpi_device_wakeup {
acpi_handle gpe_device;
u64 gpe_number;
u64 sleep_state;
- struct acpi_handle_list resources;
+ struct list_head resources;
struct acpi_device_wakeup_flags flags;
int prepare_count;
};
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -485,6 +485,7 @@ static void acpi_free_power_resources_li
{
int i;
+ acpi_power_resources_list_free(&device->wakeup.resources);
if (!device->flags.power_manageable)
return;
@@ -908,6 +909,8 @@ acpi_bus_extract_wakeup_device_power_pac
if (!wakeup)
return AE_BAD_PARAMETER;
+ INIT_LIST_HEAD(&wakeup->resources);
+
/* _PRW */
status = acpi_evaluate_object(handle, "_PRW", NULL, &buffer);
if (ACPI_FAILURE(status)) {
@@ -954,19 +957,17 @@ acpi_bus_extract_wakeup_device_power_pac
}
wakeup->sleep_state = element->integer.value;
- if ((package->package.count - 2) > ACPI_MAX_HANDLES) {
- status = AE_NO_MEMORY;
- goto out;
- }
- wakeup->resources.count = package->package.count - 2;
- for (i = 0; i < wakeup->resources.count; i++) {
- element = &(package->package.elements[i + 2]);
+ for (i = 2; i < package->package.count; i++) {
+ acpi_handle rhandle;
+
+ element = &(package->package.elements[i]);
if (element->type != ACPI_TYPE_LOCAL_REFERENCE) {
status = AE_BAD_DATA;
goto out;
}
-
- wakeup->resources.handles[i] = element->reference.handle;
+ rhandle = element->reference.handle;
+ acpi_add_power_resource(rhandle);
+ acpi_power_resources_list_add(rhandle, &wakeup->resources);
}
acpi_setup_gpe_for_wake(handle, wakeup->gpe_device, wakeup->gpe_number);
@@ -1024,6 +1025,7 @@ static void acpi_bus_get_wakeup_device_f
status = acpi_bus_extract_wakeup_device_power_package(device->handle,
&device->wakeup);
if (ACPI_FAILURE(status)) {
+ acpi_power_resources_list_free(&device->wakeup.resources);
ACPI_EXCEPTION((AE_INFO, status, "Extracting _PRW package"));
return;
}
@@ -1513,9 +1515,11 @@ static acpi_status acpi_bus_check_add(ac
acpi_handle temp;
status = acpi_get_handle(handle, "_PRW", &temp);
- if (ACPI_SUCCESS(status))
+ if (ACPI_SUCCESS(status)) {
acpi_bus_extract_wakeup_device_power_package(handle,
&wakeup);
+ acpi_power_resources_list_free(&wakeup.resources);
+ }
return AE_CTRL_DEPTH;
}
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -469,7 +469,7 @@ int acpi_device_sleep_wake(struct acpi_d
*/
int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
{
- int i, err = 0;
+ int err = 0;
if (!dev || !dev->wakeup.flags.valid)
return -EINVAL;
@@ -479,24 +479,17 @@ int acpi_enable_wakeup_device_power(stru
if (dev->wakeup.prepare_count++)
goto out;
- /* Open power resource */
- for (i = 0; i < dev->wakeup.resources.count; i++) {
- int ret = acpi_power_on(dev->wakeup.resources.handles[i]);
- if (ret) {
- printk(KERN_ERR PREFIX "Transition power state\n");
- dev->wakeup.flags.valid = 0;
- err = -ENODEV;
- goto err_out;
- }
+ err = acpi_power_on_list(&dev->wakeup.resources);
+ if (err) {
+ dev_err(&dev->dev, "Cannot turn wakeup power resources on\n");
+ dev->wakeup.flags.valid = 0;
+ } else {
+ /*
+ * Passing 3 as the third argument below means the device may be
+ * put into arbitrary power state afterward.
+ */
+ err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
}
-
- /*
- * Passing 3 as the third argument below means the device may be placed
- * in arbitrary power state afterwards.
- */
- err = acpi_device_sleep_wake(dev, 1, sleep_state, 3);
-
- err_out:
if (err)
dev->wakeup.prepare_count = 0;
@@ -513,7 +506,7 @@ int acpi_enable_wakeup_device_power(stru
*/
int acpi_disable_wakeup_device_power(struct acpi_device *dev)
{
- int i, err = 0;
+ int err = 0;
if (!dev || !dev->wakeup.flags.valid)
return -EINVAL;
@@ -534,15 +527,10 @@ int acpi_disable_wakeup_device_power(str
if (err)
goto out;
- /* Close power resource */
- for (i = 0; i < dev->wakeup.resources.count; i++) {
- int ret = acpi_power_off(dev->wakeup.resources.handles[i]);
- if (ret) {
- printk(KERN_ERR PREFIX "Transition power state\n");
- dev->wakeup.flags.valid = 0;
- err = -ENODEV;
- goto out;
- }
+ err = acpi_power_off_list(&dev->wakeup.resources);
+ if (err) {
+ dev_err(&dev->dev, "Cannot turn wakeup power resources off\n");
+ dev->wakeup.flags.valid = 0;
}
out:
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 8/12] ACPI / scan: Move power state initialization to a separate routine
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (6 preceding siblings ...)
2013-01-04 0:06 ` [PATCH 7/12] ACPI / PM: Take order attribute of wakeup " Rafael J. Wysocki
@ 2013-01-04 0:07 ` Rafael J. Wysocki
2013-01-04 0:07 ` [PATCH 9/12] ACPI / scan: Remove unnecessary initialization of local variables Rafael J. Wysocki
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:07 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
To reduce indentation level and improve code readability, move the
initialization code related to device power states from
acpi_bus_get_power_flags() to a new routine,
acpi_bus_init_power_state().
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 87 +++++++++++++++++++++++++++-------------------------
1 file changed, 46 insertions(+), 41 deletions(-)
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -1045,6 +1045,50 @@ static void acpi_bus_get_wakeup_device_f
"error in _DSW or _PSW evaluation\n"));
}
+static void acpi_bus_init_power_state(struct acpi_device *device, int state)
+{
+ struct acpi_device_power_state *ps = &device->power.states[state];
+ char object_name[5] = { '_', 'P', 'R', '0' + state, '\0' };
+ struct acpi_handle_list resources;
+ acpi_handle handle;
+ acpi_status status;
+
+ INIT_LIST_HEAD(&ps->resources);
+
+ /* Evaluate "_PRx" to se if power resources are referenced */
+ acpi_evaluate_reference(device->handle, object_name, NULL, &resources);
+ if (resources.count) {
+ int j;
+
+ device->power.flags.power_resources = 1;
+ for (j = 0; j < resources.count; j++) {
+ acpi_handle rhandle = resources.handles[j];
+
+ acpi_add_power_resource(rhandle);
+ acpi_power_resources_list_add(rhandle, &ps->resources);
+ }
+ }
+
+ /* Evaluate "_PSx" to see if we can do explicit sets */
+ object_name[2] = 'S';
+ status = acpi_get_handle(device->handle, object_name, &handle);
+ if (ACPI_SUCCESS(status))
+ ps->flags.explicit_set = 1;
+
+ /*
+ * State is valid if there are means to put the device into it.
+ * D3hot is only valid if _PR3 present.
+ */
+ if (resources.count
+ || (ps->flags.explicit_set && state < ACPI_STATE_D3_HOT)) {
+ ps->flags.valid = 1;
+ ps->flags.os_accessible = 1;
+ }
+
+ ps->power = -1; /* Unknown - driver assigned */
+ ps->latency = -1; /* Unknown - driver assigned */
+}
+
static void acpi_bus_get_power_flags(struct acpi_device *device)
{
acpi_status status = 0;
@@ -1074,47 +1118,8 @@ static void acpi_bus_get_power_flags(str
/*
* Enumerate supported power management states
*/
- for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++) {
- struct acpi_device_power_state *ps = &device->power.states[i];
- char object_name[5] = { '_', 'P', 'R', '0' + i, '\0' };
- struct acpi_handle_list resources;
-
- INIT_LIST_HEAD(&ps->resources);
- /* Evaluate "_PRx" to se if power resources are referenced */
- acpi_evaluate_reference(device->handle, object_name, NULL,
- &resources);
- if (resources.count) {
- int j;
-
- device->power.flags.power_resources = 1;
- for (j = 0; j < resources.count; j++) {
- acpi_handle rhandle = resources.handles[j];
-
- acpi_add_power_resource(rhandle);
- acpi_power_resources_list_add(rhandle,
- &ps->resources);
- }
- }
-
- /* Evaluate "_PSx" to see if we can do explicit sets */
- object_name[2] = 'S';
- status = acpi_get_handle(device->handle, object_name, &handle);
- if (ACPI_SUCCESS(status))
- ps->flags.explicit_set = 1;
-
- /*
- * State is valid if there are means to put the device into it.
- * D3hot is only valid if _PR3 present.
- */
- if (resources.count ||
- (ps->flags.explicit_set && i < ACPI_STATE_D3_HOT)) {
- ps->flags.valid = 1;
- ps->flags.os_accessible = 1;
- }
-
- ps->power = -1; /* Unknown - driver assigned */
- ps->latency = -1; /* Unknown - driver assigned */
- }
+ for (i = ACPI_STATE_D0; i <= ACPI_STATE_D3_HOT; i++)
+ acpi_bus_init_power_state(device, i);
INIT_LIST_HEAD(&device->power.states[ACPI_STATE_D3_COLD].resources);
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 9/12] ACPI / scan: Remove unnecessary initialization of local variables
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (7 preceding siblings ...)
2013-01-04 0:07 ` [PATCH 8/12] ACPI / scan: Move power state initialization to a separate routine Rafael J. Wysocki
@ 2013-01-04 0:07 ` Rafael J. Wysocki
2013-01-04 0:08 ` [PATCH 10/12] ACPI / scan: Consolidate extraction of power resources lists Rafael J. Wysocki
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:07 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The local variables in acpi_bus_get_power_flags() need not be
initialized upfront, so change the code accordingly.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -1091,9 +1091,9 @@ static void acpi_bus_init_power_state(st
static void acpi_bus_get_power_flags(struct acpi_device *device)
{
- acpi_status status = 0;
- acpi_handle handle = NULL;
- u32 i = 0;
+ acpi_status status;
+ acpi_handle handle;
+ u32 i;
/* Presence of _PS0|_PR0 indicates 'power manageable' */
status = acpi_get_handle(device->handle, "_PS0", &handle);
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 10/12] ACPI / scan: Consolidate extraction of power resources lists
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (8 preceding siblings ...)
2013-01-04 0:07 ` [PATCH 9/12] ACPI / scan: Remove unnecessary initialization of local variables Rafael J. Wysocki
@ 2013-01-04 0:08 ` Rafael J. Wysocki
2013-01-04 0:09 ` [PATCH 11/12] ACPI: Take power resource initialization errors into account Rafael J. Wysocki
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:08 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The lists of ACPI power resources are currently extracted in two
different ways, one for wakeup power resources and one for power
resources that device power states depend on. There is no reason
why it should be done differently in those two cases, so introduce
a common routine for extracting power resources lists from data
returned by AML, acpi_extract_power_resources(), and make the
namespace scanning code use it for both wakeup and device power
states power resources.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/internal.h | 4 ++-
drivers/acpi/power.c | 32 +++++++++++++++++++++++++++++-
drivers/acpi/scan.c | 51 +++++++++++++++++++-----------------------------
3 files changed, 55 insertions(+), 32 deletions(-)
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -904,7 +904,6 @@ acpi_bus_extract_wakeup_device_power_pac
union acpi_object *package = NULL;
union acpi_object *element = NULL;
acpi_status status;
- int i = 0;
if (!wakeup)
return AE_BAD_PARAMETER;
@@ -957,18 +956,9 @@ acpi_bus_extract_wakeup_device_power_pac
}
wakeup->sleep_state = element->integer.value;
- for (i = 2; i < package->package.count; i++) {
- acpi_handle rhandle;
-
- element = &(package->package.elements[i]);
- if (element->type != ACPI_TYPE_LOCAL_REFERENCE) {
- status = AE_BAD_DATA;
- goto out;
- }
- rhandle = element->reference.handle;
- acpi_add_power_resource(rhandle);
- acpi_power_resources_list_add(rhandle, &wakeup->resources);
- }
+ status = acpi_extract_power_resources(package, 2, &wakeup->resources);
+ if (ACPI_FAILURE(status))
+ goto out;
acpi_setup_gpe_for_wake(handle, wakeup->gpe_device, wakeup->gpe_number);
@@ -1025,7 +1015,6 @@ static void acpi_bus_get_wakeup_device_f
status = acpi_bus_extract_wakeup_device_power_package(device->handle,
&device->wakeup);
if (ACPI_FAILURE(status)) {
- acpi_power_resources_list_free(&device->wakeup.resources);
ACPI_EXCEPTION((AE_INFO, status, "Extracting _PRW package"));
return;
}
@@ -1048,30 +1037,32 @@ static void acpi_bus_get_wakeup_device_f
static void acpi_bus_init_power_state(struct acpi_device *device, int state)
{
struct acpi_device_power_state *ps = &device->power.states[state];
- char object_name[5] = { '_', 'P', 'R', '0' + state, '\0' };
- struct acpi_handle_list resources;
+ char pathname[5] = { '_', 'P', 'R', '0' + state, '\0' };
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
acpi_handle handle;
acpi_status status;
INIT_LIST_HEAD(&ps->resources);
- /* Evaluate "_PRx" to se if power resources are referenced */
- acpi_evaluate_reference(device->handle, object_name, NULL, &resources);
- if (resources.count) {
- int j;
-
- device->power.flags.power_resources = 1;
- for (j = 0; j < resources.count; j++) {
- acpi_handle rhandle = resources.handles[j];
-
- acpi_add_power_resource(rhandle);
- acpi_power_resources_list_add(rhandle, &ps->resources);
+ /* Evaluate "_PRx" to get referenced power resources */
+ status = acpi_evaluate_object(device->handle, pathname, NULL, &buffer);
+ if (ACPI_SUCCESS(status)) {
+ union acpi_object *package = buffer.pointer;
+
+ if (buffer.length && package
+ && package->type == ACPI_TYPE_PACKAGE
+ && package->package.count) {
+ status = acpi_extract_power_resources(package, 0,
+ &ps->resources);
+ if (ACPI_SUCCESS(status))
+ device->power.flags.power_resources = 1;
}
+ ACPI_FREE(buffer.pointer);
}
/* Evaluate "_PSx" to see if we can do explicit sets */
- object_name[2] = 'S';
- status = acpi_get_handle(device->handle, object_name, &handle);
+ pathname[2] = 'S';
+ status = acpi_get_handle(device->handle, pathname, &handle);
if (ACPI_SUCCESS(status))
ps->flags.explicit_set = 1;
@@ -1079,7 +1070,7 @@ static void acpi_bus_init_power_state(st
* State is valid if there are means to put the device into it.
* D3hot is only valid if _PR3 present.
*/
- if (resources.count
+ if (!list_empty(&ps->resources)
|| (ps->flags.explicit_set && state < ACPI_STATE_D3_HOT)) {
ps->flags.valid = 1;
ps->flags.os_accessible = 1;
Index: linux/drivers/acpi/internal.h
===================================================================
--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h
@@ -50,8 +50,10 @@ void acpi_free_ids(struct acpi_device *d
Power Resource
-------------------------------------------------------------------------- */
int acpi_power_init(void);
-void acpi_power_resources_list_add(acpi_handle handle, struct list_head *list);
void acpi_power_resources_list_free(struct list_head *list);
+acpi_status acpi_extract_power_resources(union acpi_object *package,
+ unsigned int start,
+ struct list_head *list);
void acpi_add_power_resource(acpi_handle handle);
void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
int acpi_device_sleep_wake(struct acpi_device *dev,
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -97,7 +97,8 @@ struct acpi_power_resource *acpi_power_g
return container_of(device, struct acpi_power_resource, device);
}
-void acpi_power_resources_list_add(acpi_handle handle, struct list_head *list)
+static void acpi_power_resources_list_add(acpi_handle handle,
+ struct list_head *list)
{
struct acpi_power_resource *resource = acpi_power_get_context(handle);
struct acpi_power_resource_entry *entry;
@@ -132,6 +133,35 @@ void acpi_power_resources_list_free(stru
}
}
+acpi_status acpi_extract_power_resources(union acpi_object *package,
+ unsigned int start,
+ struct list_head *list)
+{
+ acpi_status status = AE_OK;
+ unsigned int i;
+
+ for (i = start; i < package->package.count; i++) {
+ union acpi_object *element = &package->package.elements[i];
+ acpi_handle rhandle;
+
+ if (element->type != ACPI_TYPE_LOCAL_REFERENCE) {
+ status = AE_BAD_DATA;
+ break;
+ }
+ rhandle = element->reference.handle;
+ if (!rhandle) {
+ status = AE_NULL_ENTRY;
+ break;
+ }
+ acpi_add_power_resource(rhandle);
+ acpi_power_resources_list_add(rhandle, list);
+ }
+ if (ACPI_FAILURE(status))
+ acpi_power_resources_list_free(list);
+
+ return status;
+}
+
static int acpi_power_get_state(acpi_handle handle, int *state)
{
acpi_status status = AE_OK;
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 11/12] ACPI: Take power resource initialization errors into account
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (9 preceding siblings ...)
2013-01-04 0:08 ` [PATCH 10/12] ACPI / scan: Consolidate extraction of power resources lists Rafael J. Wysocki
@ 2013-01-04 0:09 ` Rafael J. Wysocki
2013-01-04 0:10 ` [PATCH 12/12] ACPI: Use system level attribute of wakeup power resources Rafael J. Wysocki
2013-01-11 21:57 ` [PATCH 0/12] ACPI: Rework the handling of " Rafael J. Wysocki
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:09 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Some ACPI power resource initialization errors, like memory
allocation errors, are not taken into account appropriately in some
cases, which may lead to a device having an incomplete list of power
resources that one of its power states depends on, for one example.
Rework the power resource initialization and namespace scanning code
so that power resource initialization errors are treated more
seriously.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/internal.h | 7 ++---
drivers/acpi/power.c | 44 +++++++++++++++++++++----------------
drivers/acpi/scan.c | 57 +++++++++++++++++++++---------------------------
3 files changed, 53 insertions(+), 55 deletions(-)
Index: linux/drivers/acpi/internal.h
===================================================================
--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h
@@ -51,10 +51,9 @@ void acpi_free_ids(struct acpi_device *d
-------------------------------------------------------------------------- */
int acpi_power_init(void);
void acpi_power_resources_list_free(struct list_head *list);
-acpi_status acpi_extract_power_resources(union acpi_object *package,
- unsigned int start,
- struct list_head *list);
-void acpi_add_power_resource(acpi_handle handle);
+int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
+ struct list_head *list);
+int acpi_add_power_resource(acpi_handle handle);
void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
int acpi_device_sleep_wake(struct acpi_device *dev,
int enable, int sleep_state, int dev_state);
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -97,18 +97,18 @@ struct acpi_power_resource *acpi_power_g
return container_of(device, struct acpi_power_resource, device);
}
-static void acpi_power_resources_list_add(acpi_handle handle,
- struct list_head *list)
+static int acpi_power_resources_list_add(acpi_handle handle,
+ struct list_head *list)
{
struct acpi_power_resource *resource = acpi_power_get_context(handle);
struct acpi_power_resource_entry *entry;
if (!resource || !list)
- return;
+ return -EINVAL;
entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
- return;
+ return -ENOMEM;
entry->resource = resource;
if (!list_empty(list)) {
@@ -117,10 +117,11 @@ static void acpi_power_resources_list_ad
list_for_each_entry(e, list, node)
if (e->resource->order > resource->order) {
list_add_tail(&entry->node, &e->node);
- return;
+ return 0;
}
}
list_add_tail(&entry->node, list);
+ return 0;
}
void acpi_power_resources_list_free(struct list_head *list)
@@ -133,33 +134,37 @@ void acpi_power_resources_list_free(stru
}
}
-acpi_status acpi_extract_power_resources(union acpi_object *package,
- unsigned int start,
- struct list_head *list)
+int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
+ struct list_head *list)
{
- acpi_status status = AE_OK;
unsigned int i;
+ int err = 0;
for (i = start; i < package->package.count; i++) {
union acpi_object *element = &package->package.elements[i];
acpi_handle rhandle;
if (element->type != ACPI_TYPE_LOCAL_REFERENCE) {
- status = AE_BAD_DATA;
+ err = -ENODATA;
break;
}
rhandle = element->reference.handle;
if (!rhandle) {
- status = AE_NULL_ENTRY;
+ err = -ENODEV;
break;
}
- acpi_add_power_resource(rhandle);
- acpi_power_resources_list_add(rhandle, list);
+ err = acpi_add_power_resource(rhandle);
+ if (err)
+ break;
+
+ err = acpi_power_resources_list_add(rhandle, list);
+ if (err)
+ break;
}
- if (ACPI_FAILURE(status))
+ if (err)
acpi_power_resources_list_free(list);
- return status;
+ return err;
}
static int acpi_power_get_state(acpi_handle handle, int *state)
@@ -662,7 +667,7 @@ static void acpi_release_power_resource(
kfree(resource);
}
-void acpi_add_power_resource(acpi_handle handle)
+int acpi_add_power_resource(acpi_handle handle)
{
struct acpi_power_resource *resource;
struct acpi_device *device = NULL;
@@ -673,11 +678,11 @@ void acpi_add_power_resource(acpi_handle
acpi_bus_get_device(handle, &device);
if (device)
- return;
+ return 0;
resource = kzalloc(sizeof(*resource), GFP_KERNEL);
if (!resource)
- return;
+ return -ENOMEM;
device = &resource->device;
acpi_init_device_object(device, handle, ACPI_BUS_TYPE_POWER,
@@ -712,10 +717,11 @@ void acpi_add_power_resource(acpi_handle
mutex_lock(&power_resource_list_lock);
list_add(&resource->list_node, &acpi_power_resource_list);
mutex_unlock(&power_resource_list_lock);
- return;
+ return 0;
err:
acpi_release_power_resource(&device->dev);
+ return result;
}
#ifdef CONFIG_ACPI_SLEEP
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -896,17 +896,17 @@ void acpi_bus_data_handler(acpi_handle h
return;
}
-static acpi_status
-acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
- struct acpi_device_wakeup *wakeup)
+static int acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
+ struct acpi_device_wakeup *wakeup)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
union acpi_object *package = NULL;
union acpi_object *element = NULL;
acpi_status status;
+ int err = -ENODATA;
if (!wakeup)
- return AE_BAD_PARAMETER;
+ return -EINVAL;
INIT_LIST_HEAD(&wakeup->resources);
@@ -914,29 +914,25 @@ acpi_bus_extract_wakeup_device_power_pac
status = acpi_evaluate_object(handle, "_PRW", NULL, &buffer);
if (ACPI_FAILURE(status)) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRW"));
- return status;
+ return err;
}
package = (union acpi_object *)buffer.pointer;
- if (!package || (package->package.count < 2)) {
- status = AE_BAD_DATA;
+ if (!package || package->package.count < 2)
goto out;
- }
element = &(package->package.elements[0]);
- if (!element) {
- status = AE_BAD_DATA;
+ if (!element)
goto out;
- }
+
if (element->type == ACPI_TYPE_PACKAGE) {
if ((element->package.count < 2) ||
(element->package.elements[0].type !=
ACPI_TYPE_LOCAL_REFERENCE)
- || (element->package.elements[1].type != ACPI_TYPE_INTEGER)) {
- status = AE_BAD_DATA;
+ || (element->package.elements[1].type != ACPI_TYPE_INTEGER))
goto out;
- }
+
wakeup->gpe_device =
element->package.elements[0].reference.handle;
wakeup->gpe_number =
@@ -945,27 +941,24 @@ acpi_bus_extract_wakeup_device_power_pac
wakeup->gpe_device = NULL;
wakeup->gpe_number = element->integer.value;
} else {
- status = AE_BAD_DATA;
goto out;
}
element = &(package->package.elements[1]);
- if (element->type != ACPI_TYPE_INTEGER) {
- status = AE_BAD_DATA;
+ if (element->type != ACPI_TYPE_INTEGER)
goto out;
- }
+
wakeup->sleep_state = element->integer.value;
- status = acpi_extract_power_resources(package, 2, &wakeup->resources);
- if (ACPI_FAILURE(status))
+ err = acpi_extract_power_resources(package, 2, &wakeup->resources);
+ if (err)
goto out;
acpi_setup_gpe_for_wake(handle, wakeup->gpe_device, wakeup->gpe_number);
out:
kfree(buffer.pointer);
-
- return status;
+ return err;
}
static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
@@ -1005,17 +998,17 @@ static void acpi_bus_get_wakeup_device_f
{
acpi_handle temp;
acpi_status status = 0;
- int psw_error;
+ int err;
/* Presence of _PRW indicates wake capable */
status = acpi_get_handle(device->handle, "_PRW", &temp);
if (ACPI_FAILURE(status))
return;
- status = acpi_bus_extract_wakeup_device_power_package(device->handle,
- &device->wakeup);
- if (ACPI_FAILURE(status)) {
- ACPI_EXCEPTION((AE_INFO, status, "Extracting _PRW package"));
+ err = acpi_bus_extract_wakeup_device_power_package(device->handle,
+ &device->wakeup);
+ if (err) {
+ dev_err(&device->dev, "_PRW evaluation error: %d\n", err);
return;
}
@@ -1028,8 +1021,8 @@ static void acpi_bus_get_wakeup_device_f
* So it is necessary to call _DSW object first. Only when it is not
* present will the _PSW object used.
*/
- psw_error = acpi_device_sleep_wake(device, 0, 0, 0);
- if (psw_error)
+ err = acpi_device_sleep_wake(device, 0, 0, 0);
+ if (err)
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"error in _DSW or _PSW evaluation\n"));
}
@@ -1052,9 +1045,9 @@ static void acpi_bus_init_power_state(st
if (buffer.length && package
&& package->type == ACPI_TYPE_PACKAGE
&& package->package.count) {
- status = acpi_extract_power_resources(package, 0,
- &ps->resources);
- if (ACPI_SUCCESS(status))
+ int err = acpi_extract_power_resources(package, 0,
+ &ps->resources);
+ if (!err)
device->power.flags.power_resources = 1;
}
ACPI_FREE(buffer.pointer);
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 12/12] ACPI: Use system level attribute of wakeup power resources
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (10 preceding siblings ...)
2013-01-04 0:09 ` [PATCH 11/12] ACPI: Take power resource initialization errors into account Rafael J. Wysocki
@ 2013-01-04 0:10 ` Rafael J. Wysocki
2013-01-11 21:57 ` [PATCH 0/12] ACPI: Rework the handling of " Rafael J. Wysocki
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-04 0:10 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The system level attribute of ACPI power resources is the lowest
system sleep level (S0, S2 etc.) in which the given resource can be
"on" (ACPI 5.0, Section 7.1). On the other hand, wakeup power
resources have to be "on" for devices depending on them to be able to
signal wakeup. Therefore devices cannot wake up the system from
sleep states higher than the minimum of the system level attributes
of their wakeup power resources.
Use the wakeup power resources' system level values to get the
deepest system sleep state (highest system sleep level) the given
device can wake up the system from.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/internal.h | 1 +
drivers/acpi/power.c | 14 ++++++++++++++
drivers/acpi/scan.c | 11 +++++++++++
3 files changed, 26 insertions(+)
Index: linux/drivers/acpi/internal.h
===================================================================
--- linux.orig/drivers/acpi/internal.h
+++ linux/drivers/acpi/internal.h
@@ -55,6 +55,7 @@ int acpi_extract_power_resources(union a
struct list_head *list);
int acpi_add_power_resource(acpi_handle handle);
void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
+int acpi_power_min_system_level(struct list_head *list);
int acpi_device_sleep_wake(struct acpi_device *dev,
int enable, int sleep_state, int dev_state);
int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
Index: linux/drivers/acpi/power.c
===================================================================
--- linux.orig/drivers/acpi/power.c
+++ linux/drivers/acpi/power.c
@@ -429,6 +429,20 @@ void acpi_power_add_remove_device(struct
}
}
+int acpi_power_min_system_level(struct list_head *list)
+{
+ struct acpi_power_resource_entry *entry;
+ int system_level = 5;
+
+ list_for_each_entry(entry, list, node) {
+ struct acpi_power_resource *resource = entry->resource;
+
+ if (system_level > resource->system_level)
+ system_level = resource->system_level;
+ }
+ return system_level;
+}
+
/* --------------------------------------------------------------------------
Device Power Management
-------------------------------------------------------------------------- */
Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -954,6 +954,17 @@ static int acpi_bus_extract_wakeup_devic
if (err)
goto out;
+ if (!list_empty(&wakeup->resources)) {
+ int sleep_state;
+
+ sleep_state = acpi_power_min_system_level(&wakeup->resources);
+ if (sleep_state < wakeup->sleep_state) {
+ acpi_handle_warn(handle, "Overriding _PRW sleep state "
+ "(S%d) by S%d from power resources\n",
+ (int)wakeup->sleep_state, sleep_state);
+ wakeup->sleep_state = sleep_state;
+ }
+ }
acpi_setup_gpe_for_wake(handle, wakeup->gpe_device, wakeup->gpe_number);
out:
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/12] ACPI: Rework the handling of power resources
2013-01-03 23:16 [PATCH 0/12] ACPI: Rework the handling of power resources Rafael J. Wysocki
` (11 preceding siblings ...)
2013-01-04 0:10 ` [PATCH 12/12] ACPI: Use system level attribute of wakeup power resources Rafael J. Wysocki
@ 2013-01-11 21:57 ` Rafael J. Wysocki
12 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2013-01-11 21:57 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML
On Friday, January 04, 2013 12:16:48 AM Rafael J. Wysocki wrote:
> Hi All,
>
> There are some problems with our handling of ACPI power resources that have
> accumulated over time or have never been addressed.
>
> First of all, the interface represented by
> acpi_power_resource_[un]register_device() doesn't really work as it should,
> so it needs to be redesigned (details in the changelog of patch [1/12]).
>
> Second, power resources really have to be treated in a special way by the
> ACPI namespace scanning code, because they may need to be ready to use as
> soon as they are discovered. Moreover, the are represented by struct
> acpi_device objects, so that they appear in sysfs along with "regular" ACPI
> device nodes, they really don't represent devices, so they need special
> configuration treatment in some cases. This special casing is obnoxiously
> present in scan.c, but if we accept the fact that power resources really are
> a special case, things can be simplified quite a bit (patch [3/12] with some
> preparations in [2/12]).
>
> If the above changes are made, then it turns out that the entire
> acpi_power_driver structure is not necessary any more and we don't need the
> device power states of power resources for anything (patches [4-5/12]).
>
> Moreover, ACPI power resources are supposed to be ordered. Namely, they have
> an attribute defining the order in which they are supposed to turned on and
> off. Currently, though, we don't take that attribute into accound, although
> we obviously should do that. After the previous changes that may be
> implemented at a relatively low cost (patch [6-7/12]).
>
> Patches [8-9/12] move code around and clean it up in preparation for patch
> [10/12], which consolidates the way in which power resources lists are
> extracted from the output of _PRW and _PR[0-3].
>
> Patch [11/12] improves the handling of power resource initialization errors
> and patch [12/12] causes the system level attribute of wakeup power resources
> to be taken into account during device setup.
>
> All patches are on top of the git branch at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
>
> Tested on HP nx6325 (and other boxes where ACPI power resources aren't used).
There has been a week without a single comment, so I gather the thing is
not controversial at all. I'll put it into linux-next for v3.9, then.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 14+ messages in thread