On Friday, July 08, 2011, Rafael J. Wysocki wrote: > On Friday, July 08, 2011, Kevin Hilman wrote: > > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: > > > > > On Fri, 8 Jul 2011, Rafael J. Wysocki wrote: > > > > > >> On Friday, July 08, 2011, Kevin Hilman wrote: > > >> > "Rafael J. Wysocki" <rjw@xxxxxxx> writes: > > >> > > > >> > > From: Rafael J. Wysocki <rjw@xxxxxxx> > > >> > > > > >> > > 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) _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
