From patchwork Fri Jul 8 19:24:24 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 957212 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p68JNNp8016467 for ; Fri, 8 Jul 2011 19:23:23 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753260Ab1GHTXX (ORCPT ); Fri, 8 Jul 2011 15:23:23 -0400 Received: from ogre.sisk.pl ([217.79.144.158]:40015 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750847Ab1GHTXW (ORCPT ); Fri, 8 Jul 2011 15:23:22 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by ogre.sisk.pl (Postfix) with ESMTP id 16D0F1B4C91; Fri, 8 Jul 2011 20:57:30 +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 22687-03; Fri, 8 Jul 2011 20:57:06 +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 93F071B4C2D; Fri, 8 Jul 2011 20:57:06 +0200 (CEST) From: "Rafael J. Wysocki" To: Kevin Hilman Subject: Re: [Update][PATCH 6/10] PM / Domains: System-wide transitions support for generic domains (v5) Date: Fri, 8 Jul 2011 21:24:24 +0200 User-Agent: KMail/1.13.6 (Linux/3.0.0-rc6+; KDE/4.6.0; x86_64; ; ) Cc: Alan Stern , Linux PM mailing list , "Greg Kroah-Hartman" , Magnus Damm , Paul Walmsley , LKML , linux-sh@vger.kernel.org, Paul Mundt References: <87y6088z9n.fsf@ti.com> <201107082006.49660.rjw@sisk.pl> In-Reply-To: <201107082006.49660.rjw@sisk.pl> MIME-Version: 1.0 Message-Id: <201107082124.24921.rjw@sisk.pl> X-Virus-Scanned: amavisd-new at ogre.sisk.pl using MkS_Vir for Linux Sender: linux-sh-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sh@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Fri, 08 Jul 2011 19:23:24 +0000 (UTC) On Friday, July 08, 2011, Rafael J. Wysocki wrote: > On Friday, July 08, 2011, Kevin Hilman wrote: > > Alan Stern writes: > > > > > On Fri, 8 Jul 2011, Rafael J. Wysocki wrote: > > > > > >> On Friday, July 08, 2011, Kevin Hilman wrote: > > >> > "Rafael J. Wysocki" writes: > > >> > > > >> > > From: Rafael J. Wysocki > > >> > > > > >> > > 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(+) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" 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/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)