[PATCH 0/6] Allow UVC devices to remain runtime-suspended when sleeping

[PATCH 0/6] Allow UVC devices to remain runtime-suspended when sleeping

[Tomeu Vizoso]

Hi,

this series contain what I needed to do in order to have my USB webcam to not be resumed when the system resumes, reducing considerably the total time that resuming takes.

It makes use of the facility that Rafael Wysocki added in aae4518b3 ("PM / sleep: Mechanism to avoid resuming runtime-suspended devices unnecessarily"), which requires that a devices and all its descendants opt-in by having their dev_pm_ops.prepare callback return 1, to have runtime PM enabled, and to be runtime suspended when the system goes to a sleep state.

Thanks,

Tomeu

Tomeu Vizoso (6):
  [media] uvcvideo: Enable runtime PM of descendant devices
  [media] v4l2-core: Implement dev_pm_ops.prepare()
  Input: Implement dev_pm_ops.prepare()
  [media] media-devnode: Implement dev_pm_ops.prepare callback
  Input: evdev - Enable runtime PM of the evdev input handler
  USB / PM: Allow USB devices to remain runtime-suspended when sleeping

 drivers/input/evdev.c              |  3 +++
 drivers/input/input.c              | 13 +++++++++++++
 drivers/media/media-devnode.c      | 10 ++++++++++
 drivers/media/usb/uvc/uvc_driver.c |  4 ++++
 drivers/media/usb/uvc/uvc_status.c |  3 +++
 drivers/media/v4l2-core/v4l2-dev.c | 10 ++++++++++
 drivers/usb/core/endpoint.c        | 17 +++++++++++++++++
 drivers/usb/core/message.c         | 16 ++++++++++++++++
 drivers/usb/core/port.c            |  6 ++++++
 drivers/usb/core/usb.c             |  2 +-
 10 files changed, 83 insertions(+), 1 deletion(-)

-- 
2.3.4

[PATCH 1/6] [media] uvcvideo: Enable runtime PM of descendant devices

[Tomeu Vizoso]

So UVC devices can remain runtime-suspended when the system goes into a
sleep state, they and all of their descendant devices need to have
runtime PM enable.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 4 ++++
 drivers/media/usb/uvc/uvc_status.c | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index cf27006..e98830a1 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1792,6 +1792,8 @@ static int uvc_register_video(struct uvc_device *dev,
 		return ret;
 	}
 
+	pm_runtime_enable(&vdev->dev);
+
 	if (stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		stream->chain->caps |= V4L2_CAP_VIDEO_CAPTURE;
 	else
@@ -1932,6 +1934,8 @@ static int uvc_probe(struct usb_interface *intf,
 	if (media_device_register(&dev->mdev) < 0)
 		goto error;
 
+	pm_runtime_enable(&dev->mdev.devnode.dev);
+
 	dev->vdev.mdev = &dev->mdev;
 #endif
 	if (v4l2_device_register(&intf->dev, &dev->vdev) < 0)
diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index f552ab9..b1d3d8c 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -13,6 +13,7 @@
 
 #include <linux/kernel.h>
 #include <linux/input.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
 #include <linux/usb/input.h>
@@ -46,6 +47,8 @@ static int uvc_input_init(struct uvc_device *dev)
 	if ((ret = input_register_device(input)) < 0)
 		goto error;
 
+	pm_runtime_enable(&input->dev);
+
 	dev->input = input;
 	return 0;
 
-- 
2.3.4

[PATCH 2/6] [media] v4l2-core: Implement dev_pm_ops.prepare()

[Tomeu Vizoso]

Have it return 1 so that video devices that are runtime-suspended won't
be suspended when the system goes to a sleep state. This can make resume
times considerably shorter because these devices don't need to be
resumed when the system is awaken.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/media/v4l2-core/v4l2-dev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index e2b8b3e..b74e3d3 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -219,9 +219,19 @@ static void v4l2_device_release(struct device *cd)
 		v4l2_device_put(v4l2_dev);
 }
 
+static int video_device_prepare(struct device *dev)
+{
+	return 1;
+}
+
+static const struct dev_pm_ops video_device_pm_ops = {
+	.prepare = video_device_prepare,
+};
+
 static struct class video_class = {
 	.name = VIDEO_NAME,
 	.dev_groups = video_device_groups,
+	.pm = &video_device_pm_ops,
 };
 
 struct video_device *video_devdata(struct file *file)
-- 
2.3.4

[PATCH 3/6] Input: Implement dev_pm_ops.prepare()

[Tomeu Vizoso]

Have it return 1 in both input_dev_type and input_class (for evdev
handlers) so that input devices that are runtime-suspended won't be
suspended when the system goes to a sleep state. This can make resume
times considerably shorter because these devices don't need to be
resumed when the system is awaken.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/input/input.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index cc357f1..cbbd391 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1741,12 +1741,22 @@ static int input_dev_poweroff(struct device *dev)
 	return 0;
 }
 
+static int input_dev_prepare(struct device *dev)
+{
+	return 1;
+}
+
 static const struct dev_pm_ops input_dev_pm_ops = {
 	.suspend	= input_dev_suspend,
 	.resume		= input_dev_resume,
 	.freeze		= input_dev_freeze,
 	.poweroff	= input_dev_poweroff,
 	.restore	= input_dev_resume,
+	.prepare	= input_dev_prepare,
+};
+
+static const struct dev_pm_ops input_class_pm_ops = {
+	.prepare	= input_dev_prepare,
 };
 #endif /* CONFIG_PM */
 
@@ -1767,6 +1777,9 @@ static char *input_devnode(struct device *dev, umode_t *mode)
 struct class input_class = {
 	.name		= "input",
 	.devnode	= input_devnode,
+#ifdef CONFIG_PM_SLEEP
+	.pm		= &input_class_pm_ops,
+#endif
 };
 EXPORT_SYMBOL_GPL(input_class);
 
-- 
2.3.4

[PATCH 4/6] [media] media-devnode: Implement dev_pm_ops.prepare callback

[Tomeu Vizoso]

Have it return 1 so that media device nodes that are runtime-suspended
won't be suspended when the system goes to a sleep state. This can make
resume times considerably shorter because these devices don't need to be
resumed when the system is awaken.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/media/media-devnode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index ebf9626..2c36d0a 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -76,8 +76,18 @@ static void media_devnode_release(struct device *cd)
 		mdev->release(mdev);
 }
 
+static int media_bus_prepare(struct device *dev)
+{
+	return 1;
+}
+
+static const struct dev_pm_ops media_bus_pm_ops = {
+	.prepare = media_bus_prepare,
+};
+
 static struct bus_type media_bus_type = {
 	.name = MEDIA_NAME,
+	.pm = &media_bus_pm_ops,
 };
 
 static ssize_t media_read(struct file *filp, char __user *buf,
-- 
2.3.4

[PATCH 5/6] Input: evdev - Enable runtime PM of the evdev input handler

[Tomeu Vizoso]

So ancestor devices can remain runtime-suspended when the system goes
into a sleep state, they and all of their descendant devices need to
have runtime PM enabled.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/input/evdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index a18f41b..3d60c20 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -26,6 +26,7 @@
 #include <linux/major.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/pm_runtime.h>
 #include "input-compat.h"
 
 enum evdev_clock_type {
@@ -1201,6 +1202,8 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
 	if (error)
 		goto err_cleanup_evdev;
 
+	pm_runtime_enable(&evdev->dev);
+
 	return 0;
 
  err_cleanup_evdev:
-- 
2.3.4

[PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Tomeu Vizoso]

Have dev_pm_ops.prepare return 1 for USB devices, interfaces, endpoints
and ports so that USB devices can remain runtime-suspended when the
system goes to a sleep state.

Also enable runtime PM for endpoints, which is another requirement for
the above to work.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/usb/core/endpoint.c | 17 +++++++++++++++++
 drivers/usb/core/message.c  | 16 ++++++++++++++++
 drivers/usb/core/port.c     |  6 ++++++
 drivers/usb/core/usb.c      |  2 +-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/endpoint.c b/drivers/usb/core/endpoint.c
index 39a2402..7c82bb7 100644
--- a/drivers/usb/core/endpoint.c
+++ b/drivers/usb/core/endpoint.c
@@ -160,6 +160,19 @@ static const struct attribute_group *ep_dev_groups[] = {
 	NULL
 };
 
+#ifdef	CONFIG_PM
+
+static int usb_ep_device_prepare(struct device *dev)
+{
+	return 1;
+}
+
+static const struct dev_pm_ops usb_ep_device_pm_ops = {
+	.prepare =	usb_ep_device_prepare,
+};
+
+#endif	/* CONFIG_PM */
+
 static void ep_device_release(struct device *dev)
 {
 	struct ep_device *ep_dev = to_ep_device(dev);
@@ -170,6 +183,9 @@ static void ep_device_release(struct device *dev)
 struct device_type usb_ep_device_type = {
 	.name =		"usb_endpoint",
 	.release = ep_device_release,
+#ifdef CONFIG_PM
+	.pm =		&usb_ep_device_pm_ops,
+#endif
 };
 
 int usb_create_ep_devs(struct device *parent,
@@ -197,6 +213,7 @@ int usb_create_ep_devs(struct device *parent,
 		goto error_register;
 
 	device_enable_async_suspend(&ep_dev->dev);
+	pm_runtime_enable(&ep_dev->dev);
 	endpoint->ep_dev = ep_dev;
 	return retval;
 
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f368d20..9041aee 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1589,10 +1589,26 @@ static int usb_if_uevent(struct device *dev, struct kobj_uevent_env *env)
 	return 0;
 }
 
+#ifdef	CONFIG_PM
+
+static int usb_if_prepare(struct device *dev)
+{
+	return 1;
+}
+
+static const struct dev_pm_ops usb_if_pm_ops = {
+	.prepare =	usb_if_prepare,
+};
+
+#endif	/* CONFIG_PM */
+
 struct device_type usb_if_device_type = {
 	.name =		"usb_interface",
 	.release =	usb_release_interface,
 	.uevent =	usb_if_uevent,
+#ifdef CONFIG_PM
+	.pm =		&usb_if_pm_ops,
+#endif
 };
 
 static struct usb_interface_assoc_descriptor *find_iad(struct usb_device *dev,
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 2106183..f49707d 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 b1fb9ae..1498faa 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -300,7 +300,7 @@ 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? */
+	return 1;		/* Implement eventually? */
 }
 
 static void usb_dev_complete(struct device *dev)
-- 
2.3.4

Re: [PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Alan Stern]

On Tue, 31 Mar 2015, Tomeu Vizoso wrote:

> Have dev_pm_ops.prepare return 1 for USB devices, interfaces, endpoints
> and ports so that USB devices can remain runtime-suspended when the
> system goes to a sleep state.
> 
> Also enable runtime PM for endpoints, which is another requirement for
> the above to work.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

...

> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index b1fb9ae..1498faa 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -300,7 +300,7 @@ 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? */
> +	return 1;		/* Implement eventually? */
>  }

The rest of the patch is okay, but this part is wrong.  The documented
requirement is that the prepare callback should return 1 if the device
is already in the appropriate state.  This means it has to have the
right wakeup setting.

In other words, if the device is currently in runtime suspend with 
remote wakeup enabled, but device_may_wakeup() returns 0 (so that the 
device should be disabled for wakeup when the system goes into 
suspend), then the prepare callback has to return 0.

Therefore what you need to do here is something like this:

	struct usb_device	*udev = to_usb_device(dev);

	/* Return 0 if the current wakeup setting is wrong, otherwise 1 */
	if (udev->do_remote_wakeup && !device_may_wakeup(dev))
		return 0;
	return 1;

Aside from this issue, I like the patch set.  Do you think you can do 
something similar for drivers/scsi/scsi_pm.c?  I'm not aware of any 
wakeup-capable SCSI devices -- not disk or CD/DVD drives, anyway.

Alan Stern

Re: [PATCH 5/6] Input: evdev - Enable runtime PM of the evdev input handler

[Dmitry Torokhov]

On Tue, Mar 31, 2015 at 06:14:49PM +0200, Tomeu Vizoso wrote:
> So ancestor devices can remain runtime-suspended when the system goes
> into a sleep state, they and all of their descendant devices need to
> have runtime PM enabled.

I am confused. Input devices are not runtime-PM-enabled, so what
enabling this on evdev handler buys us? And what about joydev and
mousedev? Other handlers that might be attached?

The stubbing of prepare also feels wrong: we do want to suspend/resume
input devices since we want to shut off and restore their leds even if
device (keyboard) happens to be sleeping.

Thanks.

> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>  drivers/input/evdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index a18f41b..3d60c20 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -26,6 +26,7 @@
>  #include <linux/major.h>
>  #include <linux/device.h>
>  #include <linux/cdev.h>
> +#include <linux/pm_runtime.h>
>  #include "input-compat.h"
>  
>  enum evdev_clock_type {
> @@ -1201,6 +1202,8 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
>  	if (error)
>  		goto err_cleanup_evdev;
>  
> +	pm_runtime_enable(&evdev->dev);
> +
>  	return 0;
>  
>   err_cleanup_evdev:
> -- 
> 2.3.4
> 

-- 
Dmitry

Re: [PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Oliver Neukum]

On Tue, 2015-03-31 at 13:09 -0400, Alan Stern wrote:
> In other words, if the device is currently in runtime suspend with 
> remote wakeup enabled, but device_may_wakeup() returns 0 (so that the 
> device should be disabled for wakeup when the system goes into 
> suspend), then the prepare callback has to return 0.
> 
> Therefore what you need to do here is something like this:
> 
>         struct usb_device       *udev = to_usb_device(dev);
> 
>         /* Return 0 if the current wakeup setting is wrong, otherwise
> 1 */

And the other way round?

	Regards
		Oliver

Re: [PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Alan Stern]

On Wed, 1 Apr 2015, Oliver Neukum wrote:

> On Tue, 2015-03-31 at 13:09 -0400, Alan Stern wrote:
> > In other words, if the device is currently in runtime suspend with 
> > remote wakeup enabled, but device_may_wakeup() returns 0 (so that the 
> > device should be disabled for wakeup when the system goes into 
> > suspend), then the prepare callback has to return 0.
> > 
> > Therefore what you need to do here is something like this:
> > 
> >         struct usb_device       *udev = to_usb_device(dev);
> > 
> >         /* Return 0 if the current wakeup setting is wrong, otherwise
> > 1 */
> 
> And the other way round?

Your meaning isn't clear.  Are you asking what should happen if the 
device is in runtime suspend with remote wakeup disabled, but 
device_may_wakeup() returns 1?  That case should never happen -- but 
if it does then the prepare callback should return 0.

Alan Stern

Re: [PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Oliver Neukum]

On Wed, 2015-04-01 at 10:01 -0400, Alan Stern wrote:
> On Wed, 1 Apr 2015, Oliver Neukum wrote:
> 
> > On Tue, 2015-03-31 at 13:09 -0400, Alan Stern wrote:
> > > In other words, if the device is currently in runtime suspend with 
> > > remote wakeup enabled, but device_may_wakeup() returns 0 (so that the 
> > > device should be disabled for wakeup when the system goes into 
> > > suspend), then the prepare callback has to return 0.
> > > 
> > > Therefore what you need to do here is something like this:
> > > 
> > >         struct usb_device       *udev = to_usb_device(dev);
> > > 
> > >         /* Return 0 if the current wakeup setting is wrong, otherwise
> > > 1 */
> > 
> > And the other way round?
> 
> Your meaning isn't clear.  Are you asking what should happen if the 
> device is in runtime suspend with remote wakeup disabled, but 
> device_may_wakeup() returns 1?

Yes. I was thinking about that case.

>   That case should never happen -- but 
> if it does then the prepare callback should return 0.

Exactly. It didn't seem to do so.

	Regards
		Oliver


	

Re: [PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Alan Stern]

On Wed, 1 Apr 2015, Oliver Neukum wrote:

> On Wed, 2015-04-01 at 10:01 -0400, Alan Stern wrote:
> > On Wed, 1 Apr 2015, Oliver Neukum wrote:
> > 
> > > On Tue, 2015-03-31 at 13:09 -0400, Alan Stern wrote:
> > > > In other words, if the device is currently in runtime suspend with 
> > > > remote wakeup enabled, but device_may_wakeup() returns 0 (so that the 
> > > > device should be disabled for wakeup when the system goes into 
> > > > suspend), then the prepare callback has to return 0.
> > > > 
> > > > Therefore what you need to do here is something like this:
> > > > 
> > > >         struct usb_device       *udev = to_usb_device(dev);
> > > > 
> > > >         /* Return 0 if the current wakeup setting is wrong, otherwise
> > > > 1 */
> > > 
> > > And the other way round?
> > 
> > Your meaning isn't clear.  Are you asking what should happen if the 
> > device is in runtime suspend with remote wakeup disabled, but 
> > device_may_wakeup() returns 1?
> 
> Yes. I was thinking about that case.
> 
> >   That case should never happen -- but 
> > if it does then the prepare callback should return 0.
> 
> Exactly. It didn't seem to do so.

Well, Tomeu can fix it up to handle both cases properly.

Alan Stern

Re: [PATCH 5/6] Input: evdev - Enable runtime PM of the evdev input handler

[Tomeu Vizoso]

On 31 March 2015 at 22:31, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Tue, Mar 31, 2015 at 06:14:49PM +0200, Tomeu Vizoso wrote:
>> So ancestor devices can remain runtime-suspended when the system goes
>> into a sleep state, they and all of their descendant devices need to
>> have runtime PM enabled.
>
> I am confused. Input devices are not runtime-PM-enabled, so what
> enabling this on evdev handler buys us?

The UVC driver would be enabling runtime PM for the input device that
it registers.

> And what about joydev and
> mousedev? Other handlers that might be attached?

Not sure, tbh. What I have ended up doing in v2 is descending the tree
from the UVC device and enabling runtime PM for all descendants. The
idea is that the UVC driver knows whether it should remain runtime
suspended and that its descendants should copy it.

> The stubbing of prepare also feels wrong: we do want to suspend/resume
> input devices since we want to shut off and restore their leds even if
> device (keyboard) happens to be sleeping.

Fair enough, I have introduced a flag that allows the creator of the
input device to decide that.

Thanks,

Tomeu

> Thanks.
>
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>  drivers/input/evdev.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>> index a18f41b..3d60c20 100644
>> --- a/drivers/input/evdev.c
>> +++ b/drivers/input/evdev.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/major.h>
>>  #include <linux/device.h>
>>  #include <linux/cdev.h>
>> +#include <linux/pm_runtime.h>
>>  #include "input-compat.h"
>>
>>  enum evdev_clock_type {
>> @@ -1201,6 +1202,8 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,
>>       if (error)
>>               goto err_cleanup_evdev;
>>
>> +     pm_runtime_enable(&evdev->dev);
>> +
>>       return 0;
>>
>>   err_cleanup_evdev:
>> --
>> 2.3.4
>>
>
> --
> Dmitry
> --
> 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/

Re: [PATCH 6/6] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

[Tomeu Vizoso]

On 31 March 2015 at 19:09, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Tue, 31 Mar 2015, Tomeu Vizoso wrote:
>
>> Have dev_pm_ops.prepare return 1 for USB devices, interfaces, endpoints
>> and ports so that USB devices can remain runtime-suspended when the
>> system goes to a sleep state.
>>
>> Also enable runtime PM for endpoints, which is another requirement for
>> the above to work.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>
> ...
>
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index b1fb9ae..1498faa 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -300,7 +300,7 @@ 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? */
>> +     return 1;               /* Implement eventually? */
>>  }
>
> The rest of the patch is okay, but this part is wrong.  The documented
> requirement is that the prepare callback should return 1 if the device
> is already in the appropriate state.  This means it has to have the
> right wakeup setting.
>
> In other words, if the device is currently in runtime suspend with
> remote wakeup enabled, but device_may_wakeup() returns 0 (so that the
> device should be disabled for wakeup when the system goes into
> suspend), then the prepare callback has to return 0.
>
> Therefore what you need to do here is something like this:
>
>         struct usb_device       *udev = to_usb_device(dev);
>
>         /* Return 0 if the current wakeup setting is wrong, otherwise 1 */
>         if (udev->do_remote_wakeup && !device_may_wakeup(dev))
>                 return 0;
>         return 1;

Thanks, in v2 I have also covered the other case, as suggested by Oliver.

> Aside from this issue, I like the patch set.  Do you think you can do
> something similar for drivers/scsi/scsi_pm.c?  I'm not aware of any
> wakeup-capable SCSI devices -- not disk or CD/DVD drives, anyway.

I think I will be able to allocate some time on monday to look at this.

Thanks,

Tomeu

> Alan Stern
>
> --
> 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/