Message-ID: <2027798.rmHWafy1WQ@vostro.rjw.lan>
Date:	Sun, 30 Dec 2012 00:21:56 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	ACPI Devel Maling List <linux-acpi@...r.kernel.org>
Cc:	LKML <linux-kernel@...r.kernel.org>, Len Brown <lenb@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Huang Ying <ying.huang@...el.com>
Subject: [Update][PATCH] ACPI / PM: Rework the handling of devices depending on power resources

From: Rafael J. Wysocki <rafael.j.wysocki@...el.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 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.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---

The update is necessary to prevent device nodes from being added multiple times
to the list of dependent devices of a given power resource.  To be precise,
acpi_power_add_dependent() needs to check if there is an entry in
resource->dependent for the given device and only add a new one if that's not
the case.

Thanks,
Rafael

---
 drivers/acpi/internal.h   |    2 
 drivers/acpi/power.c      |  210 ++++++++++++++++------------------------------
 drivers/acpi/scan.c       |   26 ++++-
 drivers/ata/libata-acpi.c |    4 
 drivers/pci/pci-acpi.c    |    2 
 include/acpi/acpi_bus.h   |    2 
 6 files changed, 98 insertions(+), 148 deletions(-)

Index: linux/drivers/acpi/scan.c
===================================================================
--- linux.orig/drivers/acpi/scan.c
+++ linux/drivers/acpi/scan.c
@@ -755,6 +755,14 @@ static void acpi_device_unregister(struc
 
 	acpi_device_remove_files(device);
 	device_unregister(&device->dev);
+
+	/*
+	 * Remove the device from the dependent devices lists of all of its
+	 * power resources, drop the reference counts of all power resources it
+	 * depends on and turn off the ones that have no more references.
+	 */
+	acpi_power_remove_device(device);
+	acpi_power_transition(device, ACPI_STATE_D3_COLD);
 }
 
 /* --------------------------------------------------------------------------
@@ -1053,6 +1061,19 @@ static void acpi_bus_get_wakeup_device_f
 
 static void acpi_bus_add_power_resource(acpi_handle handle);
 
+static void acpi_bus_update_power_resources(struct acpi_device *device,
+					    struct acpi_device_power_state *ps)
+{
+	int j;
+
+	for (j = 0; j < ps->resources.count; j++) {
+		acpi_handle rhandle = ps->resources.handles[j];
+
+		acpi_bus_add_power_resource(rhandle);
+		acpi_power_add_dependent(rhandle, device);
+	}
+}
+
 static int acpi_bus_get_power_flags(struct acpi_device *device)
 {
 	acpi_status status = 0;
@@ -1081,11 +1102,8 @@ static int acpi_bus_get_power_flags(stru
 		acpi_evaluate_reference(device->handle, object_name, NULL,
 					&ps->resources);
 		if (ps->resources.count) {
-			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]);
+			acpi_bus_update_power_resources(device, ps);
 		}
 
 		/* Evaluate "_PSx" to see if we can do explicit sets */
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,27 @@ 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);
+
+	mutex_unlock(&adev->physical_node_lock);
 }
 
 static int __acpi_power_on(struct acpi_power_resource *resource)
@@ -244,9 +239,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 +253,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;
-	}
+		} else {
+			struct acpi_power_dependent_device *dep;
 
-	mutex_unlock(&resource->resource_lock);
-
-	if (!resume_device)
-		return result;
-
-	mutex_lock(&resource->devices_lock);
-
-	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 +341,78 @@ static int acpi_power_on_list(struct acp
 	return result;
 }
 
-static void __acpi_power_resource_unregister_device(struct device *dev,
-		acpi_handle res_handle)
+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_remove_device(struct acpi_device *adev)
 {
-	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;
-
-	powered_device = kzalloc(sizeof(*powered_device), GFP_KERNEL);
-	if (!powered_device)
-		return -ENOMEM;
-
-	powered_device->dev = dev;
-	powered_device->handle = handle;
+	int state;
 
-	list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
+	for (state = ACPI_STATE_D0; state <= ACPI_STATE_D3_HOT; state++) {
+		struct acpi_device_power_state *ps = &adev->power.states[state];
+		int j;
 
-	for (i = 0; i < list->count; i++) {
-		ret = __acpi_power_resource_register_device(powered_device,
-			list->handles[i]);
+		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;
+			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 +566,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 +664,6 @@ static int acpi_power_add(struct acpi_de
 
 	resource->device = device;
 	mutex_init(&resource->resource_lock);
-	mutex_init(&resource->devices_lock);
 	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
@@ -42,6 +42,8 @@ static inline void acpi_debugfs_init(voi
                                   Power Resource
    -------------------------------------------------------------------------- */
 int acpi_power_init(void);
+void acpi_power_add_dependent(acpi_handle rhandle, struct acpi_device *adev);
+void acpi_power_remove_device(struct acpi_device *adev);
 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
@@ -330,7 +330,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;
 
@@ -353,7 +352,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
@@ -377,8 +377,6 @@ static inline bool acpi_bus_can_wakeup(a
 }
 #endif /* !CONFIG_PM */
 
-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
@@ -1036,8 +1036,6 @@ static void ata_acpi_register_power_reso
 		return;
 
 	device = &sdev->sdev_gendev;
-
-	acpi_power_resource_register_device(device, handle);
 }
 
 static void ata_acpi_unregister_power_resource(struct ata_device *dev)
@@ -1051,8 +1049,6 @@ static void ata_acpi_unregister_power_re
 		return;
 
 	device = &sdev->sdev_gendev;
-
-	acpi_power_resource_unregister_device(device, handle);
 }
 
 void ata_acpi_bind(struct ata_device *dev)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists