Since commit c93d73c9c2cf ("media: uvcvideo: Use vb2 ioctl and fop
helpers"), the IOCTLs are serialized. Due to this there is no more need
to protect ctrl, cur_format or cur_frame from concurrent access.
Drop stream->mutex after thanking it for years of good service.
Use this opportunity to do fix some CodeStyle.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_driver.c | 4 ----
drivers/media/usb/uvc/uvc_metadata.c | 10 +++------
drivers/media/usb/uvc/uvc_v4l2.c | 41 +++++++-----------------------------
drivers/media/usb/uvc/uvcvideo.h | 6 ------
4 files changed, 11 insertions(+), 50 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 775bede0d93d9b3e5391914aa395326d3de6a3b1..3039e6a533b82dd917050d416c9ced8756d69170 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -183,8 +183,6 @@ static void uvc_stream_delete(struct uvc_streaming *stream)
if (stream->async_wq)
destroy_workqueue(stream->async_wq);
- mutex_destroy(&stream->mutex);
-
usb_put_intf(stream->intf);
kfree(stream->formats);
@@ -201,8 +199,6 @@ static struct uvc_streaming *uvc_stream_new(struct uvc_device *dev,
if (stream == NULL)
return NULL;
- mutex_init(&stream->mutex);
-
stream->dev = dev;
stream->intf = usb_get_intf(intf);
stream->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 229e08ff323eed9129d835b24ea2e8085bb713b8..649844e2ad60ed9e9951daec871f2000f48702a6 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -100,16 +100,12 @@ static int uvc_meta_v4l2_set_format(struct file *file, void *fh,
* Metadata buffers would still be perfectly parseable, but it's more
* consistent and cleaner to disallow that.
*/
- mutex_lock(&stream->mutex);
-
if (vb2_is_busy(&stream->meta.queue.queue))
- ret = -EBUSY;
- else
- stream->meta.format = fmt->dataformat;
+ return -EBUSY;
- mutex_unlock(&stream->mutex);
+ stream->meta.format = fmt->dataformat;
- return ret;
+ return 0;
}
static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 160f9cf6e6dbdbf39e3eff56a5d5ea1d977fbe22..fdc4520a7bb42af7cd5cb9c1fa49957c31e0041c 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -329,14 +329,12 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
* developers test their webcams with the Linux driver as well as with
* the Windows driver).
*/
- mutex_lock(&stream->mutex);
if (stream->dev->quirks & UVC_QUIRK_PROBE_EXTRAFIELDS)
probe->dwMaxVideoFrameSize =
stream->ctrl.dwMaxVideoFrameSize;
/* Probe the device. */
ret = uvc_probe_video(stream, probe);
- mutex_unlock(&stream->mutex);
if (ret < 0)
return ret;
@@ -395,19 +393,15 @@ static int uvc_ioctl_g_fmt(struct file *file, void *fh,
struct uvc_streaming *stream = handle->stream;
const struct uvc_format *format;
const struct uvc_frame *frame;
- int ret = 0;
if (fmt->type != stream->type)
return -EINVAL;
- mutex_lock(&stream->mutex);
format = stream->cur_format;
frame = stream->cur_frame;
- if (format == NULL || frame == NULL) {
- ret = -EINVAL;
- goto done;
- }
+ if (!format || !frame)
+ return -EINVAL;
fmt->fmt.pix.pixelformat = format->fcc;
fmt->fmt.pix.width = frame->wWidth;
@@ -419,9 +413,7 @@ static int uvc_ioctl_g_fmt(struct file *file, void *fh,
fmt->fmt.pix.xfer_func = format->xfer_func;
fmt->fmt.pix.ycbcr_enc = format->ycbcr_enc;
-done:
- mutex_unlock(&stream->mutex);
- return ret;
+ return 0;
}
static int uvc_ioctl_s_fmt(struct file *file, void *fh,
@@ -441,19 +433,14 @@ static int uvc_ioctl_s_fmt(struct file *file, void *fh,
if (ret < 0)
return ret;
- mutex_lock(&stream->mutex);
- if (vb2_is_busy(&stream->queue.queue)) {
- ret = -EBUSY;
- goto done;
- }
+ if (vb2_is_busy(&stream->queue.queue))
+ return -EBUSY;
stream->ctrl = probe;
stream->cur_format = format;
stream->cur_frame = frame;
-done:
- mutex_unlock(&stream->mutex);
- return ret;
+ return 0;
}
static int uvc_ioctl_g_parm(struct file *file, void *fh,
@@ -466,10 +453,7 @@ static int uvc_ioctl_g_parm(struct file *file, void *fh,
if (parm->type != stream->type)
return -EINVAL;
- mutex_lock(&stream->mutex);
numerator = stream->ctrl.dwFrameInterval;
- mutex_unlock(&stream->mutex);
-
denominator = 10000000;
v4l2_simplify_fraction(&numerator, &denominator, 8, 333);
@@ -519,12 +503,8 @@ static int uvc_ioctl_s_parm(struct file *file, void *fh,
uvc_dbg(stream->dev, FORMAT, "Setting frame interval to %u/%u (%u)\n",
timeperframe.numerator, timeperframe.denominator, interval);
- mutex_lock(&stream->mutex);
-
- if (uvc_queue_streaming(&stream->queue)) {
- mutex_unlock(&stream->mutex);
+ if (uvc_queue_streaming(&stream->queue))
return -EBUSY;
- }
format = stream->cur_format;
frame = stream->cur_frame;
@@ -556,14 +536,11 @@ static int uvc_ioctl_s_parm(struct file *file, void *fh,
/* Probe the device with the new settings. */
ret = uvc_probe_video(stream, &probe);
- if (ret < 0) {
- mutex_unlock(&stream->mutex);
+ if (ret < 0)
return ret;
- }
stream->ctrl = probe;
stream->cur_frame = frame;
- mutex_unlock(&stream->mutex);
/* Return the actual frame period. */
timeperframe.numerator = probe.dwFrameInterval;
@@ -941,10 +918,8 @@ static int uvc_ioctl_g_selection(struct file *file, void *fh,
sel->r.left = 0;
sel->r.top = 0;
- mutex_lock(&stream->mutex);
sel->r.width = stream->cur_frame->wWidth;
sel->r.height = stream->cur_frame->wHeight;
- mutex_unlock(&stream->mutex);
return 0;
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 757254fc4fe930ae61c9d0425f04d4cd074a617e..86765b9d7935f0888476249c3fb826cd7f36b35c 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -469,12 +469,6 @@ struct uvc_streaming {
const struct uvc_format *cur_format;
const struct uvc_frame *cur_frame;
- /*
- * Protect access to ctrl, cur_format, cur_frame and hardware video
- * probe control.
- */
- struct mutex mutex;
-
/* Buffers queue. */
unsigned int frozen : 1;
struct uvc_video_queue queue;
--
2.50.1.470.g6ba607880d-googHi Ricardo,
Thank you for the patch.
On Fri, Jul 25, 2025 at 01:12:48PM +0000, Ricardo Ribalda wrote:
> Since commit c93d73c9c2cf ("media: uvcvideo: Use vb2 ioctl and fop
> helpers"), the IOCTLs are serialized. Due to this there is no more need
> to protect ctrl, cur_format or cur_frame from concurrent access.
>
> Drop stream->mutex after thanking it for years of good service.
>
> Use this opportunity to do fix some CodeStyle.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 4 ----
> drivers/media/usb/uvc/uvc_metadata.c | 10 +++------
> drivers/media/usb/uvc/uvc_v4l2.c | 41 +++++++-----------------------------
> drivers/media/usb/uvc/uvcvideo.h | 6 ------
> 4 files changed, 11 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 775bede0d93d9b3e5391914aa395326d3de6a3b1..3039e6a533b82dd917050d416c9ced8756d69170 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -183,8 +183,6 @@ static void uvc_stream_delete(struct uvc_streaming *stream)
> if (stream->async_wq)
> destroy_workqueue(stream->async_wq);
>
> - mutex_destroy(&stream->mutex);
> -
> usb_put_intf(stream->intf);
>
> kfree(stream->formats);
> @@ -201,8 +199,6 @@ static struct uvc_streaming *uvc_stream_new(struct uvc_device *dev,
> if (stream == NULL)
> return NULL;
>
> - mutex_init(&stream->mutex);
> -
> stream->dev = dev;
> stream->intf = usb_get_intf(intf);
> stream->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 229e08ff323eed9129d835b24ea2e8085bb713b8..649844e2ad60ed9e9951daec871f2000f48702a6 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -100,16 +100,12 @@ static int uvc_meta_v4l2_set_format(struct file *file, void *fh,
> * Metadata buffers would still be perfectly parseable, but it's more
> * consistent and cleaner to disallow that.
> */
> - mutex_lock(&stream->mutex);
> -
> if (vb2_is_busy(&stream->meta.queue.queue))
> - ret = -EBUSY;
> - else
> - stream->meta.format = fmt->dataformat;
> + return -EBUSY;
>
> - mutex_unlock(&stream->mutex);
> + stream->meta.format = fmt->dataformat;
>
> - return ret;
> + return 0;
> }
>
> static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 160f9cf6e6dbdbf39e3eff56a5d5ea1d977fbe22..fdc4520a7bb42af7cd5cb9c1fa49957c31e0041c 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -329,14 +329,12 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> * developers test their webcams with the Linux driver as well as with
> * the Windows driver).
> */
> - mutex_lock(&stream->mutex);
> if (stream->dev->quirks & UVC_QUIRK_PROBE_EXTRAFIELDS)
> probe->dwMaxVideoFrameSize =
> stream->ctrl.dwMaxVideoFrameSize;
>
> /* Probe the device. */
> ret = uvc_probe_video(stream, probe);
> - mutex_unlock(&stream->mutex);
> if (ret < 0)
> return ret;
>
> @@ -395,19 +393,15 @@ static int uvc_ioctl_g_fmt(struct file *file, void *fh,
> struct uvc_streaming *stream = handle->stream;
> const struct uvc_format *format;
> const struct uvc_frame *frame;
> - int ret = 0;
>
> if (fmt->type != stream->type)
> return -EINVAL;
>
> - mutex_lock(&stream->mutex);
> format = stream->cur_format;
> frame = stream->cur_frame;
>
> - if (format == NULL || frame == NULL) {
> - ret = -EINVAL;
> - goto done;
> - }
> + if (!format || !frame)
> + return -EINVAL;
>
> fmt->fmt.pix.pixelformat = format->fcc;
> fmt->fmt.pix.width = frame->wWidth;
> @@ -419,9 +413,7 @@ static int uvc_ioctl_g_fmt(struct file *file, void *fh,
> fmt->fmt.pix.xfer_func = format->xfer_func;
> fmt->fmt.pix.ycbcr_enc = format->ycbcr_enc;
>
> -done:
> - mutex_unlock(&stream->mutex);
> - return ret;
> + return 0;
> }
>
> static int uvc_ioctl_s_fmt(struct file *file, void *fh,
> @@ -441,19 +433,14 @@ static int uvc_ioctl_s_fmt(struct file *file, void *fh,
> if (ret < 0)
> return ret;
>
> - mutex_lock(&stream->mutex);
> - if (vb2_is_busy(&stream->queue.queue)) {
> - ret = -EBUSY;
> - goto done;
> - }
> + if (vb2_is_busy(&stream->queue.queue))
> + return -EBUSY;
>
> stream->ctrl = probe;
> stream->cur_format = format;
> stream->cur_frame = frame;
>
> -done:
> - mutex_unlock(&stream->mutex);
> - return ret;
> + return 0;
> }
>
> static int uvc_ioctl_g_parm(struct file *file, void *fh,
> @@ -466,10 +453,7 @@ static int uvc_ioctl_g_parm(struct file *file, void *fh,
> if (parm->type != stream->type)
> return -EINVAL;
>
> - mutex_lock(&stream->mutex);
> numerator = stream->ctrl.dwFrameInterval;
> - mutex_unlock(&stream->mutex);
> -
> denominator = 10000000;
> v4l2_simplify_fraction(&numerator, &denominator, 8, 333);
>
> @@ -519,12 +503,8 @@ static int uvc_ioctl_s_parm(struct file *file, void *fh,
> uvc_dbg(stream->dev, FORMAT, "Setting frame interval to %u/%u (%u)\n",
> timeperframe.numerator, timeperframe.denominator, interval);
>
> - mutex_lock(&stream->mutex);
> -
> - if (uvc_queue_streaming(&stream->queue)) {
> - mutex_unlock(&stream->mutex);
> + if (uvc_queue_streaming(&stream->queue))
> return -EBUSY;
> - }
>
> format = stream->cur_format;
> frame = stream->cur_frame;
> @@ -556,14 +536,11 @@ static int uvc_ioctl_s_parm(struct file *file, void *fh,
>
> /* Probe the device with the new settings. */
> ret = uvc_probe_video(stream, &probe);
> - if (ret < 0) {
> - mutex_unlock(&stream->mutex);
> + if (ret < 0)
> return ret;
> - }
>
> stream->ctrl = probe;
> stream->cur_frame = frame;
> - mutex_unlock(&stream->mutex);
>
> /* Return the actual frame period. */
> timeperframe.numerator = probe.dwFrameInterval;
> @@ -941,10 +918,8 @@ static int uvc_ioctl_g_selection(struct file *file, void *fh,
>
> sel->r.left = 0;
> sel->r.top = 0;
> - mutex_lock(&stream->mutex);
> sel->r.width = stream->cur_frame->wWidth;
> sel->r.height = stream->cur_frame->wHeight;
> - mutex_unlock(&stream->mutex);
>
> return 0;
> }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 757254fc4fe930ae61c9d0425f04d4cd074a617e..86765b9d7935f0888476249c3fb826cd7f36b35c 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -469,12 +469,6 @@ struct uvc_streaming {
> const struct uvc_format *cur_format;
> const struct uvc_frame *cur_frame;
>
> - /*
> - * Protect access to ctrl, cur_format, cur_frame and hardware video
> - * probe control.
> - */
> - struct mutex mutex;
> -
> /* Buffers queue. */
> unsigned int frozen : 1;
> struct uvc_video_queue queue;
--
Regards,
Laurent Pinchart
© 2016 - 2026 Red Hat, Inc.