| Message ID | 1436964008-15151-4-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wednesday, July 15, 2015 02:40:08 PM Tomeu Vizoso wrote: > Have dev_pm_ops.prepare return 1 for USB devices and ports so that USB > devices can remain runtime-suspended when the system goes to a sleep > state, if their wakeup state is correct and they have runtime PM enabled. > > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > --- > > drivers/usb/core/port.c | 6 ++++++ > drivers/usb/core/usb.c | 11 ++++++++++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > index 210618319f10..f49707d73b5a 100644 > --- a/drivers/usb/core/port.c > +++ b/drivers/usb/core/port.c > @@ -168,12 +168,18 @@ static int usb_port_runtime_suspend(struct device *dev) > > return retval; > } > + > +static int usb_port_prepare(struct device *dev) > +{ > + return 1; > +} > #endif > > static const struct dev_pm_ops usb_port_pm_ops = { > #ifdef CONFIG_PM > .runtime_suspend = usb_port_runtime_suspend, > .runtime_resume = usb_port_runtime_resume, > + .prepare = usb_port_prepare, > #endif > }; > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c > index 8d5b2f4113cd..cf4dde11db1c 100644 > --- a/drivers/usb/core/usb.c > +++ b/drivers/usb/core/usb.c > @@ -316,7 +316,16 @@ static int usb_dev_uevent(struct device *dev, struct kobj_uevent_env *env) > > static int usb_dev_prepare(struct device *dev) > { > - return 0; /* Implement eventually? */ > + struct usb_device *udev = to_usb_device(dev); > + > + if (!pm_runtime_enabled(dev)) Why just enabled and not suspended? > + return 0; > + > + /* Return 0 if the current wakeup setting is wrong, otherwise 1 */ > + if (udev->do_remote_wakeup != device_may_wakeup(dev)) > + return 0; > + > + return 1; > } > > static void usb_dev_complete(struct device *dev) >
On 16 July 2015 at 02:42, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Wednesday, July 15, 2015 02:40:08 PM Tomeu Vizoso wrote: >> Have dev_pm_ops.prepare return 1 for USB devices and ports so that USB >> devices can remain runtime-suspended when the system goes to a sleep >> state, if their wakeup state is correct and they have runtime PM enabled. >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> --- >> >> drivers/usb/core/port.c | 6 ++++++ >> drivers/usb/core/usb.c | 11 ++++++++++- >> 2 files changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c >> index 210618319f10..f49707d73b5a 100644 >> --- a/drivers/usb/core/port.c >> +++ b/drivers/usb/core/port.c >> @@ -168,12 +168,18 @@ static int usb_port_runtime_suspend(struct device *dev) >> >> return retval; >> } >> + >> +static int usb_port_prepare(struct device *dev) >> +{ >> + return 1; >> +} >> #endif >> >> static const struct dev_pm_ops usb_port_pm_ops = { >> #ifdef CONFIG_PM >> .runtime_suspend = usb_port_runtime_suspend, >> .runtime_resume = usb_port_runtime_resume, >> + .prepare = usb_port_prepare, >> #endif >> }; >> >> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c >> index 8d5b2f4113cd..cf4dde11db1c 100644 >> --- a/drivers/usb/core/usb.c >> +++ b/drivers/usb/core/usb.c >> @@ -316,7 +316,16 @@ static int usb_dev_uevent(struct device *dev, struct kobj_uevent_env *env) >> >> static int usb_dev_prepare(struct device *dev) >> { >> - return 0; /* Implement eventually? */ >> + struct usb_device *udev = to_usb_device(dev); >> + >> + if (!pm_runtime_enabled(dev)) > > Why just enabled and not suspended? Hmm, the core checks if it's runtime suspended before going direct_complete, but it's true that the API docs say that it should return a positive value only if it's runtime suspended. Is there a reason why the prepare() implementations have to check that instead of leaving it to the core? Thanks, Tomeu >> + return 0; >> + >> + /* Return 0 if the current wakeup setting is wrong, otherwise 1 */ >> + if (udev->do_remote_wakeup != device_may_wakeup(dev)) >> + return 0; >> + >> + return 1; >> } >> >> static void usb_dev_complete(struct device *dev) >> > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, July 16, 2015 02:09:41 PM Tomeu Vizoso wrote: > On 16 July 2015 at 02:42, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Wednesday, July 15, 2015 02:40:08 PM Tomeu Vizoso wrote: > >> Have dev_pm_ops.prepare return 1 for USB devices and ports so that USB > >> devices can remain runtime-suspended when the system goes to a sleep > >> state, if their wakeup state is correct and they have runtime PM enabled. > >> > >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> > >> --- > >> > >> drivers/usb/core/port.c | 6 ++++++ > >> drivers/usb/core/usb.c | 11 ++++++++++- > >> 2 files changed, 16 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > >> index 210618319f10..f49707d73b5a 100644 > >> --- a/drivers/usb/core/port.c > >> +++ b/drivers/usb/core/port.c > >> @@ -168,12 +168,18 @@ static int usb_port_runtime_suspend(struct device *dev) > >> > >> return retval; > >> } > >> + > >> +static int usb_port_prepare(struct device *dev) > >> +{ > >> + return 1; > >> +} > >> #endif > >> > >> static const struct dev_pm_ops usb_port_pm_ops = { > >> #ifdef CONFIG_PM > >> .runtime_suspend = usb_port_runtime_suspend, > >> .runtime_resume = usb_port_runtime_resume, > >> + .prepare = usb_port_prepare, > >> #endif > >> }; > >> > >> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c > >> index 8d5b2f4113cd..cf4dde11db1c 100644 > >> --- a/drivers/usb/core/usb.c > >> +++ b/drivers/usb/core/usb.c > >> @@ -316,7 +316,16 @@ static int usb_dev_uevent(struct device *dev, struct kobj_uevent_env *env) > >> > >> static int usb_dev_prepare(struct device *dev) > >> { > >> - return 0; /* Implement eventually? */ > >> + struct usb_device *udev = to_usb_device(dev); > >> + > >> + if (!pm_runtime_enabled(dev)) > > > > Why just enabled and not suspended? > > Hmm, the core checks if it's runtime suspended before going > direct_complete, but it's true that the API docs say that it should > return a positive value only if it's runtime suspended. > > Is there a reason why the prepare() implementations have to check that > instead of leaving it to the core? The concern is that they generally need to check the state of the device which is meaningless from the suspend suitability (so to speak) perspective if the device is not (runtime) suspended when the check is made. But this particular case seems to be exceptional as ->prepare doesn't realy check the device's (hardware) state, but only its software configuration (if I'm not mistaken). So checking the RPM status is not needed here.
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 210618319f10..f49707d73b5a 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -168,12 +168,18 @@ static int usb_port_runtime_suspend(struct device *dev) return retval; } + +static int usb_port_prepare(struct device *dev) +{ + return 1; +} #endif static const struct dev_pm_ops usb_port_pm_ops = { #ifdef CONFIG_PM .runtime_suspend = usb_port_runtime_suspend, .runtime_resume = usb_port_runtime_resume, + .prepare = usb_port_prepare, #endif }; diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 8d5b2f4113cd..cf4dde11db1c 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -316,7 +316,16 @@ static int usb_dev_uevent(struct device *dev, struct kobj_uevent_env *env) static int usb_dev_prepare(struct device *dev) { - return 0; /* Implement eventually? */ + struct usb_device *udev = to_usb_device(dev); + + if (!pm_runtime_enabled(dev)) + return 0; + + /* Return 0 if the current wakeup setting is wrong, otherwise 1 */ + if (udev->do_remote_wakeup != device_may_wakeup(dev)) + return 0; + + return 1; } static void usb_dev_complete(struct device *dev)
Have dev_pm_ops.prepare return 1 for USB devices and ports so that USB devices can remain runtime-suspended when the system goes to a sleep state, if their wakeup state is correct and they have runtime PM enabled. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- drivers/usb/core/port.c | 6 ++++++ drivers/usb/core/usb.c | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-)