[PATCH 0/5 v1] PM / Domains: Generic PM domains improvements

[PATCH 0/5 v1] PM / Domains: Generic PM domains improvements

[Rafael J. Wysocki]

Hi,

For the last few days I've been working on modifications of the generic
PM domains code allowing .start_device()/.stop_device() and drivers'
.runtime_suspend()/.runtime_resume() callbacks to execute things like
pm_runtime_resume() for other devices in the same PM domain safely
without deadlocking (the original motivation was a console driver on
shmobile that pm_runtime_get_*()/pm_runtime_put_*() are called for from
the other devices' .runtime_suspend()/.runtime_resume() callbacks).

I think I solved that problem, but in the process I found a few places
where fixes and/on improvements were necessary.  The patches in this
series address those issues.  They are on top of the branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-domains

Patches [1-2/5] are regarded as 3.1 material, the others not necessarily,
but I'd like to push them for 3.1 too if there are no complaints.

Thanks,
Rafael

[PATCH 1/5 v1] PM / Domains: Set device state to "active" during system resume

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The runtime PM status of devices in a power domain that is not
powered off in pm_genpd_complete() should be set to "active", because
those devices are operational at this point.  Some of them may not be
in use, though, so make pm_genpd_complete() call pm_runtime_idle()
in addition to pm_runtime_set_active() for each of them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -786,7 +786,9 @@ static void pm_genpd_complete(struct dev
 
 	if (run_complete) {
 		pm_generic_complete(dev);
+		pm_runtime_set_active(dev);
 		pm_runtime_enable(dev);
+		pm_runtime_idle(dev);
 	}
 }
 

[PATCH 2/5 v1] PM / Domains: Make failing pm_genpd_prepare() clean up properly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

If pm_generic_prepare() in pm_genpd_prepare() returns error code,
the PM domains counter of "prepared" devices should be decremented
and its suspend_power_off flag should be reset if this counter drops
down to zero.  Otherwise, the PM domain runtime PM code will not
handle the domain correctly (it will permanently think that system
suspend is in progress).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -367,6 +367,7 @@ static void pm_genpd_sync_poweroff(struc
 static int pm_genpd_prepare(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -400,7 +401,16 @@ static int pm_genpd_prepare(struct devic
 
 	mutex_unlock(&genpd->lock);
 
-	return pm_generic_prepare(dev);
+	ret = pm_generic_prepare(dev);
+	if (ret) {
+		mutex_lock(&genpd->lock);
+
+		if (--genpd->prepared_count == 0)
+			genpd->suspend_power_off = false;
+
+		mutex_unlock(&genpd->lock);
+	}
+	return ret;
 }
 
 /**

[PATCH 3/5 v1] PM / Domains: Rework locking

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

CUrrently, the .start_device() and .stop_device() callbacks from
struct generic_pm_domain() as well as the device drivers' runtime PM
callbacks used by the generic PM domains code are executed under
the generic PM domain lock.  This, unfortunately, is prone to
deadlocks, for example if a device and its parent are boths members
of the same PM domain.  For this reason, it would be better if the
PM domains code didn't execute device callbacks under the lock.

Rework the locking in the generic PM domains code so that the lock
is dropped for the execution of device callbacks.  To this end,
introduce PM domains states reflecting the current status of a PM
domain and such that the PM domain lock cannot be acquired if the
status is GPD_STATE_BUSY.  Make threads attempting to acquire a PM
domain's lock wait until the status changes to either
GPD_STATE_ACTIVE or GPD_STATE_POWER_OFF.

This change by itself doesn't fix the deadlock problem mentioned
above, but the mechanism introduced by it will be used for for this
purpose by a subsequent patch.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |  249 ++++++++++++++++++++++++++++++--------------
 include/linux/pm_domain.h   |   10 +
 2 files changed, 181 insertions(+), 78 deletions(-)

Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,8 +11,11 @@
 
 #include <linux/device.h>
 
-#define GPD_IN_SUSPEND	1
-#define GPD_POWER_OFF	2
+enum gpd_status {
+	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
+	GPD_STATE_BUSY,		/* Something is happening to the PM domain */
+	GPD_STATE_POWER_OFF,	/* PM domain is off */
+};
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
@@ -29,7 +32,8 @@ struct generic_pm_domain {
 	struct work_struct power_off_work;
 	unsigned int in_progress;	/* Number of devices being suspended now */
 	unsigned int sd_count;	/* Number of subdomains with power "on" */
-	bool power_is_off;	/* Whether or not power has been removed */
+	enum gpd_status status;	/* Current state of the domain */
+	wait_queue_head_t status_wait_queue;
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -13,6 +13,8 @@
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
 
 #ifdef CONFIG_PM
 
@@ -30,6 +32,34 @@ static void genpd_sd_counter_dec(struct
 			genpd->sd_count--;
 }
 
+static void genpd_acquire_lock(struct generic_pm_domain *genpd)
+{
+	DEFINE_WAIT(wait);
+
+	mutex_lock(&genpd->lock);
+	/*
+	 * Wait for the domain to transition into either the active,
+	 * or the power off state.
+	 */
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (genpd->status != GPD_STATE_BUSY)
+			break;
+		mutex_unlock(&genpd->lock);
+
+		schedule();
+
+		mutex_lock(&genpd->lock);
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
+}
+
+static void genpd_release_lock(struct generic_pm_domain *genpd)
+{
+	mutex_unlock(&genpd->lock);
+}
+
 /**
  * pm_genpd_poweron - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
@@ -39,22 +69,50 @@ static void genpd_sd_counter_dec(struct
  */
 static int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
+	struct generic_pm_domain *parent = genpd->parent;
+	DEFINE_WAIT(wait);
 	int ret = 0;
 
  start:
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	if (parent) {
+		mutex_lock(&parent->lock);
+		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	} else {
+		mutex_lock(&genpd->lock);
+	}
+	/*
+	 * Wait for the domain to transition into either the active,
+	 * or the power off state.
+	 */
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (genpd->status != GPD_STATE_BUSY)
+			break;
+		mutex_unlock(&genpd->lock);
+		if (parent)
+			mutex_unlock(&parent->lock);
 
-	if (!genpd->power_is_off
+		schedule();
+
+		if (parent) {
+			mutex_lock(&parent->lock);
+			mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+		} else {
+			mutex_lock(&genpd->lock);
+		}
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
+
+	if (genpd->status == GPD_STATE_ACTIVE
 	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
 		goto out;
 
-	if (genpd->parent && genpd->parent->power_is_off) {
+	if (parent && parent->status != GPD_STATE_ACTIVE) {
 		mutex_unlock(&genpd->lock);
-		mutex_unlock(&genpd->parent->lock);
+		mutex_unlock(&parent->lock);
 
-		ret = pm_genpd_poweron(genpd->parent);
+		ret = pm_genpd_poweron(parent);
 		if (ret)
 			return ret;
 
@@ -67,14 +125,14 @@ static int pm_genpd_poweron(struct gener
 			goto out;
 	}
 
-	genpd->power_is_off = false;
-	if (genpd->parent)
-		genpd->parent->sd_count++;
+	genpd->status = GPD_STATE_ACTIVE;
+	if (parent)
+		parent->sd_count++;
 
  out:
 	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	if (parent)
+		mutex_unlock(&parent->lock);
 
 	return ret;
 }
@@ -90,6 +148,7 @@ static int pm_genpd_poweron(struct gener
  */
 static int __pm_genpd_save_device(struct dev_list_entry *dle,
 				  struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct device *dev = dle->dev;
 	struct device_driver *drv = dev->driver;
@@ -98,6 +157,8 @@ static int __pm_genpd_save_device(struct
 	if (dle->need_restore)
 		return 0;
 
+	mutex_unlock(&genpd->lock);
+
 	if (drv && drv->pm && drv->pm->runtime_suspend) {
 		if (genpd->start_device)
 			genpd->start_device(dev);
@@ -108,6 +169,8 @@ static int __pm_genpd_save_device(struct
 			genpd->stop_device(dev);
 	}
 
+	mutex_lock(&genpd->lock);
+
 	if (!ret)
 		dle->need_restore = true;
 
@@ -121,6 +184,7 @@ static int __pm_genpd_save_device(struct
  */
 static void __pm_genpd_restore_device(struct dev_list_entry *dle,
 				      struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct device *dev = dle->dev;
 	struct device_driver *drv = dev->driver;
@@ -128,6 +192,8 @@ static void __pm_genpd_restore_device(st
 	if (!dle->need_restore)
 		return;
 
+	mutex_unlock(&genpd->lock);
+
 	if (drv && drv->pm && drv->pm->runtime_resume) {
 		if (genpd->start_device)
 			genpd->start_device(dev);
@@ -138,6 +204,8 @@ static void __pm_genpd_restore_device(st
 			genpd->stop_device(dev);
 	}
 
+	mutex_lock(&genpd->lock);
+
 	dle->need_restore = false;
 }
 
@@ -150,13 +218,14 @@ static void __pm_genpd_restore_device(st
  * the @genpd's devices' drivers and remove power from @genpd.
  */
 static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct generic_pm_domain *parent;
 	struct dev_list_entry *dle;
 	unsigned int not_suspended;
 	int ret;
 
-	if (genpd->power_is_off || genpd->prepared_count > 0)
+	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0)
 		return 0;
 
 	if (genpd->sd_count > 0)
@@ -175,22 +244,36 @@ static int pm_genpd_poweroff(struct gene
 			return -EAGAIN;
 	}
 
+	genpd->status = GPD_STATE_BUSY;
+
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
 		if (ret)
 			goto err_dev;
 	}
 
+	mutex_unlock(&genpd->lock);
+
+	parent = genpd->parent;
+	if (parent) {
+		genpd_acquire_lock(parent);
+		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	} else {
+		mutex_lock(&genpd->lock);
+	}
+
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
+	wake_up_all(&genpd->status_wait_queue);
 
-	parent = genpd->parent;
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		if (parent->sd_count == 0)
 			queue_work(pm_wq, &parent->power_off_work);
+
+		genpd_release_lock(parent);
 	}
 
 	return 0;
@@ -199,6 +282,9 @@ static int pm_genpd_poweroff(struct gene
 	list_for_each_entry_continue(dle, &genpd->dev_list, node)
 		__pm_genpd_restore_device(dle, genpd);
 
+	genpd->status = GPD_STATE_ACTIVE;
+	wake_up_all(&genpd->status_wait_queue);
+
 	return ret;
 }
 
@@ -212,13 +298,9 @@ static void genpd_power_off_work_fn(stru
 
 	genpd = container_of(work, struct generic_pm_domain, power_off_work);
 
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	genpd_acquire_lock(genpd);
 	pm_genpd_poweroff(genpd);
-	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	genpd_release_lock(genpd);
 }
 
 /**
@@ -239,23 +321,17 @@ static int pm_genpd_runtime_suspend(stru
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-
 	if (genpd->stop_device) {
 		int ret = genpd->stop_device(dev);
 		if (ret)
-			goto out;
+			return ret;
 	}
+
+	genpd_acquire_lock(genpd);
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-
- out:
-	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	genpd_release_lock(genpd);
 
 	return 0;
 }
@@ -276,9 +352,6 @@ static void __pm_genpd_runtime_resume(st
 			break;
 		}
 	}
-
-	if (genpd->start_device)
-		genpd->start_device(dev);
 }
 
 /**
@@ -304,9 +377,15 @@ static int pm_genpd_runtime_resume(struc
 	if (ret)
 		return ret;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
+	genpd->status = GPD_STATE_BUSY;
 	__pm_genpd_runtime_resume(dev, genpd);
-	mutex_unlock(&genpd->lock);
+	genpd->status = GPD_STATE_ACTIVE;
+	wake_up_all(&genpd->status_wait_queue);
+	genpd_release_lock(genpd);
+
+	if (genpd->start_device)
+		genpd->start_device(dev);
 
 	return 0;
 }
@@ -339,7 +418,7 @@ static void pm_genpd_sync_poweroff(struc
 {
 	struct generic_pm_domain *parent = genpd->parent;
 
-	if (genpd->power_is_off)
+	if (genpd->status == GPD_STATE_POWER_OFF)
 		return;
 
 	if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
@@ -348,7 +427,7 @@ static void pm_genpd_sync_poweroff(struc
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		pm_genpd_sync_poweroff(parent);
@@ -375,33 +454,35 @@ static int pm_genpd_prepare(struct devic
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count++ == 0)
-		genpd->suspend_power_off = genpd->power_is_off;
+		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
 
-	if (genpd->suspend_power_off) {
-		mutex_unlock(&genpd->lock);
-		return 0;
-	}
+	genpd_release_lock(genpd);
 
-	/*
-	 * If the device is in the (runtime) "suspended" state, call
-	 * .start_device() for it, if defined.
-	 */
-	if (pm_runtime_suspended(dev))
-		__pm_genpd_runtime_resume(dev, genpd);
+	if (genpd->suspend_power_off)
+		return 0;
 
 	/*
-	 * Do not check if runtime resume is pending at this point, because it
-	 * has been taken care of already and if pm_genpd_poweron() ran at this
-	 * point as a result of the check, it would deadlock.
+	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
+	 * so pm_genpd_poweron() will return immediately, but if the device
+	 * is suspended (e.g. it's been stopped by .stop_device()), we need
+	 * to make it operational.  However, we need to prevent already
+	 * received wakeup events from being lost too.
 	 */
-	__pm_runtime_disable(dev, false);
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
+
+	if (pm_wakeup_pending()) {
+		ret = -EBUSY;
+	} else {
+		pm_runtime_resume(dev);
+		__pm_runtime_disable(dev, false);
 
-	mutex_unlock(&genpd->lock);
-
-	ret = pm_generic_prepare(dev);
+		ret = pm_generic_prepare(dev);
+	}
 	if (ret) {
 		mutex_lock(&genpd->lock);
 
@@ -410,6 +491,8 @@ static int pm_genpd_prepare(struct devic
 
 		mutex_unlock(&genpd->lock);
 	}
+
+	pm_runtime_put_sync(dev);
 	return ret;
 }
 
@@ -726,7 +809,7 @@ static int pm_genpd_restore_noirq(struct
 	 * guaranteed that this function will never run twice in parallel for
 	 * the same PM domain, so it is not necessary to use locking here.
 	 */
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
 	if (genpd->suspend_power_off) {
 		/*
 		 * The boot kernel might put the domain into the power on state,
@@ -836,9 +919,9 @@ int pm_genpd_add_device(struct generic_p
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
-	if (genpd->power_is_off) {
+	if (genpd->status == GPD_STATE_POWER_OFF) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -870,7 +953,7 @@ int pm_genpd_add_device(struct generic_p
 	spin_unlock_irq(&dev->power.lock);
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -891,7 +974,7 @@ int pm_genpd_remove_device(struct generi
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;
@@ -915,7 +998,7 @@ int pm_genpd_remove_device(struct generi
 	}
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -934,9 +1017,19 @@ int pm_genpd_add_subdomain(struct generi
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(new_subdomain))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+ start:
+	genpd_acquire_lock(genpd);
+	mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
+
+	if (new_subdomain->status != GPD_STATE_POWER_OFF
+	    && new_subdomain->status != GPD_STATE_ACTIVE) {
+		mutex_unlock(&new_subdomain->lock);
+		genpd_release_lock(genpd);
+		goto start;
+	}
 
-	if (genpd->power_is_off && !new_subdomain->power_is_off) {
+	if (genpd->status == GPD_STATE_POWER_OFF
+	    &&  new_subdomain->status != GPD_STATE_POWER_OFF) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -948,17 +1041,14 @@ int pm_genpd_add_subdomain(struct generi
 		}
 	}
 
-	mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
-
 	list_add_tail(&new_subdomain->sd_node, &genpd->sd_list);
 	new_subdomain->parent = genpd;
-	if (!subdomain->power_is_off)
+	if (subdomain->status != GPD_STATE_POWER_OFF)
 		genpd->sd_count++;
 
-	mutex_unlock(&new_subdomain->lock);
-
  out:
-	mutex_unlock(&genpd->lock);
+	mutex_unlock(&new_subdomain->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -977,7 +1067,8 @@ int pm_genpd_remove_subdomain(struct gen
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(target))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+ start:
+	genpd_acquire_lock(genpd);
 
 	list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
 		if (subdomain != target)
@@ -985,9 +1076,16 @@ int pm_genpd_remove_subdomain(struct gen
 
 		mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);
 
+		if (subdomain->status != GPD_STATE_POWER_OFF
+		    && subdomain->status != GPD_STATE_ACTIVE) {
+			mutex_unlock(&subdomain->lock);
+			genpd_release_lock(genpd);
+			goto start;
+		}
+
 		list_del(&subdomain->sd_node);
 		subdomain->parent = NULL;
-		if (!subdomain->power_is_off)
+		if (subdomain->status != GPD_STATE_POWER_OFF)
 			genpd_sd_counter_dec(genpd);
 
 		mutex_unlock(&subdomain->lock);
@@ -996,7 +1094,7 @@ int pm_genpd_remove_subdomain(struct gen
 		break;
 	}
 
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -1022,7 +1120,8 @@ void pm_genpd_init(struct generic_pm_dom
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
 	genpd->in_progress = 0;
 	genpd->sd_count = 0;
-	genpd->power_is_off = is_off;
+	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
+	init_waitqueue_head(&genpd->status_wait_queue);
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;

[Update][PATCH 3/5] PM / Domains: Rework locking

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

CUrrently, the .start_device() and .stop_device() callbacks from
struct generic_pm_domain() as well as the device drivers' runtime PM
callbacks used by the generic PM domains code are executed under
the generic PM domain lock.  This, unfortunately, is prone to
deadlocks, for example if a device and its parent are boths members
of the same PM domain.  For this reason, it would be better if the
PM domains code didn't execute device callbacks under the lock.

Rework the locking in the generic PM domains code so that the lock
is dropped for the execution of device callbacks.  To this end,
introduce PM domains states reflecting the current status of a PM
domain and such that the PM domain lock cannot be acquired if the
status is GPD_STATE_BUSY.  Make threads attempting to acquire a PM
domain's lock wait until the status changes to either
GPD_STATE_ACTIVE or GPD_STATE_POWER_OFF.

This change by itself doesn't fix the deadlock problem mentioned
above, but the mechanism introduced by it will be used for for this
purpose by a subsequent patch.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

pm_genpd_prepare() has to check pending wakeup requests before it changes
prepared_count, because otherwise the domain won't be powered on in
response to the wakeup request.

Thanks,
Rafael

---
 drivers/base/power/domain.c |  249 +++++++++++++++++++++++++++++++-------------
 include/linux/pm_domain.h   |   10 +
 2 files changed, 185 insertions(+), 74 deletions(-)

Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,8 +11,11 @@
 
 #include <linux/device.h>
 
-#define GPD_IN_SUSPEND	1
-#define GPD_POWER_OFF	2
+enum gpd_status {
+	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
+	GPD_STATE_BUSY,		/* Something is happening to the PM domain */
+	GPD_STATE_POWER_OFF,	/* PM domain is off */
+};
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
@@ -29,7 +32,8 @@ struct generic_pm_domain {
 	struct work_struct power_off_work;
 	unsigned int in_progress;	/* Number of devices being suspended now */
 	unsigned int sd_count;	/* Number of subdomains with power "on" */
-	bool power_is_off;	/* Whether or not power has been removed */
+	enum gpd_status status;	/* Current state of the domain */
+	wait_queue_head_t status_wait_queue;
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -13,6 +13,8 @@
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
 
 #ifdef CONFIG_PM
 
@@ -30,6 +32,34 @@ static void genpd_sd_counter_dec(struct
 			genpd->sd_count--;
 }
 
+static void genpd_acquire_lock(struct generic_pm_domain *genpd)
+{
+	DEFINE_WAIT(wait);
+
+	mutex_lock(&genpd->lock);
+	/*
+	 * Wait for the domain to transition into either the active,
+	 * or the power off state.
+	 */
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (genpd->status != GPD_STATE_BUSY)
+			break;
+		mutex_unlock(&genpd->lock);
+
+		schedule();
+
+		mutex_lock(&genpd->lock);
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
+}
+
+static void genpd_release_lock(struct generic_pm_domain *genpd)
+{
+	mutex_unlock(&genpd->lock);
+}
+
 /**
  * pm_genpd_poweron - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
@@ -39,22 +69,50 @@ static void genpd_sd_counter_dec(struct
  */
 static int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
+	struct generic_pm_domain *parent = genpd->parent;
+	DEFINE_WAIT(wait);
 	int ret = 0;
 
  start:
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	if (parent) {
+		mutex_lock(&parent->lock);
+		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	} else {
+		mutex_lock(&genpd->lock);
+	}
+	/*
+	 * Wait for the domain to transition into either the active,
+	 * or the power off state.
+	 */
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (genpd->status != GPD_STATE_BUSY)
+			break;
+		mutex_unlock(&genpd->lock);
+		if (parent)
+			mutex_unlock(&parent->lock);
+
+		schedule();
 
-	if (!genpd->power_is_off
+		if (parent) {
+			mutex_lock(&parent->lock);
+			mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+		} else {
+			mutex_lock(&genpd->lock);
+		}
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
+
+	if (genpd->status == GPD_STATE_ACTIVE
 	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
 		goto out;
 
-	if (genpd->parent && genpd->parent->power_is_off) {
+	if (parent && parent->status != GPD_STATE_ACTIVE) {
 		mutex_unlock(&genpd->lock);
-		mutex_unlock(&genpd->parent->lock);
+		mutex_unlock(&parent->lock);
 
-		ret = pm_genpd_poweron(genpd->parent);
+		ret = pm_genpd_poweron(parent);
 		if (ret)
 			return ret;
 
@@ -67,14 +125,14 @@ static int pm_genpd_poweron(struct gener
 			goto out;
 	}
 
-	genpd->power_is_off = false;
-	if (genpd->parent)
-		genpd->parent->sd_count++;
+	genpd->status = GPD_STATE_ACTIVE;
+	if (parent)
+		parent->sd_count++;
 
  out:
 	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	if (parent)
+		mutex_unlock(&parent->lock);
 
 	return ret;
 }
@@ -90,6 +148,7 @@ static int pm_genpd_poweron(struct gener
  */
 static int __pm_genpd_save_device(struct dev_list_entry *dle,
 				  struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct device *dev = dle->dev;
 	struct device_driver *drv = dev->driver;
@@ -98,6 +157,8 @@ static int __pm_genpd_save_device(struct
 	if (dle->need_restore)
 		return 0;
 
+	mutex_unlock(&genpd->lock);
+
 	if (drv && drv->pm && drv->pm->runtime_suspend) {
 		if (genpd->start_device)
 			genpd->start_device(dev);
@@ -108,6 +169,8 @@ static int __pm_genpd_save_device(struct
 			genpd->stop_device(dev);
 	}
 
+	mutex_lock(&genpd->lock);
+
 	if (!ret)
 		dle->need_restore = true;
 
@@ -121,6 +184,7 @@ static int __pm_genpd_save_device(struct
  */
 static void __pm_genpd_restore_device(struct dev_list_entry *dle,
 				      struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct device *dev = dle->dev;
 	struct device_driver *drv = dev->driver;
@@ -128,6 +192,8 @@ static void __pm_genpd_restore_device(st
 	if (!dle->need_restore)
 		return;
 
+	mutex_unlock(&genpd->lock);
+
 	if (drv && drv->pm && drv->pm->runtime_resume) {
 		if (genpd->start_device)
 			genpd->start_device(dev);
@@ -138,6 +204,8 @@ static void __pm_genpd_restore_device(st
 			genpd->stop_device(dev);
 	}
 
+	mutex_lock(&genpd->lock);
+
 	dle->need_restore = false;
 }
 
@@ -150,13 +218,14 @@ static void __pm_genpd_restore_device(st
  * the @genpd's devices' drivers and remove power from @genpd.
  */
 static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct generic_pm_domain *parent;
 	struct dev_list_entry *dle;
 	unsigned int not_suspended;
 	int ret;
 
-	if (genpd->power_is_off || genpd->prepared_count > 0)
+	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0)
 		return 0;
 
 	if (genpd->sd_count > 0)
@@ -175,22 +244,36 @@ static int pm_genpd_poweroff(struct gene
 			return -EAGAIN;
 	}
 
+	genpd->status = GPD_STATE_BUSY;
+
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
 		if (ret)
 			goto err_dev;
 	}
 
+	mutex_unlock(&genpd->lock);
+
+	parent = genpd->parent;
+	if (parent) {
+		genpd_acquire_lock(parent);
+		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	} else {
+		mutex_lock(&genpd->lock);
+	}
+
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
+	wake_up_all(&genpd->status_wait_queue);
 
-	parent = genpd->parent;
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		if (parent->sd_count == 0)
 			queue_work(pm_wq, &parent->power_off_work);
+
+		genpd_release_lock(parent);
 	}
 
 	return 0;
@@ -199,6 +282,9 @@ static int pm_genpd_poweroff(struct gene
 	list_for_each_entry_continue(dle, &genpd->dev_list, node)
 		__pm_genpd_restore_device(dle, genpd);
 
+	genpd->status = GPD_STATE_ACTIVE;
+	wake_up_all(&genpd->status_wait_queue);
+
 	return ret;
 }
 
@@ -212,13 +298,9 @@ static void genpd_power_off_work_fn(stru
 
 	genpd = container_of(work, struct generic_pm_domain, power_off_work);
 
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	genpd_acquire_lock(genpd);
 	pm_genpd_poweroff(genpd);
-	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	genpd_release_lock(genpd);
 }
 
 /**
@@ -239,23 +321,17 @@ static int pm_genpd_runtime_suspend(stru
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-
 	if (genpd->stop_device) {
 		int ret = genpd->stop_device(dev);
 		if (ret)
-			goto out;
+			return ret;
 	}
+
+	genpd_acquire_lock(genpd);
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-
- out:
-	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	genpd_release_lock(genpd);
 
 	return 0;
 }
@@ -276,9 +352,6 @@ static void __pm_genpd_runtime_resume(st
 			break;
 		}
 	}
-
-	if (genpd->start_device)
-		genpd->start_device(dev);
 }
 
 /**
@@ -304,9 +377,15 @@ static int pm_genpd_runtime_resume(struc
 	if (ret)
 		return ret;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
+	genpd->status = GPD_STATE_BUSY;
 	__pm_genpd_runtime_resume(dev, genpd);
-	mutex_unlock(&genpd->lock);
+	genpd->status = GPD_STATE_ACTIVE;
+	wake_up_all(&genpd->status_wait_queue);
+	genpd_release_lock(genpd);
+
+	if (genpd->start_device)
+		genpd->start_device(dev);
 
 	return 0;
 }
@@ -339,7 +418,7 @@ static void pm_genpd_sync_poweroff(struc
 {
 	struct generic_pm_domain *parent = genpd->parent;
 
-	if (genpd->power_is_off)
+	if (genpd->status == GPD_STATE_POWER_OFF)
 		return;
 
 	if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
@@ -348,7 +427,7 @@ static void pm_genpd_sync_poweroff(struc
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		pm_genpd_sync_poweroff(parent);
@@ -375,32 +454,41 @@ static int pm_genpd_prepare(struct devic
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	/*
+	 * If a wakeup request is pending for the device, it should be woken up
+	 * at this point and a system wakeup event should be reported if it's
+	 * set up to wake up the system from sleep states.
+	 */
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
+
+	if (pm_wakeup_pending()) {
+		pm_runtime_put_sync(dev);
+		return -EBUSY;
+	}
+
+	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count++ == 0)
-		genpd->suspend_power_off = genpd->power_is_off;
+		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
+
+	genpd_release_lock(genpd);
 
 	if (genpd->suspend_power_off) {
-		mutex_unlock(&genpd->lock);
+		pm_runtime_put_noidle(dev);
 		return 0;
 	}
 
 	/*
-	 * If the device is in the (runtime) "suspended" state, call
-	 * .start_device() for it, if defined.
-	 */
-	if (pm_runtime_suspended(dev))
-		__pm_genpd_runtime_resume(dev, genpd);
-
-	/*
-	 * Do not check if runtime resume is pending at this point, because it
-	 * has been taken care of already and if pm_genpd_poweron() ran at this
-	 * point as a result of the check, it would deadlock.
+	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
+	 * so pm_genpd_poweron() will return immediately, but if the device
+	 * is suspended (e.g. it's been stopped by .stop_device()), we need
+	 * to make it operational.
 	 */
+	pm_runtime_resume(dev);
 	__pm_runtime_disable(dev, false);
 
-	mutex_unlock(&genpd->lock);
-
 	ret = pm_generic_prepare(dev);
 	if (ret) {
 		mutex_lock(&genpd->lock);
@@ -409,7 +497,10 @@ static int pm_genpd_prepare(struct devic
 			genpd->suspend_power_off = false;
 
 		mutex_unlock(&genpd->lock);
+		pm_runtime_enable(dev);
 	}
+
+	pm_runtime_put_sync(dev);
 	return ret;
 }
 
@@ -726,7 +817,7 @@ static int pm_genpd_restore_noirq(struct
 	 * guaranteed that this function will never run twice in parallel for
 	 * the same PM domain, so it is not necessary to use locking here.
 	 */
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
 	if (genpd->suspend_power_off) {
 		/*
 		 * The boot kernel might put the domain into the power on state,
@@ -836,9 +927,9 @@ int pm_genpd_add_device(struct generic_p
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
-	if (genpd->power_is_off) {
+	if (genpd->status == GPD_STATE_POWER_OFF) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -870,7 +961,7 @@ int pm_genpd_add_device(struct generic_p
 	spin_unlock_irq(&dev->power.lock);
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -891,7 +982,7 @@ int pm_genpd_remove_device(struct generi
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;
@@ -915,7 +1006,7 @@ int pm_genpd_remove_device(struct generi
 	}
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -934,9 +1025,19 @@ int pm_genpd_add_subdomain(struct generi
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(new_subdomain))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+ start:
+	genpd_acquire_lock(genpd);
+	mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
 
-	if (genpd->power_is_off && !new_subdomain->power_is_off) {
+	if (new_subdomain->status != GPD_STATE_POWER_OFF
+	    && new_subdomain->status != GPD_STATE_ACTIVE) {
+		mutex_unlock(&new_subdomain->lock);
+		genpd_release_lock(genpd);
+		goto start;
+	}
+
+	if (genpd->status == GPD_STATE_POWER_OFF
+	    &&  new_subdomain->status != GPD_STATE_POWER_OFF) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -948,17 +1049,14 @@ int pm_genpd_add_subdomain(struct generi
 		}
 	}
 
-	mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
-
 	list_add_tail(&new_subdomain->sd_node, &genpd->sd_list);
 	new_subdomain->parent = genpd;
-	if (!subdomain->power_is_off)
+	if (subdomain->status != GPD_STATE_POWER_OFF)
 		genpd->sd_count++;
 
-	mutex_unlock(&new_subdomain->lock);
-
  out:
-	mutex_unlock(&genpd->lock);
+	mutex_unlock(&new_subdomain->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -977,7 +1075,8 @@ int pm_genpd_remove_subdomain(struct gen
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(target))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+ start:
+	genpd_acquire_lock(genpd);
 
 	list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
 		if (subdomain != target)
@@ -985,9 +1084,16 @@ int pm_genpd_remove_subdomain(struct gen
 
 		mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);
 
+		if (subdomain->status != GPD_STATE_POWER_OFF
+		    && subdomain->status != GPD_STATE_ACTIVE) {
+			mutex_unlock(&subdomain->lock);
+			genpd_release_lock(genpd);
+			goto start;
+		}
+
 		list_del(&subdomain->sd_node);
 		subdomain->parent = NULL;
-		if (!subdomain->power_is_off)
+		if (subdomain->status != GPD_STATE_POWER_OFF)
 			genpd_sd_counter_dec(genpd);
 
 		mutex_unlock(&subdomain->lock);
@@ -996,7 +1102,7 @@ int pm_genpd_remove_subdomain(struct gen
 		break;
 	}
 
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -1022,7 +1128,8 @@ void pm_genpd_init(struct generic_pm_dom
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
 	genpd->in_progress = 0;
 	genpd->sd_count = 0;
-	genpd->power_is_off = is_off;
+	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
+	init_waitqueue_head(&genpd->status_wait_queue);
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;

[PATCH 4/5 v1] PM / Domains: Allow callbacks to execute arbitrary runtime PM helpers

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

A deadlock may occur if one of the PM domains' .start_device() or
.stop_device() callbacks or a device driver's .runtime_suspend() or
.runtime_resume() callback executed by the core generic PM domain
code uses a "wrong" runtime PM helper function.  This happens, for
example, if .runtime_resume() from one device's driver calls
pm_runtime_resume() for another device in the same PM domain.
A similar situation may take place if a device's parent is in the
same PM domain, in which case the runtime PM framework may execute
pm_genpd_runtime_resume() automatically for the parent (if it is
suspended at the moment).  This, of course, is undesirable, so
the generic PM domains code should be modified to prevent it from
happening.

The runtime PM framework guarantees that pm_genpd_runtime_suspend()
and pm_genpd_runtime_resume() won't be executed in parallel for
the same device, so the generic PM domains code need not worry
about those cases.  Still, it needs to prevent the other possible
race conditions between pm_genpd_runtime_suspend(),
pm_genpd_runtime_resume(), pm_genpd_poweron() and pm_genpd_poweroff()
from happening and it needs to avoid deadlocks at the same time.
To this end, modify the generic PM domains code to relax
synchronization rules so that:

* pm_genpd_poweron() doesn't wait for the PM domain status to
  change from GPD_STATE_BUSY.  If it finds that the status is
  not GPD_STATE_POWER_OFF, it returns without powering the domain on
  (it may modify the status depending on the circumstances).

* pm_genpd_poweroff() returns as soon as it finds that the PM
  domain's status changed from GPD_STATE_BUSY after it's released
  the PM domain's lock.

* pm_genpd_runtime_suspend() doesn't wait for the PM domain status
  to change from GPD_STATE_BUSY after executing the domain's
  .stop_device() callback and executes pm_genpd_poweroff() only
  if pm_genpd_runtime_resume() is not executed in parallel.

* pm_genpd_runtime_resume() doesn't wait for the PM domain status
  to change from GPD_STATE_BUSY after executing pm_genpd_poweron()
  and sets the domain's status to GPD_STATE_BUSY and increments its
  counter of resuming devices (introduced by this change) immediately
  after acquiring the lock.  The counter of resuming devices is then
  decremented after executing __pm_genpd_runtime_resume() for the
  device and the domain's status is reset to GPD_STATE_ACTIVE (unless
  there are more resuming devices in the domain, in which case the
  status remains GPD_STATE_BUSY).

This way, for example, if a device driver's .runtime_resume()
callback executes pm_runtime_resume() for another device in the same
PM domain, pm_genpd_poweron() called by pm_genpd_runtime_resume()
invoked by the runtime PM framework will not block and it will see
that there's nothing to do for it.  Next, the PM domain's lock will
be taken without waiting for its status to change from GPD_STATE_BUSY
and the device driver's .runtime_resume() callback will be executed.
In turn, if pm_runtime_suspend() is executed by one device driver's
.runtime_resume() callback for another device in the same PM domain,
pm_genpd_runtime_suspend() invoked by the runtime PM framework as
a result will notice that one of the devices in the domain is being
resumed, so it won't call pm_genpd_poweroff().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   97 +++++++++++++++++++++++++++-----------------
 include/linux/pm_domain.h   |    1 
 2 files changed, 62 insertions(+), 36 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -60,6 +60,12 @@ static void genpd_release_lock(struct ge
 	mutex_unlock(&genpd->lock);
 }
 
+static void genpd_set_active(struct generic_pm_domain *genpd)
+{
+	if (genpd->resume_count == 0)
+		genpd->status = GPD_STATE_ACTIVE;
+}
+
 /**
  * pm_genpd_poweron - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
@@ -75,42 +81,24 @@ static int pm_genpd_poweron(struct gener
 
  start:
 	if (parent) {
-		mutex_lock(&parent->lock);
+		genpd_acquire_lock(parent);
 		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
 	} else {
 		mutex_lock(&genpd->lock);
 	}
-	/*
-	 * Wait for the domain to transition into either the active,
-	 * or the power off state.
-	 */
-	for (;;) {
-		prepare_to_wait(&genpd->status_wait_queue, &wait,
-				TASK_UNINTERRUPTIBLE);
-		if (genpd->status != GPD_STATE_BUSY)
-			break;
-		mutex_unlock(&genpd->lock);
-		if (parent)
-			mutex_unlock(&parent->lock);
-
-		schedule();
-
-		if (parent) {
-			mutex_lock(&parent->lock);
-			mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-		} else {
-			mutex_lock(&genpd->lock);
-		}
-	}
-	finish_wait(&genpd->status_wait_queue, &wait);
 
 	if (genpd->status == GPD_STATE_ACTIVE
 	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
 		goto out;
 
+	if (genpd->status == GPD_STATE_BUSY) {
+		genpd_set_active(genpd);
+		goto out;
+	}
+
 	if (parent && parent->status != GPD_STATE_ACTIVE) {
 		mutex_unlock(&genpd->lock);
-		mutex_unlock(&parent->lock);
+		genpd_release_lock(parent);
 
 		ret = pm_genpd_poweron(parent);
 		if (ret)
@@ -125,14 +113,14 @@ static int pm_genpd_poweron(struct gener
 			goto out;
 	}
 
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd_set_active(genpd);
 	if (parent)
 		parent->sd_count++;
 
  out:
 	mutex_unlock(&genpd->lock);
 	if (parent)
-		mutex_unlock(&parent->lock);
+		genpd_release_lock(parent);
 
 	return ret;
 }
@@ -210,6 +198,20 @@ static void __pm_genpd_restore_device(st
 }
 
 /**
+ * genpd_abort_poweroff - Check if a PM domain power off should be aborted.
+ * @genpd: PM domain to check.
+ *
+ * Return true if a PM domain's status changed from GPD_STATE_BUSY during
+ * a "power off" operation, which means that a "power on" has occured in the
+ * meantime, or if its resume_count field is different from zero, which means
+ * that one of its devices has been resumed in the meantime.
+ */
+static bool genpd_abort_poweroff(struct generic_pm_domain *genpd)
+{
+	return genpd->status != GPD_STATE_BUSY || genpd->resume_count > 0;
+}
+
+/**
  * pm_genpd_poweroff - Remove power from a given PM domain.
  * @genpd: PM domain to power down.
  *
@@ -239,6 +241,14 @@ static int pm_genpd_poweroff(struct gene
 	if (not_suspended > genpd->in_progress)
 		return -EBUSY;
 
+	/*
+	 * Check if another instance of pm_genpd_poweroff() for the same domain
+	 * that has already started to call drivers' runtime suspend routines
+	 * is running in parallel with us.
+	 */
+	if (genpd->status == GPD_STATE_BUSY)
+		return 0;
+
 	if (genpd->gov && genpd->gov->power_down_ok) {
 		if (!genpd->gov->power_down_ok(&genpd->domain))
 			return -EAGAIN;
@@ -250,6 +260,9 @@ static int pm_genpd_poweroff(struct gene
 		ret = __pm_genpd_save_device(dle, genpd);
 		if (ret)
 			goto err_dev;
+
+		if (genpd_abort_poweroff(genpd))
+			return 0;
 	}
 
 	mutex_unlock(&genpd->lock);
@@ -262,6 +275,13 @@ static int pm_genpd_poweroff(struct gene
 		mutex_lock(&genpd->lock);
 	}
 
+	if (genpd_abort_poweroff(genpd)) {
+		if (parent)
+			genpd_release_lock(parent);
+
+		return 0;
+	}
+
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
@@ -282,7 +302,7 @@ static int pm_genpd_poweroff(struct gene
 	list_for_each_entry_continue(dle, &genpd->dev_list, node)
 		__pm_genpd_restore_device(dle, genpd);
 
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
 
 	return ret;
@@ -327,11 +347,13 @@ static int pm_genpd_runtime_suspend(stru
 			return ret;
 	}
 
-	genpd_acquire_lock(genpd);
-	genpd->in_progress++;
-	pm_genpd_poweroff(genpd);
-	genpd->in_progress--;
-	genpd_release_lock(genpd);
+	mutex_lock(&genpd->lock);
+	if (genpd->resume_count == 0) {
+		genpd->in_progress++;
+		pm_genpd_poweroff(genpd);
+		genpd->in_progress--;
+	}
+	mutex_unlock(&genpd->lock);
 
 	return 0;
 }
@@ -377,12 +399,14 @@ static int pm_genpd_runtime_resume(struc
 	if (ret)
 		return ret;
 
-	genpd_acquire_lock(genpd);
+	mutex_lock(&genpd->lock);
 	genpd->status = GPD_STATE_BUSY;
+	genpd->resume_count++;
 	__pm_genpd_runtime_resume(dev, genpd);
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd->resume_count--;
+	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
-	genpd_release_lock(genpd);
+	mutex_unlock(&genpd->lock);
 
 	if (genpd->start_device)
 		genpd->start_device(dev);
@@ -1122,6 +1146,7 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->sd_count = 0;
 	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
 	init_waitqueue_head(&genpd->status_wait_queue);
+	genpd->resume_count = 0;
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -34,6 +34,7 @@ struct generic_pm_domain {
 	unsigned int sd_count;	/* Number of subdomains with power "on" */
 	enum gpd_status status;	/* Current state of the domain */
 	wait_queue_head_t status_wait_queue;
+	unsigned int resume_count;	/* Number of devices being resumed */
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */

[Update][PATCH 4/5] PM / Domains: Allow callbacks to execute all runtime PM helpers

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

A deadlock may occur if one of the PM domains' .start_device() or
.stop_device() callbacks or a device driver's .runtime_suspend() or
.runtime_resume() callback executed by the core generic PM domain
code uses a "wrong" runtime PM helper function.  This happens, for
example, if .runtime_resume() from one device's driver calls
pm_runtime_resume() for another device in the same PM domain.
A similar situation may take place if a device's parent is in the
same PM domain, in which case the runtime PM framework may execute
pm_genpd_runtime_resume() automatically for the parent (if it is
suspended at the moment).  This, of course, is undesirable, so
the generic PM domains code should be modified to prevent it from
happening.

The runtime PM framework guarantees that pm_genpd_runtime_suspend()
and pm_genpd_runtime_resume() won't be executed in parallel for
the same device, so the generic PM domains code need not worry
about those cases.  Still, it needs to prevent the other possible
race conditions between pm_genpd_runtime_suspend(),
pm_genpd_runtime_resume(), pm_genpd_poweron() and pm_genpd_poweroff()
from happening and it needs to avoid deadlocks at the same time.
To this end, modify the generic PM domains code to relax
synchronization rules so that:

* pm_genpd_poweron() doesn't wait for the PM domain status to
  change from GPD_STATE_BUSY.  If it finds that the status is
  not GPD_STATE_POWER_OFF, it returns without powering the domain on
  (it may modify the status depending on the circumstances).

* pm_genpd_poweroff() returns as soon as it finds that the PM
  domain's status changed from GPD_STATE_BUSY after it's released
  the PM domain's lock.

* pm_genpd_runtime_suspend() doesn't wait for the PM domain status
  to change from GPD_STATE_BUSY after executing the domain's
  .stop_device() callback and executes pm_genpd_poweroff() only
  if pm_genpd_runtime_resume() is not executed in parallel.

* pm_genpd_runtime_resume() doesn't wait for the PM domain status
  to change from GPD_STATE_BUSY after executing pm_genpd_poweron()
  and sets the domain's status to GPD_STATE_BUSY and increments its
  counter of resuming devices (introduced by this change) immediately
  after acquiring the lock.  The counter of resuming devices is then
  decremented after executing __pm_genpd_runtime_resume() for the
  device and the domain's status is reset to GPD_STATE_ACTIVE (unless
  there are more resuming devices in the domain, in which case the
  status remains GPD_STATE_BUSY).

This way, for example, if a device driver's .runtime_resume()
callback executes pm_runtime_resume() for another device in the same
PM domain, pm_genpd_poweron() called by pm_genpd_runtime_resume()
invoked by the runtime PM framework will not block and it will see
that there's nothing to do for it.  Next, the PM domain's lock will
be acquired without waiting for its status to change from
GPD_STATE_BUSY and the device driver's .runtime_resume() callback
will be executed.  In turn, if pm_runtime_suspend() is executed by
one device driver's .runtime_resume() callback for another device in
the same PM domain, pm_genpd_poweroff() executed by
pm_genpd_runtime_suspend() invoked by the runtime PM framework as a
result will notice that one of the devices in the domain is being
resumed, so it will return immediately.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

Well, this turns out to be more tricky than I thought initially, because I
forgot about some "exotic" combinations of calls, like "runtime resume
immediately followed by runtime suspend called from a device driver's
.runtime_suspend() callback for another device in the same PM domain".

Updated patch, hopefully taking all of these into account, is below.

Thanks,
Rafael

---
 drivers/base/power/domain.c |  144 ++++++++++++++++++++++++++++++--------------
 include/linux/pm_domain.h   |    3 
 2 files changed, 102 insertions(+), 45 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -44,7 +44,8 @@ static void genpd_acquire_lock(struct ge
 	for (;;) {
 		prepare_to_wait(&genpd->status_wait_queue, &wait,
 				TASK_UNINTERRUPTIBLE);
-		if (genpd->status != GPD_STATE_BUSY)
+		if (genpd->status == GPD_STATE_ACTIVE
+		    || genpd->status == GPD_STATE_POWER_OFF)
 			break;
 		mutex_unlock(&genpd->lock);
 
@@ -60,6 +61,12 @@ static void genpd_release_lock(struct ge
 	mutex_unlock(&genpd->lock);
 }
 
+static void genpd_set_active(struct generic_pm_domain *genpd)
+{
+	if (genpd->resume_count == 0)
+		genpd->status = GPD_STATE_ACTIVE;
+}
+
 /**
  * pm_genpd_poweron - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
@@ -75,42 +82,24 @@ static int pm_genpd_poweron(struct gener
 
  start:
 	if (parent) {
-		mutex_lock(&parent->lock);
+		genpd_acquire_lock(parent);
 		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
 	} else {
 		mutex_lock(&genpd->lock);
 	}
-	/*
-	 * Wait for the domain to transition into either the active,
-	 * or the power off state.
-	 */
-	for (;;) {
-		prepare_to_wait(&genpd->status_wait_queue, &wait,
-				TASK_UNINTERRUPTIBLE);
-		if (genpd->status != GPD_STATE_BUSY)
-			break;
-		mutex_unlock(&genpd->lock);
-		if (parent)
-			mutex_unlock(&parent->lock);
-
-		schedule();
-
-		if (parent) {
-			mutex_lock(&parent->lock);
-			mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-		} else {
-			mutex_lock(&genpd->lock);
-		}
-	}
-	finish_wait(&genpd->status_wait_queue, &wait);
 
 	if (genpd->status == GPD_STATE_ACTIVE
 	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
 		goto out;
 
+	if (genpd->status != GPD_STATE_POWER_OFF) {
+		genpd_set_active(genpd);
+		goto out;
+	}
+
 	if (parent && parent->status != GPD_STATE_ACTIVE) {
 		mutex_unlock(&genpd->lock);
-		mutex_unlock(&parent->lock);
+		genpd_release_lock(parent);
 
 		ret = pm_genpd_poweron(parent);
 		if (ret)
@@ -125,14 +114,14 @@ static int pm_genpd_poweron(struct gener
 			goto out;
 	}
 
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd_set_active(genpd);
 	if (parent)
 		parent->sd_count++;
 
  out:
 	mutex_unlock(&genpd->lock);
 	if (parent)
-		mutex_unlock(&parent->lock);
+		genpd_release_lock(parent);
 
 	return ret;
 }
@@ -210,6 +199,20 @@ static void __pm_genpd_restore_device(st
 }
 
 /**
+ * genpd_abort_poweroff - Check if a PM domain power off should be aborted.
+ * @genpd: PM domain to check.
+ *
+ * Return true if a PM domain's status changed to GPD_STATE_ACTIVE during
+ * a "power off" operation, which means that a "power on" has occured in the
+ * meantime, or if its resume_count field is different from zero, which means
+ * that one of its devices has been resumed in the meantime.
+ */
+static bool genpd_abort_poweroff(struct generic_pm_domain *genpd)
+{
+	return genpd->status == GPD_STATE_ACTIVE || genpd->resume_count > 0;
+}
+
+/**
  * pm_genpd_poweroff - Remove power from a given PM domain.
  * @genpd: PM domain to power down.
  *
@@ -223,9 +226,17 @@ static int pm_genpd_poweroff(struct gene
 	struct generic_pm_domain *parent;
 	struct dev_list_entry *dle;
 	unsigned int not_suspended;
-	int ret;
+	int ret = 0;
 
-	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0)
+ start:
+	/*
+	 * Do not try to power off the domain in the following situations:
+	 * (1) The domain is already in the "power off" state.
+	 * (2) System suspend is in progress.
+	 * (3) One of the domain's devices is being resumed right now.
+	 */
+	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0
+	    || genpd->resume_count > 0)
 		return 0;
 
 	if (genpd->sd_count > 0)
@@ -239,34 +250,54 @@ static int pm_genpd_poweroff(struct gene
 	if (not_suspended > genpd->in_progress)
 		return -EBUSY;
 
+	if (genpd->poweroff_task) {
+		/*
+		 * Another instance of pm_genpd_poweroff() is executing
+		 * callbacks, so tell it to start over and return.
+		 */
+		genpd->status = GPD_STATE_REPEAT;
+		return 0;
+	}
+
 	if (genpd->gov && genpd->gov->power_down_ok) {
 		if (!genpd->gov->power_down_ok(&genpd->domain))
 			return -EAGAIN;
 	}
 
 	genpd->status = GPD_STATE_BUSY;
+	genpd->poweroff_task = current;
 
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
 		if (ret)
 			goto err_dev;
-	}
 
-	mutex_unlock(&genpd->lock);
+		if (genpd_abort_poweroff(genpd))
+			goto out;
+
+		if (genpd->status == GPD_STATE_REPEAT) {
+			genpd->poweroff_task = NULL;
+			goto start;
+		}
+	}
 
 	parent = genpd->parent;
 	if (parent) {
+		mutex_unlock(&genpd->lock);
+
 		genpd_acquire_lock(parent);
 		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-	} else {
-		mutex_lock(&genpd->lock);
+
+		if (genpd_abort_poweroff(genpd)) {
+			genpd_release_lock(parent);
+			goto out;
+		}
 	}
 
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
 	genpd->status = GPD_STATE_POWER_OFF;
-	wake_up_all(&genpd->status_wait_queue);
 
 	if (parent) {
 		genpd_sd_counter_dec(parent);
@@ -276,16 +307,17 @@ static int pm_genpd_poweroff(struct gene
 		genpd_release_lock(parent);
 	}
 
-	return 0;
+ out:
+	genpd->poweroff_task = NULL;
+	wake_up_all(&genpd->status_wait_queue);
+	return ret;
 
  err_dev:
 	list_for_each_entry_continue(dle, &genpd->dev_list, node)
 		__pm_genpd_restore_device(dle, genpd);
 
-	genpd->status = GPD_STATE_ACTIVE;
-	wake_up_all(&genpd->status_wait_queue);
-
-	return ret;
+	genpd_set_active(genpd);
+	goto out;
 }
 
 /**
@@ -327,11 +359,11 @@ static int pm_genpd_runtime_suspend(stru
 			return ret;
 	}
 
-	genpd_acquire_lock(genpd);
+	mutex_lock(&genpd->lock);
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-	genpd_release_lock(genpd);
+	mutex_unlock(&genpd->lock);
 
 	return 0;
 }
@@ -365,6 +397,7 @@ static void __pm_genpd_runtime_resume(st
 static int pm_genpd_runtime_resume(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	DEFINE_WAIT(wait);
 	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
@@ -377,12 +410,31 @@ static int pm_genpd_runtime_resume(struc
 	if (ret)
 		return ret;
 
-	genpd_acquire_lock(genpd);
+	mutex_lock(&genpd->lock);
 	genpd->status = GPD_STATE_BUSY;
+	genpd->resume_count++;
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		/*
+		 * If current is the powering off task, we have been called
+		 * reentrantly from one of the device callbacks, so we should
+		 * not wait.
+		 */
+		if (!genpd->poweroff_task || genpd->poweroff_task == current)
+			break;
+		mutex_unlock(&genpd->lock);
+
+		schedule();
+
+		mutex_lock(&genpd->lock);
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
 	__pm_genpd_runtime_resume(dev, genpd);
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd->resume_count--;
+	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
-	genpd_release_lock(genpd);
+	mutex_unlock(&genpd->lock);
 
 	if (genpd->start_device)
 		genpd->start_device(dev);
@@ -1122,6 +1174,8 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->sd_count = 0;
 	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
 	init_waitqueue_head(&genpd->status_wait_queue);
+	genpd->poweroff_task = NULL;
+	genpd->resume_count = 0;
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -14,6 +14,7 @@
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
 	GPD_STATE_BUSY,		/* Something is happening to the PM domain */
+	GPD_STATE_REPEAT,	/* Power off in progress, to be repeated */
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
 };
 
@@ -34,6 +35,8 @@ struct generic_pm_domain {
 	unsigned int sd_count;	/* Number of subdomains with power "on" */
 	enum gpd_status status;	/* Current state of the domain */
 	wait_queue_head_t status_wait_queue;
+	struct task_struct *poweroff_task;	/* Powering off task */
+	unsigned int resume_count;	/* Number of devices being resumed */
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */

[PATCH 5/5 v1] PM / Domains: Do not restore all devices on power off error

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since every device in a PM domain has its own need_restore
flag, which is set by __pm_genpd_save_device(), there's no need to
walk the domain's device list and restore all devices on an error
from one of the drivers' .runtime_suspend() callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |    3 ---
 1 file changed, 3 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -299,9 +299,6 @@ static int pm_genpd_poweroff(struct gene
 	return 0;
 
  err_dev:
-	list_for_each_entry_continue(dle, &genpd->dev_list, node)
-		__pm_genpd_restore_device(dle, genpd);
-
 	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
 

[Update][PATCH 5/5] PM / Domains: Do not restore all devices on power off error

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since every device in a PM domain has its own need_restore
flag, which is set by __pm_genpd_save_device(), there's no need to
walk the domain's device list and restore all devices on an error
from one of the drivers' .runtime_suspend() callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

This update is on top of the [4/5] update I've just sent.  It makes the code
more straightforward IMHO.

Thanks,
Rafael

---
 drivers/base/power/domain.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -269,8 +269,10 @@ static int pm_genpd_poweroff(struct gene
 
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
-		if (ret)
-			goto err_dev;
+		if (ret) {
+			genpd_set_active(genpd);
+			goto out;
+		}
 
 		if (genpd_abort_poweroff(genpd))
 			goto out;
@@ -311,13 +313,6 @@ static int pm_genpd_poweroff(struct gene
 	genpd->poweroff_task = NULL;
 	wake_up_all(&genpd->status_wait_queue);
 	return ret;
-
- err_dev:
-	list_for_each_entry_continue(dle, &genpd->dev_list, node)
-		__pm_genpd_restore_device(dle, genpd);
-
-	genpd_set_active(genpd);
-	goto out;
 }
 
 /**

Re: [PATCH 0/5 v1] PM / Domains: Generic PM domains improvements

[Greg KH]

On Wed, Jul 06, 2011 at 10:48:35PM +0200, Rafael J. Wysocki wrote:
> Hi,
> 
> For the last few days I've been working on modifications of the generic
> PM domains code allowing .start_device()/.stop_device() and drivers'
> .runtime_suspend()/.runtime_resume() callbacks to execute things like
> pm_runtime_resume() for other devices in the same PM domain safely
> without deadlocking (the original motivation was a console driver on
> shmobile that pm_runtime_get_*()/pm_runtime_put_*() are called for from
> the other devices' .runtime_suspend()/.runtime_resume() callbacks).
> 
> I think I solved that problem, but in the process I found a few places
> where fixes and/on improvements were necessary.  The patches in this
> series address those issues.  They are on top of the branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-domains
> 
> Patches [1-2/5] are regarded as 3.1 material, the others not necessarily,
> but I'd like to push them for 3.1 too if there are no complaints.

No complaints from me, looks nice.

greg k-h

Re: [PATCH 0/5 v1] PM / Domains: Generic PM domains improvements

[Rafael J. Wysocki]

On Wednesday, July 06, 2011, Greg KH wrote:
> On Wed, Jul 06, 2011 at 10:48:35PM +0200, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > For the last few days I've been working on modifications of the generic
> > PM domains code allowing .start_device()/.stop_device() and drivers'
> > .runtime_suspend()/.runtime_resume() callbacks to execute things like
> > pm_runtime_resume() for other devices in the same PM domain safely
> > without deadlocking (the original motivation was a console driver on
> > shmobile that pm_runtime_get_*()/pm_runtime_put_*() are called for from
> > the other devices' .runtime_suspend()/.runtime_resume() callbacks).
> > 
> > I think I solved that problem, but in the process I found a few places
> > where fixes and/on improvements were necessary.  The patches in this
> > series address those issues.  They are on top of the branch at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-domains
> > 
> > Patches [1-2/5] are regarded as 3.1 material, the others not necessarily,
> > but I'd like to push them for 3.1 too if there are no complaints.
> 
> No complaints from me, looks nice.

Thanks!

Rafael

[PATCH 0/6 v2] PM / Domains: Generic PM domains improvements

[Rafael J. Wysocki]

Hi,

The majority of patches have been updated since the last submission and there's
one new patch, so it's the time for a refresh. :-)

All patches in this series have been posted already (the last one without
a changelog, though).

On Wednesday, July 06, 2011, Rafael J. Wysocki wrote:
> 
> For the last few days I've been working on modifications of the generic
> PM domains code allowing .start_device()/.stop_device() and drivers'
> .runtime_suspend()/.runtime_resume() callbacks to execute things like
> pm_runtime_resume() for other devices in the same PM domain safely
> without deadlocking (the original motivation was a console driver on
> shmobile that pm_runtime_get_*()/pm_runtime_put_*() are called for from
> the other devices' .runtime_suspend()/.runtime_resume() callbacks).
> 
> I think I solved that problem, but in the process I found a few places
> where fixes and/on improvements were necessary.  The patches in this
> series address those issues.

[1/6] - Set device state to "active" during system resume.

[2/6] - Make failing pm_genpd_prepare() clean up properly.

[3/6] - Rework generic PM domains locking so that device callbacks are not
        executed under the genpd lock.

[4/6] - Allow device callbacks executed by the generic PM domains code to
        run runtime PM helpers safely (ie. without deadlocking).

[5/6] - Avoid restoring the states of all devices on failing domain power off.

[6/6] - Imorove the handling of system wakeup devices during system suspend.

The series is on top of the branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/suspend-2.6.git pm-domains
 
All patches in this series are regarded as 3.1 material.

Thanks,
Rafael

[PATCH 1/6 v2] PM / Domains: Set device state to "active" during system resume

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The runtime PM status of devices in a power domain that is not
powered off in pm_genpd_complete() should be set to "active", because
those devices are operational at this point.  Some of them may not be
in use, though, so make pm_genpd_complete() call pm_runtime_idle()
in addition to pm_runtime_set_active() for each of them.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -786,7 +786,9 @@ static void pm_genpd_complete(struct dev
 
 	if (run_complete) {
 		pm_generic_complete(dev);
+		pm_runtime_set_active(dev);
 		pm_runtime_enable(dev);
+		pm_runtime_idle(dev);
 	}
 }
 

[PATCH 2/6 v2] PM / Domains: Make failing pm_genpd_prepare() clean up properly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

If pm_generic_prepare() in pm_genpd_prepare() returns error code,
the PM domains counter of "prepared" devices should be decremented
and its suspend_power_off flag should be reset if this counter drops
down to zero.  Otherwise, the PM domain runtime PM code will not
handle the domain correctly (it will permanently think that system
suspend is in progress).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -367,6 +367,7 @@ static void pm_genpd_sync_poweroff(struc
 static int pm_genpd_prepare(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -400,7 +401,16 @@ static int pm_genpd_prepare(struct devic
 
 	mutex_unlock(&genpd->lock);
 
-	return pm_generic_prepare(dev);
+	ret = pm_generic_prepare(dev);
+	if (ret) {
+		mutex_lock(&genpd->lock);
+
+		if (--genpd->prepared_count == 0)
+			genpd->suspend_power_off = false;
+
+		mutex_unlock(&genpd->lock);
+	}
+	return ret;
 }
 
 /**

[PATCH 3/6 v2] PM / Domains: Do not execute device callbacks under locks

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Currently, the .start_device() and .stop_device() callbacks from
struct generic_pm_domain() as well as the device drivers' runtime PM
callbacks used by the generic PM domains code are executed under
the generic PM domain lock.  This, unfortunately, is prone to
deadlocks, for example if a device and its parent are boths members
of the same PM domain.  For this reason, it would be better if the
PM domains code didn't execute device callbacks under the lock.

Rework the locking in the generic PM domains code so that the lock
is dropped for the execution of device callbacks.  To this end,
introduce PM domains states reflecting the current status of a PM
domain and such that the PM domain lock cannot be acquired if the
status is GPD_STATE_BUSY.  Make threads attempting to acquire a PM
domain's lock wait until the status changes to either
GPD_STATE_ACTIVE or GPD_STATE_POWER_OFF.

This change by itself doesn't fix the deadlock problem mentioned
above, but the mechanism introduced by it will be used for for this
purpose by a subsequent patch.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |  249 +++++++++++++++++++++++++++++++-------------
 include/linux/pm_domain.h   |   10 +
 2 files changed, 185 insertions(+), 74 deletions(-)

Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,8 +11,11 @@
 
 #include <linux/device.h>
 
-#define GPD_IN_SUSPEND	1
-#define GPD_POWER_OFF	2
+enum gpd_status {
+	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
+	GPD_STATE_BUSY,		/* Something is happening to the PM domain */
+	GPD_STATE_POWER_OFF,	/* PM domain is off */
+};
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
@@ -29,7 +32,8 @@ struct generic_pm_domain {
 	struct work_struct power_off_work;
 	unsigned int in_progress;	/* Number of devices being suspended now */
 	unsigned int sd_count;	/* Number of subdomains with power "on" */
-	bool power_is_off;	/* Whether or not power has been removed */
+	enum gpd_status status;	/* Current state of the domain */
+	wait_queue_head_t status_wait_queue;
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -13,6 +13,8 @@
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/sched.h>
+#include <linux/suspend.h>
 
 #ifdef CONFIG_PM
 
@@ -30,6 +32,34 @@ static void genpd_sd_counter_dec(struct
 			genpd->sd_count--;
 }
 
+static void genpd_acquire_lock(struct generic_pm_domain *genpd)
+{
+	DEFINE_WAIT(wait);
+
+	mutex_lock(&genpd->lock);
+	/*
+	 * Wait for the domain to transition into either the active,
+	 * or the power off state.
+	 */
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (genpd->status != GPD_STATE_BUSY)
+			break;
+		mutex_unlock(&genpd->lock);
+
+		schedule();
+
+		mutex_lock(&genpd->lock);
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
+}
+
+static void genpd_release_lock(struct generic_pm_domain *genpd)
+{
+	mutex_unlock(&genpd->lock);
+}
+
 /**
  * pm_genpd_poweron - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
@@ -39,22 +69,50 @@ static void genpd_sd_counter_dec(struct
  */
 int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
+	struct generic_pm_domain *parent = genpd->parent;
+	DEFINE_WAIT(wait);
 	int ret = 0;
 
  start:
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	if (parent) {
+		mutex_lock(&parent->lock);
+		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	} else {
+		mutex_lock(&genpd->lock);
+	}
+	/*
+	 * Wait for the domain to transition into either the active,
+	 * or the power off state.
+	 */
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		if (genpd->status != GPD_STATE_BUSY)
+			break;
+		mutex_unlock(&genpd->lock);
+		if (parent)
+			mutex_unlock(&parent->lock);
+
+		schedule();
 
-	if (!genpd->power_is_off
+		if (parent) {
+			mutex_lock(&parent->lock);
+			mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+		} else {
+			mutex_lock(&genpd->lock);
+		}
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
+
+	if (genpd->status == GPD_STATE_ACTIVE
 	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
 		goto out;
 
-	if (genpd->parent && genpd->parent->power_is_off) {
+	if (parent && parent->status != GPD_STATE_ACTIVE) {
 		mutex_unlock(&genpd->lock);
-		mutex_unlock(&genpd->parent->lock);
+		mutex_unlock(&parent->lock);
 
-		ret = pm_genpd_poweron(genpd->parent);
+		ret = pm_genpd_poweron(parent);
 		if (ret)
 			return ret;
 
@@ -67,14 +125,14 @@ int pm_genpd_poweron(struct generic_pm_d
 			goto out;
 	}
 
-	genpd->power_is_off = false;
-	if (genpd->parent)
-		genpd->parent->sd_count++;
+	genpd->status = GPD_STATE_ACTIVE;
+	if (parent)
+		parent->sd_count++;
 
  out:
 	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	if (parent)
+		mutex_unlock(&parent->lock);
 
 	return ret;
 }
@@ -90,6 +148,7 @@ int pm_genpd_poweron(struct generic_pm_d
  */
 static int __pm_genpd_save_device(struct dev_list_entry *dle,
 				  struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct device *dev = dle->dev;
 	struct device_driver *drv = dev->driver;
@@ -98,6 +157,8 @@ static int __pm_genpd_save_device(struct
 	if (dle->need_restore)
 		return 0;
 
+	mutex_unlock(&genpd->lock);
+
 	if (drv && drv->pm && drv->pm->runtime_suspend) {
 		if (genpd->start_device)
 			genpd->start_device(dev);
@@ -108,6 +169,8 @@ static int __pm_genpd_save_device(struct
 			genpd->stop_device(dev);
 	}
 
+	mutex_lock(&genpd->lock);
+
 	if (!ret)
 		dle->need_restore = true;
 
@@ -121,6 +184,7 @@ static int __pm_genpd_save_device(struct
  */
 static void __pm_genpd_restore_device(struct dev_list_entry *dle,
 				      struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct device *dev = dle->dev;
 	struct device_driver *drv = dev->driver;
@@ -128,6 +192,8 @@ static void __pm_genpd_restore_device(st
 	if (!dle->need_restore)
 		return;
 
+	mutex_unlock(&genpd->lock);
+
 	if (drv && drv->pm && drv->pm->runtime_resume) {
 		if (genpd->start_device)
 			genpd->start_device(dev);
@@ -138,6 +204,8 @@ static void __pm_genpd_restore_device(st
 			genpd->stop_device(dev);
 	}
 
+	mutex_lock(&genpd->lock);
+
 	dle->need_restore = false;
 }
 
@@ -150,13 +218,14 @@ static void __pm_genpd_restore_device(st
  * the @genpd's devices' drivers and remove power from @genpd.
  */
 static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+	__releases(&genpd->lock) __acquires(&genpd->lock)
 {
 	struct generic_pm_domain *parent;
 	struct dev_list_entry *dle;
 	unsigned int not_suspended;
 	int ret;
 
-	if (genpd->power_is_off || genpd->prepared_count > 0)
+	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0)
 		return 0;
 
 	if (genpd->sd_count > 0)
@@ -175,22 +244,36 @@ static int pm_genpd_poweroff(struct gene
 			return -EAGAIN;
 	}
 
+	genpd->status = GPD_STATE_BUSY;
+
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
 		if (ret)
 			goto err_dev;
 	}
 
+	mutex_unlock(&genpd->lock);
+
+	parent = genpd->parent;
+	if (parent) {
+		genpd_acquire_lock(parent);
+		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	} else {
+		mutex_lock(&genpd->lock);
+	}
+
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
+	wake_up_all(&genpd->status_wait_queue);
 
-	parent = genpd->parent;
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		if (parent->sd_count == 0)
 			queue_work(pm_wq, &parent->power_off_work);
+
+		genpd_release_lock(parent);
 	}
 
 	return 0;
@@ -199,6 +282,9 @@ static int pm_genpd_poweroff(struct gene
 	list_for_each_entry_continue(dle, &genpd->dev_list, node)
 		__pm_genpd_restore_device(dle, genpd);
 
+	genpd->status = GPD_STATE_ACTIVE;
+	wake_up_all(&genpd->status_wait_queue);
+
 	return ret;
 }
 
@@ -212,13 +298,9 @@ static void genpd_power_off_work_fn(stru
 
 	genpd = container_of(work, struct generic_pm_domain, power_off_work);
 
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
+	genpd_acquire_lock(genpd);
 	pm_genpd_poweroff(genpd);
-	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	genpd_release_lock(genpd);
 }
 
 /**
@@ -239,23 +321,17 @@ static int pm_genpd_runtime_suspend(stru
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->parent)
-		mutex_lock(&genpd->parent->lock);
-	mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-
 	if (genpd->stop_device) {
 		int ret = genpd->stop_device(dev);
 		if (ret)
-			goto out;
+			return ret;
 	}
+
+	genpd_acquire_lock(genpd);
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-
- out:
-	mutex_unlock(&genpd->lock);
-	if (genpd->parent)
-		mutex_unlock(&genpd->parent->lock);
+	genpd_release_lock(genpd);
 
 	return 0;
 }
@@ -276,9 +352,6 @@ static void __pm_genpd_runtime_resume(st
 			break;
 		}
 	}
-
-	if (genpd->start_device)
-		genpd->start_device(dev);
 }
 
 /**
@@ -304,9 +377,15 @@ static int pm_genpd_runtime_resume(struc
 	if (ret)
 		return ret;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
+	genpd->status = GPD_STATE_BUSY;
 	__pm_genpd_runtime_resume(dev, genpd);
-	mutex_unlock(&genpd->lock);
+	genpd->status = GPD_STATE_ACTIVE;
+	wake_up_all(&genpd->status_wait_queue);
+	genpd_release_lock(genpd);
+
+	if (genpd->start_device)
+		genpd->start_device(dev);
 
 	return 0;
 }
@@ -339,7 +418,7 @@ static void pm_genpd_sync_poweroff(struc
 {
 	struct generic_pm_domain *parent = genpd->parent;
 
-	if (genpd->power_is_off)
+	if (genpd->status == GPD_STATE_POWER_OFF)
 		return;
 
 	if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
@@ -348,7 +427,7 @@ static void pm_genpd_sync_poweroff(struc
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		pm_genpd_sync_poweroff(parent);
@@ -375,32 +454,41 @@ static int pm_genpd_prepare(struct devic
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	/*
+	 * If a wakeup request is pending for the device, it should be woken up
+	 * at this point and a system wakeup event should be reported if it's
+	 * set up to wake up the system from sleep states.
+	 */
+	pm_runtime_get_noresume(dev);
+	if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+		pm_wakeup_event(dev, 0);
+
+	if (pm_wakeup_pending()) {
+		pm_runtime_put_sync(dev);
+		return -EBUSY;
+	}
+
+	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count++ == 0)
-		genpd->suspend_power_off = genpd->power_is_off;
+		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
+
+	genpd_release_lock(genpd);
 
 	if (genpd->suspend_power_off) {
-		mutex_unlock(&genpd->lock);
+		pm_runtime_put_noidle(dev);
 		return 0;
 	}
 
 	/*
-	 * If the device is in the (runtime) "suspended" state, call
-	 * .start_device() for it, if defined.
-	 */
-	if (pm_runtime_suspended(dev))
-		__pm_genpd_runtime_resume(dev, genpd);
-
-	/*
-	 * Do not check if runtime resume is pending at this point, because it
-	 * has been taken care of already and if pm_genpd_poweron() ran at this
-	 * point as a result of the check, it would deadlock.
+	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
+	 * so pm_genpd_poweron() will return immediately, but if the device
+	 * is suspended (e.g. it's been stopped by .stop_device()), we need
+	 * to make it operational.
 	 */
+	pm_runtime_resume(dev);
 	__pm_runtime_disable(dev, false);
 
-	mutex_unlock(&genpd->lock);
-
 	ret = pm_generic_prepare(dev);
 	if (ret) {
 		mutex_lock(&genpd->lock);
@@ -409,7 +497,10 @@ static int pm_genpd_prepare(struct devic
 			genpd->suspend_power_off = false;
 
 		mutex_unlock(&genpd->lock);
+		pm_runtime_enable(dev);
 	}
+
+	pm_runtime_put_sync(dev);
 	return ret;
 }
 
@@ -726,7 +817,7 @@ static int pm_genpd_restore_noirq(struct
 	 * guaranteed that this function will never run twice in parallel for
 	 * the same PM domain, so it is not necessary to use locking here.
 	 */
-	genpd->power_is_off = true;
+	genpd->status = GPD_STATE_POWER_OFF;
 	if (genpd->suspend_power_off) {
 		/*
 		 * The boot kernel might put the domain into the power on state,
@@ -836,9 +927,9 @@ int pm_genpd_add_device(struct generic_p
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
-	if (genpd->power_is_off) {
+	if (genpd->status == GPD_STATE_POWER_OFF) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -870,7 +961,7 @@ int pm_genpd_add_device(struct generic_p
 	spin_unlock_irq(&dev->power.lock);
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -891,7 +982,7 @@ int pm_genpd_remove_device(struct generi
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;
@@ -915,7 +1006,7 @@ int pm_genpd_remove_device(struct generi
 	}
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -934,9 +1025,19 @@ int pm_genpd_add_subdomain(struct generi
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(new_subdomain))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+ start:
+	genpd_acquire_lock(genpd);
+	mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
 
-	if (genpd->power_is_off && !new_subdomain->power_is_off) {
+	if (new_subdomain->status != GPD_STATE_POWER_OFF
+	    && new_subdomain->status != GPD_STATE_ACTIVE) {
+		mutex_unlock(&new_subdomain->lock);
+		genpd_release_lock(genpd);
+		goto start;
+	}
+
+	if (genpd->status == GPD_STATE_POWER_OFF
+	    &&  new_subdomain->status != GPD_STATE_POWER_OFF) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -948,17 +1049,14 @@ int pm_genpd_add_subdomain(struct generi
 		}
 	}
 
-	mutex_lock_nested(&new_subdomain->lock, SINGLE_DEPTH_NESTING);
-
 	list_add_tail(&new_subdomain->sd_node, &genpd->sd_list);
 	new_subdomain->parent = genpd;
-	if (!subdomain->power_is_off)
+	if (subdomain->status != GPD_STATE_POWER_OFF)
 		genpd->sd_count++;
 
-	mutex_unlock(&new_subdomain->lock);
-
  out:
-	mutex_unlock(&genpd->lock);
+	mutex_unlock(&new_subdomain->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -977,7 +1075,8 @@ int pm_genpd_remove_subdomain(struct gen
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(target))
 		return -EINVAL;
 
-	mutex_lock(&genpd->lock);
+ start:
+	genpd_acquire_lock(genpd);
 
 	list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
 		if (subdomain != target)
@@ -985,9 +1084,16 @@ int pm_genpd_remove_subdomain(struct gen
 
 		mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);
 
+		if (subdomain->status != GPD_STATE_POWER_OFF
+		    && subdomain->status != GPD_STATE_ACTIVE) {
+			mutex_unlock(&subdomain->lock);
+			genpd_release_lock(genpd);
+			goto start;
+		}
+
 		list_del(&subdomain->sd_node);
 		subdomain->parent = NULL;
-		if (!subdomain->power_is_off)
+		if (subdomain->status != GPD_STATE_POWER_OFF)
 			genpd_sd_counter_dec(genpd);
 
 		mutex_unlock(&subdomain->lock);
@@ -996,7 +1102,7 @@ int pm_genpd_remove_subdomain(struct gen
 		break;
 	}
 
-	mutex_unlock(&genpd->lock);
+	genpd_release_lock(genpd);
 
 	return ret;
 }
@@ -1022,7 +1128,8 @@ void pm_genpd_init(struct generic_pm_dom
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
 	genpd->in_progress = 0;
 	genpd->sd_count = 0;
-	genpd->power_is_off = is_off;
+	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
+	init_waitqueue_head(&genpd->status_wait_queue);
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;

[PATCH 4/6 v2] PM / Domains: Allow callbacks to execute all runtime PM helpers

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

A deadlock may occur if one of the PM domains' .start_device() or
.stop_device() callbacks or a device driver's .runtime_suspend() or
.runtime_resume() callback executed by the core generic PM domain
code uses a "wrong" runtime PM helper function.  This happens, for
example, if .runtime_resume() from one device's driver calls
pm_runtime_resume() for another device in the same PM domain.
A similar situation may take place if a device's parent is in the
same PM domain, in which case the runtime PM framework may execute
pm_genpd_runtime_resume() automatically for the parent (if it is
suspended at the moment).  This, of course, is undesirable, so
the generic PM domains code should be modified to prevent it from
happening.

The runtime PM framework guarantees that pm_genpd_runtime_suspend()
and pm_genpd_runtime_resume() won't be executed in parallel for
the same device, so the generic PM domains code need not worry
about those cases.  Still, it needs to prevent the other possible
race conditions between pm_genpd_runtime_suspend(),
pm_genpd_runtime_resume(), pm_genpd_poweron() and pm_genpd_poweroff()
from happening and it needs to avoid deadlocks at the same time.
To this end, modify the generic PM domains code to relax
synchronization rules so that:

* pm_genpd_poweron() doesn't wait for the PM domain status to
  change from GPD_STATE_BUSY.  If it finds that the status is
  not GPD_STATE_POWER_OFF, it returns without powering the domain on
  (it may modify the status depending on the circumstances).

* pm_genpd_poweroff() returns as soon as it finds that the PM
  domain's status changed from GPD_STATE_BUSY after it's released
  the PM domain's lock.

* pm_genpd_runtime_suspend() doesn't wait for the PM domain status
  to change from GPD_STATE_BUSY after executing the domain's
  .stop_device() callback and executes pm_genpd_poweroff() only
  if pm_genpd_runtime_resume() is not executed in parallel.

* pm_genpd_runtime_resume() doesn't wait for the PM domain status
  to change from GPD_STATE_BUSY after executing pm_genpd_poweron()
  and sets the domain's status to GPD_STATE_BUSY and increments its
  counter of resuming devices (introduced by this change) immediately
  after acquiring the lock.  The counter of resuming devices is then
  decremented after executing __pm_genpd_runtime_resume() for the
  device and the domain's status is reset to GPD_STATE_ACTIVE (unless
  there are more resuming devices in the domain, in which case the
  status remains GPD_STATE_BUSY).

This way, for example, if a device driver's .runtime_resume()
callback executes pm_runtime_resume() for another device in the same
PM domain, pm_genpd_poweron() called by pm_genpd_runtime_resume()
invoked by the runtime PM framework will not block and it will see
that there's nothing to do for it.  Next, the PM domain's lock will
be acquired without waiting for its status to change from
GPD_STATE_BUSY and the device driver's .runtime_resume() callback
will be executed.  In turn, if pm_runtime_suspend() is executed by
one device driver's .runtime_resume() callback for another device in
the same PM domain, pm_genpd_poweroff() executed by
pm_genpd_runtime_suspend() invoked by the runtime PM framework as a
result will notice that one of the devices in the domain is being
resumed, so it will return immediately.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |  144 ++++++++++++++++++++++++++++++--------------
 include/linux/pm_domain.h   |    3 
 2 files changed, 102 insertions(+), 45 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -44,7 +44,8 @@ static void genpd_acquire_lock(struct ge
 	for (;;) {
 		prepare_to_wait(&genpd->status_wait_queue, &wait,
 				TASK_UNINTERRUPTIBLE);
-		if (genpd->status != GPD_STATE_BUSY)
+		if (genpd->status == GPD_STATE_ACTIVE
+		    || genpd->status == GPD_STATE_POWER_OFF)
 			break;
 		mutex_unlock(&genpd->lock);
 
@@ -60,6 +61,12 @@ static void genpd_release_lock(struct ge
 	mutex_unlock(&genpd->lock);
 }
 
+static void genpd_set_active(struct generic_pm_domain *genpd)
+{
+	if (genpd->resume_count == 0)
+		genpd->status = GPD_STATE_ACTIVE;
+}
+
 /**
  * pm_genpd_poweron - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
@@ -75,42 +82,24 @@ static int pm_genpd_poweron(struct gener
 
  start:
 	if (parent) {
-		mutex_lock(&parent->lock);
+		genpd_acquire_lock(parent);
 		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
 	} else {
 		mutex_lock(&genpd->lock);
 	}
-	/*
-	 * Wait for the domain to transition into either the active,
-	 * or the power off state.
-	 */
-	for (;;) {
-		prepare_to_wait(&genpd->status_wait_queue, &wait,
-				TASK_UNINTERRUPTIBLE);
-		if (genpd->status != GPD_STATE_BUSY)
-			break;
-		mutex_unlock(&genpd->lock);
-		if (parent)
-			mutex_unlock(&parent->lock);
-
-		schedule();
-
-		if (parent) {
-			mutex_lock(&parent->lock);
-			mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-		} else {
-			mutex_lock(&genpd->lock);
-		}
-	}
-	finish_wait(&genpd->status_wait_queue, &wait);
 
 	if (genpd->status == GPD_STATE_ACTIVE
 	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
 		goto out;
 
+	if (genpd->status != GPD_STATE_POWER_OFF) {
+		genpd_set_active(genpd);
+		goto out;
+	}
+
 	if (parent && parent->status != GPD_STATE_ACTIVE) {
 		mutex_unlock(&genpd->lock);
-		mutex_unlock(&parent->lock);
+		genpd_release_lock(parent);
 
 		ret = pm_genpd_poweron(parent);
 		if (ret)
@@ -125,14 +114,14 @@ static int pm_genpd_poweron(struct gener
 			goto out;
 	}
 
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd_set_active(genpd);
 	if (parent)
 		parent->sd_count++;
 
  out:
 	mutex_unlock(&genpd->lock);
 	if (parent)
-		mutex_unlock(&parent->lock);
+		genpd_release_lock(parent);
 
 	return ret;
 }
@@ -210,6 +199,20 @@ static void __pm_genpd_restore_device(st
 }
 
 /**
+ * genpd_abort_poweroff - Check if a PM domain power off should be aborted.
+ * @genpd: PM domain to check.
+ *
+ * Return true if a PM domain's status changed to GPD_STATE_ACTIVE during
+ * a "power off" operation, which means that a "power on" has occured in the
+ * meantime, or if its resume_count field is different from zero, which means
+ * that one of its devices has been resumed in the meantime.
+ */
+static bool genpd_abort_poweroff(struct generic_pm_domain *genpd)
+{
+	return genpd->status == GPD_STATE_ACTIVE || genpd->resume_count > 0;
+}
+
+/**
  * pm_genpd_poweroff - Remove power from a given PM domain.
  * @genpd: PM domain to power down.
  *
@@ -223,9 +226,17 @@ static int pm_genpd_poweroff(struct gene
 	struct generic_pm_domain *parent;
 	struct dev_list_entry *dle;
 	unsigned int not_suspended;
-	int ret;
+	int ret = 0;
 
-	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0)
+ start:
+	/*
+	 * Do not try to power off the domain in the following situations:
+	 * (1) The domain is already in the "power off" state.
+	 * (2) System suspend is in progress.
+	 * (3) One of the domain's devices is being resumed right now.
+	 */
+	if (genpd->status == GPD_STATE_POWER_OFF || genpd->prepared_count > 0
+	    || genpd->resume_count > 0)
 		return 0;
 
 	if (genpd->sd_count > 0)
@@ -239,34 +250,54 @@ static int pm_genpd_poweroff(struct gene
 	if (not_suspended > genpd->in_progress)
 		return -EBUSY;
 
+	if (genpd->poweroff_task) {
+		/*
+		 * Another instance of pm_genpd_poweroff() is executing
+		 * callbacks, so tell it to start over and return.
+		 */
+		genpd->status = GPD_STATE_REPEAT;
+		return 0;
+	}
+
 	if (genpd->gov && genpd->gov->power_down_ok) {
 		if (!genpd->gov->power_down_ok(&genpd->domain))
 			return -EAGAIN;
 	}
 
 	genpd->status = GPD_STATE_BUSY;
+	genpd->poweroff_task = current;
 
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
 		if (ret)
 			goto err_dev;
-	}
 
-	mutex_unlock(&genpd->lock);
+		if (genpd_abort_poweroff(genpd))
+			goto out;
+
+		if (genpd->status == GPD_STATE_REPEAT) {
+			genpd->poweroff_task = NULL;
+			goto start;
+		}
+	}
 
 	parent = genpd->parent;
 	if (parent) {
+		mutex_unlock(&genpd->lock);
+
 		genpd_acquire_lock(parent);
 		mutex_lock_nested(&genpd->lock, SINGLE_DEPTH_NESTING);
-	} else {
-		mutex_lock(&genpd->lock);
+
+		if (genpd_abort_poweroff(genpd)) {
+			genpd_release_lock(parent);
+			goto out;
+		}
 	}
 
 	if (genpd->power_off)
 		genpd->power_off(genpd);
 
 	genpd->status = GPD_STATE_POWER_OFF;
-	wake_up_all(&genpd->status_wait_queue);
 
 	if (parent) {
 		genpd_sd_counter_dec(parent);
@@ -276,16 +307,17 @@ static int pm_genpd_poweroff(struct gene
 		genpd_release_lock(parent);
 	}
 
-	return 0;
+ out:
+	genpd->poweroff_task = NULL;
+	wake_up_all(&genpd->status_wait_queue);
+	return ret;
 
  err_dev:
 	list_for_each_entry_continue(dle, &genpd->dev_list, node)
 		__pm_genpd_restore_device(dle, genpd);
 
-	genpd->status = GPD_STATE_ACTIVE;
-	wake_up_all(&genpd->status_wait_queue);
-
-	return ret;
+	genpd_set_active(genpd);
+	goto out;
 }
 
 /**
@@ -327,11 +359,11 @@ static int pm_genpd_runtime_suspend(stru
 			return ret;
 	}
 
-	genpd_acquire_lock(genpd);
+	mutex_lock(&genpd->lock);
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-	genpd_release_lock(genpd);
+	mutex_unlock(&genpd->lock);
 
 	return 0;
 }
@@ -365,6 +397,7 @@ static void __pm_genpd_runtime_resume(st
 static int pm_genpd_runtime_resume(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	DEFINE_WAIT(wait);
 	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
@@ -377,12 +410,31 @@ static int pm_genpd_runtime_resume(struc
 	if (ret)
 		return ret;
 
-	genpd_acquire_lock(genpd);
+	mutex_lock(&genpd->lock);
 	genpd->status = GPD_STATE_BUSY;
+	genpd->resume_count++;
+	for (;;) {
+		prepare_to_wait(&genpd->status_wait_queue, &wait,
+				TASK_UNINTERRUPTIBLE);
+		/*
+		 * If current is the powering off task, we have been called
+		 * reentrantly from one of the device callbacks, so we should
+		 * not wait.
+		 */
+		if (!genpd->poweroff_task || genpd->poweroff_task == current)
+			break;
+		mutex_unlock(&genpd->lock);
+
+		schedule();
+
+		mutex_lock(&genpd->lock);
+	}
+	finish_wait(&genpd->status_wait_queue, &wait);
 	__pm_genpd_runtime_resume(dev, genpd);
-	genpd->status = GPD_STATE_ACTIVE;
+	genpd->resume_count--;
+	genpd_set_active(genpd);
 	wake_up_all(&genpd->status_wait_queue);
-	genpd_release_lock(genpd);
+	mutex_unlock(&genpd->lock);
 
 	if (genpd->start_device)
 		genpd->start_device(dev);
@@ -1130,6 +1182,8 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->sd_count = 0;
 	genpd->status = is_off ? GPD_STATE_POWER_OFF : GPD_STATE_ACTIVE;
 	init_waitqueue_head(&genpd->status_wait_queue);
+	genpd->poweroff_task = NULL;
+	genpd->resume_count = 0;
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -14,6 +14,7 @@
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
 	GPD_STATE_BUSY,		/* Something is happening to the PM domain */
+	GPD_STATE_REPEAT,	/* Power off in progress, to be repeated */
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
 };
 
@@ -34,6 +35,8 @@ struct generic_pm_domain {
 	unsigned int sd_count;	/* Number of subdomains with power "on" */
 	enum gpd_status status;	/* Current state of the domain */
 	wait_queue_head_t status_wait_queue;
+	struct task_struct *poweroff_task;	/* Powering off task */
+	unsigned int resume_count;	/* Number of devices being resumed */
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */

[PATCH 5/6 v2] PM / Domains: Do not restore all devices on power off error

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since every device in a PM domain has its own need_restore
flag, which is set by __pm_genpd_save_device(), there's no need to
walk the domain's device list and restore all devices on an error
from one of the drivers' .runtime_suspend() callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -269,8 +269,10 @@ static int pm_genpd_poweroff(struct gene
 
 	list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
 		ret = __pm_genpd_save_device(dle, genpd);
-		if (ret)
-			goto err_dev;
+		if (ret) {
+			genpd_set_active(genpd);
+			goto out;
+		}
 
 		if (genpd_abort_poweroff(genpd))
 			goto out;
@@ -311,13 +313,6 @@ static int pm_genpd_poweroff(struct gene
 	genpd->poweroff_task = NULL;
 	wake_up_all(&genpd->status_wait_queue);
 	return ret;
-
- err_dev:
-	list_for_each_entry_continue(dle, &genpd->dev_list, node)
-		__pm_genpd_restore_device(dle, genpd);
-
-	genpd_set_active(genpd);
-	goto out;
 }
 
 /**

[PATCH 6/6 v2] PM / Domains: Improve handling of wakeup devices during system suspend

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Kevin points out that if there's a device that can wake up the system
from sleep states, but it doesn't generate wakeup signals by itself
(they are generated on its behalf by other parts of the system) and
it currently is not enabled to wake up the system (that is,
device_may_wakeup() returns "false" for it), we may need to change
its wakeup settings during system suspend (for example, the device
might have been configured to signal remote wakeup from the system's
working state, as needed by runtime PM).  Therefore the generic PM
domains code should invoke the system suspend callbacks provided by
the device's driver, which it doesn't do if the PM domain is powered
off during the system suspend's "prepare" stage.  This is a valid
point.  Moreover, this code also should make sure that system wakeup
devices that are enabled to wake up the system from sleep states and
have to remain active for this purpose are not suspended while the
system is in a sleep state.

To avoid the above issues, make the generic PM domains' .prepare()
routine, pm_genpd_prepare(), force runtime resume of devices whose
system wakeup settings may need to be changed during system suspend
or that should remain active while the system is in a sleep state to
be able to wake it up from that state.

Reported-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -482,6 +482,33 @@ static void pm_genpd_sync_poweroff(struc
 }
 
 /**
+ * resume_needed - Check whether to resume a device before system suspend.
+ * @dev: Device to check.
+ * @genpd: PM domain the device belongs to.
+ *
+ * There are two cases in which a device that can wake up the system from sleep
+ * states should be resumed by pm_genpd_prepare(): (1) if the device is enabled
+ * to wake up the system and it has to remain active for this purpose while the
+ * system is in the sleep state and (2) if the device is not enabled to wake up
+ * the system from sleep states and it generally doesn't generate wakeup signals
+ * by itself (those signals are generated on its behalf by other parts of the
+ * system).  In the latter case it may be necessary to reconfigure the device's
+ * wakeup settings during system suspend, because it may have been set up to
+ * signal remote wakeup from the system's working state as needed by runtime PM.
+ * Return 'true' in either of the above cases.
+ */
+static bool resume_needed(struct device *dev, struct generic_pm_domain *genpd)
+{
+	bool active_wakeup;
+
+	if (!device_can_wakeup(dev))
+		return false;
+
+	active_wakeup = genpd->active_wakeup && genpd->active_wakeup(dev);
+	return device_may_wakeup(dev) ? active_wakeup : !active_wakeup;
+}
+
+/**
  * pm_genpd_prepare - Start power transition of a device in a PM domain.
  * @dev: Device to start the transition of.
  *
@@ -515,6 +542,9 @@ static int pm_genpd_prepare(struct devic
 		return -EBUSY;
 	}
 
+	if (resume_needed(dev, genpd))
+		pm_runtime_resume(dev);
+
 	genpd_acquire_lock(genpd);
 
 	if (genpd->prepared_count++ == 0)

[PATCH 7 v2] PM / Domains: Queue up power off work only if it is not pending

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

In theory it is possible that pm_genpd_poweroff() for two different
subdomains of the same parent domain will attempt to queue up the
execution of pm_genpd_poweroff() for the parent twice in a row.  This
would lead to unpleasant consequences, so prevent it from happening
by checking if genpd->power_off_work is pending before attempting to
queue it up.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

One more patch to the "PM / Domains: Generic PM domains improvements" series,
the changelog should explain everything. :-)

Thanks,
Rafael

---
 drivers/base/power/domain.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -213,6 +213,19 @@ static bool genpd_abort_poweroff(struct
 }
 
 /**
+ * genpd_queue_power_off_work - Queue up the execution of pm_genpd_poweroff().
+ * @genpd: PM domait to power off.
+ *
+ * Queue up the execution of pm_genpd_poweroff() unless it's already been done
+ * before.
+ */
+static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
+{
+	if (!work_pending(&genpd->power_off_work))
+		queue_work(pm_wq, &genpd->power_off_work);
+}
+
+/**
  * pm_genpd_poweroff - Remove power from a given PM domain.
  * @genpd: PM domain to power down.
  *
@@ -304,7 +317,7 @@ static int pm_genpd_poweroff(struct gene
 	if (parent) {
 		genpd_sd_counter_dec(parent);
 		if (parent->sd_count == 0)
-			queue_work(pm_wq, &parent->power_off_work);
+			genpd_queue_power_off_work(parent);
 
 		genpd_release_lock(parent);
 	}