Re: [PATCH v6 2/5] media: uvcvideo: Create uvc_pm_(get|put) functions

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Ricardo,

Thank you for the patch.

On Thu, Mar 27, 2025 at 09:05:28PM +0000, Ricardo Ribalda wrote:
> Most of the times that we have to call uvc_status_(get|put) we need to
> call the usb_autopm_ functions.
> 
> Create a new pair of functions that automate this for us. This
> simplifies the current code and future PM changes in the driver.
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 36 ++++++++++++++++++++++++------------
>  drivers/media/usb/uvc/uvcvideo.h |  4 ++++
>  2 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 22886b47d81c2cfd0a744f34a50d296d606e54e8..1d5be045d04ecbf17e65e14b390e494a294b735f 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -26,6 +26,27 @@
>  
>  #include "uvcvideo.h"
>  
> +int uvc_pm_get(struct uvc_device *dev)
> +{
> +	int ret;
> +
> +	ret = usb_autopm_get_interface(dev->intf);
> +	if (ret)
> +		return ret;
> +
> +	ret = uvc_status_get(dev);
> +	if (ret)
> +		usb_autopm_put_interface(dev->intf);
> +
> +	return ret;
> +}
> +
> +void uvc_pm_put(struct uvc_device *dev)
> +{
> +	uvc_status_put(dev);
> +	usb_autopm_put_interface(dev->intf);
> +}
> +
>  static int uvc_acquire_privileges(struct uvc_fh *handle);
>  
>  static int uvc_control_add_xu_mapping(struct uvc_video_chain *chain,
> @@ -642,20 +663,13 @@ static int uvc_v4l2_open(struct file *file)
>  	stream = video_drvdata(file);
>  	uvc_dbg(stream->dev, CALLS, "%s\n", __func__);
>  
> -	ret = usb_autopm_get_interface(stream->dev->intf);
> -	if (ret < 0)
> -		return ret;
> -
>  	/* Create the device handle. */
>  	handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> -	if (handle == NULL) {
> -		usb_autopm_put_interface(stream->dev->intf);
> +	if (!handle)
>  		return -ENOMEM;
> -	}
>  
> -	ret = uvc_status_get(stream->dev);
> +	ret = uvc_pm_get(stream->dev);
>  	if (ret) {
> -		usb_autopm_put_interface(stream->dev->intf);
>  		kfree(handle);
>  		return ret;
>  	}
> @@ -690,9 +704,7 @@ static int uvc_v4l2_release(struct file *file)
>  	kfree(handle);
>  	file->private_data = NULL;
>  
> -	uvc_status_put(stream->dev);
> -
> -	usb_autopm_put_interface(stream->dev->intf);
> +	uvc_pm_put(stream->dev);
>  	return 0;
>  }
>  
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 5ceb01e7831a83507550e1d3313e63da7494b2e4..b9f8eb62ba1d82ea7788cf6c10cc838a429dbc9e 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -768,6 +768,10 @@ void uvc_status_suspend(struct uvc_device *dev);
>  int uvc_status_get(struct uvc_device *dev);
>  void uvc_status_put(struct uvc_device *dev);
>  
> +/* PM */
> +int uvc_pm_get(struct uvc_device *dev);
> +void uvc_pm_put(struct uvc_device *dev);
> +
>  /* Controls */
>  extern const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops;
>  

-- 
Regards,

Laurent Pinchart