Skip to content

Commit

Permalink
PM: runtime: Update device status before letting suppliers suspend
Browse files Browse the repository at this point in the history
Because the PM-runtime status of the device is not updated in
__rpm_callback(), attempts to suspend the suppliers of the given
device triggered by rpm_put_suppliers() called by it may fail.

Fix this by making __rpm_callback() update the device's status to
RPM_SUSPENDED before calling rpm_put_suppliers() if the current
status of the device is RPM_SUSPENDING and the callback just invoked
by it has returned 0 (success).

While at it, modify the code in __rpm_callback() to always check
the device's PM-runtime status under its PM lock.

Link: https://lore.kernel.org/linux-pm/CAPDyKFqm06KDw_p8WXsM4dijDbho4bb6T4k50UqqvR1_COsp8g@mail.gmail.com/
Fixes: 21d5c57 ("PM / runtime: Use device links")
Reported-by: Elaine Zhang <zhangqing@rock-chips.com>
Diagnosed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Elaine Zhang <zhangiqng@rock-chips.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Cc: 4.10+ <stable@vger.kernel.org> # 4.10+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
  • Loading branch information
Rafael J. Wysocki committed Mar 1, 2021
1 parent fe07bfd commit 44cc89f
Showing 1 changed file with 37 additions and 25 deletions.
62 changes: 37 additions & 25 deletions drivers/base/power/runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,22 +325,22 @@ static void rpm_put_suppliers(struct device *dev)
static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
__releases(&dev->power.lock) __acquires(&dev->power.lock)
{
int retval, idx;
bool use_links = dev->power.links_count > 0;
bool get = false;
int retval, idx;
bool put;

if (dev->power.irq_safe) {
spin_unlock(&dev->power.lock);
} else if (!use_links) {
spin_unlock_irq(&dev->power.lock);
} else {
get = dev->power.runtime_status == RPM_RESUMING;

spin_unlock_irq(&dev->power.lock);

/*
* Resume suppliers if necessary.
*
* The device's runtime PM status cannot change until this
* routine returns, so it is safe to read the status outside of
* the lock.
*/
if (use_links && dev->power.runtime_status == RPM_RESUMING) {
/* Resume suppliers if necessary. */
if (get) {
idx = device_links_read_lock();

retval = rpm_get_suppliers(dev);
Expand All @@ -355,24 +355,36 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev)

if (dev->power.irq_safe) {
spin_lock(&dev->power.lock);
} else {
/*
* If the device is suspending and the callback has returned
* success, drop the usage counters of the suppliers that have
* been reference counted on its resume.
*
* Do that if resume fails too.
*/
if (use_links
&& ((dev->power.runtime_status == RPM_SUSPENDING && !retval)
|| (dev->power.runtime_status == RPM_RESUMING && retval))) {
idx = device_links_read_lock();
return retval;
}

fail:
rpm_put_suppliers(dev);
spin_lock_irq(&dev->power.lock);

device_links_read_unlock(idx);
}
if (!use_links)
return retval;

/*
* If the device is suspending and the callback has returned success,
* drop the usage counters of the suppliers that have been reference
* counted on its resume.
*
* Do that if the resume fails too.
*/
put = dev->power.runtime_status == RPM_SUSPENDING && !retval;
if (put)
__update_runtime_status(dev, RPM_SUSPENDED);
else
put = get && retval;

if (put) {
spin_unlock_irq(&dev->power.lock);

idx = device_links_read_lock();

fail:
rpm_put_suppliers(dev);

device_links_read_unlock(idx);

spin_lock_irq(&dev->power.lock);
}
Expand Down

0 comments on commit 44cc89f

Please sign in to comment.