Skip to content

Commit

Permalink
ACPI: PM: Fix sharing of wakeup power resources
Browse files Browse the repository at this point in the history
If an ACPI wakeup power resource is shared between multiple devices,
it may not be managed correctly.

Suppose, for example, that two devices, A and B, share a wakeup power
resource P whose wakeup_enabled flag is 0 initially.  Next, suppose
that wakeup power is enabled for A and B, in this order, and disabled
for B.  When wakeup power is enabled for A, P will be turned on and
its wakeup_enabled flag will be set.  Next, when wakeup power is
enabled for B, P will not be touched, because its wakeup_enabled flag
is set.  Now, when wakeup power is disabled for B, P will be turned
off which is incorrect, because A will still need P in order to signal
wakeup.

Moreover, if wakeup power is enabled for A and then disabled for B,
the latter will cause P to be turned off incorrectly (it will be still
needed by A), because acpi_disable_wakeup_device_power() is allowed
to manipulate power resources when the wakeup.prepare_count counter
of the given device is 0.

While the first issue could be addressed by changing the
wakeup_enabled power resource flag into a counter, addressing the
second one requires modifying acpi_disable_wakeup_device_power() to
do nothing when the target device's wakeup.prepare_count reference
counter is zero and that would cause the new counter to be redundant.
Namely, if acpi_disable_wakeup_device_power() is modified as per the
above, every change of the new counter following a wakeup.prepare_count
change would be reflected by the analogous change of the main reference
counter of the given power resource.

Accordingly, modify acpi_disable_wakeup_device_power() to do nothing
when the target device's wakeup.prepare_count reference counter is
zero and drop the power resource wakeup_enabled flag altogether.

While at it, ensure that all of the power resources that can be
turned off will be turned off when disabling device wakeup due to
a power resource manipulation error, to prevent energy from being
wasted.

Fixes: b5d667e ("ACPI / PM: Take unusual configurations of power resources into account")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
rafaeljw committed Oct 19, 2021
1 parent 7a63296 commit a2d7b2e
Showing 1 changed file with 24 additions and 45 deletions.
69 changes: 24 additions & 45 deletions drivers/acpi/power.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ struct acpi_power_resource {
u32 order;
unsigned int ref_count;
u8 state;
bool wakeup_enabled;
struct mutex resource_lock;
struct list_head dependents;
};
Expand Down Expand Up @@ -710,7 +709,6 @@ int acpi_device_sleep_wake(struct acpi_device *dev,
*/
int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
{
struct acpi_power_resource_entry *entry;
int err = 0;

if (!dev || !dev->wakeup.flags.valid)
Expand All @@ -721,26 +719,13 @@ int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state)
if (dev->wakeup.prepare_count++)
goto out;

list_for_each_entry(entry, &dev->wakeup.resources, node) {
struct acpi_power_resource *resource = entry->resource;

mutex_lock(&resource->resource_lock);

if (!resource->wakeup_enabled) {
err = acpi_power_on_unlocked(resource);
if (!err)
resource->wakeup_enabled = true;
}

mutex_unlock(&resource->resource_lock);

if (err) {
dev_err(&dev->dev,
"Cannot turn wakeup power resources on\n");
dev->wakeup.flags.valid = 0;
goto out;
}
err = acpi_power_on_list(&dev->wakeup.resources);
if (err) {
dev_err(&dev->dev, "Cannot turn on wakeup power resources\n");
dev->wakeup.flags.valid = 0;
goto out;
}

/*
* Passing 3 as the third argument below means the device may be
* put into arbitrary power state afterward.
Expand Down Expand Up @@ -770,39 +755,33 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev)

mutex_lock(&acpi_device_lock);

if (--dev->wakeup.prepare_count > 0)
if (dev->wakeup.prepare_count > 1) {
dev->wakeup.prepare_count--;
goto out;
}

/*
* Executing the code below even if prepare_count is already zero when
* the function is called may be useful, for example for initialisation.
*/
if (dev->wakeup.prepare_count < 0)
dev->wakeup.prepare_count = 0;
/* Do nothing if wakeup power has not been enabled for this device. */
if (!dev->wakeup.prepare_count)
goto out;

err = acpi_device_sleep_wake(dev, 0, 0, 0);
if (err)
goto out;

/*
* All of the power resources in the list need to be turned off even if
* there are errors.
*/
list_for_each_entry(entry, &dev->wakeup.resources, node) {
struct acpi_power_resource *resource = entry->resource;

mutex_lock(&resource->resource_lock);

if (resource->wakeup_enabled) {
err = acpi_power_off_unlocked(resource);
if (!err)
resource->wakeup_enabled = false;
}

mutex_unlock(&resource->resource_lock);
int ret;

if (err) {
dev_err(&dev->dev,
"Cannot turn wakeup power resources off\n");
dev->wakeup.flags.valid = 0;
break;
}
ret = acpi_power_off(entry->resource);
if (ret && !err)
err = ret;
}
if (err) {
dev_err(&dev->dev, "Cannot turn off wakeup power resources\n");
dev->wakeup.flags.valid = 0;
}

out:
Expand Down

0 comments on commit a2d7b2e

Please sign in to comment.