| Message ID | 20160202213916.GA19432@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 2, 2016 at 10:39 PM, Tony Lindgren <tony@atomide.com> wrote: > * Ulf Hansson <ulf.hansson@linaro.org> [160202 12:25]: >> > >> > ? pm_runtime_put_sync() _already_ does not respect the autosuspend >> > mode. If you want to respect it, you have to call >> > pm_runtime_put_sync_autosuspend() instead. >> >> Then there's a bug in the runtime PM core. >> >> From Tony's regression report and from mine own local runtime PM test >> driver, I can see that the device doesn't get RPM_SUSPENDED (the >> ->runtime_suspend() callback isn't called), even when the usage count >> is zero - when pm_runtime_put_sync() is called. > ... >> Okay, so you are saying that the pm_runtime_put_sync() should idle the >> device even if autosuspend is in use. That seems reasonable, I will >> look into this problem. > > The patch below fixes pm_runtime_put_sync() to not respect the > autosuspend mode to match what Alan is saying above. Seems to also > fixes the $subject issue for me. And seems to behave for PM runtime > for other devices during runtime too based on light testing here. > > Regards, > > Tony > > 8< --------------- > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -353,7 +353,9 @@ static int rpm_idle(struct device *dev, int rpmflags) > > out: > trace_rpm_return_int(dev, _THIS_IP_, retval); > - return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO); > + if (!(rpmflags & RPM_IGNORE_AUTO)) > + rpmflags |= RPM_AUTO; > + return retval ? retval : rpm_suspend(dev, rpmflags); > } > > /** > --- a/include/linux/pm_runtime.h > +++ b/include/linux/pm_runtime.h > @@ -23,6 +23,8 @@ > usage_count */ > #define RPM_AUTO 0x08 /* Use autosuspend_delay */ > > +#define RPM_IGNORE_AUTO 0x10 /* Ignore autosuspend */ > + > #ifdef CONFIG_PM > extern struct workqueue_struct *pm_wq; > > @@ -241,7 +243,7 @@ static inline int pm_runtime_put_autosuspend(struct device *dev) > > static inline int pm_runtime_put_sync(struct device *dev) > { > - return __pm_runtime_idle(dev, RPM_GET_PUT); > + return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_IGNORE_AUTO); > } > > static inline int pm_runtime_put_sync_suspend(struct device *dev) This changes a well-documented behavior that someone may be relying on. Not the safest thing to do I have to say. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Rafael J. Wysocki <rafael@kernel.org> [160203 05:04]: > On Tue, Feb 2, 2016 at 10:39 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Ulf Hansson <ulf.hansson@linaro.org> [160202 12:25]: > >> > > >> > ? pm_runtime_put_sync() _already_ does not respect the autosuspend > >> > mode. If you want to respect it, you have to call > >> > pm_runtime_put_sync_autosuspend() instead. > >> > >> Then there's a bug in the runtime PM core. > >> > >> From Tony's regression report and from mine own local runtime PM test > >> driver, I can see that the device doesn't get RPM_SUSPENDED (the > >> ->runtime_suspend() callback isn't called), even when the usage count > >> is zero - when pm_runtime_put_sync() is called. > > ... > >> Okay, so you are saying that the pm_runtime_put_sync() should idle the > >> device even if autosuspend is in use. That seems reasonable, I will > >> look into this problem. > > > > The patch below fixes pm_runtime_put_sync() to not respect the > > autosuspend mode to match what Alan is saying above. Seems to also > > fixes the $subject issue for me. And seems to behave for PM runtime > > for other devices during runtime too based on light testing here. > > > > Regards, > > > > Tony > > > > 8< --------------- > > --- a/drivers/base/power/runtime.c > > +++ b/drivers/base/power/runtime.c > > @@ -353,7 +353,9 @@ static int rpm_idle(struct device *dev, int rpmflags) > > > > out: > > trace_rpm_return_int(dev, _THIS_IP_, retval); > > - return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO); > > + if (!(rpmflags & RPM_IGNORE_AUTO)) > > + rpmflags |= RPM_AUTO; > > + return retval ? retval : rpm_suspend(dev, rpmflags); > > } > > > > /** > > --- a/include/linux/pm_runtime.h > > +++ b/include/linux/pm_runtime.h > > @@ -23,6 +23,8 @@ > > usage_count */ > > #define RPM_AUTO 0x08 /* Use autosuspend_delay */ > > > > +#define RPM_IGNORE_AUTO 0x10 /* Ignore autosuspend */ > > + > > #ifdef CONFIG_PM > > extern struct workqueue_struct *pm_wq; > > > > @@ -241,7 +243,7 @@ static inline int pm_runtime_put_autosuspend(struct device *dev) > > > > static inline int pm_runtime_put_sync(struct device *dev) > > { > > - return __pm_runtime_idle(dev, RPM_GET_PUT); > > + return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_IGNORE_AUTO); > > } > > > > static inline int pm_runtime_put_sync_suspend(struct device *dev) > > This changes a well-documented behavior that someone may be relying on. > > Not the safest thing to do I have to say. Yup fine with me. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -353,7 +353,9 @@ static int rpm_idle(struct device *dev, int rpmflags) out: trace_rpm_return_int(dev, _THIS_IP_, retval); - return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO); + if (!(rpmflags & RPM_IGNORE_AUTO)) + rpmflags |= RPM_AUTO; + return retval ? retval : rpm_suspend(dev, rpmflags); } /** --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -23,6 +23,8 @@ usage_count */ #define RPM_AUTO 0x08 /* Use autosuspend_delay */ +#define RPM_IGNORE_AUTO 0x10 /* Ignore autosuspend */ + #ifdef CONFIG_PM extern struct workqueue_struct *pm_wq; @@ -241,7 +243,7 @@ static inline int pm_runtime_put_autosuspend(struct device *dev) static inline int pm_runtime_put_sync(struct device *dev) { - return __pm_runtime_idle(dev, RPM_GET_PUT); + return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_IGNORE_AUTO); } static inline int pm_runtime_put_sync_suspend(struct device *dev)