On Thu, Jun 13, 2019 at 12:38:50AM +0200, Rafael J. Wysocki wrote:
> Basically, at the pci_acpi_setup() time dev and adev need to be passed to a function that
> will add dev as a "dependent device" for each of the power resources in the adev's D0
> list.
>
> Next whenever a power resource with a list of "dependent devices" goes _ON successfully,
> pm_request_resume() needs to be called for each device in that list.
>
> Finally, at the pci_acpi_cleanup() time, dev needs to be removed from the lists of
> "dependent devices" for all power resources in its ACPI companion's D0 list.
>
> At least that's how I see that.
Thanks for the suggestion. This seems to make it work only for PCI
devices, though. Is that the intention?
I went for the all ACPI devices path instead where we add all devices
sharing the power resource as "consumers" for that resource. I haven't
fully tested the approach yet but the draft patch is below. I have no
issues doing what you say above, though :)
----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index f6157d4d637a..e840299c3293 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -127,10 +127,10 @@ int __acpi_device_uevent_modalias(struct acpi_device *adev,
Power Resource
-------------------------------------------------------------------------- */
int acpi_power_init(void);
-void acpi_power_resources_list_free(struct list_head *list);
-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_resources_list_free(struct acpi_device *device, struct list_head *list);
+int acpi_extract_power_resources(struct acpi_device *device,
+ union acpi_object *package, unsigned int start, struct list_head *list);
+int acpi_add_power_resource(struct acpi_device *adev, acpi_handle handle);
void acpi_power_add_remove_device(struct acpi_device *adev, bool add);
int acpi_power_wakeup_list_init(struct list_head *list, int *system_level);
int acpi_device_sleep_wake(struct acpi_device *dev,
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index a916417b9e70..31817f931381 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -45,6 +45,8 @@ ACPI_MODULE_NAME("power");
struct acpi_power_resource {
struct acpi_device device;
struct list_head list_node;
+ struct list_head consumers;
+ struct mutex consumer_lock;
char *name;
u32 system_level;
u32 order;
@@ -58,6 +60,11 @@ struct acpi_power_resource_entry {
struct acpi_power_resource *resource;
};
+struct acpi_power_resource_consumer {
+ struct list_head node;
+ struct acpi_device *consumer;
+};
+
static LIST_HEAD(acpi_power_resource_list);
static DEFINE_MUTEX(power_resource_list_lock);
@@ -81,6 +88,111 @@ static struct acpi_power_resource *acpi_power_get_context(acpi_handle handle)
return to_power_resource(device);
}
+static int acpi_power_add_consumer(struct acpi_power_resource *resource,
+ struct acpi_device *device)
+{
+ struct acpi_power_resource_consumer *consumer;
+ int ret = 0;
+
+ if (!device)
+ return 0;
+
+ mutex_lock(&resource->consumer_lock);
+
+ list_for_each_entry(consumer, &resource->consumers, node) {
+ /* Don't add it twice */
+ if (consumer->consumer == device)
+ goto unlock;
+ }
+
+ consumer = kzalloc(sizeof(*consumer), GFP_KERNEL);
+ if (!consumer) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ consumer->consumer = device;
+ list_add_tail(&consumer->node, &resource->consumers);
+
+ acpi_handle_debug(device->handle, "added dependency to %s\n",
+ resource->name);
+
+unlock:
+ mutex_unlock(&resource->consumer_lock);
+ return ret;
+}
+
+static void acpi_power_remove_consumer(struct acpi_power_resource *resource,
+ struct acpi_device *device)
+{
+ struct acpi_power_resource_consumer *consumer;
+
+ mutex_lock(&resource->consumer_lock);
+ list_for_each_entry(consumer, &resource->consumers, node) {
+ if (consumer->consumer == device) {
+ list_del(&consumer->node);
+ kfree(consumer);
+
+ acpi_handle_debug(device->handle,
+ "removed dependency to %s\n",
+ resource->name);
+ break;
+ }
+ }
+ mutex_unlock(&resource->consumer_lock);
+}
+
+static void
+acpi_power_resume_consumer(const struct acpi_power_resource *resource,
+ struct acpi_device *device)
+{
+ struct device *dev;
+
+ /*
+ * If the device is prepared to wake we need to resume it now so
+ * that the driver can re-program it to do so. For non-wake devices
+ * we can leave them as is. The driver then restores the device
+ * when it is needed next time.
+ */
+ if (!acpi_device_can_wakeup(device) || !device->wakeup.prepare_count)
+ return;
+
+ dev = acpi_get_first_physical_node(device);
+ if (dev) {
+ acpi_handle_debug(device->handle,
+ "resuming %s because %s was turned on\n",
+ dev_name(dev), resource->name);
+ pm_runtime_resume(dev);
+ }
+}
+
+static int acpi_power_resume_consumers(struct acpi_power_resource *resource,
+ struct acpi_device *device)
+{
+ struct acpi_power_resource_consumer *consumer;
+ int ret = 0;
+
+ mutex_lock(&resource->consumer_lock);
+
+ list_for_each_entry(consumer, &resource->consumers, node) {
+ struct acpi_device *adev = consumer->consumer;
+ int state, ret;
+
+ /* Skip the device that originated the power on request */
+ if (adev == device)
+ continue;
+
+ ret = acpi_power_get_inferred_state(consumer->consumer, &state);
+ if (!ret && adev->power.state > ACPI_STATE_D0 &&
+ state == ACPI_STATE_D0) {
+ acpi_power_resume_consumer(resource, adev);
+ }
+ }
+
+ mutex_unlock(&resource->consumer_lock);
+ return ret;
+}
+
static int acpi_power_resources_list_add(acpi_handle handle,
struct list_head *list)
{
@@ -108,12 +220,14 @@ static int acpi_power_resources_list_add(acpi_handle handle,
return 0;
}
-void acpi_power_resources_list_free(struct list_head *list)
+void acpi_power_resources_list_free(struct acpi_device *device, struct list_head *list)
{
struct acpi_power_resource_entry *entry, *e;
list_for_each_entry_safe(entry, e, list, node) {
list_del(&entry->node);
+ if (device)
+ acpi_power_remove_consumer(entry->resource, device);
kfree(entry);
}
}
@@ -135,8 +249,8 @@ static bool acpi_power_resource_is_dup(union acpi_object *package,
return false;
}
-int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
- struct list_head *list)
+int acpi_extract_power_resources(struct acpi_device *device,
+ union acpi_object *package, unsigned int start, struct list_head *list)
{
unsigned int i;
int err = 0;
@@ -159,7 +273,7 @@ int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
if (acpi_power_resource_is_dup(package, start, i))
continue;
- err = acpi_add_power_resource(rhandle);
+ err = acpi_add_power_resource(device, rhandle);
if (err)
break;
@@ -168,7 +282,7 @@ int acpi_extract_power_resources(union acpi_object *package, unsigned int start,
break;
}
if (err)
- acpi_power_resources_list_free(list);
+ acpi_power_resources_list_free(device, list);
return err;
}
@@ -258,18 +372,33 @@ static int acpi_power_on_unlocked(struct acpi_power_resource *resource)
result = __acpi_power_on(resource);
if (result)
resource->ref_count--;
+ else
+ result = 1;
}
return result;
}
-static int acpi_power_on(struct acpi_power_resource *resource)
+static int acpi_power_on(struct acpi_device *device,
+ struct acpi_power_resource *resource)
{
int result;
mutex_lock(&resource->resource_lock);
result = acpi_power_on_unlocked(resource);
mutex_unlock(&resource->resource_lock);
- return result;
+
+ if (result <= 0)
+ return result;
+
+ /*
+ * The power resource was physically turned on. Because of this
+ * some of the devices sharing it may have been transitioned into
+ * D0 so we need to runtime resume them to make sure their driver
+ * re-initializes them properly. This is important for PCI devices
+ * that go into D0uninitialized and lose their wakeup settings
+ * otherwise.
+ */
+ return acpi_power_resume_consumers(resource, device);
}
static int __acpi_power_off(struct acpi_power_resource *resource)
@@ -319,7 +448,7 @@ static int acpi_power_off(struct acpi_power_resource *resource)
return result;
}
-static int acpi_power_off_list(struct list_head *list)
+static int acpi_power_off_list(struct acpi_device *device, struct list_head *list)
{
struct acpi_power_resource_entry *entry;
int result = 0;
@@ -333,18 +462,18 @@ static int acpi_power_off_list(struct list_head *list)
err:
list_for_each_entry_continue(entry, list, node)
- acpi_power_on(entry->resource);
+ acpi_power_on(device, entry->resource);
return result;
}
-static int acpi_power_on_list(struct list_head *list)
+static int acpi_power_on_list(struct acpi_device *device, struct list_head *list)
{
struct acpi_power_resource_entry *entry;
int result = 0;
list_for_each_entry(entry, list, node) {
- result = acpi_power_on(entry->resource);
+ result = acpi_power_on(device, entry->resource);
if (result)
goto err;
}
@@ -582,7 +711,7 @@ int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
if (!resource->wakeup_enabled) {
err = acpi_power_on_unlocked(resource);
- if (!err)
+ if (err >= 0)
resource->wakeup_enabled = true;
}
@@ -703,7 +832,7 @@ int acpi_power_on_resources(struct acpi_device *device, int state)
if (!device || state < ACPI_STATE_D0 || state > ACPI_STATE_D3_HOT)
return -EINVAL;
- return acpi_power_on_list(&device->power.states[state].resources);
+ return acpi_power_on_list(device, &device->power.states[state].resources);
}
int acpi_power_transition(struct acpi_device *device, int state)
@@ -726,11 +855,11 @@ int acpi_power_transition(struct acpi_device *device, int state)
* we dereference all power resources used in the current list.
*/
if (state < ACPI_STATE_D3_COLD)
- result = acpi_power_on_list(
+ result = acpi_power_on_list(device,
&device->power.states[state].resources);
if (!result && device->power.state < ACPI_STATE_D3_COLD)
- acpi_power_off_list(
+ acpi_power_off_list(device,
&device->power.states[device->power.state].resources);
/* We shouldn't change the state unless the above operations succeed. */
@@ -788,7 +917,7 @@ static void acpi_power_add_resource_to_list(struct acpi_power_resource *resource
mutex_unlock(&power_resource_list_lock);
}
-int acpi_add_power_resource(acpi_handle handle)
+int acpi_add_power_resource(struct acpi_device *adev, acpi_handle handle)
{
struct acpi_power_resource *resource;
struct acpi_device *device = NULL;
@@ -798,8 +927,10 @@ int acpi_add_power_resource(acpi_handle handle)
int state, result = -ENODEV;
acpi_bus_get_device(handle, &device);
- if (device)
- return 0;
+ if (device) {
+ resource = to_power_resource(device);
+ goto add_consumer;
+ }
resource = kzalloc(sizeof(*resource), GFP_KERNEL);
if (!resource)
@@ -810,6 +941,8 @@ int acpi_add_power_resource(acpi_handle handle)
ACPI_STA_DEFAULT);
mutex_init(&resource->resource_lock);
INIT_LIST_HEAD(&resource->list_node);
+ INIT_LIST_HEAD(&resource->consumers);
+ mutex_init(&resource->consumer_lock);
resource->name = device->pnp.bus_id;
strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME);
strcpy(acpi_device_class(device), ACPI_POWER_CLASS);
@@ -840,7 +973,11 @@ int acpi_add_power_resource(acpi_handle handle)
acpi_power_add_resource_to_list(resource);
acpi_device_add_finalize(device);
- return 0;
+
+ add_consumer:
+ result = acpi_power_add_consumer(resource, adev);
+ if (!result)
+ return 0;
err:
acpi_release_power_resource(&device->dev);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6153030451eb..3af0abe5b5e2 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -451,14 +451,14 @@ static void acpi_free_power_resources_lists(struct acpi_device *device)
int i;
if (device->wakeup.flags.valid)
- acpi_power_resources_list_free(&device->wakeup.resources);
+ acpi_power_resources_list_free(NULL, &device->wakeup.resources);
if (!device->power.flags.power_resources)
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);
+ acpi_power_resources_list_free(device, &ps->resources);
}
}
@@ -816,7 +816,7 @@ static int acpi_bus_extract_wakeup_device_power_package(struct acpi_device *dev)
wakeup->sleep_state = element->integer.value;
- err = acpi_extract_power_resources(package, 2, &wakeup->resources);
+ err = acpi_extract_power_resources(NULL, package, 2, &wakeup->resources);
if (err)
goto out;
@@ -828,7 +828,7 @@ static int acpi_bus_extract_wakeup_device_power_package(struct acpi_device *dev)
if (err) {
acpi_handle_warn(handle, "Retrieving current states "
"of wakeup power resources failed\n");
- acpi_power_resources_list_free(&wakeup->resources);
+ acpi_power_resources_list_free(NULL, &wakeup->resources);
goto out;
}
if (sleep_state < wakeup->sleep_state) {
@@ -920,7 +920,7 @@ static void acpi_bus_init_power_state(struct acpi_device *device, int state)
if (buffer.length && package
&& package->type == ACPI_TYPE_PACKAGE
&& package->package.count) {
- int err = acpi_extract_power_resources(package, 0,
+ int err = acpi_extract_power_resources(device, package, 0,
&ps->resources);
if (!err)
device->power.flags.power_resources = 1;
@@ -1867,7 +1867,7 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used,
return AE_OK;
if (type == ACPI_BUS_TYPE_POWER) {
- acpi_add_power_resource(handle);
+ acpi_add_power_resource(NULL, handle);
return AE_OK;
}
--
2.20.1
