* Re: 3.0-rc6-git6: Reported regressions from 2.6.39
From: Johannes Berg @ 2011-07-10 10:54 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux SCSI List, Linux ACPI, Network Development,
Linux Wireless List, Linux Kernel Mailing List, DRI,
Florian Mickler, Andrew Morton, Kernel Testers List,
Linus Torvalds, Linux PM List, Maciej Rutecki
In-Reply-To: <Y7LyOsOBwFM.A.kJG.kBYGOB@chimera>
On Sun, 2011-07-10 at 12:19 +0200, Rafael J. Wysocki wrote:
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38702
> Subject : 3.0.0-rc4-git6 - INFO: possible circular locking dependency detected - (&rdev->mtx){+.+.+.}, at: [<ffffffffa00e27e0>] cfg80211_netdev_notifier_call+0x275/0x4ff [cfg80211]
> Submitter : Miles Lane <miles.lane@gmail.com>
> Date : 2011-06-26 21:54 (15 days old)
> Message-ID : <BANLkTikztsFnPkJZ7VxRC-zsitwCCvVmeg@mail.gmail.com>
> References : http://marc.info/?l=linux-kernel&m=130912559804336&w=2
Should be fixed by c10841ca722a0bc960dc541c51582773f9a24f98 which was
part of John's latest pull request and is in net-2.6 now on its way up.
johannes
^ permalink raw reply
* 3.0-rc6-git6: Reported regressions from 2.6.39
From: Rafael J. Wysocki @ 2011-07-10 10:19 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Linux SCSI List, Linux ACPI, Network Development,
Linux Wireless List, DRI, Florian Mickler, Andrew Morton,
Kernel Testers List, Linus Torvalds, Linux PM List,
Maciej Rutecki
This message contains a list of some regressions from 2.6.39,
for which there are no fixes in the mainline known to the tracking team.
If any of them have been fixed already, please let us know.
If you know of any other unresolved regressions from 2.6.39, please let us
know either and we'll add them to the list. Also, please let us know
if any of the entries below are invalid.
Each entry from the list will be sent additionally in an automatic reply
to this message with CCs to the people involved in reporting and handling
the issue.
Listed regressions statistics:
Date Total Pending Unresolved
----------------------------------------
2011-07-10 38 16 13
2011-06-26 22 14 12
2011-06-12 8 7 7
Unresolved regressions
----------------------
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=39052
Subject : .tmp_depmod owned by root after modules_install
Submitter : Christian Kujau <lists@nerdbynature.de>
Date : 2011-07-04 9:49 (7 days old)
Message-ID : <alpine.DEB.2.01.1107040242180.9183@trent.utfs.org>
References : http://marc.info/?l=linux-kernel&m=130977296812731&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38802
Subject : Not work hotkey of Microsoft Comfort Curve Keyboard 2000
Submitter : Cristian Aravena Romero <caravena@gmail.com>
Date : 2011-07-05 20:43 (6 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38732
Subject : Linux 3.0-rc5 doesnt boot and hangs at rcu_sched_state ()
Submitter : RKK <kulkarni.ravi4@gmail.com>
Date : 2011-06-29 13:38 (12 days old)
Message-ID : <BANLkTikW6Xk8UMsOHLCnLn7STRzy8RaiJA@mail.gmail.com>
References : http://marc.info/?l=linux-kernel&m=130935402722869&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38702
Subject : 3.0.0-rc4-git6 - INFO: possible circular locking dependency detected - (&rdev->mtx){+.+.+.}, at: [<ffffffffa00e27e0>] cfg80211_netdev_notifier_call+0x275/0x4ff [cfg80211]
Submitter : Miles Lane <miles.lane@gmail.com>
Date : 2011-06-26 21:54 (15 days old)
Message-ID : <BANLkTikztsFnPkJZ7VxRC-zsitwCCvVmeg@mail.gmail.com>
References : http://marc.info/?l=linux-kernel&m=130912559804336&w=2
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38672
Subject : KVM guest boot crashed
Submitter : Steve <stefan.bosak@gmail.com>
Date : 2011-07-02 06:56 (9 days old)
First-Bad-Commit: http://git.kernel.org/linus/6506e4f995967b1a48cc34418c77b318df92ce35
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38622
Subject : [radeon cayman regresion] artefacts on screen
Submitter : Marek Hobler, 'neutrinus' <kernellist@neutrinus.com>
Date : 2011-07-01 09:35 (10 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38612
Subject : Boot time warnings in APIC
Submitter : Bill Huey <bill.huey@gmail.com>
Date : 2011-07-01 06:44 (10 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38582
Subject : T510 43495KG won't resume with 32bit installation
Submitter : Marc Koschewski <marc@osknowledge.org>
Date : 2011-06-30 22:39 (11 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38522
Subject : address space collision error message on boot
Submitter : Jools Wills <jools@oxfordinspire.co.uk>
Date : 2011-06-29 02:28 (12 days old)
First-Bad-Commit: http://git.kernel.org/linus/5d94e81f69d4b1d1102d3ab557ce0a817c11fbbb
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38332
Subject : System hang when enable rc6
Submitter : Gu Rui <chaos.proton@gmail.com>
Date : 2011-06-27 08:16 (14 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=37872
Subject : CPU lockup during boot
Submitter : Bruno Wolff III <bruno@wolff.to>
Date : 2011-06-19 15:00 (22 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=37482
Subject : Driver: i915 - Starting with kernel 3.0.0-rc3, dual monitor freezes a Dell Latitude E6410 when a second monitor is connected at boot time
Submitter : David Hill <hilld@binarystorm.net>
Date : 2011-06-14 18:42 (27 days old)
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=37432
Subject : sdhci-pci fails on 3.0.0-rc1 on Dell E6510
Submitter : Oliver Hartkopp <socketcan@hartkopp.net>
Date : 2011-06-07 20:06 (34 days old)
First-Bad-Commit: http://git.kernel.org/linus/da7822e5ad71ec9b745b412639f1e5e0ba795a20
References : http://article.gmane.org/gmane.linux.kernel.mmc/8442
https://lkml.org/lkml/2011/6/23/660
Regressions with patches
------------------------
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38972
Subject : Kernel panic when syncing while cifs is mounted
Submitter : <farbing@web.de>
Date : 2011-07-08 15:39 (3 days old)
First-Bad-Commit: http://git.kernel.org/linus/dd8544661947ad6d8d87b3c9d4333bfa1583d1bc
Handled-By : Jeff Layton <jlayton@redhat.com>
Patch : https://bugzilla.kernel.org/attachment.cgi?id=65082
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38302
Subject : NFS crash in un-modified 3.0.0-rc3+, list corruption.
Submitter : Ben Greear <greearb@candelatech.com>
Date : 2011-06-20 22:42 (21 days old)
Message-ID : <4DFFCCDC.2070106@candelatech.com>
References : http://marc.info/?l=linux-kernel&m=130860979610651&w=2
Patch : https://bugzilla.kernel.org/attachment.cgi?id=63902
Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=38152
Subject : acpi_power_off lockdep splat
Submitter : Borislav Petkov <bp@alien8.de>
Date : 2011-06-19 13:30 (22 days old)
Message-ID : <20110619133049.GA18168@liondog.tnic>
References : http://marc.info/?l=linux-kernel&m=130849028317365&w=2
Handled-By : Rafael J. Wysocki <rjw@sisk.pl>
Patch : https://patchwork.kernel.org/patch/950852/
For details, please visit the bug entries and follow the links given in
references.
As you can see, there is a Bugzilla entry for each of the listed regressions.
There also is a Bugzilla entry used for tracking the regressions from 2.6.39,
unresolved as well as resolved, at:
http://bugzilla.kernel.org/show_bug.cgi?id=36912
Please let the tracking team know if there are any Bugzilla entries that
should be added to the list in there.
Thanks!
^ permalink raw reply
* [PATCH 6/6 v2] PM / Domains: Improve handling of wakeup devices during system suspend
From: Rafael J. Wysocki @ 2011-07-10 9:09 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107101059.30322.rjw@sisk.pl>
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)
^ permalink raw reply
* [PATCH 5/6 v2] PM / Domains: Do not restore all devices on power off error
From: Rafael J. Wysocki @ 2011-07-10 9:08 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107101059.30322.rjw@sisk.pl>
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;
}
/**
^ permalink raw reply
* [PATCH 4/6 v2] PM / Domains: Allow callbacks to execute all runtime PM helpers
From: Rafael J. Wysocki @ 2011-07-10 9:08 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107101059.30322.rjw@sisk.pl>
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 */
^ permalink raw reply
* [PATCH 3/6 v2] PM / Domains: Do not execute device callbacks under locks
From: Rafael J. Wysocki @ 2011-07-10 9:07 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107101059.30322.rjw@sisk.pl>
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;
^ permalink raw reply
* [PATCH 2/6 v2] PM / Domains: Make failing pm_genpd_prepare() clean up properly
From: Rafael J. Wysocki @ 2011-07-10 9:06 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107101059.30322.rjw@sisk.pl>
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;
}
/**
^ permalink raw reply
* [PATCH 1/6 v2] PM / Domains: Set device state to "active" during system resume
From: Rafael J. Wysocki @ 2011-07-10 9:05 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107101059.30322.rjw@sisk.pl>
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);
}
}
^ permalink raw reply
* [PATCH 0/6 v2] PM / Domains: Generic PM domains improvements
From: Rafael J. Wysocki @ 2011-07-10 8:59 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Greg KH, LKML, MyungJoo Ham
In-Reply-To: <201107062248.35586.rjw@sisk.pl>
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
^ permalink raw reply
* Re: [PATCH 00/05] ARM: mach-shmobile: another sh7372 power domain update
From: Rafael J. Wysocki @ 2011-07-10 8:46 UTC (permalink / raw)
To: Magnus Damm; +Cc: linux-pm, linux-sh
In-Reply-To: <20110707133212.22347.68775.sendpatchset@t400s>
On Thursday, July 07, 2011, Magnus Damm wrote:
> ARM: mach-shmobile: another sh7372 power domain update
>
> [PATCH 01/05] ARM: mach-shmobile: sh7372 D4 support
> [PATCH 02/05] ARM: mach-shmobile: Runtime PM late init callback
> [PATCH 03/05] ARM: mach-shmobile: sh7372 late pm domain off
> [PATCH 04/05] PM: export pm_genpd_poweron() in header
> [PATCH 05/05] ARM: mach-shmobile: sh7372 A3RV requires A4LC
>
> These patches update the partial hardware power domain support
> included in the pm-domains branch of the suspend-2.6 git tree.
All patches applied to suspend-2.6/pm-domains.
Thanks,
Rafael
^ permalink raw reply
* [PATCH v2] Input: enable i8042-level wakeup control
From: Daniel Drake @ 2011-07-09 21:13 UTC (permalink / raw)
To: dmitry.torokhov, dtor; +Cc: linux-pm, dilinger, linux-input
The OLPC XO laptop is able to use the PS/2 controller as a wakeup source.
When used as a wakeup source, the key press/mouse motion must not be lost.
Add infrastructure to allow controllers to be marked as wakeup-capable,
and avoid doing power control/reset on the i8042/serio/input devices when
running in this mode. Default behaviour is unchanged - hardware platforms
must explicitly enable this functionality if they can support it.
Signed-off-by: Daniel Drake <dsd@laptop.org>
---
drivers/input/input.c | 18 +++++++++++
drivers/input/serio/i8042-io.h | 4 ++
drivers/input/serio/i8042-ip22io.h | 4 ++
drivers/input/serio/i8042-jazzio.h | 4 ++
drivers/input/serio/i8042-ppcio.h | 4 ++
drivers/input/serio/i8042-snirm.h | 4 ++
drivers/input/serio/i8042-sparcio.h | 4 ++
drivers/input/serio/i8042-x86ia64io.h | 4 ++
drivers/input/serio/i8042.c | 54 ++++++++++++++++++++++++++++++--
drivers/input/serio/serio.c | 28 ++++++++++++++--
10 files changed, 120 insertions(+), 8 deletions(-)
A followup patch will come soon, hooking this into OLPC's embedded controller:
http://dev.laptop.org/~dsd/20110114/0015-i8042-Enable-OLPC-s-EC-based-i8042-wakeup-control.patch
The underlying infrastructure for this work has now been merged in linux-next.
On last submission, Dmitry was worried about this functionality not working
at all on other platforms. I agree, it will only work where the hardware
has been specifically designed with this consideration. v2 of the patch
therefore removes the module param option, meaning that it will only be
activated on platforms that explicitly enable it at the code level.
v2 also performs a more extensive job. We avoid resetting the device
at the input_device level during suspend/resume - but this is ugly. Rafael,
is there a better way? Please see the input.c hunks. We also disable
i8042 interrupts when going into suspend, to avoid races handling interrupts
in the wrong order during resume.
diff --git a/drivers/input/input.c b/drivers/input/input.c
index da38d97..81a87bc 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1588,6 +1588,15 @@ static int input_dev_suspend(struct device *dev)
{
struct input_dev *input_dev = to_input_dev(dev);
+ /*
+ * If this device is a child of serio, which is a child of i8042, and
+ * i8042 wakeup is enabled (i.e. this input device is not being
+ * suspended), do nothing.
+ */
+ if (dev->parent && dev->parent->parent
+ && device_may_wakeup(dev->parent->parent))
+ return 0;
+
mutex_lock(&input_dev->mutex);
if (input_dev->users)
@@ -1602,6 +1611,15 @@ static int input_dev_resume(struct device *dev)
{
struct input_dev *input_dev = to_input_dev(dev);
+ /*
+ * If this device is a child of serio, which is a child of i8042, and
+ * i8042 wakeup is enabled (i.e. this input device was not suspended),
+ * do nothing.
+ */
+ if (dev->parent && dev->parent->parent
+ && device_may_wakeup(dev->parent->parent))
+ return 0;
+
input_reset_device(input_dev);
return 0;
diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
index 5d48bb6..296633c 100644
--- a/drivers/input/serio/i8042-io.h
+++ b/drivers/input/serio/i8042-io.h
@@ -92,4 +92,8 @@ static inline void i8042_platform_exit(void)
#endif
}
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
#endif /* _I8042_IO_H */
diff --git a/drivers/input/serio/i8042-ip22io.h b/drivers/input/serio/i8042-ip22io.h
index ee1ad27..c5b76a4 100644
--- a/drivers/input/serio/i8042-ip22io.h
+++ b/drivers/input/serio/i8042-ip22io.h
@@ -73,4 +73,8 @@ static inline void i8042_platform_exit(void)
#endif
}
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
#endif /* _I8042_IP22_H */
diff --git a/drivers/input/serio/i8042-jazzio.h b/drivers/input/serio/i8042-jazzio.h
index 13fd710..a11913a 100644
--- a/drivers/input/serio/i8042-jazzio.h
+++ b/drivers/input/serio/i8042-jazzio.h
@@ -66,4 +66,8 @@ static inline void i8042_platform_exit(void)
#endif
}
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
#endif /* _I8042_JAZZ_H */
diff --git a/drivers/input/serio/i8042-ppcio.h b/drivers/input/serio/i8042-ppcio.h
index f708c75..c9f4292 100644
--- a/drivers/input/serio/i8042-ppcio.h
+++ b/drivers/input/serio/i8042-ppcio.h
@@ -52,6 +52,10 @@ static inline void i8042_platform_exit(void)
{
}
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
#else
#include "i8042-io.h"
diff --git a/drivers/input/serio/i8042-snirm.h b/drivers/input/serio/i8042-snirm.h
index 409a934..96d034f 100644
--- a/drivers/input/serio/i8042-snirm.h
+++ b/drivers/input/serio/i8042-snirm.h
@@ -72,4 +72,8 @@ static inline void i8042_platform_exit(void)
}
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
#endif /* _I8042_SNIRM_H */
diff --git a/drivers/input/serio/i8042-sparcio.h b/drivers/input/serio/i8042-sparcio.h
index 395a9af..e5381d3 100644
--- a/drivers/input/serio/i8042-sparcio.h
+++ b/drivers/input/serio/i8042-sparcio.h
@@ -154,4 +154,8 @@ static inline void i8042_platform_exit(void)
}
#endif /* !CONFIG_PCI */
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
#endif /* _I8042_SPARCIO_H */
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index bb9f5d3..76b2e58 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -875,6 +875,10 @@ static inline int i8042_pnp_init(void) { return 0; }
static inline void i8042_pnp_exit(void) { }
#endif
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
static int __init i8042_platform_init(void)
{
int retval;
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index d37a48e..e944667 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -87,6 +87,7 @@ MODULE_PARM_DESC(debug, "Turn i8042 debugging mode on and off");
#endif
static bool i8042_bypass_aux_irq_test;
+static bool i8042_enable_wakeup;
#include "i8042.h"
@@ -1082,10 +1083,17 @@ static void i8042_dritek_enable(void)
* before suspending.
*/
-static int i8042_controller_resume(bool force_reset)
+static int i8042_controller_resume(bool force_reset, bool soft_resume)
{
int error;
+ /*
+ * If device is selected as a wakeup source, it was not powered down
+ * or reset during suspend, so we have very little to do.
+ */
+ if (soft_resume)
+ goto soft;
+
error = i8042_controller_check();
if (error)
return error;
@@ -1129,6 +1137,7 @@ static int i8042_controller_resume(bool force_reset)
if (i8042_ports[I8042_KBD_PORT_NO].serio)
i8042_enable_kbd_port();
+soft:
i8042_interrupt(0, NULL);
return 0;
@@ -1146,14 +1155,48 @@ static int i8042_pm_reset(struct device *dev)
return 0;
}
+static int i8042_pm_suspend(struct device *dev)
+{
+ i8042_platform_suspend(dev, device_may_wakeup(dev));
+
+ /*
+ * If device is selected as a wakeup source, don't powerdown or reset
+ * during suspend. Just disable IRQs to ensure race-free resume.
+ */
+ if (device_may_wakeup(dev)) {
+ if (i8042_kbd_irq_registered)
+ disable_irq(I8042_KBD_IRQ);
+ if (i8042_aux_irq_registered)
+ disable_irq(I8042_AUX_IRQ);
+ return 0;
+ }
+
+ return i8042_pm_reset(dev);
+}
+
static int i8042_pm_resume(struct device *dev)
{
/*
* On resume from S2R we always try to reset the controller
* to bring it in a sane state. (In case of S2D we expect
* BIOS to reset the controller for us.)
+ * This function call will also handle any pending keypress event
+ * (perhaps the system wakeup reason)
+ */
+ int r = i8042_controller_resume(true, device_may_wakeup(dev));
+
+ /* If the device was left running during suspend, enable IRQs again
+ * now. Must be done last to avoid races with interrupt processing
+ * inside i8042_controller_resume.
*/
- return i8042_controller_resume(true);
+ if (device_may_wakeup(dev)) {
+ if (i8042_kbd_irq_registered)
+ enable_irq(I8042_KBD_IRQ);
+ if (i8042_aux_irq_registered)
+ enable_irq(I8042_AUX_IRQ);
+ }
+
+ return r;
}
static int i8042_pm_thaw(struct device *dev)
@@ -1165,11 +1208,11 @@ static int i8042_pm_thaw(struct device *dev)
static int i8042_pm_restore(struct device *dev)
{
- return i8042_controller_resume(false);
+ return i8042_controller_resume(false, false);
}
static const struct dev_pm_ops i8042_pm_ops = {
- .suspend = i8042_pm_reset,
+ .suspend = i8042_pm_suspend,
.resume = i8042_pm_resume,
.thaw = i8042_pm_thaw,
.poweroff = i8042_pm_reset,
@@ -1403,6 +1446,9 @@ static int __init i8042_probe(struct platform_device *dev)
i8042_dritek_enable();
#endif
+ if (i8042_enable_wakeup)
+ device_set_wakeup_capable(&dev->dev, true);
+
if (!i8042_noaux) {
error = i8042_setup_aux();
if (error && error != -ENODEV && error != -EBUSY)
diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index ba70058..3d5793a 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -931,7 +931,7 @@ static int serio_uevent(struct device *dev, struct kobj_uevent_env *env)
#endif /* CONFIG_HOTPLUG */
#ifdef CONFIG_PM
-static int serio_suspend(struct device *dev)
+static int serio_poweroff(struct device *dev)
{
struct serio *serio = to_serio_port(dev);
@@ -940,7 +940,17 @@ static int serio_suspend(struct device *dev)
return 0;
}
-static int serio_resume(struct device *dev)
+static int serio_suspend(struct device *dev)
+{
+ /* If parent controller is configured as a wakeup source, don't
+ * power off. */
+ if (device_may_wakeup(dev->parent))
+ return 0;
+
+ return serio_poweroff(dev);
+}
+
+static int serio_restore(struct device *dev)
{
struct serio *serio = to_serio_port(dev);
@@ -953,11 +963,21 @@ static int serio_resume(struct device *dev)
return 0;
}
+static int serio_resume(struct device *dev)
+{
+ /* If parent controller is configured as a wakeup source, we didn't
+ * power off during suspend, and hence have nothing to do. */
+ if (device_may_wakeup(dev->parent))
+ return 0;
+
+ return serio_restore(dev);
+}
+
static const struct dev_pm_ops serio_pm_ops = {
.suspend = serio_suspend,
.resume = serio_resume,
- .poweroff = serio_suspend,
- .restore = serio_resume,
+ .poweroff = serio_poweroff,
+ .restore = serio_restore,
};
#endif /* CONFIG_PM */
--
1.7.5.4
^ permalink raw reply related
* Re: PM domain using _noirq methods to "finish" pending runtime PM transistions
From: Rafael J. Wysocki @ 2011-07-09 21:02 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-pm, linux-omap
In-Reply-To: <87iprc2t34.fsf@ti.com>
Hi,
On Saturday, July 09, 2011, Kevin Hilman wrote:
> Hi Rafael,
>
> So I'm now experimenting with your suggestion of using the noirq
> callbacks of the PM domain to ensure device low-power state transitions
> for the cases where runtime PM has been disabled from userspace (or a
> driver has used runtime PM calls in it's suspend/resume path but they
> have no effect since runtime PM is disabled.)
>
> Before I get too far, I want to be sure I understood your suggestion
> correctly, and run my current implemtation past you.
>
> For starters, I just set the driver's noirq callbacks to point to the
> runtime PM callbacks.
>
> Then, in the PM domain's suspend_noirq, I basically do
>
> ret = pm_generic_suspend_noirq(dev);
> if (!ret && !(dev->power.runtime_status == RPM_SUSPENDED))
> magic_device_power_down(dev); /* shared with pm_domain runtime PM methods */
> priv->flags |= MAGIC_DEVICE_SUSPENDED;
> }
> return ret;
>
> and in the PM domain's resume_norq, I basically do:
>
> if ((priv->flags & MAGIC_DEVICE_SUSPENDED) &&
> !(dev->power.runtime_status == RPM_SUSPENDED)) {
> priv->flags &= ~OMAP_DEVICE_SUSPENDED;
> magic_device_power_up(dev);
> }
> return pm_generic_resume_noirq(dev);
>
> Note: The MAGIC_DEVICE_SUSPENDED flag is used so resume_noirq only
> reenables devices that were disabled by suspend_noirq so that devices
> which were disabled by runtime PM are left untouched (similar to how
> you've implmented generic PM domains.)
>
> Since the driver's noirq callbacks point to the runtime PM callbacks
> this all works quite well.
>
> I believe this was basically the suggestion you were recommending, right?
That's correct.
> However, what I need is for the driver's runtime PM callbacks not to be
> called unconditionally, but only if the PM domain's noirq methods are
> actually going to disable/power-down the device (e.g. it's not already
> runtime suspended.)
>
> The first obvious solution is to create new driver noirq methods that
> check if the device is not already RPM_SUSPENDED and only then call the
> runtime PM callbacks. That works, but hey, it's a Friday night so I
> decided to take it to the next level...
>
> Instead of making all the drivers add new noirq methods that just
> conditionally call the runtime PM methods, what if I just handle it in
> the PM domain? IOW, what if I just call pm_generic_runtime_* from the
> PM domain's noirq methods? e.g. call pm_generic_runtime_suspend() just
> before magic_device_power_down() and call pm_generic_runtime_resume()
> just after magic_device_power_up()?
That should be fine.
> I tested this today with a handful of our drivers that do all their PM
> in terms of the runtime PM API (including get_sync/put_sync in their
> suspend path) and they all work as expected without modification.
>
> I know it's not much to add a couple new callbacks to each of these
> drivers, but if I can handle it at the PM domain level, I'd rather allow
> the drivers to stay simple.
>
> What do you think?
I don't see anything wrong with your approach. :-)
Thanks,
Rafael
PS
I wonder what you think about this patch:
https://patchwork.kernel.org/patch/959672/
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Rafael J. Wysocki @ 2011-07-09 14:15 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <201107082124.24921.rjw@sisk.pl>
On Friday, July 08, 2011, Rafael J. Wysocki wrote:
...
>
> Well, in fact, since we already have .active_wakeup(), what about the
> following patch (on top of https://lkml.org/lkml/2011/7/8/223):
Well, it is wrong, because we've been discussing devices that are _not_
enabled to wake up the system.
> ---
> drivers/base/power/domain.c | 4 ++++
> 1 file changed, 4 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
> @@ -519,6 +519,10 @@ static int pm_genpd_prepare(struct devic
> return -EBUSY;
> }
>
> + if (device_may_wakeup(dev)
So, this should have been !device_may_wakeup(dev), _but_ ...
> + && !(genpd->active_wakeup && genpd->active_wakeup(dev)))
> + pm_runtime_resume(dev);
> +
> genpd_acquire_lock(genpd);
>
> if (genpd->prepared_count++ == 0)
There's one more case to consider, namely devices that are runtime
suspended, set up to wake up the system from sleep states (ie. device_may_wakeup(dev)
returns "true") and such that genpd->active_wakeup(dev) returns "true" for them,
because they need to be resumed at this point too (arguably, it makes a little
sense to runtime suspend such devices, but that's possible in principle).
So, IMO, the patch should look like this:
---
drivers/base/power/domain.c | 19 +++++++++++++++++++
1 file changed, 19 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
@@ -486,6 +486,22 @@ static void pm_genpd_sync_poweroff(struc
}
/**
+ * resume_needed - Check whether to resume a device before system suspend.
+ * @dev: Device to handle.
+ * @genpd: PM domain the device belongs to.
+ */
+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.
*
@@ -519,6 +535,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)
^ permalink raw reply
* Re: pm_runtime_suspended() can be false if RPM_SUSPENDED
From: Rafael J. Wysocki @ 2011-07-09 10:19 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-pm, Linux OMAP
In-Reply-To: <871uy0733n.fsf@ti.com>
Hi,
On Saturday, July 09, 2011, Kevin Hilman wrote:
> Hi Rafael,
>
> Just curious why pm_runtime_suspended() requires the device to be
> enabled for it to return true:
>
> static inline bool pm_runtime_suspended(struct device *dev)
> {
> return dev->power.runtime_status == RPM_SUSPENDED
> && !dev->power.disable_depth;
> }
>
> I must be misunderstanding something, but I would consider a device that
> has been runtime suspended before runtime PM was disabled to still be
> runtime suspended.
The problem is that while the runtime PM of the device is disabled
(ie. dev->power.disable_depth > 0), its status may be switched from
RPM_SUSPENDED to RPM_ACTIVE on the fly, using pm_runtime_set_active()
(and the other way around) and it doesn't reflect the real status in
principle. So it was a tough choice what to do about that and I decided
to go with returning false (in many cases runtime PM disabled means the
device is always operational).
> I just noticed this when testing with your pm-domains branch. when I
> noticed that an 'if (pm_runtime_suspended(dev))' check in my PM domain's
> ->suspend_noirq() was always failing since it's after the PM core calls
> pm_runtime_disable(). I had to change my PM domain code to only check
> dev->power.runtime_status for it to work.
Well, at this point I'm kind of cautious about changing pm_runtime_suspended(),
so perhaps we need a separate callback returnig true in the status is
RPM_SUSPENDED regardless of the value of power.disable_depth, like
pm_runtime_status_suspended() or something similar.
Thanks,
Rafael
^ permalink raw reply
* Re: Runtime PM discussion notes
From: Mark Brown @ 2011-07-09 4:06 UTC (permalink / raw)
To: Paul Walmsley; +Cc: linux-pm, linux-omap
In-Reply-To: <alpine.DEB.2.00.1106281346160.2874@utopia.booyaka.com>
On Tue, Jun 28, 2011 at 01:47:47PM -0600, Paul Walmsley wrote:
> On Fri, 24 Jun 2011, Arve Hj?nnev?g wrote:
> > On Fri, Jun 24, 2011 at 12:53 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > > "On the hardware that shipped we enter the same power state from idle
> > > and suspend, so the only power savings we get from suspend that we
> > > don't get in idle is from not respecting the scheduler and timers."
> > This is no longer the case. Both the Nexus-S and Xoom enter lower
> > power states from suspend than idle.
> Just out of curiosity, is that due to some kind of hardware limitation on
> those platforms, or is it because the software infrastructure for dynamic
> deep idle hasn't been fully implemented in that subarchitecture code?
At least the Nexus S doesn't implmeent any of the deep idle
infrastructure. However, I'd expect that you can achieve some power
saving from entering system suspend as if *everything* is off then the
PMIC can be suspended which can enable additional power savings. Unless
I'm missing something that'd be hard to hit with cpuidle only stuff.
^ permalink raw reply
* PM domain using _noirq methods to "finish" pending runtime PM transistions
From: Kevin Hilman @ 2011-07-09 0:30 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm, linux-omap@vger.kernel.org
Hi Rafael,
So I'm now experimenting with your suggestion of using the noirq
callbacks of the PM domain to ensure device low-power state transitions
for the cases where runtime PM has been disabled from userspace (or a
driver has used runtime PM calls in it's suspend/resume path but they
have no effect since runtime PM is disabled.)
Before I get too far, I want to be sure I understood your suggestion
correctly, and run my current implemtation past you.
For starters, I just set the driver's noirq callbacks to point to the
runtime PM callbacks.
Then, in the PM domain's suspend_noirq, I basically do
ret = pm_generic_suspend_noirq(dev);
if (!ret && !(dev->power.runtime_status == RPM_SUSPENDED))
magic_device_power_down(dev); /* shared with pm_domain runtime PM methods */
priv->flags |= MAGIC_DEVICE_SUSPENDED;
}
return ret;
and in the PM domain's resume_norq, I basically do:
if ((priv->flags & MAGIC_DEVICE_SUSPENDED) &&
!(dev->power.runtime_status == RPM_SUSPENDED)) {
priv->flags &= ~OMAP_DEVICE_SUSPENDED;
magic_device_power_up(dev);
}
return pm_generic_resume_noirq(dev);
Note: The MAGIC_DEVICE_SUSPENDED flag is used so resume_noirq only
reenables devices that were disabled by suspend_noirq so that devices
which were disabled by runtime PM are left untouched (similar to how
you've implmented generic PM domains.)
Since the driver's noirq callbacks point to the runtime PM callbacks
this all works quite well.
I believe this was basically the suggestion you were recommending, right?
However, what I need is for the driver's runtime PM callbacks not to be
called unconditionally, but only if the PM domain's noirq methods are
actually going to disable/power-down the device (e.g. it's not already
runtime suspended.)
The first obvious solution is to create new driver noirq methods that
check if the device is not already RPM_SUSPENDED and only then call the
runtime PM callbacks. That works, but hey, it's a Friday night so I
decided to take it to the next level...
Instead of making all the drivers add new noirq methods that just
conditionally call the runtime PM methods, what if I just handle it in
the PM domain? IOW, what if I just call pm_generic_runtime_* from the
PM domain's noirq methods? e.g. call pm_generic_runtime_suspend() just
before magic_device_power_down() and call pm_generic_runtime_resume()
just after magic_device_power_up()?
I tested this today with a handful of our drivers that do all their PM
in terms of the runtime PM API (including get_sync/put_sync in their
suspend path) and they all work as expected without modification.
I know it's not much to add a couple new callbacks to each of these
drivers, but if I can handle it at the PM domain level, I'd rather allow
the drivers to stay simple.
What do you think?
Kevin
^ permalink raw reply
* pm_runtime_suspended() can be false if RPM_SUSPENDED
From: Kevin Hilman @ 2011-07-08 23:41 UTC (permalink / raw)
To: Rafael J. Wysocki, linux-pm; +Cc: linux-omap@vger.kernel.org
Hi Rafael,
Just curious why pm_runtime_suspended() requires the device to be
enabled for it to return true:
static inline bool pm_runtime_suspended(struct device *dev)
{
return dev->power.runtime_status == RPM_SUSPENDED
&& !dev->power.disable_depth;
}
I must be misunderstanding something, but I would consider a device that
has been runtime suspended before runtime PM was disabled to still be
runtime suspended.
I just noticed this when testing with your pm-domains branch. when I
noticed that an 'if (pm_runtime_suspended(dev))' check in my PM domain's
->suspend_noirq() was always failing since it's after the PM core calls
pm_runtime_disable(). I had to change my PM domain code to only check
dev->power.runtime_status for it to work.
Kevin
^ permalink raw reply
* Old messages
From: Rafael J. Wysocki @ 2011-07-08 22:30 UTC (permalink / raw)
To: Linux PM mailing list; +Cc: Eric Searcy
Dear Listmembers,
Somebody has been sending old linux-pm messages back to linux-pm recently,
which causes quite some confusion. We've been able to track the offender down
to the IP address that the messages are sent from, which is:
203.33-179-91.adsl-dyn.isp.belgacom.be
Please check if you're using this IP and if so, please fix your system.
Thanks,
Rafael
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Rafael J. Wysocki @ 2011-07-08 19:24 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <201107082006.49660.rjw@sisk.pl>
On Friday, July 08, 2011, Rafael J. Wysocki wrote:
> On Friday, July 08, 2011, Kevin Hilman wrote:
> > Alan Stern <stern@rowland.harvard.edu> writes:
> >
> > > On Fri, 8 Jul 2011, Rafael J. Wysocki wrote:
> > >
> > >> On Friday, July 08, 2011, Kevin Hilman wrote:
> > >> > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> > >> >
> > >> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > >> > >
> > >> > > Make generic PM domains support system-wide power transitions
> > >> > > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> > >> > > poweroff and restore callbacks to be associated with struct
> > >> > > generic_pm_domain objects and make pm_genpd_init() use them as
> > >> > > appropriate.
> > >> > >
> > >> > > The new callbacks do nothing for devices belonging to power domains
> > >> > > that were powered down at run time (before the transition).
> > >> >
> > >> > Thinking about this some more, how is a driver supposed to reconfigure
> > >> > wakeups during suspend if it has already been runtime suspended?
> > >>
> > >> If the device belongs to a PM domain that has been powered off, it
> > >> won't be notified.
> > >>
> > >> > For example, assume a device where device_may_wakeup() == false. This
> > >> > means wakeups during *suspend* are disabled, but wakeups wakeups are
> > >> > assumed to enabled when it is runtime suspended.
> > >> >
> > >> > So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
> > >> > and then system suspend comes along.
> > >> >
> > >> > With this current patch, the driver will never receive any callbacks, so
> > >> > it can never disable its wakeups.
> > >> >
> > >> > Am I missing something?
> > >>
> > >> As I said above, this only happens with devices that belog to PM domains
> > >> that were powered off before system suspend has started, so the problem
> > >> is limited to devices that wakeup is signaled on behalf of even when they
> > >> have no power.
> >
> > Which on OMAP, is *all* devices, so that's a pretty major limitation. :)
>
> So we'll need to avoid it _for_ _OMAP_. Any of the platforms that will use
> this code in near future doesn't have this limitation, AFAICS, so I don't
> really see a point coding for it right now.
>
> Your point seems to be that the whole thing has to be ready for OMAP
> in advance and my point is that we can make it ready for OMAP later.
> Which I believe is more pragmatic, because if every platform had adopted
> the other viewpoint, we wouldn't have made any progress at all.
>
> > >> So this is a limitation, but not affecting all platforms.
> > >>
> > >> There are a few ways to avoid this limitation I can think of:
> > >> (1) Add a "make me operational during system suspend" flag to struct dev_pm_info
> > >> and run pm_runtime_resume() on such devices from the core (either dpm_prepare()
> > >> core, or pm_genpd_prepare()).
> > >
> > > What's to prevent the device from being runtime-suspended again before
> > > the wakeup setting can be changed?
> > >
> > >> (2) Add a "my .prepare() is safe to run if device is not accessible" flag to
> > >> struct dev_pm_info and make pm_genpd_prepare() execute .prepare() for such
> > >> devices regardless of whether or not their PM domains are off.
> > >> (3) Call .prepare() from all drivers unconditionally during system suspend
> > >> (and probably .complete() too) in the hope they won't access inaccessible
> > >> devices.
> >
> > Like Alan's comment above for (1), I think the same applies for (3)
> > since runtime PM transitions can still happen between .prepare() and
> > .suspend()
>
> Not with the current PM domains code (and please see my reply to Alan).
Well, in fact, since we already have .active_wakeup(), what about the
following patch (on top of https://lkml.org/lkml/2011/7/8/223):
---
drivers/base/power/domain.c | 4 ++++
1 file changed, 4 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
@@ -519,6 +519,10 @@ static int pm_genpd_prepare(struct devic
return -EBUSY;
}
+ if (device_may_wakeup(dev)
+ && !(genpd->active_wakeup && genpd->active_wakeup(dev)))
+ pm_runtime_resume(dev);
+
genpd_acquire_lock(genpd);
if (genpd->prepared_count++ == 0)
^ permalink raw reply
* Re: [PATCH 1/7] pm: improve error code of pm_notifier_call_chain()
From: Rafael J. Wysocki @ 2011-07-08 18:59 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-s390, Jiri Kosina, Heiko Carstens, linux-kernel,
Martin Schwidefsky, linux390, akpm, linux-pm
In-Reply-To: <CAC5umyhZ35AXtwA=Uy1+zp9N=NwLths=h6jKUK53f0U+tZMZZA@mail.gmail.com>
On Friday, July 08, 2011, Akinobu Mita wrote:
> 2011/7/8 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Monday, July 04, 2011, Akinobu Mita wrote:
> >> 2011/7/4 Pavel Machek <pavel@ucw.cz>:
> >> > On Sun 2011-07-03 23:16:15, Akinobu Mita wrote:
> >> >> This enables pm_notifier_call_chain() to get the actual error code
> >> >> in the callback rather than always assume -EINVAL by converting all
> >> >> PM notifier calls to return encapsulate error code with
> >> >> notifier_from_errno().
> >> >>
> >> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> >> Cc: Pavel Machek <pavel@ucw.cz>
> >> >
> >> > Nothing obviously wrong here.
> >>
> >> Thanks. Can I add your ACK for this patch?
> >
> > Do you want me to take this patch?
>
> Yes, please merge it to your tree.
Applied.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH] runtime-pm: consistent utilization of deferred_resume
From: Rafael J. Wysocki @ 2011-07-08 18:59 UTC (permalink / raw)
To: Liu, ShuoX
Cc: Brown, Len, linux-pm@lists.linux-foundation.org, gregkh@suse.de,
linux-kernel@vger.kernel.org
In-Reply-To: <6E3BC7F7C9A4BF4286DD4C043110F30B59E040012A@shsmsx502.ccr.corp.intel.com>
On Friday, July 08, 2011, Liu, ShuoX wrote:
> From 10bb3c851a0a9c9ca4d552743a371e3cfda81075 Mon Sep 17 00:00:00 2001
> From: ShuoX Liu <shuox.liu@intel.com>
> Date: Thu, 7 Jul 2011 15:59:06 +0800
> Subject: [PATCH] runtime-pm: consistent utilization of deferred_resume
>
> dev->power.deferred_resume is used as a bool typically, so change
> one assignment to false from 0, like other places.
>
> Signed-off-by: ShuoX Liu <shuox.liu@intel.com>
Applied to suspend-2.6/linux-next, will push for 3.1.
Thanks,
Rafael
> ---
> drivers/base/power/runtime.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 0d4587b..af3d1a5 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -388,7 +388,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> retval = rpm_callback(callback, dev);
> if (retval) {
> __update_runtime_status(dev, RPM_ACTIVE);
> - dev->power.deferred_resume = 0;
> + dev->power.deferred_resume = false;
> if (retval == -EAGAIN || retval == -EBUSY)
> dev->power.runtime_error = 0;
> else
>
^ permalink raw reply
* Re: [PATCH 6/7] ARM / Samsung: Use struct syscore_ops for "core" power management
From: Rafael J. Wysocki @ 2011-07-08 18:12 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Russell King, Greg KH, LKML, Ben Dooks, Kay Sievers,
Linux PM mailing list, linux-omap
In-Reply-To: <20110327234521.GH9995@trinity.fluff.org>
On Monday, March 28, 2011, Ben Dooks wrote:
> On Mon, Mar 28, 2011 at 01:29:49AM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Replace sysdev classes and struct sys_device objects used for "core"
> > power management by Samsung platforms with struct syscore_ops objects
> > that are simpler.
> >
> > This generally reduces the code size and the kernel memory footprint.
> > It also is necessary for removing sysdevs entirely from the kernel in
> > the future.
>
> Hmm, does it still allow the system to choose which bits are bound
> depending on the cpu being registered, as for the s3c stuff it isn't
> just about the suspend/resume, it's binding items that get registered
> early in the startup sequence?
Yes, it does, AFAICS. It didn't change the bits that weren't directly
related to suspend/resume (at least that wasn't the intention).
That said, using sysdevs for the initialization of things the way you describe
will have to change anyway, because sysdevs are going to be removed entirely
from the kernel at one point.
Thanks,
Rafael
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Rafael J. Wysocki @ 2011-07-08 18:06 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <87y6088z9n.fsf@ti.com>
On Friday, July 08, 2011, Kevin Hilman wrote:
> Alan Stern <stern@rowland.harvard.edu> writes:
>
> > On Fri, 8 Jul 2011, Rafael J. Wysocki wrote:
> >
> >> On Friday, July 08, 2011, Kevin Hilman wrote:
> >> > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> >> >
> >> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> >> > >
> >> > > Make generic PM domains support system-wide power transitions
> >> > > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> >> > > poweroff and restore callbacks to be associated with struct
> >> > > generic_pm_domain objects and make pm_genpd_init() use them as
> >> > > appropriate.
> >> > >
> >> > > The new callbacks do nothing for devices belonging to power domains
> >> > > that were powered down at run time (before the transition).
> >> >
> >> > Thinking about this some more, how is a driver supposed to reconfigure
> >> > wakeups during suspend if it has already been runtime suspended?
> >>
> >> If the device belongs to a PM domain that has been powered off, it
> >> won't be notified.
> >>
> >> > For example, assume a device where device_may_wakeup() == false. This
> >> > means wakeups during *suspend* are disabled, but wakeups wakeups are
> >> > assumed to enabled when it is runtime suspended.
> >> >
> >> > So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
> >> > and then system suspend comes along.
> >> >
> >> > With this current patch, the driver will never receive any callbacks, so
> >> > it can never disable its wakeups.
> >> >
> >> > Am I missing something?
> >>
> >> As I said above, this only happens with devices that belog to PM domains
> >> that were powered off before system suspend has started, so the problem
> >> is limited to devices that wakeup is signaled on behalf of even when they
> >> have no power.
>
> Which on OMAP, is *all* devices, so that's a pretty major limitation. :)
So we'll need to avoid it _for_ _OMAP_. Any of the platforms that will use
this code in near future doesn't have this limitation, AFAICS, so I don't
really see a point coding for it right now.
Your point seems to be that the whole thing has to be ready for OMAP
in advance and my point is that we can make it ready for OMAP later.
Which I believe is more pragmatic, because if every platform had adopted
the other viewpoint, we wouldn't have made any progress at all.
> >> So this is a limitation, but not affecting all platforms.
> >>
> >> There are a few ways to avoid this limitation I can think of:
> >> (1) Add a "make me operational during system suspend" flag to struct dev_pm_info
> >> and run pm_runtime_resume() on such devices from the core (either dpm_prepare()
> >> core, or pm_genpd_prepare()).
> >
> > What's to prevent the device from being runtime-suspended again before
> > the wakeup setting can be changed?
> >
> >> (2) Add a "my .prepare() is safe to run if device is not accessible" flag to
> >> struct dev_pm_info and make pm_genpd_prepare() execute .prepare() for such
> >> devices regardless of whether or not their PM domains are off.
> >> (3) Call .prepare() from all drivers unconditionally during system suspend
> >> (and probably .complete() too) in the hope they won't access inaccessible
> >> devices.
>
> Like Alan's comment above for (1), I think the same applies for (3)
> since runtime PM transitions can still happen between .prepare() and
> .suspend()
Not with the current PM domains code (and please see my reply to Alan).
Thanks,
Rafael
^ permalink raw reply
* Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5)
From: Rafael J. Wysocki @ 2011-07-08 17:56 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-sh, Greg Kroah-Hartman, LKML, Linux PM mailing list
In-Reply-To: <Pine.LNX.4.44L0.1107081033000.2208-100000@iolanthe.rowland.org>
On Friday, July 08, 2011, Alan Stern wrote:
> On Fri, 8 Jul 2011, Rafael J. Wysocki wrote:
>
> > On Friday, July 08, 2011, Kevin Hilman wrote:
> > > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> > >
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > >
> > > > Make generic PM domains support system-wide power transitions
> > > > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> > > > poweroff and restore callbacks to be associated with struct
> > > > generic_pm_domain objects and make pm_genpd_init() use them as
> > > > appropriate.
> > > >
> > > > The new callbacks do nothing for devices belonging to power domains
> > > > that were powered down at run time (before the transition).
> > >
> > > Thinking about this some more, how is a driver supposed to reconfigure
> > > wakeups during suspend if it has already been runtime suspended?
> >
> > If the device belongs to a PM domain that has been powered off, it
> > won't be notified.
> >
> > > For example, assume a device where device_may_wakeup() == false. This
> > > means wakeups during *suspend* are disabled, but wakeups wakeups are
> > > assumed to enabled when it is runtime suspended.
> > >
> > > So now, assume this device is RPM_SUSPENDED, it has wakeups *enabled*,
> > > and then system suspend comes along.
> > >
> > > With this current patch, the driver will never receive any callbacks, so
> > > it can never disable its wakeups.
> > >
> > > Am I missing something?
> >
> > As I said above, this only happens with devices that belog to PM domains
> > that were powered off before system suspend has started, so the problem
> > is limited to devices that wakeup is signaled on behalf of even when they
> > have no power.
> >
> > So this is a limitation, but not affecting all platforms.
> >
> > There are a few ways to avoid this limitation I can think of:
> > (1) Add a "make me operational during system suspend" flag to struct dev_pm_info
> > and run pm_runtime_resume() on such devices from the core (either dpm_prepare()
> > core, or pm_genpd_prepare()).
>
> What's to prevent the device from being runtime-suspended again before
> the wakeup setting can be changed?
If we do it from pm_genpd_prepare() after this patch:
https://lkml.org/lkml/2011/7/8/223
the pm_runtime_get_noresume() (and the subsequent disabling of runtime PM)
will prevent this from happening.
> > (2) Add a "my .prepare() is safe to run if device is not accessible" flag to
> > struct dev_pm_info and make pm_genpd_prepare() execute .prepare() for such
> > devices regardless of whether or not their PM domains are off.
> > (3) Call .prepare() from all drivers unconditionally during system suspend
> > (and probably .complete() too) in the hope they won't access inaccessible
> > devices.
> > Probably, there's more.
>
> In the PM domain's suspend code, do a runtime resume if the wakeup
> setting needs to be changed, rather than simply skipping over the
> device.
.suspend() is too late, we'd need to do that in .prepare(). If we did it
in .suspend(), we'd need to run .prepare() for all drivers, because at
this point we wouldn't know if the driver's .suspend() was going to be
called. However, the driver's .prepare() may need to access the device,
which is going to fail if the whole domain is off at this point.
Also, we need a new flag that will tell the core code whether or not
the wakeup setting needs to be changed.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH 5/7] PM: PM notifier error injection
From: Rafael J. Wysocki @ 2011-07-08 17:43 UTC (permalink / raw)
To: Akinobu Mita; +Cc: akpm, linux-kernel, linux-pm
In-Reply-To: <CAC5umyifMvVTG29JyvBtc_YThorK=AZHZypheUvbtvmShnbTog@mail.gmail.com>
On Friday, July 08, 2011, Akinobu Mita wrote:
> >> --- a/kernel/power/main.c
> >> +++ b/kernel/power/main.c
> >> @@ -42,6 +42,36 @@ int pm_notifier_call_chain(unsigned long val)
> >> return notifier_to_errno(ret);
> >> }
> >>
> >> +#ifdef CONFIG_PM_NOTIFIER_ERROR_INJECTION
> >> +
> >> +static struct err_inject_notifier_block err_inject_pm_notifier = {
> >> + .actions = {
> >> + { ERR_INJECT_NOTIFIER_ACTION(PM_HIBERNATION_PREPARE) },
> >> + { ERR_INJECT_NOTIFIER_ACTION(PM_SUSPEND_PREPARE) },
> >> + { ERR_INJECT_NOTIFIER_ACTION(PM_RESTORE_PREPARE) },
> >> + {}
> >
> > Why have you omitted the PM_POST_* actions?
>
> Because the callbacks of PM_POST_* are not supposed to fail (i.e.
> the return value of pm_notifier_call() is ignored).
OK, fair enough.
> >> --- a/lib/Kconfig.debug
> >> +++ b/lib/Kconfig.debug
> >> @@ -1043,6 +1043,14 @@ config CPU_NOTIFIER_ERROR_INJECTION
> >> # echo 0 > /sys/devices/system/cpu/cpu1/online
> >> bash: echo: write error: Operation not permitted
> >>
> >> +config PM_NOTIFIER_ERROR_INJECTION
> >> + bool "PM notifier error injection"
> >> + depends on PM_DEBUG && NOTIFIER_ERROR_INJECTION
> >> + help
> >> + This option provides the ability to inject artifical errors to
> >> + PM notifier chain callbacks. It is controlled through debugfs
> >> + interface under /sys/kernel/debug/pm-notifier-error-inject/
> >
> > I'm not sure the help is necessary. I think it should be selected
> > automatically if both PM_DEBUG and NOTIFIER_ERROR_INJECTION are set.
>
> OK, I'll try to tweak the option to be better.
Thanks!
Rafael
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox