On Friday, September 10, 2010, Alan Stern wrote:
> On Fri, 10 Sep 2010, Rafael J. Wysocki wrote:
>
> > On Friday, September 10, 2010, Alan Stern wrote:
> > > Rafael:
> >
> > Hi Alan,
> >
> > First, I think we should CC linux-pm at least with this.
>
> Quite right. I have also removed linux-usb and Oliver since they
> aren't directly involved, and updated the Subject: line.
>
> > > It turns out that the existing "from_wq" arguments can be subsumed into
> > > an "rpmflags" variable, along with the other things we've been talking
> > > about. Even better, one of the flag bits can be used to indicate
> > > whether a call should be synchronous.
> > >
> > > Which leads to the possibility of merging the __pm_runtime_X and
> > > __pm_request_X pairs of routines. They all make similar checks when
> > > they start; this is needlessly duplicated code. Unfortunately, they
> > > are slightly inconsistent in a few obscure respects:
> > >
> > > If the current status is RPM_RESUMING then __rpm_request_suspend
> > > succeeds but __rpm_runtime_suspend fails. Similarly for idle.
> >
> > That was intentional. __rpm_runtime_suspend has to fail in that case, but
> > __rpm_request_suspend need not, because __rpm_runtime_suspend is going to
> > be called after it anyway. So, the decision whether or not to suspend the
> > device is always made by __rpm_runtime_suspend and I'd like that to stay this
> > way.
>
> Okay, so you would like __rpm_runtime_suspend to fail while the device
> is resuming but __rpm_request_suspend to succeed.
Yes.
> > > The routines in each pair sometimes differ in whether they
> > > cancel other pending requests first vs. check for invalid
> > > conditions (such as wrong usage_count) first.
> >
> > That might be intentional as well, at least in some cases.
> >
> > > It seems to me that the __pm_runtime_X and __pm_request_X functions
> > > should behave the same, as nearly as possible.
> > >
> > > To make everything more uniform, we should agree on some fixed scheme like
> > > this:
> > >
> > > First check for things that make the function impossible:
> > > power.runtime_error set, usage_count or child_count set
> > > wrong, or disable_depth > 0. If these checks fail, return
> > > immediately.
> >
> > OK
> >
> > > Then check the current status and the pending requests to
> > > see if they rule out the current function:
> > >
> > > idle is allowed only in RPM_ACTIVE and when there
> > > are no pending requests (but the suspend timer can
> > > be running);
> > >
> > > suspend is not allowed if the status is RPM_SUSPENDED
> > > or RPM_RESUMING, or if there is a pending resume
> > > (either a request or a deferred resume);
> > >
> > > resume is not allowed if the status is RPM_ACTIVE.
> >
> > OK, but that need not apply to the _ldle() variants.
>
> I don't understand. Are you referring to the "idle is allowed only..."
> paragraph above? What's the connection with resume?
Sorry, I thought *_request_*() and wrote _idle(). Must be too tired. :-)
> > > If this check succeeds, then cancel any pending requests
> > > (exception: doing an idle should not cause the suspend timer to
> > > be cancelled) and either carry out the action, put it on the
> > > workqueue, defer it, or start the timer.
> > >
> > > This almost agrees with the rules laid out in
> > > Documentation/power/runtime.txt. The only difference I can see is what
> > > should happen if __pm_{runtime|request}_resume is called while the
> > > status is RPM_ACTIVE and there is a pending idle or suspend request or
> > > a scheduled suspend. Under these conditions I think we probably
> > > shouldn't cancel a pending idle request.
> >
> > That would be fine.
> >
> > > I'm not sure about pending or scheduled suspends.
> >
> > I actually think we may try not to cancel any pending/scheduled operations
> > at all and let things work out. If the caller is not careful enough to use the
> > reference counting, so be it.
>
> Well, pending or scheduled operations can certainly be cancelled safely
> when one of the synchronous operations starts. For example, when
> a runtime resume is starting we can cancel a pending resume request.
> (It should not be possible for there to be any pending idle or suspend
> requests at this time, because the status is RPM_SUSPENDED.)
Cancelling a pending operation of the same kind is reasonable.
> However, if you want to avoid cancelling pending or scheduled
> operations when one of the routines fails because it was called at the
> wrong time, I'm okay with that. In fact, it's what I suggested above.
OK, then.
> As for the asynchronous routines... If a request is successfully
> queued then it _has_ to cancel pending requests, since only one request
> can be pending at a time. But we can theoretically have both a pending
> request and a scheduled suspend. I don't see any point in allowing it,
> though. In general we should let the most recent successful function
> call win.
OK
Thanks,
Rafael
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
