From patchwork Sat Jun 18 11:08:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 892992 X-Patchwork-Delegate: khilman@deeprootsystems.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p5IB7pU0028982 for ; Sat, 18 Jun 2011 11:07:52 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754947Ab1FRLHu (ORCPT ); Sat, 18 Jun 2011 07:07:50 -0400 Received: from ogre.sisk.pl ([217.79.144.158]:38468 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754687Ab1FRLHt (ORCPT ); Sat, 18 Jun 2011 07:07:49 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by ogre.sisk.pl (Postfix) with ESMTP id D1AB91AC6DF; Sat, 18 Jun 2011 12:48:33 +0200 (CEST) Received: from ogre.sisk.pl ([127.0.0.1]) by localhost (ogre.sisk.pl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 24145-04; Sat, 18 Jun 2011 12:48:14 +0200 (CEST) Received: from ferrari.rjw.lan (220-bem-13.acn.waw.pl [82.210.184.220]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ogre.sisk.pl (Postfix) with ESMTP id 8D7441AC682; Sat, 18 Jun 2011 12:48:14 +0200 (CEST) From: "Rafael J. Wysocki" To: Alan Stern Subject: Re: [linux-pm] calling runtime PM from system PM methods Date: Sat, 18 Jun 2011 13:08:42 +0200 User-Agent: KMail/1.13.6 (Linux/3.0.0-rc3+; KDE/4.6.0; x86_64; ; ) Cc: linux-pm@lists.linux-foundation.org, linux-omap@vger.kernel.org, Kevin Hilman , Paul Walmsley , Magnus Damm References: <201106172329.37087.rjw@sisk.pl> In-Reply-To: <201106172329.37087.rjw@sisk.pl> MIME-Version: 1.0 Message-Id: <201106181308.42298.rjw@sisk.pl> X-Virus-Scanned: amavisd-new at ogre.sisk.pl using MkS_Vir for Linux Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 18 Jun 2011 11:07:52 +0000 (UTC) On Friday, June 17, 2011, Rafael J. Wysocki wrote: > On Friday, June 17, 2011, Alan Stern wrote: > > On Fri, 17 Jun 2011, Rafael J. Wysocki wrote: > > > > > Having considered that a bit more I see that, in fact, commit > > > e8665002477f0278f84f898145b1f141ba26ee26 (PM: Allow pm_runtime_suspend() to > > > succeed during system suspend) has introduced at least one regression. > > > Namely, the PCI bus type runs pm_runtime_resume() in its .prepare() > > > callback to guarantee that devices will be in a well known state before > > > the PCI .suspend() and .suspend_noirq() callbacks are executed. > > > Unfortunately, after commit e8665002477f0278f84f898145b1f141ba26ee26 this > > > isn't valid any more, because devices can be runtime-suspend after the > > > pm_runtime_resume() in .prepare() has run. > > > > > > USB seems to do something similar in choose_wakeup(). > > > > > > So, either the both of these subsystems should be modified to use > > > pm_runtime_get_sync() and then pm_runtime_put_() some time > > > during resume, or we should revert commit e8665002477f0278f84f898145b1f141ba26ee26. > > > > pm_runtime_put_noidle would be appropriate. > > > > > Quite frankly, which shouldn't be a surprise to anyone at this point, I'd > > > prefer to revert that commit for 3.0. > > > > Maybe we can compromise. Instead of reverting that commit outright, > > put the get_noresume just before the suspend callback and put the > > put_sync just after the resume callback. > > That wouldn't fix the PCI problem, though, because it would leave a small > window in which the device could be suspended after the pm_runtime_resume() > in pci_pm_prepare() had run. That said, the PCI case can be solved with a separate patch and if the other subsystems are not affected, perhaps that's the best approach. Still, I'd like to make sure that there won't be any races between runtime PM and .suspend_noirq() and .resume_noirq() callbacks, so I'd like to apply the patch below. Thanks, Rafael --- drivers/base/power/main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: linux-2.6/drivers/base/power/main.c =================================================================== --- linux-2.6.orig/drivers/base/power/main.c +++ linux-2.6/drivers/base/power/main.c @@ -591,6 +591,8 @@ void dpm_resume(pm_message_t state) async_error = 0; list_for_each_entry(dev, &dpm_suspended_list, power.entry) { + pm_runtime_get_noresume(dev); + pm_runtime_enable(dev); INIT_COMPLETION(dev->power.completion); if (is_async(dev)) { get_device(dev); @@ -614,6 +616,7 @@ void dpm_resume(pm_message_t state) } if (!list_empty(&dev->power.entry)) list_move_tail(&dev->power.entry, &dpm_prepared_list); + pm_runtime_put_noidle(dev); put_device(dev); } mutex_unlock(&dpm_list_mtx); @@ -939,8 +942,10 @@ int dpm_suspend(pm_message_t state) put_device(dev); break; } - if (!list_empty(&dev->power.entry)) + if (!list_empty(&dev->power.entry)) { list_move(&dev->power.entry, &dpm_suspended_list); + pm_runtime_disable(dev); + } put_device(dev); if (async_error) break;