Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752471AbaBTB1u (ORCPT ); Wed, 19 Feb 2014 20:27:50 -0500 Received: from v094114.home.net.pl ([79.96.170.134]:57941 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751717AbaBTB1q (ORCPT ); Wed, 19 Feb 2014 20:27:46 -0500 From: "Rafael J. Wysocki" To: Alan Stern Cc: Linux PM list , Mika Westerberg , Aaron Lu , ACPI Devel Maling List , LKML Subject: Re: [PATCH 1/3] PM / sleep: New flag to speed up suspend-resume of suspended devices Date: Thu, 20 Feb 2014 02:42:38 +0100 Message-ID: <2436342.FaiucBZtkf@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/3.13.0+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1757715.oeT3oQZ2Zz@vostro.rjw.lan> References: <1757715.oeT3oQZ2Zz@vostro.rjw.lan> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday, February 20, 2014 02:23:30 AM Rafael J. Wysocki wrote: > On Wednesday, February 19, 2014 12:01:20 PM Alan Stern wrote: > > On Mon, 17 Feb 2014, Rafael J. Wysocki wrote: > > > > > From: Rafael J. Wysocki > > > > > > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to > > > resume all runtime-suspended devices during system suspend, mostly > > > because those devices may need to be reprogrammed due to different > > > wakeup settings for system sleep and for runtime PM. However, at > > > least in some cases, that isn't really necessary, because the wakeup > > > settings may not be really different. > > > > > > The idea here is that subsystems should know whether or not it is > > > necessary to reprogram a given device during system suspend and they > > > should be able to tell the PM core about that. For this reason, > > > modify the PM core so that if the .prepare() callback returns a > > > positive value for certain device, the core will set a new > > > power.fast_suspend flag for it. Then, if that flag is set, the core > > > will skip all of the subsequent suspend callbacks for that device. > > > It also will skip all of the system resume callbacks for the device > > > during the subsequent system resume and pm_request_resume() will be > > > executed to trigger a runtime PM resume of the device after the > > > system device resume sequence has been finished. > > I was worried that you wouldn't comment at all. ;-) > > > Does the PM core really need to get involved in this? > > Yes, it does, in my opinion, and the reason is because it may be necessary > to resume parents in order to reprogram their children and the parents and > the children may be on different bus types. > > > Can't the subsystem do the right thing on its own? > > No, I don't think so. > > > In the USB subsystem, the .suspend routine checks the required wakeup > > settings. If they are different for runtime suspend and system > > suspend, and if the device is runtime suspended, then we call > > pm_runtime_resume. After that, if the device is still in runtime > > suspend then we return immediately. > > > > Of course, this addresses only the suspend side of the issue. Skipping > > the resume callbacks is a separate matter, and the USB subsystem > > doesn't try to do it. Still, I don't see any reason why we couldn't > > take care of that as well. > > What about USB controllers that are PCI devices? > > > > However, since parents may need to be resumed so that their children > > > can be reprogrammed, make the PM core clear power.fast_suspend for > > > devices whose children don't have power.fast_suspend set (the > > > power.ignore_children flag doesn't matter here, because a parent > > > whose children are normally ignored for runtime PM may still need to > > > be accessible for their children to be prepare for system suspend > > > properly). > > > > I have run across a similar issue. It's a general problem that a > > device may try to remain in runtime suspend during a system resume, but > > a descendant of the device may need to perform I/O as part of its own > > resume routine. A natural solution would be to use the regular runtime > > PM facilities to wake up the device. But since the PM work queue is > > frozen, we can't rely on pm_runtime_get or the equivalent. I'm not > > sure what the best solution will be. > > We can rely on pm_runtime_get_sync(), though, which would be the right thing to > use here. > > However, given that the parent's .prepare() has run already, I'm not sure > if we want runtime PM to be involved at all. > > > After a quick look, I noticed a couple of questionable things in this > > patch. This is after reading just the second half... > > > > > @@ -1296,8 +1301,10 @@ static int __device_suspend(struct devic > > > * for it, this is equivalent to the device signaling wakeup, so the > > > * system suspend operation should be aborted. > > > */ > > > - if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) > > > + if (pm_runtime_barrier(dev) && device_may_wakeup(dev)) { > > > pm_wakeup_event(dev, 0); > > > + dev->power.fast_suspend = false; > > > + } > > > > Is this change needed? We're aborting the sleep transition anyway, > > right? > > Yes, we are. The goal was to ensure that power.fast_suspend would be cleared > in that case, but dpm_resume_end() should take care of this anyway. Sorry, I wanted to say "the clearing of it in device_prepare()", not sure why I mentioned dpm_resume_end(). Well, maybe I'm too tired ... Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/