Re: [PATCH 1/1] PM / Runtime: let rpm_resume fail if rpm disabled and device suspended.

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Monday, June 16, 2014 01:40:05 PM Alan Stern wrote:
> On Sat, 14 Jun 2014, Allen Yu wrote:
> 
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -608,7 +608,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
> >   repeat:
> >  	if (dev->power.runtime_error)
> >  		retval = -EINVAL;
> > -	else if (dev->power.disable_depth == 1 && dev->power.is_suspended
> > +	else if (dev->power.disable_depth == 1 && !dev->power.is_suspended
> >  	    && dev->power.runtime_status == RPM_ACTIVE)
> >  		retval = 1;
> 
> For reasons having nothing to do with Allen's suggested change, I
> wonder if we shouldn't replace this line with something like:
> 
> -	else if (dev->power.disable_depth == 1 && dev->power.is_suspended
> +	else if (dev->power.disable > 0 && !dev->power.is_suspended
> 	    && dev->power.runtime_status == RPM_ACTIVE)
> 		retval = 1;
> 
> It seems that I've been bitten by this several times in the past.  
> When a device is disabled for runtime PM, and more or less permanently
> stuck in the RPM_ACTIVE state, calls to pm_runtime_resume() or
> pm_runtime_get_sync() shouldn't fail.
> 
> For example, suppose some devices of a certain type support runtime 
> power management but others don't.  We naturally want to call 
> pm_runtime_disable() for the ones that don't.  But we also want the 
> same driver to work for all the devices, which means that 
> pm_runtime_get_sync() should return success -- otherwise the driver 
> will think that something has gone wrong.
> 
> Rafael, what do you think?

That condition is there specifically to take care of the system suspend
code path.  It means that if runtime PM is disabled, but it only has been
disabled by the system suspend code path, we should treat the device as
"active" (ie. return 1).  That won't work after the proposed change.

I guess drivers that want to work with devices where runtime PM may be
disabled can just check the return value of rpm_resume() for -EACCES?

Rafael