| Message ID | 32970649.AWpGt5zezN@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ma, 2015-12-14 at 23:22 +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce a new runtime PM function, pm_runtime_get_if_in_use(), > that will increment the device's runtime PM usage counter and > return 'true' if its status is RPM_ACTIVE and its usage counter > is greater than 0 at the same time ('false' will be returned > otherwise). > > This is useful for things that should only be done if the device > is active (from the runtime PM perspective) and used by somebody > (as indicated by the usage counter) already and they are not worth > bothering otherwise. > > Requested-by: Imre Deak <imre.deak@intel.com> > Acked-by: Imre Deak <imre.deak@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > Documentation/power/runtime_pm.txt | 5 +++++ > drivers/base/power/runtime.c | 21 +++++++++++++++++++++ > include/linux/pm_runtime.h | 5 +++++ > 3 files changed, 31 insertions(+) > > Index: linux-pm/drivers/base/power/runtime.c > =================================================================== > --- linux-pm.orig/drivers/base/power/runtime.c > +++ linux-pm/drivers/base/power/runtime.c > @@ -966,6 +966,27 @@ int __pm_runtime_resume(struct device *d > EXPORT_SYMBOL_GPL(__pm_runtime_resume); > > /** > + * pm_runtime_get_if_in_use - Conditionally bump up the device's > usage counter. > + * @dev: Device to handle. > + * > + * Increment the device's runtime PM usage counter and return 'true' > if its > + * runtime PM status is RPM_ACTIVE and its usage counter is already > different > + * from zero at the same time. Otherwise, return 'false'. > + */ > +bool pm_runtime_get_if_in_use(struct device *dev) > +{ > + unsigned long flags; > + bool retval; > + > + spin_lock_irqsave(&dev->power.lock, flags); > + retval = dev->power.runtime_status == RPM_ACTIVE > + && atomic_inc_not_zero(&dev->power.usage_count); > + spin_unlock_irqrestore(&dev->power.lock, flags); > + return retval; > +} > +EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); > + > +/** > * __pm_runtime_set_status - Set runtime PM status of a device. > * @dev: Device to handle. > * @status: New runtime PM status of the device. > Index: linux-pm/include/linux/pm_runtime.h > =================================================================== > --- linux-pm.orig/include/linux/pm_runtime.h > +++ linux-pm/include/linux/pm_runtime.h > @@ -39,6 +39,7 @@ extern int pm_runtime_force_resume(struc > extern int __pm_runtime_idle(struct device *dev, int rpmflags); > extern int __pm_runtime_suspend(struct device *dev, int rpmflags); > extern int __pm_runtime_resume(struct device *dev, int rpmflags); > +extern bool pm_runtime_get_if_in_use(struct device *dev); > extern int pm_schedule_suspend(struct device *dev, unsigned int > delay); > extern int __pm_runtime_set_status(struct device *dev, unsigned int > status); > extern int pm_runtime_barrier(struct device *dev); > @@ -143,6 +144,10 @@ static inline int pm_schedule_suspend(st > { > return -ENOSYS; > } > +static inline bool pm_runtime_get_if_in_use(struct device *dev) > +{ > + return true; > +} > static inline int __pm_runtime_set_status(struct device *dev, > unsigned int status) { > return 0; } > static inline int pm_runtime_barrier(struct device *dev) { return 0; > } > Index: linux-pm/Documentation/power/runtime_pm.txt > =================================================================== > --- linux-pm.orig/Documentation/power/runtime_pm.txt > +++ linux-pm/Documentation/power/runtime_pm.txt > @@ -371,6 +371,11 @@ drivers/base/power/runtime.c and include > - increment the device's usage counter, run > pm_runtime_resume(dev) and > return its result > > + bool pm_runtime_get_if_in_use(struct device *dev); > + - increment the device's usage counter and return 'true' if its > runtime PM > + status is 'active' and its usage counter is greater than 0 at > the same > + time; return 'false' otherwise > + > void pm_runtime_put_noidle(struct device *dev); > - decrement the device's usage counter > >
On 14 December 2015 at 23:22, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce a new runtime PM function, pm_runtime_get_if_in_use(), As we already have pm_runtime_set_active() and pm_runtime_active(), changing the new function name to "pm_runtime_get_if_active" may be better!? > that will increment the device's runtime PM usage counter and > return 'true' if its status is RPM_ACTIVE and its usage counter > is greater than 0 at the same time ('false' will be returned > otherwise). > > This is useful for things that should only be done if the device > is active (from the runtime PM perspective) and used by somebody > (as indicated by the usage counter) already and they are not worth > bothering otherwise. > > Requested-by: Imre Deak <imre.deak@intel.com> > Acked-by: Imre Deak <imre.deak@intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > Documentation/power/runtime_pm.txt | 5 +++++ > drivers/base/power/runtime.c | 21 +++++++++++++++++++++ > include/linux/pm_runtime.h | 5 +++++ > 3 files changed, 31 insertions(+) > > Index: linux-pm/drivers/base/power/runtime.c > =================================================================== > --- linux-pm.orig/drivers/base/power/runtime.c > +++ linux-pm/drivers/base/power/runtime.c > @@ -966,6 +966,27 @@ int __pm_runtime_resume(struct device *d > EXPORT_SYMBOL_GPL(__pm_runtime_resume); > > /** > + * pm_runtime_get_if_in_use - Conditionally bump up the device's usage counter. > + * @dev: Device to handle. > + * > + * Increment the device's runtime PM usage counter and return 'true' if its > + * runtime PM status is RPM_ACTIVE and its usage counter is already different > + * from zero at the same time. Otherwise, return 'false'. > + */ > +bool pm_runtime_get_if_in_use(struct device *dev) > +{ > + unsigned long flags; > + bool retval; > + > + spin_lock_irqsave(&dev->power.lock, flags); > + retval = dev->power.runtime_status == RPM_ACTIVE > + && atomic_inc_not_zero(&dev->power.usage_count); Don't we also need to check that runtime PM is enabled (&& !dev->power.disable_depth), or the user of this function don't care about that? > + spin_unlock_irqrestore(&dev->power.lock, flags); > + return retval; > +} > +EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); > + [...] Kind regards Uffe
On Mon, 14 Dec 2015, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce a new runtime PM function, pm_runtime_get_if_in_use(), > that will increment the device's runtime PM usage counter and > return 'true' if its status is RPM_ACTIVE and its usage counter > is greater than 0 at the same time ('false' will be returned > otherwise). > > This is useful for things that should only be done if the device > is active (from the runtime PM perspective) and used by somebody > (as indicated by the usage counter) already and they are not worth > bothering otherwise. > --- linux-pm.orig/drivers/base/power/runtime.c > +++ linux-pm/drivers/base/power/runtime.c > @@ -966,6 +966,27 @@ int __pm_runtime_resume(struct device *d > EXPORT_SYMBOL_GPL(__pm_runtime_resume); > > /** > + * pm_runtime_get_if_in_use - Conditionally bump up the device's usage counter. > + * @dev: Device to handle. > + * > + * Increment the device's runtime PM usage counter and return 'true' if its > + * runtime PM status is RPM_ACTIVE and its usage counter is already different > + * from zero at the same time. Otherwise, return 'false'. The phrasing of this comment is slightly ambiguous (it's not clear whether the "if" clause applies to both the increment and the return or just the return). IMO it would be somewhat better to write: If the runtime PM status is RPM_ACTIVE and the runtime PM usage counter is nonzero, increment the counter and return 'true'. Otherwise return false without changing the counter. > --- linux-pm.orig/Documentation/power/runtime_pm.txt > +++ linux-pm/Documentation/power/runtime_pm.txt > @@ -371,6 +371,11 @@ drivers/base/power/runtime.c and include > - increment the device's usage counter, run pm_runtime_resume(dev) and > return its result > > + bool pm_runtime_get_if_in_use(struct device *dev); > + - increment the device's usage counter and return 'true' if its runtime PM > + status is 'active' and its usage counter is greater than 0 at the same > + time; return 'false' otherwise Same thing here. Alan Stern
On Tuesday, December 15, 2015 03:28:54 PM Ulf Hansson wrote: > On 14 December 2015 at 23:22, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Introduce a new runtime PM function, pm_runtime_get_if_in_use(), > > As we already have pm_runtime_set_active() and pm_runtime_active(), > changing the new function name to "pm_runtime_get_if_active" may be > better!? "In use" is supposed to mean "active and reference counted". > > that will increment the device's runtime PM usage counter and > > return 'true' if its status is RPM_ACTIVE and its usage counter > > is greater than 0 at the same time ('false' will be returned > > otherwise). > > > > This is useful for things that should only be done if the device > > is active (from the runtime PM perspective) and used by somebody > > (as indicated by the usage counter) already and they are not worth > > bothering otherwise. > > > > Requested-by: Imre Deak <imre.deak@intel.com> > > Acked-by: Imre Deak <imre.deak@intel.com> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > Documentation/power/runtime_pm.txt | 5 +++++ > > drivers/base/power/runtime.c | 21 +++++++++++++++++++++ > > include/linux/pm_runtime.h | 5 +++++ > > 3 files changed, 31 insertions(+) > > > > Index: linux-pm/drivers/base/power/runtime.c > > =================================================================== > > --- linux-pm.orig/drivers/base/power/runtime.c > > +++ linux-pm/drivers/base/power/runtime.c > > @@ -966,6 +966,27 @@ int __pm_runtime_resume(struct device *d > > EXPORT_SYMBOL_GPL(__pm_runtime_resume); > > > > /** > > + * pm_runtime_get_if_in_use - Conditionally bump up the device's usage counter. > > + * @dev: Device to handle. > > + * > > + * Increment the device's runtime PM usage counter and return 'true' if its > > + * runtime PM status is RPM_ACTIVE and its usage counter is already different > > + * from zero at the same time. Otherwise, return 'false'. > > + */ > > +bool pm_runtime_get_if_in_use(struct device *dev) > > +{ > > + unsigned long flags; > > + bool retval; > > + > > + spin_lock_irqsave(&dev->power.lock, flags); > > + retval = dev->power.runtime_status == RPM_ACTIVE > > + && atomic_inc_not_zero(&dev->power.usage_count); > > Don't we also need to check that runtime PM is enabled (&& > !dev->power.disable_depth), or the user of this function don't care > about that? The user probably cares, but calling this for devices with runtime PM disabled doesn't really make sense to me (the status is not meaningful then). > > + spin_unlock_irqrestore(&dev->power.lock, flags); > > + return retval; > > +} > > +EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); > > + > > [...] Thanks, Rafael
On Tuesday, December 15, 2015 10:06:33 AM Alan Stern wrote: > On Mon, 14 Dec 2015, Rafael J. Wysocki wrote: > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > Introduce a new runtime PM function, pm_runtime_get_if_in_use(), > > that will increment the device's runtime PM usage counter and > > return 'true' if its status is RPM_ACTIVE and its usage counter > > is greater than 0 at the same time ('false' will be returned > > otherwise). > > > > This is useful for things that should only be done if the device > > is active (from the runtime PM perspective) and used by somebody > > (as indicated by the usage counter) already and they are not worth > > bothering otherwise. > > > > --- linux-pm.orig/drivers/base/power/runtime.c > > +++ linux-pm/drivers/base/power/runtime.c > > @@ -966,6 +966,27 @@ int __pm_runtime_resume(struct device *d > > EXPORT_SYMBOL_GPL(__pm_runtime_resume); > > > > /** > > + * pm_runtime_get_if_in_use - Conditionally bump up the device's usage counter. > > + * @dev: Device to handle. > > + * > > + * Increment the device's runtime PM usage counter and return 'true' if its > > + * runtime PM status is RPM_ACTIVE and its usage counter is already different > > + * from zero at the same time. Otherwise, return 'false'. > > The phrasing of this comment is slightly ambiguous (it's not clear > whether the "if" clause applies to both the increment and the return > or just the return). IMO it would be somewhat better to write: > > If the runtime PM status is RPM_ACTIVE and the runtime PM usage > counter is nonzero, increment the counter and return 'true'. > Otherwise return false without changing the counter. Yes, that sounds better, thanks! I'll resend the patch with that modification. Thanks, Rafael
Index: linux-pm/drivers/base/power/runtime.c =================================================================== --- linux-pm.orig/drivers/base/power/runtime.c +++ linux-pm/drivers/base/power/runtime.c @@ -966,6 +966,27 @@ int __pm_runtime_resume(struct device *d EXPORT_SYMBOL_GPL(__pm_runtime_resume); /** + * pm_runtime_get_if_in_use - Conditionally bump up the device's usage counter. + * @dev: Device to handle. + * + * Increment the device's runtime PM usage counter and return 'true' if its + * runtime PM status is RPM_ACTIVE and its usage counter is already different + * from zero at the same time. Otherwise, return 'false'. + */ +bool pm_runtime_get_if_in_use(struct device *dev) +{ + unsigned long flags; + bool retval; + + spin_lock_irqsave(&dev->power.lock, flags); + retval = dev->power.runtime_status == RPM_ACTIVE + && atomic_inc_not_zero(&dev->power.usage_count); + spin_unlock_irqrestore(&dev->power.lock, flags); + return retval; +} +EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); + +/** * __pm_runtime_set_status - Set runtime PM status of a device. * @dev: Device to handle. * @status: New runtime PM status of the device. Index: linux-pm/include/linux/pm_runtime.h =================================================================== --- linux-pm.orig/include/linux/pm_runtime.h +++ linux-pm/include/linux/pm_runtime.h @@ -39,6 +39,7 @@ extern int pm_runtime_force_resume(struc extern int __pm_runtime_idle(struct device *dev, int rpmflags); extern int __pm_runtime_suspend(struct device *dev, int rpmflags); extern int __pm_runtime_resume(struct device *dev, int rpmflags); +extern bool pm_runtime_get_if_in_use(struct device *dev); extern int pm_schedule_suspend(struct device *dev, unsigned int delay); extern int __pm_runtime_set_status(struct device *dev, unsigned int status); extern int pm_runtime_barrier(struct device *dev); @@ -143,6 +144,10 @@ static inline int pm_schedule_suspend(st { return -ENOSYS; } +static inline bool pm_runtime_get_if_in_use(struct device *dev) +{ + return true; +} static inline int __pm_runtime_set_status(struct device *dev, unsigned int status) { return 0; } static inline int pm_runtime_barrier(struct device *dev) { return 0; } Index: linux-pm/Documentation/power/runtime_pm.txt =================================================================== --- linux-pm.orig/Documentation/power/runtime_pm.txt +++ linux-pm/Documentation/power/runtime_pm.txt @@ -371,6 +371,11 @@ drivers/base/power/runtime.c and include - increment the device's usage counter, run pm_runtime_resume(dev) and return its result + bool pm_runtime_get_if_in_use(struct device *dev); + - increment the device's usage counter and return 'true' if its runtime PM + status is 'active' and its usage counter is greater than 0 at the same + time; return 'false' otherwise + void pm_runtime_put_noidle(struct device *dev); - decrement the device's usage counter