Question about expected behavior when PM runtime is disabled

Question about expected behavior when PM runtime is disabled

[Kenneth Heitke]

Hi Rafael,

Sorry if this question has been raised before.  I actually have two 
questions here.  These questions are related to PM runtime being 
disabled at runtime (i.e. call to pm_runtime_disable() )

If I call pm_runtime_enabled() to first determine if PM runtime is 
enabled followed conditionally by a call to pm_runtime_get_sync(), it 
would be possible for PM runtime to be disabled between these two calls 
and the get_sync() will fail.  Is there any reason to even use the 
enabled() call?  My goal here was to use the enabled() call to determine 
if PM runtime was configured/enabled in the kernel and then to manage my 
resources, clocks etc, in a different way if PM runtime is not present.

My second question then is what if PM runtime is enabled in the kernel 
and then gets disabled at runtime.  What is the expected behavior for a 
driver?  Should it fail all requests with EGAIN until PM runtime is 
enabled again? (in suspend state, PM runtime gets disable, new i/o 
request is made, power and clocks need to be turned on).

What about delayed autosuspend?  I believe that if PM runtime is 
disabled while there is a delayed autosuspend pending, the suspend will 
fail without notification (clocks and power will be left on).  Will PM 
runtime still be in the idle state once PM runtime is re-enabled?

thanks,
Ken


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: Question about expected behavior when PM runtime is disabled

[Alan Stern]

On Fri, 10 Jun 2011, Kenneth Heitke wrote:

> Hi Rafael,
> 
> Sorry if this question has been raised before.  I actually have two 
> questions here.  These questions are related to PM runtime being 
> disabled at runtime (i.e. call to pm_runtime_disable() )
> 
> If I call pm_runtime_enabled() to first determine if PM runtime is 
> enabled followed conditionally by a call to pm_runtime_get_sync(), it 
> would be possible for PM runtime to be disabled between these two calls 
> and the get_sync() will fail.  Is there any reason to even use the 
> enabled() call?

As a general rule, a device won't be enabled or disabled for runtime PM 
unless its driver or subsystem enables or disables it.  Since you 
should already know what the driver and subsystem are doing, there 
usually isn't any reason for using pm_runtime_enabled().

>  My goal here was to use the enabled() call to determine 
> if PM runtime was configured/enabled in the kernel and then to manage my 
> resources, clocks etc, in a different way if PM runtime is not present.

If runtime PM isn't present, why do you want to manage your clocks etc.  
at all?  The fact that it's not in the kernel means the system manager
doesn't care about power usage.

> My second question then is what if PM runtime is enabled in the kernel 
> and then gets disabled at runtime.  What is the expected behavior for a 
> driver?  Should it fail all requests with EGAIN until PM runtime is 
> enabled again? (in suspend state, PM runtime gets disable, new i/o 
> request is made, power and clocks need to be turned on).

It's up to the driver and the subsystem, since they are the entities 
that are responsible for disabling runtime PM.  If you think disabling 
runtime PM will cause problems, then don't do it.

> What about delayed autosuspend?  I believe that if PM runtime is 
> disabled while there is a delayed autosuspend pending, the suspend will 
> fail without notification (clocks and power will be left on).

That's right.

>  Will PM 
> runtime still be in the idle state once PM runtime is re-enabled?

The device will be in the same state as it was when it was disabled, 
unless you explicitly call pm_runtime_set_active() or 
pm_runtime_set_suspended().

Alan Stern

Re: Question about expected behavior when PM runtime is disabled

[Kenneth Heitke]

Alan,

Thanks for taking the time to answer my questions.  See inline for 
additional comments.


On 06/11/2011 10:12 AM, Alan Stern wrote:
> On Fri, 10 Jun 2011, Kenneth Heitke wrote:
>
>> Hi Rafael,
>>
>> Sorry if this question has been raised before.  I actually have two
>> questions here.  These questions are related to PM runtime being
>> disabled at runtime (i.e. call to pm_runtime_disable() )
>>
>> If I call pm_runtime_enabled() to first determine if PM runtime is
>> enabled followed conditionally by a call to pm_runtime_get_sync(), it
>> would be possible for PM runtime to be disabled between these two calls
>> and the get_sync() will fail.  Is there any reason to even use the
>> enabled() call?
>
> As a general rule, a device won't be enabled or disabled for runtime PM
> unless its driver or subsystem enables or disables it.  Since you
> should already know what the driver and subsystem are doing, there
> usually isn't any reason for using pm_runtime_enabled().
>
>>   My goal here was to use the enabled() call to determine
>> if PM runtime was configured/enabled in the kernel and then to manage my
>> resources, clocks etc, in a different way if PM runtime is not present.
>
> If runtime PM isn't present, why do you want to manage your clocks etc.
> at all?  The fact that it's not in the kernel means the system manager
> doesn't care about power usage.

I am trying to be backwards compatible.  There is likely a period of 
time from when the runtime PM feature was added to when it was turned on 
by default.  If the feature happens to be disabled, I think it makes 
sense for the driver to still do what it can to manage its resources. 
The power guys aren't going to let me off the hook that easily :)

  >> My second question then is what if PM runtime is enabled in the kernel
>> and then gets disabled at runtime.  What is the expected behavior for a
>> driver?  Should it fail all requests with EGAIN until PM runtime is
>> enabled again? (in suspend state, PM runtime gets disable, new i/o
>> request is made, power and clocks need to be turned on).
>
> It's up to the driver and the subsystem, since they are the entities
> that are responsible for disabling runtime PM.  If you think disabling
> runtime PM will cause problems, then don't do it.

I'm thinking about within runtime PM itself.  I believe during system 
suspend, disable() followed by enable() can be called.  If that happens, 
are there any scenarios that I need to be concerned about?  Can my 
autosuspend timer just happen to fire during that window between disable 
and enable resulting in a failure to suspend?  My driver is part of the 
i2c subsystem, do I know for a fact that disable() won't be used?

>
>> What about delayed autosuspend?  I believe that if PM runtime is
>> disabled while there is a delayed autosuspend pending, the suspend will
>> fail without notification (clocks and power will be left on).
>
> That's right.
>
>>   Will PM
>> runtime still be in the idle state once PM runtime is re-enabled?
>
> The device will be in the same state as it was when it was disabled,
> unless you explicitly call pm_runtime_set_active() or
> pm_runtime_set_suspended().
>
> Alan Stern
>
>

thanks,
Ken


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: Question about expected behavior when PM runtime is disabled

[Alan Stern]

On Mon, 13 Jun 2011, Kenneth Heitke wrote:

> > If runtime PM isn't present, why do you want to manage your clocks etc.
> > at all?  The fact that it's not in the kernel means the system manager
> > doesn't care about power usage.
> 
> I am trying to be backwards compatible.  There is likely a period of 
> time from when the runtime PM feature was added to when it was turned on 
> by default.  If the feature happens to be disabled, I think it makes 
> sense for the driver to still do what it can to manage its resources. 
> The power guys aren't going to let me off the hook that easily :)

Maybe not...  but then you can point out that it's somebody else's 
fault!  :-)  More to the point, runtime PM should be enabled at the 
same time it is added.  The code for both belongs in the subsystem and 
driver, so there's no reason not to do them together.

IMO it's silly to bypass the runtime PM core's notion of when power 
management should or should not be available.  If you're going to do 
that, why bother using the runtime PM framework in the first place?


> > It's up to the driver and the subsystem, since they are the entities
> > that are responsible for disabling runtime PM.  If you think disabling
> > runtime PM will cause problems, then don't do it.
> 
> I'm thinking about within runtime PM itself.  I believe during system 
> suspend, disable() followed by enable() can be called.  If that happens, 
> are there any scenarios that I need to be concerned about?  Can my 
> autosuspend timer just happen to fire during that window between disable 
> and enable resulting in a failure to suspend?  My driver is part of the 
> i2c subsystem, do I know for a fact that disable() won't be used?

Ah -- that's a very good question.

The PM core doesn't call disable followed by enable, but subsystems do
as part of system resume.  In fact, I'm not at all sure that the
current implementation is correct in this regard.  The failure scenario
you bring up seems entirely possible.

I think we need to have the PM core call pm_runtime_get_noresume()  
before invoking the resume_noirq (or thaw_noirq or restore_noirq)  
callback, and then call pm_runtime_put_sync() after invoking the
complete callback.  This would solve your race: The put_sync would 
invoke the runtime_idle method, which would start another runtime 
suspend or autosuspend.

(It used to be that the PM core called pm_runtime_get_noresume() 
earlier on, before the prepare callback.  This also solved your race, 
but it caused other problems and so was changed.)

It's true that subsystems could do this for themselves, but then they'd
_all_ have to do it.  So we might as well put it in the PM core.

Rafael, what do you think?

Alan Stern

Re: Question about expected behavior when PM runtime is disabled

[Rafael J. Wysocki]

On Monday, June 13, 2011, Alan Stern wrote:
> On Mon, 13 Jun 2011, Kenneth Heitke wrote:
> 
> > > If runtime PM isn't present, why do you want to manage your clocks etc.
> > > at all?  The fact that it's not in the kernel means the system manager
> > > doesn't care about power usage.
> > 
> > I am trying to be backwards compatible.  There is likely a period of 
> > time from when the runtime PM feature was added to when it was turned on 
> > by default.  If the feature happens to be disabled, I think it makes 
> > sense for the driver to still do what it can to manage its resources. 
> > The power guys aren't going to let me off the hook that easily :)
> 
> Maybe not...  but then you can point out that it's somebody else's 
> fault!  :-)  More to the point, runtime PM should be enabled at the 
> same time it is added.  The code for both belongs in the subsystem and 
> driver, so there's no reason not to do them together.
> 
> IMO it's silly to bypass the runtime PM core's notion of when power 
> management should or should not be available.  If you're going to do 
> that, why bother using the runtime PM framework in the first place?
> 
> 
> > > It's up to the driver and the subsystem, since they are the entities
> > > that are responsible for disabling runtime PM.  If you think disabling
> > > runtime PM will cause problems, then don't do it.
> > 
> > I'm thinking about within runtime PM itself.  I believe during system 
> > suspend, disable() followed by enable() can be called.  If that happens, 
> > are there any scenarios that I need to be concerned about?  Can my 
> > autosuspend timer just happen to fire during that window between disable 
> > and enable resulting in a failure to suspend?  My driver is part of the 
> > i2c subsystem, do I know for a fact that disable() won't be used?
> 
> Ah -- that's a very good question.
> 
> The PM core doesn't call disable followed by enable, but subsystems do
> as part of system resume.  In fact, I'm not at all sure that the
> current implementation is correct in this regard.  The failure scenario
> you bring up seems entirely possible.
> 
> I think we need to have the PM core call pm_runtime_get_noresume()  
> before invoking the resume_noirq (or thaw_noirq or restore_noirq)  
> callback, and then call pm_runtime_put_sync() after invoking the
> complete callback.  This would solve your race: The put_sync would 
> invoke the runtime_idle method, which would start another runtime 
> suspend or autosuspend.
> 
> (It used to be that the PM core called pm_runtime_get_noresume() 
> earlier on, before the prepare callback.  This also solved your race, 
> but it caused other problems and so was changed.)
> 
> It's true that subsystems could do this for themselves, but then they'd
> _all_ have to do it.  So we might as well put it in the PM core.
> 
> Rafael, what do you think?

Yes, we can do that.

I even suspect that all subsystems will end up calling pm_runtime_disable()
somewhere in the system suspend code path and pm_runtime_enable() during
system resume.  It might be a good idea to do that in the core too, after
calling the subsystem's .suspend() and before calling its .resume(),
respectively.

Thanks,
Rafael

Re: Question about expected behavior when PM runtime is disabled

[Alan Stern]

On Mon, 13 Jun 2011, Rafael J. Wysocki wrote:

> > I think we need to have the PM core call pm_runtime_get_noresume()  
> > before invoking the resume_noirq (or thaw_noirq or restore_noirq)  
> > callback, and then call pm_runtime_put_sync() after invoking the
> > complete callback.  This would solve your race: The put_sync would 
> > invoke the runtime_idle method, which would start another runtime 
> > suspend or autosuspend.
> > 
> > (It used to be that the PM core called pm_runtime_get_noresume() 
> > earlier on, before the prepare callback.  This also solved your race, 
> > but it caused other problems and so was changed.)
> > 
> > It's true that subsystems could do this for themselves, but then they'd
> > _all_ have to do it.  So we might as well put it in the PM core.
> > 
> > Rafael, what do you think?
> 
> Yes, we can do that.
> 
> I even suspect that all subsystems will end up calling pm_runtime_disable()
> somewhere in the system suspend code path and pm_runtime_enable() during
> system resume.  It might be a good idea to do that in the core too, after
> calling the subsystem's .suspend() and before calling its .resume(),
> respectively.

Will that bring back Kevin's problems?  There was a specific commit:  
"PM: Allow pm_runtime_suspend() to succeed during system suspend".  If
the core disables runtime PM, won't he be right back where he was
before?

Alan Stern

Re: Question about expected behavior when PM runtime is disabled

[Rafael J. Wysocki]

On Monday, June 13, 2011, Alan Stern wrote:
> On Mon, 13 Jun 2011, Rafael J. Wysocki wrote:
> 
> > > I think we need to have the PM core call pm_runtime_get_noresume()  
> > > before invoking the resume_noirq (or thaw_noirq or restore_noirq)  
> > > callback, and then call pm_runtime_put_sync() after invoking the
> > > complete callback.  This would solve your race: The put_sync would 
> > > invoke the runtime_idle method, which would start another runtime 
> > > suspend or autosuspend.
> > > 
> > > (It used to be that the PM core called pm_runtime_get_noresume() 
> > > earlier on, before the prepare callback.  This also solved your race, 
> > > but it caused other problems and so was changed.)
> > > 
> > > It's true that subsystems could do this for themselves, but then they'd
> > > _all_ have to do it.  So we might as well put it in the PM core.
> > > 
> > > Rafael, what do you think?
> > 
> > Yes, we can do that.
> > 
> > I even suspect that all subsystems will end up calling pm_runtime_disable()
> > somewhere in the system suspend code path and pm_runtime_enable() during
> > system resume.  It might be a good idea to do that in the core too, after
> > calling the subsystem's .suspend() and before calling its .resume(),
> > respectively.
> 
> Will that bring back Kevin's problems?  There was a specific commit:  
> "PM: Allow pm_runtime_suspend() to succeed during system suspend".  If
> the core disables runtime PM, won't he be right back where he was
> before?

Not exactly, because that commit removed the pm_runtime_get_noresume()
done before .prepare(), which was too early.  As I said before, I don't
see anything wrong with running pm_runtime_ helpers from .prepare() or
.complete().  However, to me, it is highly doubtful if there is any valid
reason for calling them after .suspend() has been executed.  In fact, I
think that .suspend() should ensure that they won't be executed for the
given device after it has returned, so doing pm_runtime_disable() in the
core at this point makes sense.

We really shouldn't allow any runtime PM callbacks to race with
.suspend_noirq() and .resume_noirq(), because allowing that to happen would
be asking for breakage.

Thanks,
Rafael

Re: Question about expected behavior when PM runtime is disabled

[Alan Stern]

On Mon, 13 Jun 2011, Rafael J. Wysocki wrote:

> On Monday, June 13, 2011, Alan Stern wrote:
> > On Mon, 13 Jun 2011, Rafael J. Wysocki wrote:
> > 
> > > > I think we need to have the PM core call pm_runtime_get_noresume()  
> > > > before invoking the resume_noirq (or thaw_noirq or restore_noirq)  
> > > > callback, and then call pm_runtime_put_sync() after invoking the
> > > > complete callback.  This would solve your race: The put_sync would 
> > > > invoke the runtime_idle method, which would start another runtime 
> > > > suspend or autosuspend.
> > > > 
> > > > (It used to be that the PM core called pm_runtime_get_noresume() 
> > > > earlier on, before the prepare callback.  This also solved your race, 
> > > > but it caused other problems and so was changed.)
> > > > 
> > > > It's true that subsystems could do this for themselves, but then they'd
> > > > _all_ have to do it.  So we might as well put it in the PM core.
> > > > 
> > > > Rafael, what do you think?
> > > 
> > > Yes, we can do that.
> > > 
> > > I even suspect that all subsystems will end up calling pm_runtime_disable()
> > > somewhere in the system suspend code path and pm_runtime_enable() during
> > > system resume.  It might be a good idea to do that in the core too, after
> > > calling the subsystem's .suspend() and before calling its .resume(),
> > > respectively.
> > 
> > Will that bring back Kevin's problems?  There was a specific commit:  
> > "PM: Allow pm_runtime_suspend() to succeed during system suspend".  If
> > the core disables runtime PM, won't he be right back where he was
> > before?
> 
> Not exactly, because that commit removed the pm_runtime_get_noresume()
> done before .prepare(), which was too early.  As I said before, I don't
> see anything wrong with running pm_runtime_ helpers from .prepare() or
> .complete().  However, to me, it is highly doubtful if there is any valid
> reason for calling them after .suspend() has been executed.  In fact, I
> think that .suspend() should ensure that they won't be executed for the
> given device after it has returned, so doing pm_runtime_disable() in the
> core at this point makes sense.
> 
> We really shouldn't allow any runtime PM callbacks to race with
> .suspend_noirq() and .resume_noirq(), because allowing that to happen would
> be asking for breakage.

Then you suggest:

	Call pm_runtime_disable after .suspend;

	Call pm_runtime_get_noresume and pm_runtime_enable before
	.resume;

	Call pm_runtime_put_sync after .complete.

Right?  Kevin, does this look okay?

Alan Stern

Re: Question about expected behavior when PM runtime is disabled

[Rafael J. Wysocki]

On Tuesday, June 14, 2011, Alan Stern wrote:
> On Mon, 13 Jun 2011, Rafael J. Wysocki wrote:
> 
> > On Monday, June 13, 2011, Alan Stern wrote:
> > > On Mon, 13 Jun 2011, Rafael J. Wysocki wrote:
> > > 
> > > > > I think we need to have the PM core call pm_runtime_get_noresume()  
> > > > > before invoking the resume_noirq (or thaw_noirq or restore_noirq)  
> > > > > callback, and then call pm_runtime_put_sync() after invoking the
> > > > > complete callback.  This would solve your race: The put_sync would 
> > > > > invoke the runtime_idle method, which would start another runtime 
> > > > > suspend or autosuspend.
> > > > > 
> > > > > (It used to be that the PM core called pm_runtime_get_noresume() 
> > > > > earlier on, before the prepare callback.  This also solved your race, 
> > > > > but it caused other problems and so was changed.)
> > > > > 
> > > > > It's true that subsystems could do this for themselves, but then they'd
> > > > > _all_ have to do it.  So we might as well put it in the PM core.
> > > > > 
> > > > > Rafael, what do you think?
> > > > 
> > > > Yes, we can do that.
> > > > 
> > > > I even suspect that all subsystems will end up calling pm_runtime_disable()
> > > > somewhere in the system suspend code path and pm_runtime_enable() during
> > > > system resume.  It might be a good idea to do that in the core too, after
> > > > calling the subsystem's .suspend() and before calling its .resume(),
> > > > respectively.
> > > 
> > > Will that bring back Kevin's problems?  There was a specific commit:  
> > > "PM: Allow pm_runtime_suspend() to succeed during system suspend".  If
> > > the core disables runtime PM, won't he be right back where he was
> > > before?
> > 
> > Not exactly, because that commit removed the pm_runtime_get_noresume()
> > done before .prepare(), which was too early.  As I said before, I don't
> > see anything wrong with running pm_runtime_ helpers from .prepare() or
> > .complete().  However, to me, it is highly doubtful if there is any valid
> > reason for calling them after .suspend() has been executed.  In fact, I
> > think that .suspend() should ensure that they won't be executed for the
> > given device after it has returned, so doing pm_runtime_disable() in the
> > core at this point makes sense.
> > 
> > We really shouldn't allow any runtime PM callbacks to race with
> > .suspend_noirq() and .resume_noirq(), because allowing that to happen would
> > be asking for breakage.
> 
> Then you suggest:
> 
> 	Call pm_runtime_disable after .suspend;
> 
> 	Call pm_runtime_get_noresume and pm_runtime_enable before
> 	.resume;
> 
> 	Call pm_runtime_put_sync after .complete.
> 
> Right?

Yes, that would be resonable IMO.

Thanks,
Rafael

Re: Question about expected behavior when PM runtime is disabled

[Alan Stern]

On Tue, 14 Jun 2011, Rafael J. Wysocki wrote:

> > Then you suggest:
> > 
> > 	Call pm_runtime_disable after .suspend;
> > 
> > 	Call pm_runtime_get_noresume and pm_runtime_enable before
> > 	.resume;
> > 
> > 	Call pm_runtime_put_sync after .complete.
> > 
> > Right?
> 
> Yes, that would be resonable IMO.

This turns out to be harder than it looks.  If an error occurs, we may
run the complete callback for devices that never were suspended or
resumed and hence never had their usage_count incremented.  How can we
tell that we need to skip the pm_runtime_put_sync for these devices?

Would it be okay to call pm_runtime_put_sync immediately after the
resume callback instead of after complete?

Alan Stern

Re: Question about expected behavior when PM runtime is disabled

[Rafael J. Wysocki]

On Friday, June 17, 2011, Alan Stern wrote:
> On Tue, 14 Jun 2011, Rafael J. Wysocki wrote:
> 
> > > Then you suggest:
> > > 
> > > 	Call pm_runtime_disable after .suspend;
> > > 
> > > 	Call pm_runtime_get_noresume and pm_runtime_enable before
> > > 	.resume;
> > > 
> > > 	Call pm_runtime_put_sync after .complete.
> > > 
> > > Right?
> > 
> > Yes, that would be resonable IMO.
> 
> This turns out to be harder than it looks.  If an error occurs, we may
> run the complete callback for devices that never were suspended or
> resumed and hence never had their usage_count incremented.  How can we
> tell that we need to skip the pm_runtime_put_sync for these devices?
> 
> Would it be okay to call pm_runtime_put_sync immediately after the
> resume callback instead of after complete?

Yes, it would.

That said we may be better off by simply reverting commit
e8665002477f0278f84f898145b1f141ba26ee26 (PM: Allow pm_runtime_suspend() to
succeed during system suspend).

Thanks,
Rafael

Re: Question about expected behavior when PM runtime is disabled

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Friday, June 17, 2011, Alan Stern wrote:
>> On Tue, 14 Jun 2011, Rafael J. Wysocki wrote:
>> 
>> > > Then you suggest:
>> > > 
>> > > 	Call pm_runtime_disable after .suspend;
>> > > 
>> > > 	Call pm_runtime_get_noresume and pm_runtime_enable before
>> > > 	.resume;
>> > > 
>> > > 	Call pm_runtime_put_sync after .complete.
>> > > 
>> > > Right?
>> > 
>> > Yes, that would be resonable IMO.
>> 
>> This turns out to be harder than it looks.  If an error occurs, we may
>> run the complete callback for devices that never were suspended or
>> resumed and hence never had their usage_count incremented.  How can we
>> tell that we need to skip the pm_runtime_put_sync for these devices?
>> 
>> Would it be okay to call pm_runtime_put_sync immediately after the
>> resume callback instead of after complete?
>
> Yes, it would.
>
> That said we may be better off by simply reverting commit
> e8665002477f0278f84f898145b1f141ba26ee26 (PM: Allow pm_runtime_suspend() to
> succeed during system suspend).

I'm OK with blocking runtime PM requests after .suspend (and unblocking
before .resume) e.g. protecting the _noirq callbacks, but I hope we
don't have to go back to blocking them before .prepare (and unblocking
after .complete)

Kevin

Re: Question about expected behavior when PM runtime is disabled

[Rafael J. Wysocki]

On Tuesday, June 21, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > On Friday, June 17, 2011, Alan Stern wrote:
> >> On Tue, 14 Jun 2011, Rafael J. Wysocki wrote:
> >> 
> >> > > Then you suggest:
> >> > > 
> >> > > 	Call pm_runtime_disable after .suspend;
> >> > > 
> >> > > 	Call pm_runtime_get_noresume and pm_runtime_enable before
> >> > > 	.resume;
> >> > > 
> >> > > 	Call pm_runtime_put_sync after .complete.
> >> > > 
> >> > > Right?
> >> > 
> >> > Yes, that would be resonable IMO.
> >> 
> >> This turns out to be harder than it looks.  If an error occurs, we may
> >> run the complete callback for devices that never were suspended or
> >> resumed and hence never had their usage_count incremented.  How can we
> >> tell that we need to skip the pm_runtime_put_sync for these devices?
> >> 
> >> Would it be okay to call pm_runtime_put_sync immediately after the
> >> resume callback instead of after complete?
> >
> > Yes, it would.
> >
> > That said we may be better off by simply reverting commit
> > e8665002477f0278f84f898145b1f141ba26ee26 (PM: Allow pm_runtime_suspend() to
> > succeed during system suspend).
> 
> I'm OK with blocking runtime PM requests after .suspend (and unblocking
> before .resume) e.g. protecting the _noirq callbacks, but I hope we
> don't have to go back to blocking them before .prepare (and unblocking
> after .complete)

We probably won't, but I'm not sure about .suspend() and .resume() yet.

Well, there are different things to take into consideration.  Please
see this message, for example: https://lkml.org/lkml/2011/6/20/328 (I should
have CCed it to you, sorry about that).

Thanks,
Rafael