[PATCH 0/3] media: uvcvideo: Misc fixes

[PATCH 0/3] media: uvcvideo: Misc fixes

[Laurent Pinchart]

Hello,

A small series of assorted fixes resulting from recent reviews. Nothing
much to mention here, please see individual patches for details.

Laurent Pinchart (3):
  media: uvcvideo: Drop unneeded memset() in meta device ioctl handlers
  media: uvcvideo: Add missing curly braces
  media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

 drivers/media/usb/uvc/uvc_metadata.c | 23 ++++++++---------------
 include/linux/usb/uvc.h              | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 15 deletions(-)


base-commit: d968e50b5c26642754492dea23cbd3592bde62d8
prerequisite-patch-id: 0037ebb12937e902b333498ec926b7422f00c5ae
-- 
Regards,

Laurent Pinchart

[PATCH 1/3] media: uvcvideo: Drop unneeded memset() in meta device ioctl handlers

[Laurent Pinchart]

The .vidioc_g_fmt_meta_cap() and .vidioc_enum_fmt_meta_cap() ioctl
handlers for meta capture devices memset the ioctl argument structure to
zero. This is unnecessary as the memory is already zeroed by the V4L2
ioctl core. Drop the memset(), which, in uvc_meta_v4l2_enum_formats(),
also allows further simplification as structure fields don't need to be
saved and restored.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_metadata.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 229e08ff323e..b68bfb2d47df 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -49,8 +49,6 @@ static int uvc_meta_v4l2_get_format(struct file *file, void *fh,
 	if (format->type != vfh->vdev->queue->type)
 		return -EINVAL;
 
-	memset(fmt, 0, sizeof(*fmt));
-
 	fmt->dataformat = stream->meta.format;
 	fmt->buffersize = UVC_METADATA_BUF_SIZE;
 
@@ -118,19 +116,14 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
 	struct v4l2_fh *vfh = file->private_data;
 	struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
 	struct uvc_device *dev = stream->dev;
-	u32 i = fdesc->index;
 
 	if (fdesc->type != vfh->vdev->queue->type)
 		return -EINVAL;
 
-	if (i >= dev->nmeta_formats)
+	if (fdesc->index >= dev->nmeta_formats)
 		return -EINVAL;
 
-	memset(fdesc, 0, sizeof(*fdesc));
-
-	fdesc->type = vfh->vdev->queue->type;
-	fdesc->index = i;
-	fdesc->pixelformat = dev->meta_formats[i];
+	fdesc->pixelformat = dev->meta_formats[fdesc->index];
 
 	return 0;
 }
-- 
Regards,

Laurent Pinchart

[PATCH 2/3] media: uvcvideo: Add missing curly braces

[Laurent Pinchart]

The uvc_meta_v4l2_try_format() function is missing curly braces on an
outer for loop statement to comply with the driver coding style. Add
them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_metadata.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index b68bfb2d47df..386b04a3a102 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -67,11 +67,12 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
 	if (format->type != vfh->vdev->queue->type)
 		return -EINVAL;
 
-	for (unsigned int i = 0; i < dev->nmeta_formats; i++)
+	for (unsigned int i = 0; i < dev->nmeta_formats; i++) {
 		if (dev->meta_formats[i] == fmt->dataformat) {
 			fmeta = fmt->dataformat;
 			break;
 		}
+	}
 
 	memset(fmt, 0, sizeof(*fmt));
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/3] media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

[Laurent Pinchart]

Move the MSXU_CONTROL_METADATA control definitino to the
include/linux/usb/uvc.h header, alongside the corresponding XU GUID. Add
a UVC_ prefix to avoid namespace clashes.

While at it, add the definition for the other controls for that
extension unit, as defined in
https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_metadata.c |  9 ++++-----
 include/linux/usb/uvc.h              | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 386b04a3a102..57f5455a726c 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -171,7 +171,6 @@ static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
 	return NULL;
 }
 
-#define MSXU_CONTROL_METADATA 0x9
 static int uvc_meta_detect_msxu(struct uvc_device *dev)
 {
 	u32 *data __free(kfree) = NULL;
@@ -192,7 +191,7 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
 
 	/* Check if the metadata is already enabled. */
 	ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
-			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (ret)
 		return 0;
 
@@ -207,16 +206,16 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
 	 * us, the value from GET_MAX seems to work all the time.
 	 */
 	ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
-			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (ret || !*data)
 		return 0;
 
 	/*
-	 * If we can set MSXU_CONTROL_METADATA, the device will report
+	 * If we can set UVC_MSXU_CONTROL_METADATA, the device will report
 	 * metadata.
 	 */
 	ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
-			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (!ret)
 		dev->quirks |= UVC_QUIRK_MSXU_META;
 
diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
index ee19e9f915b8..72fff9463c88 100644
--- a/include/linux/usb/uvc.h
+++ b/include/linux/usb/uvc.h
@@ -33,6 +33,22 @@
 	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
 	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
 
+#define UVC_MSXU_CONTROL_FOCUS			0x01
+#define UVC_MSXU_CONTROL_EXPOSURE		0x02
+#define UVC_MSXU_CONTROL_EVCOMPENSATION		0x03
+#define UVC_MSXU_CONTROL_WHITEBALANCE		0x04
+#define UVC_MSXU_CONTROL_FACE_AUTHENTICATION	0x06
+#define UVC_MSXU_CONTROL_CAMERA_EXTRINSICS	0x07
+#define UVC_MSXU_CONTROL_CAMERA_INTRINSICS	0x08
+#define UVC_MSXU_CONTROL_METADATA		0x09
+#define UVC_MSXU_CONTROL_IR_TORCH		0x0a
+#define UVC_MSXU_CONTROL_DIGITALWINDOW		0x0b
+#define UVC_MSXU_CONTROL_DIGITALWINDOW_CONFIG	0x0c
+#define UVC_MSXU_CONTROL_VIDEO_HDR		0x0d
+#define UVC_MSXU_CONTROL_FRAMERATE_THROTTLE	0x0e
+#define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG	0x0f
+#define UVC_MSXU_CONTROL_FIELDOFVIEW2		0x10
+
 #define UVC_GUID_FORMAT_MJPEG \
 	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
 	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

[Ricardo Ribalda]

Hi Laurent

On Tue, 15 Jul 2025 at 20:53, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Move the MSXU_CONTROL_METADATA control definitino to the
> include/linux/usb/uvc.h header, alongside the corresponding XU GUID. Add
> a UVC_ prefix to avoid namespace clashes.
>
> While at it, add the definition for the other controls for that
> extension unit, as defined in
> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.
>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/usb/uvc/uvc_metadata.c |  9 ++++-----
>  include/linux/usb/uvc.h              | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 386b04a3a102..57f5455a726c 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -171,7 +171,6 @@ static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
>         return NULL;
>  }
>
> -#define MSXU_CONTROL_METADATA 0x9
>  static int uvc_meta_detect_msxu(struct uvc_device *dev)
>  {
>         u32 *data __free(kfree) = NULL;
> @@ -192,7 +191,7 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
>
>         /* Check if the metadata is already enabled. */
>         ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> -                            MSXU_CONTROL_METADATA, data, sizeof(*data));
> +                            UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>         if (ret)
>                 return 0;
>
> @@ -207,16 +206,16 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
>          * us, the value from GET_MAX seems to work all the time.
>          */
>         ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> -                            MSXU_CONTROL_METADATA, data, sizeof(*data));
> +                            UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>         if (ret || !*data)
>                 return 0;
>
>         /*
> -        * If we can set MSXU_CONTROL_METADATA, the device will report
> +        * If we can set UVC_MSXU_CONTROL_METADATA, the device will report
>          * metadata.
>          */
>         ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> -                            MSXU_CONTROL_METADATA, data, sizeof(*data));
> +                            UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>         if (!ret)
>                 dev->quirks |= UVC_QUIRK_MSXU_META;
>
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index ee19e9f915b8..72fff9463c88 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -33,6 +33,22 @@
>         {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
>          0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
>

Maybe you want to add a link to MS doc here?
https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.
> +#define UVC_MSXU_CONTROL_FOCUS                 0x01
> +#define UVC_MSXU_CONTROL_EXPOSURE              0x02
> +#define UVC_MSXU_CONTROL_EVCOMPENSATION                0x03
> +#define UVC_MSXU_CONTROL_WHITEBALANCE          0x04
> +#define UVC_MSXU_CONTROL_FACE_AUTHENTICATION   0x06
> +#define UVC_MSXU_CONTROL_CAMERA_EXTRINSICS     0x07
> +#define UVC_MSXU_CONTROL_CAMERA_INTRINSICS     0x08
> +#define UVC_MSXU_CONTROL_METADATA              0x09
> +#define UVC_MSXU_CONTROL_IR_TORCH              0x0a
> +#define UVC_MSXU_CONTROL_DIGITALWINDOW         0x0b
> +#define UVC_MSXU_CONTROL_DIGITALWINDOW_CONFIG  0x0c
> +#define UVC_MSXU_CONTROL_VIDEO_HDR             0x0d
> +#define UVC_MSXU_CONTROL_FRAMERATE_THROTTLE    0x0e
> +#define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG   0x0f
> +#define UVC_MSXU_CONTROL_FIELDOFVIEW2          0x10
> +
>  #define UVC_GUID_FORMAT_MJPEG \
>         { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
>          0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> --
> Regards,
>
> Laurent Pinchart
>


-- 
Ricardo Ribalda

Re: [PATCH 2/3] media: uvcvideo: Add missing curly braces

[Ricardo Ribalda]

Hi Laurent

If you use a linter we could add it to media-ci to make sure that the
code follows your style.

On Tue, 15 Jul 2025 at 20:53, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The uvc_meta_v4l2_try_format() function is missing curly braces on an
> outer for loop statement to comply with the driver coding style. Add
> them.
>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_metadata.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index b68bfb2d47df..386b04a3a102 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -67,11 +67,12 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
>         if (format->type != vfh->vdev->queue->type)
>                 return -EINVAL;
>
> -       for (unsigned int i = 0; i < dev->nmeta_formats; i++)
> +       for (unsigned int i = 0; i < dev->nmeta_formats; i++) {
>                 if (dev->meta_formats[i] == fmt->dataformat) {
>                         fmeta = fmt->dataformat;
>                         break;
>                 }
> +       }
>
>         memset(fmt, 0, sizeof(*fmt));
>
> --
> Regards,
>
> Laurent Pinchart
>


-- 
Ricardo Ribalda

Re: [PATCH 1/3] media: uvcvideo: Drop unneeded memset() in meta device ioctl handlers

[Ricardo Ribalda]

On Tue, 15 Jul 2025 at 20:53, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The .vidioc_g_fmt_meta_cap() and .vidioc_enum_fmt_meta_cap() ioctl
> handlers for meta capture devices memset the ioctl argument structure to
> zero. This is unnecessary as the memory is already zeroed by the V4L2
> ioctl core. Drop the memset(), which, in uvc_meta_v4l2_enum_formats(),
> also allows further simplification as structure fields don't need to be
> saved and restored.
>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/usb/uvc/uvc_metadata.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 229e08ff323e..b68bfb2d47df 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -49,8 +49,6 @@ static int uvc_meta_v4l2_get_format(struct file *file, void *fh,
>         if (format->type != vfh->vdev->queue->type)
>                 return -EINVAL;
>
> -       memset(fmt, 0, sizeof(*fmt));
> -
>         fmt->dataformat = stream->meta.format;
>         fmt->buffersize = UVC_METADATA_BUF_SIZE;
>
> @@ -118,19 +116,14 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
>         struct v4l2_fh *vfh = file->private_data;
>         struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
>         struct uvc_device *dev = stream->dev;
> -       u32 i = fdesc->index;
>
>         if (fdesc->type != vfh->vdev->queue->type)
>                 return -EINVAL;
>
> -       if (i >= dev->nmeta_formats)
> +       if (fdesc->index >= dev->nmeta_formats)
>                 return -EINVAL;
>
> -       memset(fdesc, 0, sizeof(*fdesc));
> -
> -       fdesc->type = vfh->vdev->queue->type;
> -       fdesc->index = i;
> -       fdesc->pixelformat = dev->meta_formats[i];
> +       fdesc->pixelformat = dev->meta_formats[fdesc->index];
>
>         return 0;
>  }
> --
> Regards,
>
> Laurent Pinchart
>


-- 
Ricardo Ribalda

Re: [PATCH 2/3] media: uvcvideo: Add missing curly braces

[Laurent Pinchart]

On Wed, Jul 16, 2025 at 12:47:58PM +0200, Ricardo Ribalda wrote:
> Hi Laurent
> 
> If you use a linter we could add it to media-ci to make sure that the
> code follows your style.

I currently don't, it's all wired in the OCD part of my brain I suppose
:-)

> On Tue, 15 Jul 2025 at 20:53, Laurent Pinchart wrote:
> >
> > The uvc_meta_v4l2_try_format() function is missing curly braces on an
> > outer for loop statement to comply with the driver coding style. Add
> > them.
> 
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  drivers/media/usb/uvc/uvc_metadata.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> > index b68bfb2d47df..386b04a3a102 100644
> > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > @@ -67,11 +67,12 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
> >         if (format->type != vfh->vdev->queue->type)
> >                 return -EINVAL;
> >
> > -       for (unsigned int i = 0; i < dev->nmeta_formats; i++)
> > +       for (unsigned int i = 0; i < dev->nmeta_formats; i++) {
> >                 if (dev->meta_formats[i] == fmt->dataformat) {
> >                         fmeta = fmt->dataformat;
> >                         break;
> >                 }
> > +       }
> >
> >         memset(fmt, 0, sizeof(*fmt));
> >

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] media: uvcvideo: Drop unneeded memset() in meta device ioctl handlers

[Hans de Goede]

Hi,

On 15-Jul-25 8:52 PM, Laurent Pinchart wrote:
> The .vidioc_g_fmt_meta_cap() and .vidioc_enum_fmt_meta_cap() ioctl
> handlers for meta capture devices memset the ioctl argument structure to
> zero. This is unnecessary as the memory is already zeroed by the V4L2
> ioctl core. Drop the memset(), which, in uvc_meta_v4l2_enum_formats(),
> also allows further simplification as structure fields don't need to be
> saved and restored.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
>  drivers/media/usb/uvc/uvc_metadata.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 229e08ff323e..b68bfb2d47df 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -49,8 +49,6 @@ static int uvc_meta_v4l2_get_format(struct file *file, void *fh,
>  	if (format->type != vfh->vdev->queue->type)
>  		return -EINVAL;
>  
> -	memset(fmt, 0, sizeof(*fmt));
> -
>  	fmt->dataformat = stream->meta.format;
>  	fmt->buffersize = UVC_METADATA_BUF_SIZE;
>  
> @@ -118,19 +116,14 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
>  	struct v4l2_fh *vfh = file->private_data;
>  	struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
>  	struct uvc_device *dev = stream->dev;
> -	u32 i = fdesc->index;
>  
>  	if (fdesc->type != vfh->vdev->queue->type)
>  		return -EINVAL;
>  
> -	if (i >= dev->nmeta_formats)
> +	if (fdesc->index >= dev->nmeta_formats)
>  		return -EINVAL;
>  
> -	memset(fdesc, 0, sizeof(*fdesc));
> -
> -	fdesc->type = vfh->vdev->queue->type;
> -	fdesc->index = i;
> -	fdesc->pixelformat = dev->meta_formats[i];
> +	fdesc->pixelformat = dev->meta_formats[fdesc->index];
>  
>  	return 0;
>  }

Re: [PATCH 2/3] media: uvcvideo: Add missing curly braces

[Hans de Goede]

Hi,

On 15-Jul-25 8:52 PM, Laurent Pinchart wrote:
> The uvc_meta_v4l2_try_format() function is missing curly braces on an
> outer for loop statement to comply with the driver coding style. Add
> them.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans


> ---
>  drivers/media/usb/uvc/uvc_metadata.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index b68bfb2d47df..386b04a3a102 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -67,11 +67,12 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
>  	if (format->type != vfh->vdev->queue->type)
>  		return -EINVAL;
>  
> -	for (unsigned int i = 0; i < dev->nmeta_formats; i++)
> +	for (unsigned int i = 0; i < dev->nmeta_formats; i++) {
>  		if (dev->meta_formats[i] == fmt->dataformat) {
>  			fmeta = fmt->dataformat;
>  			break;
>  		}
> +	}
>  
>  	memset(fmt, 0, sizeof(*fmt));
>  

Re: [PATCH 3/3] media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

[Hans de Goede]

Hi,

On 15-Jul-25 8:52 PM, Laurent Pinchart wrote:
> Move the MSXU_CONTROL_METADATA control definitino to the
> include/linux/usb/uvc.h header, alongside the corresponding XU GUID. Add
> a UVC_ prefix to avoid namespace clashes.
> 
> While at it, add the definition for the other controls for that
> extension unit, as defined in
> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



>
> ---
>  drivers/media/usb/uvc/uvc_metadata.c |  9 ++++-----
>  include/linux/usb/uvc.h              | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 386b04a3a102..57f5455a726c 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -171,7 +171,6 @@ static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
>  	return NULL;
>  }
>  
> -#define MSXU_CONTROL_METADATA 0x9
>  static int uvc_meta_detect_msxu(struct uvc_device *dev)
>  {
>  	u32 *data __free(kfree) = NULL;
> @@ -192,7 +191,7 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
>  
>  	/* Check if the metadata is already enabled. */
>  	ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> -			     MSXU_CONTROL_METADATA, data, sizeof(*data));
> +			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>  	if (ret)
>  		return 0;
>  
> @@ -207,16 +206,16 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
>  	 * us, the value from GET_MAX seems to work all the time.
>  	 */
>  	ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> -			     MSXU_CONTROL_METADATA, data, sizeof(*data));
> +			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>  	if (ret || !*data)
>  		return 0;
>  
>  	/*
> -	 * If we can set MSXU_CONTROL_METADATA, the device will report
> +	 * If we can set UVC_MSXU_CONTROL_METADATA, the device will report
>  	 * metadata.
>  	 */
>  	ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> -			     MSXU_CONTROL_METADATA, data, sizeof(*data));
> +			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>  	if (!ret)
>  		dev->quirks |= UVC_QUIRK_MSXU_META;
>  
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index ee19e9f915b8..72fff9463c88 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -33,6 +33,22 @@
>  	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
>  	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
>  
> +#define UVC_MSXU_CONTROL_FOCUS			0x01
> +#define UVC_MSXU_CONTROL_EXPOSURE		0x02
> +#define UVC_MSXU_CONTROL_EVCOMPENSATION		0x03
> +#define UVC_MSXU_CONTROL_WHITEBALANCE		0x04
> +#define UVC_MSXU_CONTROL_FACE_AUTHENTICATION	0x06
> +#define UVC_MSXU_CONTROL_CAMERA_EXTRINSICS	0x07
> +#define UVC_MSXU_CONTROL_CAMERA_INTRINSICS	0x08
> +#define UVC_MSXU_CONTROL_METADATA		0x09
> +#define UVC_MSXU_CONTROL_IR_TORCH		0x0a
> +#define UVC_MSXU_CONTROL_DIGITALWINDOW		0x0b
> +#define UVC_MSXU_CONTROL_DIGITALWINDOW_CONFIG	0x0c
> +#define UVC_MSXU_CONTROL_VIDEO_HDR		0x0d
> +#define UVC_MSXU_CONTROL_FRAMERATE_THROTTLE	0x0e
> +#define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG	0x0f
> +#define UVC_MSXU_CONTROL_FIELDOFVIEW2		0x10
> +
>  #define UVC_GUID_FORMAT_MJPEG \
>  	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
>  	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}

Re: [PATCH 3/3] media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

[Hans de Goede]

Hi,

On 16-Jul-25 12:46 PM, Ricardo Ribalda wrote:
> Hi Laurent
> 
> On Tue, 15 Jul 2025 at 20:53, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Move the MSXU_CONTROL_METADATA control definitino to the
>> include/linux/usb/uvc.h header, alongside the corresponding XU GUID. Add
>> a UVC_ prefix to avoid namespace clashes.
>>
>> While at it, add the definition for the other controls for that
>> extension unit, as defined in
>> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.
>>
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  drivers/media/usb/uvc/uvc_metadata.c |  9 ++++-----
>>  include/linux/usb/uvc.h              | 16 ++++++++++++++++
>>  2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
>> index 386b04a3a102..57f5455a726c 100644
>> --- a/drivers/media/usb/uvc/uvc_metadata.c
>> +++ b/drivers/media/usb/uvc/uvc_metadata.c
>> @@ -171,7 +171,6 @@ static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
>>         return NULL;
>>  }
>>
>> -#define MSXU_CONTROL_METADATA 0x9
>>  static int uvc_meta_detect_msxu(struct uvc_device *dev)
>>  {
>>         u32 *data __free(kfree) = NULL;
>> @@ -192,7 +191,7 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
>>
>>         /* Check if the metadata is already enabled. */
>>         ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
>> -                            MSXU_CONTROL_METADATA, data, sizeof(*data));
>> +                            UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>>         if (ret)
>>                 return 0;
>>
>> @@ -207,16 +206,16 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
>>          * us, the value from GET_MAX seems to work all the time.
>>          */
>>         ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
>> -                            MSXU_CONTROL_METADATA, data, sizeof(*data));
>> +                            UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>>         if (ret || !*data)
>>                 return 0;
>>
>>         /*
>> -        * If we can set MSXU_CONTROL_METADATA, the device will report
>> +        * If we can set UVC_MSXU_CONTROL_METADATA, the device will report
>>          * metadata.
>>          */
>>         ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
>> -                            MSXU_CONTROL_METADATA, data, sizeof(*data));
>> +                            UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
>>         if (!ret)
>>                 dev->quirks |= UVC_QUIRK_MSXU_META;
>>
>> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
>> index ee19e9f915b8..72fff9463c88 100644
>> --- a/include/linux/usb/uvc.h
>> +++ b/include/linux/usb/uvc.h
>> @@ -33,6 +33,22 @@
>>         {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
>>          0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
>>
> 
> Maybe you want to add a link to MS doc here?
> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.

I've added this while merging. I'm afraid the link will likely go
404 over time, but until it goes 404 it will be useful to have.

Regards,

Hans



>> +#define UVC_MSXU_CONTROL_FOCUS                 0x01
>> +#define UVC_MSXU_CONTROL_EXPOSURE              0x02
>> +#define UVC_MSXU_CONTROL_EVCOMPENSATION                0x03
>> +#define UVC_MSXU_CONTROL_WHITEBALANCE          0x04
>> +#define UVC_MSXU_CONTROL_FACE_AUTHENTICATION   0x06
>> +#define UVC_MSXU_CONTROL_CAMERA_EXTRINSICS     0x07
>> +#define UVC_MSXU_CONTROL_CAMERA_INTRINSICS     0x08
>> +#define UVC_MSXU_CONTROL_METADATA              0x09
>> +#define UVC_MSXU_CONTROL_IR_TORCH              0x0a
>> +#define UVC_MSXU_CONTROL_DIGITALWINDOW         0x0b
>> +#define UVC_MSXU_CONTROL_DIGITALWINDOW_CONFIG  0x0c
>> +#define UVC_MSXU_CONTROL_VIDEO_HDR             0x0d
>> +#define UVC_MSXU_CONTROL_FRAMERATE_THROTTLE    0x0e
>> +#define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG   0x0f
>> +#define UVC_MSXU_CONTROL_FIELDOFVIEW2          0x10
>> +
>>  #define UVC_GUID_FORMAT_MJPEG \
>>         { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
>>          0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
>> --
>> Regards,
>>
>> Laurent Pinchart
>>
> 
> 

Re: [PATCH 0/3] media: uvcvideo: Misc fixes

[Hans de Goede]

Hi,

On 15-Jul-25 8:52 PM, Laurent Pinchart wrote:
> Hello,
> 
> A small series of assorted fixes resulting from recent reviews. Nothing
> much to mention here, please see individual patches for details.
> 
> Laurent Pinchart (3):
>   media: uvcvideo: Drop unneeded memset() in meta device ioctl handlers
>   media: uvcvideo: Add missing curly braces
>   media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header
> 
>  drivers/media/usb/uvc/uvc_metadata.c | 23 ++++++++---------------
>  include/linux/usb/uvc.h              | 16 ++++++++++++++++
>  2 files changed, 24 insertions(+), 15 deletions(-)

Thank you for your patch-series.

I have merged this series into:

https://gitlab.freedesktop.org/linux-media/users/uvc/-/commits/for-next/

now.

Regards,

Hans