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

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

[Tomeu Vizoso]

v2:	* Let creators of the input device to decide whether it should remain
	runtime suspended when the system goes into a sleep state
	* Don't enable PM runtime on all evdev handlers
	* Cope with another wrong wakeup setting in usb_dev_prepare

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 device 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 (7):
  Input: Implement dev_pm_ops.prepare in input_class
  Input: Add input_dev.stay_runtime_suspended flag
  [media] uvcvideo: Set input_dev.stay_runtime_suspended flag
  [media] uvcvideo: Enable runtime PM of descendant devices
  [media] v4l2-core: Implement dev_pm_ops.prepare()
  [media] media-devnode: Implement dev_pm_ops.prepare callback
  USB / PM: Allow USB devices to remain runtime-suspended when sleeping

 drivers/input/input.c              | 20 ++++++++++++++++++++
 drivers/media/media-devnode.c      | 10 ++++++++++
 drivers/media/usb/uvc/uvc_driver.c | 11 +++++++++++
 drivers/media/usb/uvc/uvc_status.c |  1 +
 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             |  8 +++++++-
 include/linux/input.h              |  4 ++++
 10 files changed, 102 insertions(+), 1 deletion(-)

-- 
2.3.4

[PATCH v2 1/7] Input: Implement dev_pm_ops.prepare in input_class

[Tomeu Vizoso]

Have it return 1 so that input handlers don't block input devices from
remaining runtime suspended when the system goes into a sleep state. For
this to happen, the handler device needs to have runtime PM enabled.

This can make resume times considerably shorter because these devices
won't be resumed when the system is awaken.

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

diff --git a/drivers/input/input.c b/drivers/input/input.c
index cc357f1..9666309 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1748,6 +1748,15 @@ static const struct dev_pm_ops input_dev_pm_ops = {
 	.poweroff	= input_dev_poweroff,
 	.restore	= input_dev_resume,
 };
+
+static int input_class_prepare(struct device *dev)
+{
+	return 1;
+}
+
+static const struct dev_pm_ops input_class_pm_ops = {
+	.prepare	= input_class_prepare,
+};
 #endif /* CONFIG_PM */
 
 static struct device_type input_dev_type = {
@@ -1767,6 +1776,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 v2 2/7] Input: Add input_dev.stay_runtime_suspended flag

[Tomeu Vizoso]

It indicates that if the device is runtime suspended when the system
goes into a sleep state, it will be left in that state instead of
suspended and resumed.

It's useful for devices such as UVC that register an input device for
the buttons that they may have and that should remain runtime suspended
when the system suspends.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/input/input.c | 8 ++++++++
 include/linux/input.h | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 9666309..2bcc708 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1741,12 +1741,20 @@ static int input_dev_poweroff(struct device *dev)
 	return 0;
 }
 
+static int input_dev_prepare(struct device *dev)
+{
+	struct input_dev *input_dev = to_input_dev(dev);
+
+	return input_dev->stay_runtime_suspended;
+}
+
 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 int input_class_prepare(struct device *dev)
diff --git a/include/linux/input.h b/include/linux/input.h
index 82ce323..7466961 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -117,6 +117,9 @@ struct input_value {
  * @vals: array of values queued in the current frame
  * @devres_managed: indicates that devices is managed with devres framework
  *	and needs not be explicitly unregistered or freed.
+ * @stay_runtime_suspended: indicates that if the device is runtime suspended
+ *	when the system goes into a sleep state, it will be left in that state
+ *	instead of suspended and resumed
  */
 struct input_dev {
 	const char *name;
@@ -187,6 +190,7 @@ struct input_dev {
 	struct input_value *vals;
 
 	bool devres_managed;
+	bool stay_runtime_suspended;
 };
 #define to_input_dev(d) container_of(d, struct input_dev, dev)
 
-- 
2.3.4

[PATCH v2 3/7] [media] uvcvideo: Set input_dev.stay_runtime_suspended flag

[Tomeu Vizoso]

So that the UVC device can remain runtime suspended when the system goes
into a sleep state, let the input device do the same.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/media/usb/uvc/uvc_status.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index f552ab9..78647c0 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -39,6 +39,7 @@ static int uvc_input_init(struct uvc_device *dev)
 	input->phys = dev->input_phys;
 	usb_to_input_id(dev->udev, &input->id);
 	input->dev.parent = &dev->intf->dev;
+	input->stay_runtime_suspended = true;
 
 	__set_bit(EV_KEY, input->evbit);
 	__set_bit(KEY_CAMERA, input->keybit);
-- 
2.3.4

[PATCH v2 4/7] [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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index cf27006..687e5fb 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1855,6 +1855,15 @@ static int uvc_register_chains(struct uvc_device *dev)
 	return 0;
 }
 
+static int uvc_pm_runtime_enable(struct device *dev, void *data)
+{
+	pm_runtime_enable(dev);
+
+	device_for_each_child(dev, NULL, uvc_pm_runtime_enable);
+
+	return 0;
+}
+
 /* ------------------------------------------------------------------------
  * USB probe, disconnect, suspend and resume
  */
@@ -1959,6 +1968,8 @@ static int uvc_probe(struct usb_interface *intf,
 			"supported.\n", ret);
 	}
 
+	device_for_each_child(&dev->intf->dev, NULL, uvc_pm_runtime_enable);
+
 	uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
 	usb_enable_autosuspend(udev);
 	return 0;
-- 
2.3.4

[PATCH v2 5/7] [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 v2 6/7] [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 v2 7/7] 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, if their wakeup state is correct.

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      |  8 +++++++-
 4 files changed, 46 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 8d5b2f4..3a55c91 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -316,7 +316,13 @@ 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);
+
+	/* 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)
-- 
2.3.4

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

[Dmitry Torokhov]

Hi Tomeu,

On Fri, Apr 03, 2015 at 02:57:56PM +0200, 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, if their wakeup state is correct.
> 
> Also enable runtime PM for endpoints, which is another requirement for
> the above to work.

After patching I think the 4th unrelated subsystem with stubs for
prepare() I think it is pretty clear that this approach is not the right
one.

If your driver does not care about any children hanging off it there is
dev->ignore_children flag that either already does what you want, or
maybe needs adjusted to support your use case.

Thanks.

> 
> 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      |  8 +++++++-
>  4 files changed, 46 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 8d5b2f4..3a55c91 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -316,7 +316,13 @@ 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);
> +
> +	/* 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)
> -- 
> 2.3.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Dmitry

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

[Rafael J. Wysocki]

On Friday, April 03, 2015 01:44:36 PM Dmitry Torokhov wrote:
> Hi Tomeu,
> 
> On Fri, Apr 03, 2015 at 02:57:56PM +0200, 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, if their wakeup state is correct.
> > 
> > Also enable runtime PM for endpoints, which is another requirement for
> > the above to work.
> 
> After patching I think the 4th unrelated subsystem with stubs for
> prepare() I think it is pretty clear that this approach is not the right
> one.
> 
> If your driver does not care about any children hanging off it there is
> dev->ignore_children flag that either already does what you want, or
> maybe needs adjusted to support your use case.

I wouldn't overload ignore_children with that meaning.  That said adding
a "return 1 from device_prepare() if this is set" flag might be a good idea.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v2 4/7] [media] uvcvideo: Enable runtime PM of descendant devices

[Laurent Pinchart]

Hi Tomeu,

Thank you for the patch.

Could you please CC me on the whole series for v3 ?

On Friday 03 April 2015 14:57:53 Tomeu Vizoso wrote:
> 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 | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index cf27006..687e5fb 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1855,6 +1855,15 @@ static int uvc_register_chains(struct uvc_device
> *dev) return 0;
>  }
> 
> +static int uvc_pm_runtime_enable(struct device *dev, void *data)
> +{
> +	pm_runtime_enable(dev);
> +
> +	device_for_each_child(dev, NULL, uvc_pm_runtime_enable);

How many recursion levels do we typically have with uvcvideo ?

> +
> +	return 0;
> +}

The function isn't UVC-specific, how about renaming it to 
pm_runtime_enable_recursive() (or something similar) and moving it to the 
runtime PM core ?

> +
>  /* ------------------------------------------------------------------------
> * USB probe, disconnect, suspend and resume
>   */
> @@ -1959,6 +1968,8 @@ static int uvc_probe(struct usb_interface *intf,
>  			"supported.\n", ret);
>  	}
> 
> +	device_for_each_child(&dev->intf->dev, NULL, uvc_pm_runtime_enable);

You could just call uvc_pm_runtime_enable(&dev->intf->dev, NULL) here.

> +
>  	uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
>  	usb_enable_autosuspend(udev);
>  	return 0;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 4/7] [media] uvcvideo: Enable runtime PM of descendant devices

[Tomeu Vizoso]

On 4 April 2015 at 14:33, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Tomeu,
>
> Thank you for the patch.
>
> Could you please CC me on the whole series for v3 ?

Sure.

> On Friday 03 April 2015 14:57:53 Tomeu Vizoso wrote:
>> 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 | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_driver.c
>> b/drivers/media/usb/uvc/uvc_driver.c index cf27006..687e5fb 100644
>> --- a/drivers/media/usb/uvc/uvc_driver.c
>> +++ b/drivers/media/usb/uvc/uvc_driver.c
>> @@ -1855,6 +1855,15 @@ static int uvc_register_chains(struct uvc_device
>> *dev) return 0;
>>  }
>>
>> +static int uvc_pm_runtime_enable(struct device *dev, void *data)
>> +{
>> +     pm_runtime_enable(dev);
>> +
>> +     device_for_each_child(dev, NULL, uvc_pm_runtime_enable);
>
> How many recursion levels do we typically have with uvcvideo ?

it has video%d -> input%d -> event%d, when USB_VIDEO_CLASS_INPUT_EVDEV
is enabled.

>> +
>> +     return 0;
>> +}
>
> The function isn't UVC-specific, how about renaming it to
> pm_runtime_enable_recursive() (or something similar) and moving it to the
> runtime PM core ?

Yeah, that would be handy when doing the same to other drivers.

>> +
>>  /* ------------------------------------------------------------------------
>> * USB probe, disconnect, suspend and resume
>>   */
>> @@ -1959,6 +1968,8 @@ static int uvc_probe(struct usb_interface *intf,
>>                       "supported.\n", ret);
>>       }
>>
>> +     device_for_each_child(&dev->intf->dev, NULL, uvc_pm_runtime_enable);
>
> You could just call uvc_pm_runtime_enable(&dev->intf->dev, NULL) here.

I will go with the above for now.

Thanks,

Tomeu

>> +
>>       uvc_trace(UVC_TRACE_PROBE, "UVC device initialized.\n");
>>       usb_enable_autosuspend(udev);
>>       return 0;
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> 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 v2 5/7] [media] v4l2-core: Implement dev_pm_ops.prepare()

[Mauro Carvalho Chehab]

Hi,

Em Fri,  3 Apr 2015 14:57:54 +0200
Tomeu Vizoso <tomeu.vizoso@collabora.com> escreveu:

> 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.

I'm not a PM exprert, but that patch doesn't sound right. Not all devices
supported by v4l2-dev implement runtime suspend.

So, I guess this need to be done at driver level, not at core level.

> 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)