Re: [patch update] PM: Introduce core framework for run-time PM of I/O devices (rev. 6)

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Monday 29 June 2009, Alan Stern wrote:
> On Mon, 29 Jun 2009, Rafael J. Wysocki wrote:
> 
> > IMO one can think of pm_request_resume() as a top half of pm_runtime_resume().
> 
> Normal top halves don't trigger before the circumstances are
> appropriate.  For example, if you enable remote wakeup on a USB device,
> it won't send a wakeup signal before it has been powered down.  A
> driver calling pm_request_resume while the device is still resumed is
> like a USB device sending a wakeup request while it is still powered
> up.  So IMO the analogy with top halves isn't a good one.
> 
> > Thus, it should either queue up a request to run pm_runtime_resume() or leave
> > the status as though pm_runtime_resume() ran.  Anything else would be
> > internally inconsistent.  So, if pm_runtime_resume() cancels pending suspend
> > requests, pm_request_resume() should do the same or the other way around.
> > 
> > Now, arguably, ignoring pending suspend requests is somewhat easier from
> > the core's point of view, but it may not be so for drivers.
> 
> The argument I gave in the previous email demonstrates that it doesn't
> make any difference to drivers.  Either way, they have to use two I/O
> pathways, they have to do a pm_runtime_get before pm_request_resume,
> and they have to do a pm_request_put after the I/O is done.
> 
> Of course, this is all somewhat theoretical.  I still don't know of any 
> actual drivers that do the equivalent of pm_request_resume.
> 
> > My point is that the core should always treat pending suspend requests in the
> > same way.  If they are canceled by pm_runtime_resume(), then
> > pm_request_resume() should also cancel them and it shouldn't be possible
> > to schedule a suspend request when the resume counter is greater than 0.
> > In turn, if they are ignored by pm_runtime_resume(), then pm_request_resume()
> > should also ignore them and there's no point to prevent pm_request_suspend()
> > from scheduling a suspend request if the resume counter is greater than 0.
> > 
> > Any other type of behavior has a potential to confuse driver writers.
> 
> Another possible approach you could take when the call to
> cancel_delayed_work fails (which should be rare) is to turn on RPM_WAKE
> in addition to RPM_IDLE and leave the suspend request queued.  When
> __pm_runtime_suspend sees both flags are set, it should abort and set
> the status directly back to RPM_ACTIVE.  At that time the idle
> notifications can start up again.
> 
> Is this any better?  I can't see how drivers would care, though.

There still is the problem that the suspend request is occupying the
work_struct which cannot be used for any other purpose.  I don't think this
is avoidable, though.  This way or another it is possible to have two requests
pending at a time.

Perhaps the simplest thing to do would be to simply ignore pending suspend
requests in both pm_request_resume() and pm_runtime_resume() and to allow
them to be scheduled at any time.  That shouldn't hurt anything as long as
pm_runtime_suspend() is smart enough, but it has to be anyway, because it
can be run synchronously at any time.

The only question is what pm_runtime_suspend() should do when it sees a pending
suspend request and quite frankly I think it can just ignore it as well,
leaving the RPM_IDLE bit set.  In which case the name RPM_IDLE will not really
be adequate, so perhaps it can be renamed to RPM_REQUEST or something like
this.

Then, we'll need a separate work structure for suspend requests, but I have no
problem with that.

> P.S.: What do you think should happen if there's a delayed suspend
> request pending, then pm_request_resume is called (and it leaves the
> request queued), and then someone calls pm_runtime_suspend?  You've got
> two pending requests and a synchronous call all active at the same
> time!

That's easy, pm_runtime_suspend() sees a pending resume, so it quits and the
other things work out as usual.

Best,
Rafael