From 5616ce7b780af6103d7724f119514a6eb7b05f66 Mon Sep 17 00:00:00 2001 From: Shawn Guo Date: Wed, 2 Nov 2022 22:21:01 +0800 Subject: PM: domains: Drop genpd status manipulation for hibernate restore The genpd status manipulation for hibernate restore has really never worked as intended. For example, if the genpd->status was GENPD_STATE_ON, the parent domain's `sd_count` must have been increased, so it needs to be adjusted too. So drop this status manipulation. Suggested-by: Ulf Hansson Signed-off-by: Shawn Guo Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 6471b559230e..97deae1d4e77 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1374,20 +1374,7 @@ static int genpd_restore_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - /* - * At this point suspended_count == 0 means we are being run for the - * first time for the given domain in the present cycle. - */ genpd_lock(genpd); - if (genpd->suspended_count++ == 0) { - /* - * The boot kernel might put the domain into arbitrary state, - * so make it appear as powered off to genpd_sync_power_on(), - * so that it tries to power it on in case it was really off. - */ - genpd->status = GENPD_STATE_OFF; - } - genpd_sync_power_on(genpd, true, 0); genpd_unlock(genpd); -- cgit v1.2.3 From 615db6d96c725406ae39e2822e1fbde81ecf1efd Mon Sep 17 00:00:00 2001 From: Shawn Guo Date: Wed, 2 Nov 2022 22:21:02 +0800 Subject: PM: domains: Pass generic PM noirq hooks to genpd_finish_suspend() While argument `poweroff` works fine for genpd_finish_suspend() to handle distinction between suspend and poweroff, it won't scale if we want to use it for freeze as well. Pass generic PM noirq hooks as arguments instead, so that the function can possibly cover freeze case too. Signed-off-by: Shawn Guo Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 97deae1d4e77..f18b8b1bc17a 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1189,12 +1189,15 @@ static int genpd_prepare(struct device *dev) * genpd_finish_suspend - Completion of suspend or hibernation of device in an * I/O pm domain. * @dev: Device to suspend. - * @poweroff: Specifies if this is a poweroff_noirq or suspend_noirq callback. + * @suspend_noirq: Generic suspend_noirq callback. + * @resume_noirq: Generic resume_noirq callback. * * Stop the device and remove power from the domain if all devices in it have * been stopped. */ -static int genpd_finish_suspend(struct device *dev, bool poweroff) +static int genpd_finish_suspend(struct device *dev, + int (*suspend_noirq)(struct device *dev), + int (*resume_noirq)(struct device *dev)) { struct generic_pm_domain *genpd; int ret = 0; @@ -1203,10 +1206,7 @@ static int genpd_finish_suspend(struct device *dev, bool poweroff) if (IS_ERR(genpd)) return -EINVAL; - if (poweroff) - ret = pm_generic_poweroff_noirq(dev); - else - ret = pm_generic_suspend_noirq(dev); + ret = suspend_noirq(dev); if (ret) return ret; @@ -1217,10 +1217,7 @@ static int genpd_finish_suspend(struct device *dev, bool poweroff) !pm_runtime_status_suspended(dev)) { ret = genpd_stop_dev(genpd, dev); if (ret) { - if (poweroff) - pm_generic_restore_noirq(dev); - else - pm_generic_resume_noirq(dev); + resume_noirq(dev); return ret; } } @@ -1244,7 +1241,9 @@ static int genpd_suspend_noirq(struct device *dev) { dev_dbg(dev, "%s()\n", __func__); - return genpd_finish_suspend(dev, false); + return genpd_finish_suspend(dev, + pm_generic_suspend_noirq, + pm_generic_resume_noirq); } /** @@ -1353,7 +1352,9 @@ static int genpd_poweroff_noirq(struct device *dev) { dev_dbg(dev, "%s()\n", __func__); - return genpd_finish_suspend(dev, true); + return genpd_finish_suspend(dev, + pm_generic_poweroff_noirq, + pm_generic_restore_noirq); } /** -- cgit v1.2.3 From d9cc34fe0aae7e9df236be046a853fc63e889026 Mon Sep 17 00:00:00 2001 From: Shawn Guo Date: Wed, 2 Nov 2022 22:21:03 +0800 Subject: PM: domains: Consolidate genpd_restore_noirq() and genpd_resume_noirq() Most of the logic between genpd_restore_noirq() and genpd_resume_noirq() are identical. The suspended_count decrement for restore should be the right thing to do anyway, considering there is an increment in genpd_finish_suspend() for hibernation. So consolidate these two functions into genpd_finish_resume(). Signed-off-by: Shawn Guo Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index f18b8b1bc17a..7cee9439fd21 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1247,12 +1247,14 @@ static int genpd_suspend_noirq(struct device *dev) } /** - * genpd_resume_noirq - Start of resume of device in an I/O PM domain. + * genpd_finish_resume - Completion of resume of device in an I/O PM domain. * @dev: Device to resume. + * @resume_noirq: Generic resume_noirq callback. * * Restore power to the device's PM domain, if necessary, and start the device. */ -static int genpd_resume_noirq(struct device *dev) +static int genpd_finish_resume(struct device *dev, + int (*resume_noirq)(struct device *dev)) { struct generic_pm_domain *genpd; int ret; @@ -1264,7 +1266,7 @@ static int genpd_resume_noirq(struct device *dev) return -EINVAL; if (device_wakeup_path(dev) && genpd_is_active_wakeup(genpd)) - return pm_generic_resume_noirq(dev); + return resume_noirq(dev); genpd_lock(genpd); genpd_sync_power_on(genpd, true, 0); @@ -1281,6 +1283,19 @@ static int genpd_resume_noirq(struct device *dev) return pm_generic_resume_noirq(dev); } +/** + * genpd_resume_noirq - Start of resume of device in an I/O PM domain. + * @dev: Device to resume. + * + * Restore power to the device's PM domain, if necessary, and start the device. + */ +static int genpd_resume_noirq(struct device *dev) +{ + dev_dbg(dev, "%s()\n", __func__); + + return genpd_finish_resume(dev, pm_generic_resume_noirq); +} + /** * genpd_freeze_noirq - Completion of freezing a device in an I/O PM domain. * @dev: Device to freeze. @@ -1366,27 +1381,9 @@ static int genpd_poweroff_noirq(struct device *dev) */ static int genpd_restore_noirq(struct device *dev) { - struct generic_pm_domain *genpd; - int ret = 0; - dev_dbg(dev, "%s()\n", __func__); - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - genpd_lock(genpd); - genpd_sync_power_on(genpd, true, 0); - genpd_unlock(genpd); - - if (genpd->dev_ops.stop && genpd->dev_ops.start && - !pm_runtime_status_suspended(dev)) { - ret = genpd_start_dev(genpd, dev); - if (ret) - return ret; - } - - return pm_generic_restore_noirq(dev); + return genpd_finish_resume(dev, pm_generic_restore_noirq); } /** -- cgit v1.2.3 From ebb486bed2c36c3c4053ee9d55ef66d11030694a Mon Sep 17 00:00:00 2001 From: Shawn Guo Date: Wed, 2 Nov 2022 22:21:04 +0800 Subject: PM: domains: Power off[on] domain in hibernate .freeze[thaw]_noirq hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On platforms which use SHUTDOWN as hibernation mode, the genpd noirq hooks will be called like below. genpd_freeze_noirq() genpd_restore_noirq() ↓ ↑ Create snapshot image Restore target kernel ↓ ↑ genpd_thaw_noirq() genpd_freeze_noirq() ↓ ↑ Write snapshot image Read snapshot image ↓ ↑ power_down() Kernel boot As of today suspend hooks genpd_suspend[resume]_noirq() manages domain on/off state, but hibernate hooks genpd_freeze[thaw]_noirq() doesn't. This results in a different behavior of domain power state between suspend and hibernate freeze, i.e. domain is powered off for the former while on for the later. It causes a problem on platforms like i.MX where the domain needs to be powered on/off by calling clock and regulator interface. When the platform restores from hibernation, the domain is off in hardware and genpd_restore_noirq() tries to power it on, but will never succeed because software state of domain (clock and regulator) is left on from the last hibernate freeze, so kernel thinks that clock and regulator are enabled while they are actually not turned on in hardware. The consequence would be that devices in the power domain will access registers without clock or power, and cause hardware lockup. Power off[on] domain in hibernate .freeze[thaw]_noirq hook for reasons: - Align the behavior between suspend and hibernate freeze. - Have power state of domains stay in sync between hardware and software for hibernate freeze, and thus fix the lockup issue seen on i.MX platform. Signed-off-by: Shawn Guo Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 7cee9439fd21..2f0787d5101b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1307,24 +1307,11 @@ static int genpd_resume_noirq(struct device *dev) */ static int genpd_freeze_noirq(struct device *dev) { - const struct generic_pm_domain *genpd; - int ret = 0; - dev_dbg(dev, "%s()\n", __func__); - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - ret = pm_generic_freeze_noirq(dev); - if (ret) - return ret; - - if (genpd->dev_ops.stop && genpd->dev_ops.start && - !pm_runtime_status_suspended(dev)) - ret = genpd_stop_dev(genpd, dev); - - return ret; + return genpd_finish_suspend(dev, + pm_generic_freeze_noirq, + pm_generic_thaw_noirq); } /** @@ -1336,23 +1323,9 @@ static int genpd_freeze_noirq(struct device *dev) */ static int genpd_thaw_noirq(struct device *dev) { - const struct generic_pm_domain *genpd; - int ret = 0; - dev_dbg(dev, "%s()\n", __func__); - genpd = dev_to_genpd(dev); - if (IS_ERR(genpd)) - return -EINVAL; - - if (genpd->dev_ops.stop && genpd->dev_ops.start && - !pm_runtime_status_suspended(dev)) { - ret = genpd_start_dev(genpd, dev); - if (ret) - return ret; - } - - return pm_generic_thaw_noirq(dev); + return genpd_finish_resume(dev, pm_generic_thaw_noirq); } /** -- cgit v1.2.3 From ae8ac19655e0cd7ca5ac2f45837d386745aaed7f Mon Sep 17 00:00:00 2001 From: Abel Vesa Date: Tue, 15 Nov 2022 23:25:43 +0200 Subject: PM: domains: Reverse the order of performance and enabling ops The ->set_performance_state() needs to be called before ->power_on() when a genpd is powered on, and after ->power_off() when a genpd is powered off. Do this in order to let the provider know to which performance state to power on the genpd, on the power on sequence, and also to maintain the performance for that genpd until after powering off, on power off sequence. There is no scenario where a consumer would need its genpd enabled and then its performance state increased. Instead, in every scenario, the consumer needs the genpd to be enabled from the start at a specific performance state. And same logic applies to the powering down. No consumer would need its genpd performance state dropped right before powering down. Now, there are currently two vendors which use ->set_performance_state() in their genpd providers. One of them is Tegra, but the only genpd provider (PMC) that makes use of ->set_performance_state() doesn't implement the ->power_on() or ->power_off(), and so it will not be affected by the ops reversal. The other vendor that uses it is Qualcomm, in multiple genpd providers actually (RPM, RPMh and CPR). But all Qualcomm genpd providers that make use of ->set_performance_state() need the order between enabling ops and the performance setting op to be reversed. And the reason for that is that it currently translates into two different voltages in order to power on a genpd to a specific performance state. Basically, ->power_on() switches to the minimum (enabling) voltage for that genpd, and then ->set_performance_state() sets it to the voltage level required by the consumer. By reversing the call order, we rely on the provider to know what to do on each call, but most popular usecase is to cache the performance state and postpone the voltage setting until the ->power_on() gets called. As for the reason of still needing the ->power_on() and ->power_off() for a provider which could get away with just having ->set_performance_state() implemented, there are consumers that do not (nor should) provide an opp-table. For those consumers, ->set_performance_state() will not be called, and so they will enable the genpd to its minimum performance state by a ->power_on() call. Same logic goes for the disabling. Signed-off-by: Abel Vesa Reviewed-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 2f0787d5101b..b46aa490b4cd 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -939,8 +939,8 @@ static int genpd_runtime_suspend(struct device *dev) return 0; genpd_lock(genpd); - gpd_data->rpm_pstate = genpd_drop_performance_state(dev); genpd_power_off(genpd, true, 0); + gpd_data->rpm_pstate = genpd_drop_performance_state(dev); genpd_unlock(genpd); return 0; @@ -978,9 +978,8 @@ static int genpd_runtime_resume(struct device *dev) goto out; genpd_lock(genpd); + genpd_restore_performance_state(dev, gpd_data->rpm_pstate); ret = genpd_power_on(genpd, 0); - if (!ret) - genpd_restore_performance_state(dev, gpd_data->rpm_pstate); genpd_unlock(genpd); if (ret) @@ -1018,8 +1017,8 @@ err_stop: err_poweroff: if (!pm_runtime_is_irq_safe(dev) || genpd_is_irq_safe(genpd)) { genpd_lock(genpd); - gpd_data->rpm_pstate = genpd_drop_performance_state(dev); genpd_power_off(genpd, true, 0); + gpd_data->rpm_pstate = genpd_drop_performance_state(dev); genpd_unlock(genpd); } @@ -2707,17 +2706,6 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, dev->pm_domain->detach = genpd_dev_pm_detach; dev->pm_domain->sync = genpd_dev_pm_sync; - if (power_on) { - genpd_lock(pd); - ret = genpd_power_on(pd, 0); - genpd_unlock(pd); - } - - if (ret) { - genpd_remove_device(pd, dev); - return -EPROBE_DEFER; - } - /* Set the default performance state */ pstate = of_get_required_opp_performance_state(dev->of_node, index); if (pstate < 0 && pstate != -ENODEV && pstate != -EOPNOTSUPP) { @@ -2729,6 +2717,24 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev, goto err; dev_gpd_data(dev)->default_pstate = pstate; } + + if (power_on) { + genpd_lock(pd); + ret = genpd_power_on(pd, 0); + genpd_unlock(pd); + } + + if (ret) { + /* Drop the default performance state */ + if (dev_gpd_data(dev)->default_pstate) { + dev_pm_genpd_set_performance_state(dev, 0); + dev_gpd_data(dev)->default_pstate = 0; + } + + genpd_remove_device(pd, dev); + return -EPROBE_DEFER; + } + return 1; err: -- cgit v1.2.3 From bc80c2e438dcbfcf748452ec0f7ad5b79ff3ad88 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 2 Dec 2022 15:30:28 +0100 Subject: PM: runtime: Do not call __rpm_callback() from rpm_idle() Calling __rpm_callback() from rpm_idle() after adding device links support to the former is a clear mistake. Not only it causes rpm_idle() to carry out unnecessary actions, but it is also against the assumption regarding the stability of PM-runtime status across __rpm_callback() invocations, because rpm_suspend() and rpm_resume() may run in parallel with __rpm_callback() when it is called by rpm_idle() and the device's PM-runtime status can be updated by any of them. Fixes: 21d5c57b3726 ("PM / runtime: Use device links") Link: https://lore.kernel.org/linux-pm/36aed941-a73e-d937-2721-4f0decd61ce0@quicinc.com Signed-off-by: Rafael J. Wysocki Reviewed-by: Adrian Hunter --- drivers/base/power/runtime.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b52049098d4e..14088b5adb55 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -484,7 +484,17 @@ static int rpm_idle(struct device *dev, int rpmflags) dev->power.idle_notification = true; - retval = __rpm_callback(callback, dev); + if (dev->power.irq_safe) + spin_unlock(&dev->power.lock); + else + spin_unlock_irq(&dev->power.lock); + + retval = callback(dev); + + if (dev->power.irq_safe) + spin_lock(&dev->power.lock); + else + spin_lock_irq(&dev->power.lock); dev->power.idle_notification = false; wake_up_all(&dev->power.wait_queue); -- cgit v1.2.3 From 0307f4e8fff5518f2e123f992b4cd089813dc18c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 2 Dec 2022 15:32:09 +0100 Subject: PM: runtime: Relocate rpm_callback() right after __rpm_callback() Because rpm_callback() is a wrapper around __rpm_callback(), and the only caller of it after the change eliminating an invocation of it from rpm_idle(), move the former next to the latter to make the code a bit easier to follow. Signed-off-by: Rafael J. Wysocki Reviewed-by: Adrian Hunter --- drivers/base/power/runtime.c | 64 ++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 14088b5adb55..5a5cd1d8c4a9 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -421,6 +421,38 @@ fail: return retval; } +/** + * rpm_callback - Run a given runtime PM callback for a given device. + * @cb: Runtime PM callback to run. + * @dev: Device to run the callback for. + */ +static int rpm_callback(int (*cb)(struct device *), struct device *dev) +{ + int retval; + + if (dev->power.memalloc_noio) { + unsigned int noio_flag; + + /* + * Deadlock might be caused if memory allocation with + * GFP_KERNEL happens inside runtime_suspend and + * runtime_resume callbacks of one block device's + * ancestor or the block device itself. Network + * device might be thought as part of iSCSI block + * device, so network device and its ancestor should + * be marked as memalloc_noio too. + */ + noio_flag = memalloc_noio_save(); + retval = __rpm_callback(cb, dev); + memalloc_noio_restore(noio_flag); + } else { + retval = __rpm_callback(cb, dev); + } + + dev->power.runtime_error = retval; + return retval != -EACCES ? retval : -EIO; +} + /** * rpm_idle - Notify device bus type if the device can be suspended. * @dev: Device to notify the bus type about. @@ -504,38 +536,6 @@ static int rpm_idle(struct device *dev, int rpmflags) return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO); } -/** - * rpm_callback - Run a given runtime PM callback for a given device. - * @cb: Runtime PM callback to run. - * @dev: Device to run the callback for. - */ -static int rpm_callback(int (*cb)(struct device *), struct device *dev) -{ - int retval; - - if (dev->power.memalloc_noio) { - unsigned int noio_flag; - - /* - * Deadlock might be caused if memory allocation with - * GFP_KERNEL happens inside runtime_suspend and - * runtime_resume callbacks of one block device's - * ancestor or the block device itself. Network - * device might be thought as part of iSCSI block - * device, so network device and its ancestor should - * be marked as memalloc_noio too. - */ - noio_flag = memalloc_noio_save(); - retval = __rpm_callback(cb, dev); - memalloc_noio_restore(noio_flag); - } else { - retval = __rpm_callback(cb, dev); - } - - dev->power.runtime_error = retval; - return retval != -EACCES ? retval : -EIO; -} - /** * rpm_suspend - Carry out runtime suspend of given device. * @dev: Device to suspend. -- cgit v1.2.3 From dbfa44782787dc90460bae8b500708ec83e0f611 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 5 Dec 2022 19:54:59 +0100 Subject: PM: runtime: Adjust white space in the core code Some inconsistent usage of white space in the PM-runtime core code causes that code to be somewhat harder to read that it would have been otherwise, so adjust the white space in there to be more consistent with the rest of the code. No expected functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 69 ++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 34 deletions(-) (limited to 'drivers/base/power') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 5a5cd1d8c4a9..50e726b6c2cf 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -243,8 +243,7 @@ void pm_runtime_set_memalloc_noio(struct device *dev, bool enable) * flag was set by any one of the descendants. */ if (!dev || (!enable && - device_for_each_child(dev, NULL, - dev_memalloc_noio))) + device_for_each_child(dev, NULL, dev_memalloc_noio))) break; } mutex_unlock(&dev_hotplug_mutex); @@ -265,15 +264,13 @@ static int rpm_check_suspend_allowed(struct device *dev) retval = -EACCES; else if (atomic_read(&dev->power.usage_count)) retval = -EAGAIN; - else if (!dev->power.ignore_children && - atomic_read(&dev->power.child_count)) + else if (!dev->power.ignore_children && atomic_read(&dev->power.child_count)) retval = -EBUSY; /* Pending resume requests take precedence over suspends. */ - else if ((dev->power.deferred_resume - && dev->power.runtime_status == RPM_SUSPENDING) - || (dev->power.request_pending - && dev->power.request == RPM_REQ_RESUME)) + else if ((dev->power.deferred_resume && + dev->power.runtime_status == RPM_SUSPENDING) || + (dev->power.request_pending && dev->power.request == RPM_REQ_RESUME)) retval = -EAGAIN; else if (__dev_pm_qos_resume_latency(dev) == 0) retval = -EPERM; @@ -404,9 +401,9 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev) * * Do that if resume fails too. */ - if (use_links - && ((dev->power.runtime_status == RPM_SUSPENDING && !retval) - || (dev->power.runtime_status == RPM_RESUMING && retval))) { + if (use_links && + ((dev->power.runtime_status == RPM_SUSPENDING && !retval) || + (dev->power.runtime_status == RPM_RESUMING && retval))) { idx = device_links_read_lock(); __rpm_put_suppliers(dev, false); @@ -491,6 +488,7 @@ static int rpm_idle(struct device *dev, int rpmflags) /* Act as though RPM_NOWAIT is always set. */ else if (dev->power.idle_notification) retval = -EINPROGRESS; + if (retval) goto out; @@ -574,12 +572,12 @@ static int rpm_suspend(struct device *dev, int rpmflags) /* Synchronous suspends are not allowed in the RPM_RESUMING state. */ if (dev->power.runtime_status == RPM_RESUMING && !(rpmflags & RPM_ASYNC)) retval = -EAGAIN; + if (retval) goto out; /* If the autosuspend_delay time hasn't expired yet, reschedule. */ - if ((rpmflags & RPM_AUTO) - && dev->power.runtime_status != RPM_SUSPENDING) { + if ((rpmflags & RPM_AUTO) && dev->power.runtime_status != RPM_SUSPENDING) { u64 expires = pm_runtime_autosuspend_expiration(dev); if (expires != 0) { @@ -594,7 +592,7 @@ static int rpm_suspend(struct device *dev, int rpmflags) * rest. */ if (!(dev->power.timer_expires && - dev->power.timer_expires <= expires)) { + dev->power.timer_expires <= expires)) { /* * We add a slack of 25% to gather wakeups * without sacrificing the granularity. @@ -604,9 +602,9 @@ static int rpm_suspend(struct device *dev, int rpmflags) dev->power.timer_expires = expires; hrtimer_start_range_ns(&dev->power.suspend_timer, - ns_to_ktime(expires), - slack, - HRTIMER_MODE_ABS); + ns_to_ktime(expires), + slack, + HRTIMER_MODE_ABS); } dev->power.timer_autosuspends = 1; goto out; @@ -797,8 +795,8 @@ static int rpm_resume(struct device *dev, int rpmflags) goto out; } - if (dev->power.runtime_status == RPM_RESUMING - || dev->power.runtime_status == RPM_SUSPENDING) { + if (dev->power.runtime_status == RPM_RESUMING || + dev->power.runtime_status == RPM_SUSPENDING) { DEFINE_WAIT(wait); if (rpmflags & (RPM_ASYNC | RPM_NOWAIT)) { @@ -825,8 +823,8 @@ static int rpm_resume(struct device *dev, int rpmflags) for (;;) { prepare_to_wait(&dev->power.wait_queue, &wait, TASK_UNINTERRUPTIBLE); - if (dev->power.runtime_status != RPM_RESUMING - && dev->power.runtime_status != RPM_SUSPENDING) + if (dev->power.runtime_status != RPM_RESUMING && + dev->power.runtime_status != RPM_SUSPENDING) break; spin_unlock_irq(&dev->power.lock); @@ -846,9 +844,9 @@ static int rpm_resume(struct device *dev, int rpmflags) */ if (dev->power.no_callbacks && !parent && dev->parent) { spin_lock_nested(&dev->parent->power.lock, SINGLE_DEPTH_NESTING); - if (dev->parent->power.disable_depth > 0 - || dev->parent->power.ignore_children - || dev->parent->power.runtime_status == RPM_ACTIVE) { + if (dev->parent->power.disable_depth > 0 || + dev->parent->power.ignore_children || + dev->parent->power.runtime_status == RPM_ACTIVE) { atomic_inc(&dev->parent->power.child_count); spin_unlock(&dev->parent->power.lock); retval = 1; @@ -877,6 +875,7 @@ static int rpm_resume(struct device *dev, int rpmflags) parent = dev->parent; if (dev->power.irq_safe) goto skip_parent; + spin_unlock(&dev->power.lock); pm_runtime_get_noresume(parent); @@ -886,8 +885,8 @@ static int rpm_resume(struct device *dev, int rpmflags) * Resume the parent if it has runtime PM enabled and not been * set to ignore its children. */ - if (!parent->power.disable_depth - && !parent->power.ignore_children) { + if (!parent->power.disable_depth && + !parent->power.ignore_children) { rpm_resume(parent, 0); if (parent->power.runtime_status != RPM_ACTIVE) retval = -EBUSY; @@ -897,6 +896,7 @@ static int rpm_resume(struct device *dev, int rpmflags) spin_lock(&dev->power.lock); if (retval) goto out; + goto repeat; } skip_parent: @@ -1301,9 +1301,9 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) * not active, has runtime PM enabled and the * 'power.ignore_children' flag unset. */ - if (!parent->power.disable_depth - && !parent->power.ignore_children - && parent->power.runtime_status != RPM_ACTIVE) { + if (!parent->power.disable_depth && + !parent->power.ignore_children && + parent->power.runtime_status != RPM_ACTIVE) { dev_err(dev, "runtime PM trying to activate child device %s but parent (%s) is not active\n", dev_name(dev), dev_name(parent)); @@ -1368,9 +1368,9 @@ static void __pm_runtime_barrier(struct device *dev) dev->power.request_pending = false; } - if (dev->power.runtime_status == RPM_SUSPENDING - || dev->power.runtime_status == RPM_RESUMING - || dev->power.idle_notification) { + if (dev->power.runtime_status == RPM_SUSPENDING || + dev->power.runtime_status == RPM_RESUMING || + dev->power.idle_notification) { DEFINE_WAIT(wait); /* Suspend, wake-up or idle notification in progress. */ @@ -1455,8 +1455,8 @@ void __pm_runtime_disable(struct device *dev, bool check_resume) * means there probably is some I/O to process and disabling runtime PM * shouldn't prevent the device from processing the I/O. */ - if (check_resume && dev->power.request_pending - && dev->power.request == RPM_REQ_RESUME) { + if (check_resume && dev->power.request_pending && + dev->power.request == RPM_REQ_RESUME) { /* * Prevent suspends and idle notifications from being carried * out after we have woken up the device. @@ -1616,6 +1616,7 @@ void pm_runtime_irq_safe(struct device *dev) { if (dev->parent) pm_runtime_get_sync(dev->parent); + spin_lock_irq(&dev->power.lock); dev->power.irq_safe = 1; spin_unlock_irq(&dev->power.lock); -- cgit v1.2.3