* [PATCH v2 0/6] media: uvcvideo: Map known XU controls
@ 2025-11-19 19:37 Ricardo Ribalda
2025-11-19 19:37 ` [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter Ricardo Ribalda
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Greg Kroah-Hartman
Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda,
Manav Gautama, Martin Rubli
The UVC driver uses a custom ioctl `UVCIOC_CTRL_MAP` to map XU controls
into v4l2 controls. The most well know user of this feature is the
uvcdynctrl app.
This app has a set of XML files which contains the list of mappings.
Some of these mappings are standard and other ones are custom.
This series move the standard mappings to the kernel driver, so
userspace do not need to depend on external apps to use them.
While we are at it we realized that some of the mappings can be harmful
for the privacy of the user. This series introduce a mechanism to block
those mappings.
While we are at it, we complete the deprecation of the nodrop parameter.
Ideally, this patch should belong in a different series, but then we
will have conflicts... and who wants to works twice?
I have tried this series with a Logitech Webcam Pro 9000, that has been
donated by Hans de Goede (Thanks Hans!!!).
Without this patch and uvcdynctrl the device has 14 controls. (Ctrls A)
With this patch the device has 15 controls (Ctrls B):
Ctrls A
+
control 0x009a090a `Focus, Absolute' min 0 max 255 step 0 default 0 current 0
With uvcdynctrl and this patch the device has 17 controls (Ctrls C):
Ctrls B
+
control 0x0a046d71 `Disable video processing' min 0 max 1 step 1 default 0 current 0
control 0x0a046d72 `Raw bits per pixel' min 0 max 1 step 1 default 0 current 0
With uvcdynctrl and without this patch the device has 19 controls:
Ctrls C
+
control 0x0a046d05 `LED1 Mode' min 0 max 3 step 1 default 3 current 3
0: Off
1: On
2: Blinking
3: Auto (*)
control 0x0a046d06 `LED1 Frequency' min 0 max 255 step 1 default 0 current 0
BTW, Driver tested with virtme-ng. First time that I use it for uvc
development, and it works like a charm :).
virtme-run --kimg arch/x86/boot/bzImage --mods auto --show-command \
--show-boot-console --verbose --qemu-opts -usb -device qemu-xhci \
-device usb-host,hostbus=1,hostport=4
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v2:
- Document how to get GUIDs
- Show clear deprecation intentions to the user.
- Add new patch for the Kconfig approach (Thanks Mauro).
- Remove duplicated control in commit message, provide background and
add documentation. (Thanks Laurent)
- Block also uvc_query_ctrl for privacy controls (Thanks Gergo Koteles)
- Link to v1: https://lore.kernel.org/r/20251117-uvcdynctrl-v1-0-aed70eadf3d8@chromium.org
---
Ricardo Ribalda (6):
media: uvcvideo: Remove nodrop parameter
media: uvcvideo: Import standard controls from uvcdynctrl
media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP
media: uvcvideo: Document how to format GUIDs
media: uvcvideo: Introduce allow_privacy_override param
media: uvcvideo: RFC: Convert allow_privacy_override into Kconfig
.../userspace-api/media/drivers/uvcvideo.rst | 2 +
drivers/media/usb/uvc/Kconfig | 11 ++
drivers/media/usb/uvc/uvc_ctrl.c | 212 +++++++++++++++++++++
drivers/media/usb/uvc/uvc_driver.c | 19 --
drivers/media/usb/uvc/uvc_queue.c | 25 ---
drivers/media/usb/uvc/uvc_v4l2.c | 11 ++
drivers/media/usb/uvc/uvcvideo.h | 2 +-
include/linux/usb/uvc.h | 18 ++
8 files changed, 255 insertions(+), 45 deletions(-)
---
base-commit: 1f2353f5a1af995efbf7bea44341aa0d03460b28
change-id: 20251117-uvcdynctrl-7b80f5bfbb41
Best regards,
--
Ricardo Ribalda <ribalda@chromium.org>
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda @ 2025-11-19 19:37 ` Ricardo Ribalda 2025-12-08 10:54 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl Ricardo Ribalda ` (4 subsequent siblings) 5 siblings, 1 reply; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw) To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda We announced the deprecation intentions one year ago in the commit commit 40ed9e9b2808 ("media: uvcvideo: Announce the user our deprecation intentions"). We have not hear any complains, lets remove the nodrop parameter. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_driver.c | 19 ------------------- drivers/media/usb/uvc/uvc_queue.c | 25 ------------------------- drivers/media/usb/uvc/uvcvideo.h | 1 - 3 files changed, 45 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index ee4f54d6834962414979a046afc59c5036455124..71563d8f4bcf581694ccd4b665ff52b629caa0b6 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -32,7 +32,6 @@ unsigned int uvc_clock_param = CLOCK_MONOTONIC; unsigned int uvc_hw_timestamps_param; -unsigned int uvc_no_drop_param = 1; static unsigned int uvc_quirks_param = -1; unsigned int uvc_dbg_param; unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; @@ -2468,24 +2467,6 @@ MODULE_PARM_DESC(clock, "Video buffers timestamp clock"); module_param_named(hwtimestamps, uvc_hw_timestamps_param, uint, 0644); MODULE_PARM_DESC(hwtimestamps, "Use hardware timestamps"); -static int param_set_nodrop(const char *val, const struct kernel_param *kp) -{ - pr_warn_once("uvcvideo: " - DEPRECATED - "nodrop parameter will be eventually removed.\n"); - return param_set_bool(val, kp); -} - -static const struct kernel_param_ops param_ops_nodrop = { - .set = param_set_nodrop, - .get = param_get_uint, -}; - -param_check_uint(nodrop, &uvc_no_drop_param); -module_param_cb(nodrop, ¶m_ops_nodrop, &uvc_no_drop_param, 0644); -__MODULE_PARM_TYPE(nodrop, "uint"); -MODULE_PARM_DESC(nodrop, "Don't drop incomplete frames"); - module_param_named(quirks, uvc_quirks_param, uint, 0644); MODULE_PARM_DESC(quirks, "Forced device quirks"); module_param_named(trace, uvc_dbg_param, uint, 0644); diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c index 790184c9843d211d34fa7d66801631d5a07450bd..3bc54456b4d98ed50b1ea250ce8501e67141e1ef 100644 --- a/drivers/media/usb/uvc/uvc_queue.c +++ b/drivers/media/usb/uvc/uvc_queue.c @@ -331,34 +331,9 @@ struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue) return nextbuf; } -/* - * uvc_queue_buffer_requeue: Requeue a buffer on our internal irqqueue - * - * Reuse a buffer through our internal queue without the need to 'prepare'. - * The buffer will be returned to userspace through the uvc_buffer_queue call if - * the device has been disconnected. - */ -static void uvc_queue_buffer_requeue(struct uvc_video_queue *queue, - struct uvc_buffer *buf) -{ - buf->error = 0; - buf->state = UVC_BUF_STATE_QUEUED; - buf->bytesused = 0; - vb2_set_plane_payload(&buf->buf.vb2_buf, 0, 0); - - uvc_buffer_queue(&buf->buf.vb2_buf); -} - static void uvc_queue_buffer_complete(struct kref *ref) { struct uvc_buffer *buf = container_of(ref, struct uvc_buffer, ref); - struct vb2_buffer *vb = &buf->buf.vb2_buf; - struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue); - - if (buf->error && !uvc_no_drop_param) { - uvc_queue_buffer_requeue(queue, buf); - return; - } buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE; vb2_set_plane_payload(&buf->buf.vb2_buf, 0, buf->bytesused); diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index ed7bad31f75ca474c1037d666d5310c78dd764df..9a86d7f1f6ea022dace87614030bf0fde0d260f0 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -659,7 +659,6 @@ static inline struct uvc_fh *to_uvc_fh(struct file *filp) #define UVC_WARN_XU_GET_RES 2 extern unsigned int uvc_clock_param; -extern unsigned int uvc_no_drop_param; extern unsigned int uvc_dbg_param; extern unsigned int uvc_timeout_param; extern unsigned int uvc_hw_timestamps_param; -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter 2025-11-19 19:37 ` [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter Ricardo Ribalda @ 2025-12-08 10:54 ` Hans de Goede 0 siblings, 0 replies; 21+ messages in thread From: Hans de Goede @ 2025-12-08 10:54 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb Hi, On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > We announced the deprecation intentions one year ago in the commit > commit 40ed9e9b2808 ("media: uvcvideo: Announce the user our deprecation > intentions"). > > We have not hear any complains, lets remove the nodrop parameter. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Thanks patch looks good to me: Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Since this is independent of the rest of the series I've merged this into my local uvc/for-next branch which I will push out later today. Regards, Hans > --- > drivers/media/usb/uvc/uvc_driver.c | 19 ------------------- > drivers/media/usb/uvc/uvc_queue.c | 25 ------------------------- > drivers/media/usb/uvc/uvcvideo.h | 1 - > 3 files changed, 45 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index ee4f54d6834962414979a046afc59c5036455124..71563d8f4bcf581694ccd4b665ff52b629caa0b6 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -32,7 +32,6 @@ > > unsigned int uvc_clock_param = CLOCK_MONOTONIC; > unsigned int uvc_hw_timestamps_param; > -unsigned int uvc_no_drop_param = 1; > static unsigned int uvc_quirks_param = -1; > unsigned int uvc_dbg_param; > unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; > @@ -2468,24 +2467,6 @@ MODULE_PARM_DESC(clock, "Video buffers timestamp clock"); > module_param_named(hwtimestamps, uvc_hw_timestamps_param, uint, 0644); > MODULE_PARM_DESC(hwtimestamps, "Use hardware timestamps"); > > -static int param_set_nodrop(const char *val, const struct kernel_param *kp) > -{ > - pr_warn_once("uvcvideo: " > - DEPRECATED > - "nodrop parameter will be eventually removed.\n"); > - return param_set_bool(val, kp); > -} > - > -static const struct kernel_param_ops param_ops_nodrop = { > - .set = param_set_nodrop, > - .get = param_get_uint, > -}; > - > -param_check_uint(nodrop, &uvc_no_drop_param); > -module_param_cb(nodrop, ¶m_ops_nodrop, &uvc_no_drop_param, 0644); > -__MODULE_PARM_TYPE(nodrop, "uint"); > -MODULE_PARM_DESC(nodrop, "Don't drop incomplete frames"); > - > module_param_named(quirks, uvc_quirks_param, uint, 0644); > MODULE_PARM_DESC(quirks, "Forced device quirks"); > module_param_named(trace, uvc_dbg_param, uint, 0644); > diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c > index 790184c9843d211d34fa7d66801631d5a07450bd..3bc54456b4d98ed50b1ea250ce8501e67141e1ef 100644 > --- a/drivers/media/usb/uvc/uvc_queue.c > +++ b/drivers/media/usb/uvc/uvc_queue.c > @@ -331,34 +331,9 @@ struct uvc_buffer *uvc_queue_get_current_buffer(struct uvc_video_queue *queue) > return nextbuf; > } > > -/* > - * uvc_queue_buffer_requeue: Requeue a buffer on our internal irqqueue > - * > - * Reuse a buffer through our internal queue without the need to 'prepare'. > - * The buffer will be returned to userspace through the uvc_buffer_queue call if > - * the device has been disconnected. > - */ > -static void uvc_queue_buffer_requeue(struct uvc_video_queue *queue, > - struct uvc_buffer *buf) > -{ > - buf->error = 0; > - buf->state = UVC_BUF_STATE_QUEUED; > - buf->bytesused = 0; > - vb2_set_plane_payload(&buf->buf.vb2_buf, 0, 0); > - > - uvc_buffer_queue(&buf->buf.vb2_buf); > -} > - > static void uvc_queue_buffer_complete(struct kref *ref) > { > struct uvc_buffer *buf = container_of(ref, struct uvc_buffer, ref); > - struct vb2_buffer *vb = &buf->buf.vb2_buf; > - struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue); > - > - if (buf->error && !uvc_no_drop_param) { > - uvc_queue_buffer_requeue(queue, buf); > - return; > - } > > buf->state = buf->error ? UVC_BUF_STATE_ERROR : UVC_BUF_STATE_DONE; > vb2_set_plane_payload(&buf->buf.vb2_buf, 0, buf->bytesused); > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index ed7bad31f75ca474c1037d666d5310c78dd764df..9a86d7f1f6ea022dace87614030bf0fde0d260f0 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -659,7 +659,6 @@ static inline struct uvc_fh *to_uvc_fh(struct file *filp) > #define UVC_WARN_XU_GET_RES 2 > > extern unsigned int uvc_clock_param; > -extern unsigned int uvc_no_drop_param; > extern unsigned int uvc_dbg_param; > extern unsigned int uvc_timeout_param; > extern unsigned int uvc_hw_timestamps_param; > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda 2025-11-19 19:37 ` [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter Ricardo Ribalda @ 2025-11-19 19:37 ` Ricardo Ribalda 2025-12-08 11:02 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP Ricardo Ribalda ` (3 subsequent siblings) 5 siblings, 1 reply; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw) To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda, Manav Gautama, Martin Rubli The uvcdynctrl tool from libwebcam: https://sourceforge.net/projects/libwebcam/ maps proprietary controls into v4l2 controls using the UVCIOC_CTRL_MAP ioctl. The tool has not been updated for 10+ years now, and there is no reason for the UVC driver to not do the mapping by itself. This patch adds the mappings from the uvcdynctrl into the driver. Hopefully this effort can help in deprecating the UVCIOC_CTRL_MAP ioctl. Some background about UVCIOC_CTRL_MAP (thanks Laurent for the context): ``` this was envisioned as the base of a vibrant ecosystem where a large number of vendors would submit XML files that describe their XU control mappings, at a pace faster than could be supported by adding XU mappings to the driver. This vision failed to materialize and the tool has not been updated for 10+ years now. There is no reason to believe the situation will change. ``` During the porting, the following mappings where NOT imported because they were not using standard v4l2 IDs. It is recommended that userspace moves to UVCIOC_CTRL_QUERY for non standard controls. { .id = V4L2_CID_FLASH_MODE, .entity = UVC_GUID_SIS_LED_HW_CONTROL, .selector = 4, .size = 4, .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, .menu_mask = 0x3, .menu_mapping = { 0x20, 0x22 }, .menu_names = { "Off", "On" }, }, { .id = V4L2_CID_FLASH_FREQUENCY, .entity = UVC_GUID_SIS_LED_HW_CONTROL, .selector = 4, .size = 8, .offset = 16, .v4l2_type = V4L2_CTRL_TYPE_INTEGER, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, }, { .id = V4L2_CID_LED1_MODE, .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, .selector = 1, .size = 8, .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, .menu_mask = 0xF, .menu_mapping = { 0, 1, 2, 3 }, .menu_names = { "Off", "On", "Blinking", "Auto" }, }, { .id = V4L2_CID_LED1_FREQUENCY, .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, .selector = 1, .size = 8, .offset = 16, .v4l2_type = V4L2_CTRL_TYPE_INTEGER, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, }, { .id = V4L2_CID_DISABLE_PROCESSING, .entity = UVC_GUID_LOGITECH_VIDEO_PIPE_V1, .selector = 5, .size = 8, .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_BOOLEAN, .data_type = UVC_CTRL_DATA_TYPE_BOOLEAN, }, { .id = V4L2_CID_RAW_BITS_PER_PIXEL, .entity = UVC_GUID_LOGITECH_VIDEO_PIPE_V1, .selector = 8, .size = 8, .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_INTEGER, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, }, { .id = V4L2_CID_LED1_MODE, .entity = UVC_GUID_LOGITECH_PERIPHERAL, .selector = 0x09, .size = 2, .offset = 8, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, .menu_mask = 0xF, .menu_mapping = { 0, 1, 2, 3 }, .menu_names = { "Off", "On", "Blink", "Auto" }, }, { .id = V4L2_CID_LED1_FREQUENCY, .entity = UVC_GUID_LOGITECH_PERIPHERAL, .selector = 0x09, .size = 8, .offset = 24, .v4l2_type = V4L2_CTRL_TYPE_INTEGER, .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, }, This script has been used to generate the mappings. They were then reformatted manually to follow the driver style. import sys import uuid import re import xml.etree.ElementTree as ET def get_namespace(root): return re.match(r"\{.*\}", root.tag).group(0) def get_single_guid(ns, constant): id = constant.find(ns + "id").text value = constant.find(ns + "value").text return (id, value) def get_constants(ns, root): out = dict() for constant in root.iter(ns + "constant"): attr = constant.attrib if attr["type"] == "integer": id, value = get_single_guid(ns, constant) if id in out: print(f"dupe constant {id}") out[id] = value return out def get_guids(ns, root): out = dict() for constant in root.iter(ns + "constant"): attr = constant.attrib if attr["type"] == "guid": id, value = get_single_guid(ns, constant) if id in out: print(f"dupe guid {id}") out[id] = value return out def get_single_control(ns, control): out = {} for id in "entity", "selector", "index", "size", "description": v = control.find(ns + id) if v is None and id == "description": continue out[id] = v.text reqs = set() for r in control.find(ns + "requests"): reqs.add(r.text) out["requests"] = reqs return (control.attrib["id"], out) def get_controls(ns, root): out = dict() for control in root.iter(ns + "control"): id, value = get_single_control(ns, control) if id in out: print(f"Dupe control id {id}") out[id] = value return out def get_single_mapping(ns, mapping): out = {} out["name"] = mapping.find(ns + "name").text uvc = mapping.find(ns + "uvc") for id in "size", "offset", "uvc_type": out[id] = uvc.find(ns + id).text out["control_ref"] = uvc.find(ns + "control_ref").attrib["idref"] v4l2 = mapping.find(ns + "v4l2") for id in "id", "v4l2_type": out[id] = v4l2.find(ns + id).text menu = {} for entry in v4l2.iter(ns + "menu_entry"): menu[entry.attrib["name"]] = entry.attrib["value"] if menu: out["menu"] = menu return out def get_mapping(ns, root): out = [] for control in root.iter(ns + "mapping"): mapping = get_single_mapping(ns, control) out += [mapping] return out def print_guids(guids): for g in guids: print(f"#define {g} \\") u_bytes = uuid.UUID(guids[g]).bytes_le u_bytes = [f"0x{b:02x}" for b in u_bytes] print("\t{ " + ", ".join(u_bytes) + " }") def print_flags(flags): get_range = {"GET_MIN", "GET_DEF", "GET_MAX", "GET_CUR", "GET_RES"} if get_range.issubset(flags): flags -= get_range flags.add("GET_RANGE") flags = list(flags) flags.sort() out = "" for f in flags[:-1]: out += f"UVC_CTRL_FLAG_{f}\n\t\t\t\t| " out += f"UVC_CTRL_FLAG_{flags[-1]}" return out def print_description(desc): print("/*") for line in desc.strip().splitlines(): print(f" * {line.strip()}") print("*/") def print_controls(controls, cons): for id in controls: c = controls[id] if "description" in c: print_description(c["description"]) print( f"""\t{{ \t\t.entity\t\t= {c["entity"]}, \t\t.selector\t= {cons[c["selector"]]}, \t\t.index\t\t= {c["index"]}, \t\t.size\t\t= {c["size"]}, \t\t.flags\t\t= {print_flags(c["requests"])}, \t}},""" ) def menu_mapping_txt(menu): out = f"\n\t\t.menu_mask\t= 0x{((1<<len(menu))-1):X},\n" out += f"\t\t.menu_mapping\t= {{ {", ".join(menu.values())} }},\n" out += f"\t\t.menu_names\t= {{ \"{"\", \"".join(menu.keys())}\" }},\n" return out def print_mappings(mappings, controls, cons): for m in mappings: c = controls[m["control_ref"]] if "menu" in m: menu_mapping = menu_mapping_txt(m["menu"]) else: menu_mapping = "" print( f"""\t{{ \t\t.id\t\t= {m["id"]}, \t\t.entity\t\t= {c["entity"]}, \t\t.selector\t= {cons[c["selector"]]}, \t\t.size\t\t= {m["size"]}, \t\t.offset\t\t= {m["offset"]}, \t\t.v4l2_type\t= {m["v4l2_type"]}, \t\t.data_type\t= {m["uvc_type"]},{menu_mapping} \t}},""" ) def print_code(guids, cons, controls, mappings): used_controls = set() for m in mappings: used_controls.add(m["control_ref"]) used_guids = set() for c in used_controls: used_guids.add(controls[c]["entity"]) print("\n######GUIDs#######\n") print_guids({id: guids[id] for id in guids if id in used_guids}) print("\n######CONTROLS#######\n") print_controls({id: controls[id] for id in controls if id in used_controls}, cons) print("\n######MAPPINGS#######\n") print_mappings(mappings, controls, cons) # print(guids) # print(used_controls) root = ET.fromstring(sys.stdin.read()) ns = get_namespace(root) cons = get_constants(ns, root) guids = get_guids(ns, root) controls = get_controls(ns, root) mappings = get_mapping(ns, root) print_code(guids, cons, controls, mappings) Cc: Manav Gautama <bandwidthcrunch@gmail.com> Cc: Martin Rubli <martin_rubli@logitech.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_ctrl.c | 174 +++++++++++++++++++++++++++++++++++++++ include/linux/usb/uvc.h | 6 ++ 2 files changed, 180 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 2905505c240c060e5034ea12d33b59d5702f2e1f..57ce486f22bbc404a1f127539eb2d12373431631 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -385,6 +385,99 @@ static const struct uvc_control_info uvc_ctrls[] = { | UVC_CTRL_FLAG_GET_RANGE | UVC_CTRL_FLAG_RESTORE, }, + /* + * Allows the control of pan/tilt motor movements for camera models + * that support mechanical pan/tilt. + * + * Bits 0 to 15 control pan, bits 16 to 31 control tilt. + * The unit of the pan/tilt values is 1/64th of a degree and the + * resolution is 1 degree. + */ + { + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 1, + .index = 0, + .size = 4, + .flags = UVC_CTRL_FLAG_GET_DEF + | UVC_CTRL_FLAG_GET_MAX + | UVC_CTRL_FLAG_GET_MIN + | UVC_CTRL_FLAG_SET_CUR, + }, + /* + * Reset the pan/tilt motors to their original position for camera + * models that support mechanical pan/tilt. + * + * Setting bit 0 resets the pan position. + * Setting bit 1 resets the tilt position. + * + * Both bits can be set at the same time to reset both, pan and tilt, + * at the same time. + */ + { + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 2, + .index = 1, + .size = 1, + .flags = UVC_CTRL_FLAG_GET_DEF + | UVC_CTRL_FLAG_GET_MAX + | UVC_CTRL_FLAG_GET_MIN + | UVC_CTRL_FLAG_SET_CUR, + }, + /* + * Allows the control of focus motor movements for camera models that + * support mechanical focus. + * + * Bits 0 to 7 allow selection of the desired lens position. + * There are no physical units, instead, the focus range is spread over + * 256 logical units with 0 representing infinity focus and 255 being + * macro focus. + */ + { + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 3, + .index = 2, + .size = 6, + .flags = UVC_CTRL_FLAG_GET_CUR + | UVC_CTRL_FLAG_GET_DEF + | UVC_CTRL_FLAG_GET_MAX + | UVC_CTRL_FLAG_GET_MIN + | UVC_CTRL_FLAG_SET_CUR, + }, + /* + * Allows the control of pan/tilt motor movements for camera models + * that support mechanical pan/tilt. + * + * Bits 0 to 15 control pan, bits 16 to 31 control tilt. + */ + { + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 1, + .index = 0, + .size = 4, + .flags = UVC_CTRL_FLAG_GET_DEF + | UVC_CTRL_FLAG_GET_MAX + | UVC_CTRL_FLAG_GET_MIN + | UVC_CTRL_FLAG_GET_RES + | UVC_CTRL_FLAG_SET_CUR, + }, + /* + * Reset the pan/tilt motors to their original position for camera + * models that support mechanical pan/tilt. + * + * Setting bit 0 resets the pan position. + * Setting bit 1 resets the tilt position. + */ + { + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 2, + .index = 1, + .size = 1, + .flags = UVC_CTRL_FLAG_GET_DEF + | UVC_CTRL_FLAG_GET_MAX + | UVC_CTRL_FLAG_GET_MIN + | UVC_CTRL_FLAG_GET_RES + | UVC_CTRL_FLAG_SET_CUR, + }, }; static const u32 uvc_control_classes[] = { @@ -1009,6 +1102,87 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = { .menu_mask = BIT(V4L2_COLORFX_VIVID) | BIT(V4L2_COLORFX_NONE), }, + { + .id = V4L2_CID_PAN_RELATIVE, + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 1, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + }, + { + .id = V4L2_CID_TILT_RELATIVE, + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 1, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + }, + { + .id = V4L2_CID_PAN_RESET, + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 2, + .size = 1, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + }, + { + .id = V4L2_CID_TILT_RESET, + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 2, + .size = 1, + .offset = 1, + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + }, + { + .id = V4L2_CID_PAN_RELATIVE, + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 1, + .size = 16, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + }, + { + .id = V4L2_CID_TILT_RELATIVE, + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 1, + .size = 16, + .offset = 16, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, + }, + { + .id = V4L2_CID_PAN_RESET, + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 2, + .size = 1, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + }, + { + .id = V4L2_CID_TILT_RESET, + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 2, + .size = 1, + .offset = 1, + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + }, + { + .id = V4L2_CID_FOCUS_ABSOLUTE, + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, + .selector = 3, + .size = 8, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, + }, }; /* ------------------------------------------------------------------------ diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h index 22e0dab0809e296e089940620ae0e8838e109701..b939a01da11466747249c64c72a3ea40cd364a59 100644 --- a/include/linux/usb/uvc.h +++ b/include/linux/usb/uvc.h @@ -35,6 +35,12 @@ #define UVC_GUID_MSXU_1_5 \ {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \ 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8} +#define UVC_GUID_LOGITECH_MOTOR_CONTROL_V1 \ + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \ + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x56 } +#define UVC_GUID_LOGITECH_PERIPHERAL \ + {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \ + 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd } /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */ #define UVC_MSXU_CONTROL_FOCUS 0x01 -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl 2025-11-19 19:37 ` [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl Ricardo Ribalda @ 2025-12-08 11:02 ` Hans de Goede 2025-12-08 11:12 ` Hans de Goede 0 siblings, 1 reply; 21+ messages in thread From: Hans de Goede @ 2025-12-08 11:02 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Manav Gautama, Martin Rubli Hi Ricardo, Thank you very much for doing this, this has been on my own TODO list for a long time, so it is great to finally see this happen. On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > The uvcdynctrl tool from libwebcam: > https://sourceforge.net/projects/libwebcam/ > maps proprietary controls into v4l2 controls using the UVCIOC_CTRL_MAP > ioctl. > > The tool has not been updated for 10+ years now, and there is no reason > for the UVC driver to not do the mapping by itself. > > This patch adds the mappings from the uvcdynctrl into the driver. Hopefully > this effort can help in deprecating the UVCIOC_CTRL_MAP ioctl. ... Question what happens if uvcdynctrl is run after applying this patch ? We already have the ctrl-info + mappings now, does the code detect this and ignore the duplicate info, or ... ? > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 2905505c240c060e5034ea12d33b59d5702f2e1f..57ce486f22bbc404a1f127539eb2d12373431631 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -385,6 +385,99 @@ static const struct uvc_control_info uvc_ctrls[] = { > | UVC_CTRL_FLAG_GET_RANGE > | UVC_CTRL_FLAG_RESTORE, > }, > + /* > + * Allows the control of pan/tilt motor movements for camera models > + * that support mechanical pan/tilt. > + * > + * Bits 0 to 15 control pan, bits 16 to 31 control tilt. > + * The unit of the pan/tilt values is 1/64th of a degree and the > + * resolution is 1 degree. > + */ > + { > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 1, > + .index = 0, > + .size = 4, > + .flags = UVC_CTRL_FLAG_GET_DEF > + | UVC_CTRL_FLAG_GET_MAX > + | UVC_CTRL_FLAG_GET_MIN > + | UVC_CTRL_FLAG_SET_CUR, > + }, > + /* > + * Reset the pan/tilt motors to their original position for camera > + * models that support mechanical pan/tilt. > + * > + * Setting bit 0 resets the pan position. > + * Setting bit 1 resets the tilt position. > + * > + * Both bits can be set at the same time to reset both, pan and tilt, > + * at the same time. > + */ > + { > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 2, > + .index = 1, > + .size = 1, > + .flags = UVC_CTRL_FLAG_GET_DEF > + | UVC_CTRL_FLAG_GET_MAX > + | UVC_CTRL_FLAG_GET_MIN > + | UVC_CTRL_FLAG_SET_CUR, > + }, > + /* > + * Allows the control of focus motor movements for camera models that > + * support mechanical focus. > + * > + * Bits 0 to 7 allow selection of the desired lens position. > + * There are no physical units, instead, the focus range is spread over > + * 256 logical units with 0 representing infinity focus and 255 being > + * macro focus. > + */ > + { > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 3, > + .index = 2, > + .size = 6, > + .flags = UVC_CTRL_FLAG_GET_CUR > + | UVC_CTRL_FLAG_GET_DEF > + | UVC_CTRL_FLAG_GET_MAX > + | UVC_CTRL_FLAG_GET_MIN > + | UVC_CTRL_FLAG_SET_CUR, > + }, > + /* > + * Allows the control of pan/tilt motor movements for camera models > + * that support mechanical pan/tilt. > + * > + * Bits 0 to 15 control pan, bits 16 to 31 control tilt. > + */ > + { > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, At a first read I thought this was duplicate with the pan/tilt motor control mapping above but this is for UVC_GUID_LOGITECH_PERIPHERAL, where as the ctrl_info above is for UVC_GUID_LOGITECH_MOTOR_CONTROL_V1 . I wonder if this is worth a comment ? One difference here is is the support of UVC_CTRL_FLAG_GET_RES . > + .selector = 1, > + .index = 0, > + .size = 4, > + .flags = UVC_CTRL_FLAG_GET_DEF > + | UVC_CTRL_FLAG_GET_MAX > + | UVC_CTRL_FLAG_GET_MIN > + | UVC_CTRL_FLAG_GET_RES > + | UVC_CTRL_FLAG_SET_CUR, > + }, > + /* > + * Reset the pan/tilt motors to their original position for camera > + * models that support mechanical pan/tilt. > + * > + * Setting bit 0 resets the pan position. > + * Setting bit 1 resets the tilt position. > + */ > + { > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 2, > + .index = 1, > + .size = 1, > + .flags = UVC_CTRL_FLAG_GET_DEF > + | UVC_CTRL_FLAG_GET_MAX > + | UVC_CTRL_FLAG_GET_MIN > + | UVC_CTRL_FLAG_GET_RES > + | UVC_CTRL_FLAG_SET_CUR, > + }, > }; > > static const u32 uvc_control_classes[] = { > @@ -1009,6 +1102,87 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = { > .menu_mask = BIT(V4L2_COLORFX_VIVID) | > BIT(V4L2_COLORFX_NONE), > }, > + { > + .id = V4L2_CID_PAN_RELATIVE, > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 1, > + .size = 16, > + .offset = 0, > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > + }, > + { > + .id = V4L2_CID_TILT_RELATIVE, > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 1, > + .size = 16, > + .offset = 16, > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > + }, > + { > + .id = V4L2_CID_PAN_RESET, > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 2, > + .size = 1, > + .offset = 0, > + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, > + }, > + { > + .id = V4L2_CID_TILT_RESET, > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 2, > + .size = 1, > + .offset = 1, > + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, > + }, > + { > + .id = V4L2_CID_PAN_RELATIVE, > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 1, > + .size = 16, > + .offset = 0, > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > + }, > + { > + .id = V4L2_CID_TILT_RELATIVE, > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 1, > + .size = 16, > + .offset = 16, > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > + .data_type = UVC_CTRL_DATA_TYPE_SIGNED, > + }, > + { > + .id = V4L2_CID_PAN_RESET, > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 2, > + .size = 1, > + .offset = 0, > + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, > + }, > + { > + .id = V4L2_CID_TILT_RESET, > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 2, > + .size = 1, > + .offset = 1, > + .v4l2_type = V4L2_CTRL_TYPE_BUTTON, > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, > + }, > + { > + .id = V4L2_CID_FOCUS_ABSOLUTE, > + .entity = UVC_GUID_LOGITECH_MOTOR_CONTROL_V1, > + .selector = 3, > + .size = 8, > + .offset = 0, > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER, > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED, > + }, > }; > > /* ------------------------------------------------------------------------ > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h > index 22e0dab0809e296e089940620ae0e8838e109701..b939a01da11466747249c64c72a3ea40cd364a59 100644 > --- a/include/linux/usb/uvc.h > +++ b/include/linux/usb/uvc.h > @@ -35,6 +35,12 @@ > #define UVC_GUID_MSXU_1_5 \ > {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \ > 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8} > +#define UVC_GUID_LOGITECH_MOTOR_CONTROL_V1 \ > + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \ > + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x56 } > +#define UVC_GUID_LOGITECH_PERIPHERAL \ > + {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \ > + 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd } > > /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */ > #define UVC_MSXU_CONTROL_FOCUS 0x01 > Regards, Hans ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl 2025-12-08 11:02 ` Hans de Goede @ 2025-12-08 11:12 ` Hans de Goede 2025-12-09 6:28 ` Ricardo Ribalda 0 siblings, 1 reply; 21+ messages in thread From: Hans de Goede @ 2025-12-08 11:12 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Manav Gautama, Martin Rubli Hi, On 8-Dec-25 12:02 PM, Hans de Goede wrote: > Hi Ricardo, > > Thank you very much for doing this, this has been on my own TODO list for > a long time, so it is great to finally see this happen. > > On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: >> The uvcdynctrl tool from libwebcam: >> https://sourceforge.net/projects/libwebcam/ >> maps proprietary controls into v4l2 controls using the UVCIOC_CTRL_MAP >> ioctl. >> >> The tool has not been updated for 10+ years now, and there is no reason >> for the UVC driver to not do the mapping by itself. >> >> This patch adds the mappings from the uvcdynctrl into the driver. Hopefully >> this effort can help in deprecating the UVCIOC_CTRL_MAP ioctl. > > ... > > Question what happens if uvcdynctrl is run after applying this patch ? Answering my own question here, we will hit: drivers/media/usb/uvc/uvc_ctrl.c: 3166: list_for_each_entry(map, &ctrl->info.mappings, list) { if (mapping->id == map->id) { uvc_dbg(dev, CONTROL, "Can't add mapping '%s', control id 0x%08x already exists\n", uvc_map_get_name(mapping), mapping->id); ret = -EEXIST; goto done; } } So uvcdynctrl will see an EEXIST error. I think we need to add an -EEXIST check to uvc_ctrl_add_mapping() )or uvc_ioctl_xu_ctrl_map() which is the only caller of uvc_ctrl_add_mapping()) and if -EEXIST is returned do a uvc_warn_once() that duplicate mappings are being ignored and return 0 instead of -EEXIST to avoid breaking existing userspace. Regards, Hans ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl 2025-12-08 11:12 ` Hans de Goede @ 2025-12-09 6:28 ` Ricardo Ribalda 2025-12-09 16:22 ` Hans de Goede 0 siblings, 1 reply; 21+ messages in thread From: Ricardo Ribalda @ 2025-12-09 6:28 UTC (permalink / raw) To: Hans de Goede Cc: Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb, Manav Gautama, Martin Rubli Hi Hans On Mon, 8 Dec 2025 at 20:12, Hans de Goede <hansg@kernel.org> wrote: > > Hi, > > On 8-Dec-25 12:02 PM, Hans de Goede wrote: > > Hi Ricardo, > > > > Thank you very much for doing this, this has been on my own TODO list for > > a long time, so it is great to finally see this happen. > > > > On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > >> The uvcdynctrl tool from libwebcam: > >> https://sourceforge.net/projects/libwebcam/ > >> maps proprietary controls into v4l2 controls using the UVCIOC_CTRL_MAP > >> ioctl. > >> > >> The tool has not been updated for 10+ years now, and there is no reason > >> for the UVC driver to not do the mapping by itself. > >> > >> This patch adds the mappings from the uvcdynctrl into the driver. Hopefully > >> this effort can help in deprecating the UVCIOC_CTRL_MAP ioctl. > > > > ... > > > > Question what happens if uvcdynctrl is run after applying this patch ? > > Answering my own question here, we will hit: > > drivers/media/usb/uvc/uvc_ctrl.c: 3166: > > list_for_each_entry(map, &ctrl->info.mappings, list) { > if (mapping->id == map->id) { > uvc_dbg(dev, CONTROL, > "Can't add mapping '%s', control id 0x%08x already exists\n", > uvc_map_get_name(mapping), mapping->id); > ret = -EEXIST; > goto done; > } > } > > So uvcdynctrl will see an EEXIST error. I think we need to add an -EEXIST check > to uvc_ctrl_add_mapping() )or uvc_ioctl_xu_ctrl_map() which is the only caller of > uvc_ctrl_add_mapping()) and if -EEXIST is returned do a uvc_warn_once() that duplicate > mappings are being ignored and return 0 instead of -EEXIST to avoid breaking existing uvcdynctrl seems to handle this kind of error ok: https://sourceforge.net/p/libwebcam/code/ci/master/tree/libwebcam/dynctrl.c#l1215 while(node_mapping) { CResult ret = process_mapping(node_mapping, ctx); if(ctx->info) { if(ret) ctx->info->stats.mappings.successful++; else ctx->info->stats.mappings.failed++; } node_mapping = xml_get_next_sibling_by_name(node_mapping, "mapping"); } https://sourceforge.net/p/libwebcam/code/ci/master/tree/libwebcam/dynctrl.c#l1199 if(v4l2_ret != 0 #ifdef DYNCTRL_IGNORE_EEXIST_AFTER_PASS1 && (ctx->pass == 1 || errno != EEXIST) #endif ) https://sourceforge.net/p/libwebcam/code/ci/master/tree/libwebcam/libwebcam.h#l69 /// Ignore EEXIST errors for the UVCIOC_CTRL_ADD and UVCIOC_CTRL_MAP ioctls for /// all but the first device. This is required if the driver uses global controls /// instead of per-device controls. #define DYNCTRL_IGNORE_EEXIST_AFTER_PASS1 (BTW, I think the last comment ^^^ is wrong, it should be. Ignore EEXIST errors, or errors for second passes. But I might need a coffee :P) Looking at the debian codesearch: https://codesearch.debian.net/search?q=UVCIOC_CTRL_MAP+-path%3Aioctl.rs+-path%3Auvc_v4l2.c+-file%3Auvcvideo.rst+-file%3Auvc_ctrl.c+-file%3Auvcvideo.h&literal=1 the only occurrence that I am no sure if it will properly handle -EEXIST is: https://sources.debian.org/src/chromium/143.0.7499.40-1/chrome/browser/ash/chromebox_for_meetings/xu_camera/xu_camera_service.cc?hl=400#L400 But that is ash-> ChromeOS browser. I can ask the code owner to fix it if needed. I'd rather not add the quirk that you are proposing if possible. I would expect that any/all the userspace handles -EEXIST because the uvc control state outlives the userspace. Let me know what you think. > userspace. > > Regards, > > Hans > > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl 2025-12-09 6:28 ` Ricardo Ribalda @ 2025-12-09 16:22 ` Hans de Goede 0 siblings, 0 replies; 21+ messages in thread From: Hans de Goede @ 2025-12-09 16:22 UTC (permalink / raw) To: Ricardo Ribalda Cc: Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb, Manav Gautama, Martin Rubli Hi, On 9-Dec-25 7:28 AM, Ricardo Ribalda wrote: > Hi Hans > > > On Mon, 8 Dec 2025 at 20:12, Hans de Goede <hansg@kernel.org> wrote: >> >> Hi, >> >> On 8-Dec-25 12:02 PM, Hans de Goede wrote: >>> Hi Ricardo, >>> >>> Thank you very much for doing this, this has been on my own TODO list for >>> a long time, so it is great to finally see this happen. >>> >>> On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: >>>> The uvcdynctrl tool from libwebcam: >>>> https://sourceforge.net/projects/libwebcam/ >>>> maps proprietary controls into v4l2 controls using the UVCIOC_CTRL_MAP >>>> ioctl. >>>> >>>> The tool has not been updated for 10+ years now, and there is no reason >>>> for the UVC driver to not do the mapping by itself. >>>> >>>> This patch adds the mappings from the uvcdynctrl into the driver. Hopefully >>>> this effort can help in deprecating the UVCIOC_CTRL_MAP ioctl. >>> >>> ... >>> >>> Question what happens if uvcdynctrl is run after applying this patch ? >> >> Answering my own question here, we will hit: >> >> drivers/media/usb/uvc/uvc_ctrl.c: 3166: >> >> list_for_each_entry(map, &ctrl->info.mappings, list) { >> if (mapping->id == map->id) { >> uvc_dbg(dev, CONTROL, >> "Can't add mapping '%s', control id 0x%08x already exists\n", >> uvc_map_get_name(mapping), mapping->id); >> ret = -EEXIST; >> goto done; >> } >> } >> >> So uvcdynctrl will see an EEXIST error. I think we need to add an -EEXIST check >> to uvc_ctrl_add_mapping() )or uvc_ioctl_xu_ctrl_map() which is the only caller of >> uvc_ctrl_add_mapping()) and if -EEXIST is returned do a uvc_warn_once() that duplicate >> mappings are being ignored and return 0 instead of -EEXIST to avoid breaking existing > > uvcdynctrl seems to handle this kind of error ok: > > https://sourceforge.net/p/libwebcam/code/ci/master/tree/libwebcam/dynctrl.c#l1215 > > while(node_mapping) { > CResult ret = process_mapping(node_mapping, ctx); > if(ctx->info) { > if(ret) > ctx->info->stats.mappings.successful++; > else > ctx->info->stats.mappings.failed++; > } > node_mapping = xml_get_next_sibling_by_name(node_mapping, "mapping"); > } > > https://sourceforge.net/p/libwebcam/code/ci/master/tree/libwebcam/dynctrl.c#l1199 > if(v4l2_ret != 0 > #ifdef DYNCTRL_IGNORE_EEXIST_AFTER_PASS1 > && (ctx->pass == 1 || errno != EEXIST) > #endif > ) > > https://sourceforge.net/p/libwebcam/code/ci/master/tree/libwebcam/libwebcam.h#l69 > /// Ignore EEXIST errors for the UVCIOC_CTRL_ADD and UVCIOC_CTRL_MAP ioctls for > /// all but the first device. This is required if the driver uses > global controls > /// instead of per-device controls. > #define DYNCTRL_IGNORE_EEXIST_AFTER_PASS1 > > > (BTW, I think the last comment ^^^ is wrong, it should be. Ignore > EEXIST errors, or errors for second passes. > But I might need a coffee :P) > > > Looking at the debian codesearch: > https://codesearch.debian.net/search?q=UVCIOC_CTRL_MAP+-path%3Aioctl.rs+-path%3Auvc_v4l2.c+-file%3Auvcvideo.rst+-file%3Auvc_ctrl.c+-file%3Auvcvideo.h&literal=1 > > the only occurrence that I am no sure if it will properly handle -EEXIST is: > https://sources.debian.org/src/chromium/143.0.7499.40-1/chrome/browser/ash/chromebox_for_meetings/xu_camera/xu_camera_service.cc?hl=400#L400 > But that is ash-> ChromeOS browser. I can ask the code owner to fix it > if needed. > > > I'd rather not add the quirk that you are proposing if possible. I > would expect that any/all the userspace handles -EEXIST because the > uvc control state outlives the userspace. > > > Let me know what you think. Based on what you wrote above I think that going with -EEXIST is ok for now, we can always revisit if we get complaints about this. Regards, Hans ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda 2025-11-19 19:37 ` [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter Ricardo Ribalda 2025-11-19 19:37 ` [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl Ricardo Ribalda @ 2025-11-19 19:37 ` Ricardo Ribalda 2025-12-08 11:17 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs Ricardo Ribalda ` (2 subsequent siblings) 5 siblings, 1 reply; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw) To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda The UVCIOC_CTRL_MAP lets userspace create a mapping for a custom control. This mapping is usually created by the uvcdynctrl userspace utility. We would like to get the mappings into the driver instead. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- Documentation/userspace-api/media/drivers/uvcvideo.rst | 2 ++ drivers/media/usb/uvc/uvc_v4l2.c | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst index dbb30ad389ae4d53bc734b4269ebea20ecdd7535..b09d2f8ba66ecde67f1e35fd77858a505ad44eb1 100644 --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst @@ -109,6 +109,8 @@ IOCTL reference UVCIOC_CTRL_MAP - Map a UVC control to a V4L2 control ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +**This IOCTL is deprecated and will be eventually removed** + Argument: struct uvc_xu_control_mapping **Description**: diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 9e4a251eca88085a1b4e0e854370015855be92ee..03c64b5698bf4331fed8437fa6e9c726a07450bd 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -1044,6 +1044,8 @@ static long uvc_ioctl_default(struct file *file, void *priv, bool valid_prio, switch (cmd) { /* Dynamic controls. */ case UVCIOC_CTRL_MAP: + pr_warn_once("uvcvideo: " DEPRECATED + "UVCIOC_CTRL_MAP ioctl will be eventually removed.\n"); return uvc_ioctl_xu_ctrl_map(chain, arg); case UVCIOC_CTRL_QUERY: @@ -1158,6 +1160,8 @@ static long uvc_v4l2_compat_ioctl32(struct file *file, switch (cmd) { case UVCIOC_CTRL_MAP32: + pr_warn_once("uvcvideo: " DEPRECATED + "UVCIOC_CTRL_MAP32 ioctl will be eventually removed.\n"); ret = uvc_v4l2_get_xu_mapping(&karg.xmap, up); if (ret) break; -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP 2025-11-19 19:37 ` [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP Ricardo Ribalda @ 2025-12-08 11:17 ` Hans de Goede 2025-12-09 6:41 ` Ricardo Ribalda 0 siblings, 1 reply; 21+ messages in thread From: Hans de Goede @ 2025-12-08 11:17 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb Hi, On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > The UVCIOC_CTRL_MAP lets userspace create a mapping for a custom > control. > > This mapping is usually created by the uvcdynctrl userspace utility. We > would like to get the mappings into the driver instead. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > Documentation/userspace-api/media/drivers/uvcvideo.rst | 2 ++ > drivers/media/usb/uvc/uvc_v4l2.c | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst > index dbb30ad389ae4d53bc734b4269ebea20ecdd7535..b09d2f8ba66ecde67f1e35fd77858a505ad44eb1 100644 > --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst > +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst > @@ -109,6 +109,8 @@ IOCTL reference > UVCIOC_CTRL_MAP - Map a UVC control to a V4L2 control > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > +**This IOCTL is deprecated and will be eventually removed** > + > Argument: struct uvc_xu_control_mapping > > **Description**: > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 9e4a251eca88085a1b4e0e854370015855be92ee..03c64b5698bf4331fed8437fa6e9c726a07450bd 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -1044,6 +1044,8 @@ static long uvc_ioctl_default(struct file *file, void *priv, bool valid_prio, > switch (cmd) { > /* Dynamic controls. */ > case UVCIOC_CTRL_MAP: > + pr_warn_once("uvcvideo: " DEPRECATED > + "UVCIOC_CTRL_MAP ioctl will be eventually removed.\n"); > return uvc_ioctl_xu_ctrl_map(chain, arg); > > case UVCIOC_CTRL_QUERY: Deprecating and then removing this is going to be a long slow process. I was thinking that rather then remove it we would keep accepting the ioctl but instead of calling uvc_ioctl_xu_ctrl_map() we would simply return 0. E.g. change the above to: case UVCIOC_CTRL_MAP: pr_warn_once("uvcvideo: " DEPRECATED "UVCIOC_CTRL_MAP ioctl will eventually be ignored.\n"); return uvc_ioctl_xu_ctrl_map(chain, arg); And then say in one year after a kernel with the above is released change it to: case UVCIOC_CTRL_MAP: pr_warn_once("uvcvideo: " DEPRECATED "UVCIOC_CTRL_MAP ioctls are ignored.\n"); return 0; I think removing it in 1 year is too soon, but ignoring it is ok. This does mean that people will loose the custom v4l2-ctrls for which patch 2/6 is not adding mappings into the driver in 1 year after a kernel with the warning is released... I'm not 100% sure about this plan, so please let me know what you think. For outright deprecation warning + full removal I think we need to wait at least 2 years after shipping a kernel with the deprecation warning. Regards, Hans ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP 2025-12-08 11:17 ` Hans de Goede @ 2025-12-09 6:41 ` Ricardo Ribalda 2025-12-09 16:27 ` Hans de Goede 0 siblings, 1 reply; 21+ messages in thread From: Ricardo Ribalda @ 2025-12-09 6:41 UTC (permalink / raw) To: Hans de Goede Cc: Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb Hi Hans On Mon, 8 Dec 2025 at 20:17, Hans de Goede <hansg@kernel.org> wrote: > > Hi, > > On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > > The UVCIOC_CTRL_MAP lets userspace create a mapping for a custom > > control. > > > > This mapping is usually created by the uvcdynctrl userspace utility. We > > would like to get the mappings into the driver instead. > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > Documentation/userspace-api/media/drivers/uvcvideo.rst | 2 ++ > > drivers/media/usb/uvc/uvc_v4l2.c | 4 ++++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst > > index dbb30ad389ae4d53bc734b4269ebea20ecdd7535..b09d2f8ba66ecde67f1e35fd77858a505ad44eb1 100644 > > --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst > > +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst > > @@ -109,6 +109,8 @@ IOCTL reference > > UVCIOC_CTRL_MAP - Map a UVC control to a V4L2 control > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > +**This IOCTL is deprecated and will be eventually removed** > > + > > Argument: struct uvc_xu_control_mapping > > > > **Description**: > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index 9e4a251eca88085a1b4e0e854370015855be92ee..03c64b5698bf4331fed8437fa6e9c726a07450bd 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -1044,6 +1044,8 @@ static long uvc_ioctl_default(struct file *file, void *priv, bool valid_prio, > > switch (cmd) { > > /* Dynamic controls. */ > > case UVCIOC_CTRL_MAP: > > + pr_warn_once("uvcvideo: " DEPRECATED > > + "UVCIOC_CTRL_MAP ioctl will be eventually removed.\n"); > > return uvc_ioctl_xu_ctrl_map(chain, arg); > > > > case UVCIOC_CTRL_QUERY: > > Deprecating and then removing this is going to be a long slow process. > > I was thinking that rather then remove it we would keep accepting the ioctl but instead > of calling uvc_ioctl_xu_ctrl_map() we would simply return 0. E.g. change the above to: > > case UVCIOC_CTRL_MAP: > pr_warn_once("uvcvideo: " DEPRECATED > "UVCIOC_CTRL_MAP ioctl will eventually be ignored.\n"); > return uvc_ioctl_xu_ctrl_map(chain, arg); > > And then say in one year after a kernel with the above is released change it to: > > case UVCIOC_CTRL_MAP: > pr_warn_once("uvcvideo: " DEPRECATED > "UVCIOC_CTRL_MAP ioctls are ignored.\n"); > return 0; > > > I think removing it in 1 year is too soon, but ignoring it is ok. This does mean > that people will lose the custom v4l2-ctrls for which patch 2/6 is not adding > mappings into the driver in 1 year after a kernel with the warning is released... > > I'm not 100% sure about this plan, so please let me know what you think. For > outright deprecation warning + full removal I think we need to wait at least > 2 years after shipping a kernel with the deprecation warning. Let me rephrase what you have written: today: pr_warn_once("uvcvideo: " DEPRECATED "UVCIOC_CTRL_MAP ioctl will be eventually ignored.\n"); return uvc_ioctl_xu_ctrl_map(chain, arg); in 1 year: pr_warn_once("uvcvideo: " DEPRECATED "UVCIOC_CTRL_MAP ioctl is ignored.\n"); return 0; in 2 years: return -ENOIOCTLCMD; Normally I would prefer not to lie to userspace (saying that the mapping was done, but not doing it). But in this case, UVCIOC_CTRL_MAP does not seem to be very widely used (check previous email), so I do not think it really matters if we skip the "1 year step" and just return -ENOIOCTLCMD in 2 years. I leave it up to you to decide the deprecation steps. Best regards! > > Regards, > > Hans > > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP 2025-12-09 6:41 ` Ricardo Ribalda @ 2025-12-09 16:27 ` Hans de Goede 0 siblings, 0 replies; 21+ messages in thread From: Hans de Goede @ 2025-12-09 16:27 UTC (permalink / raw) To: Ricardo Ribalda Cc: Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb Hi Ricaro, On 9-Dec-25 7:41 AM, Ricardo Ribalda wrote: > Hi Hans > > > On Mon, 8 Dec 2025 at 20:17, Hans de Goede <hansg@kernel.org> wrote: >> >> Hi, >> >> On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: >>> The UVCIOC_CTRL_MAP lets userspace create a mapping for a custom >>> control. >>> >>> This mapping is usually created by the uvcdynctrl userspace utility. We >>> would like to get the mappings into the driver instead. >>> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> >>> --- >>> Documentation/userspace-api/media/drivers/uvcvideo.rst | 2 ++ >>> drivers/media/usb/uvc/uvc_v4l2.c | 4 ++++ >>> 2 files changed, 6 insertions(+) >>> >>> diff --git a/Documentation/userspace-api/media/drivers/uvcvideo.rst b/Documentation/userspace-api/media/drivers/uvcvideo.rst >>> index dbb30ad389ae4d53bc734b4269ebea20ecdd7535..b09d2f8ba66ecde67f1e35fd77858a505ad44eb1 100644 >>> --- a/Documentation/userspace-api/media/drivers/uvcvideo.rst >>> +++ b/Documentation/userspace-api/media/drivers/uvcvideo.rst >>> @@ -109,6 +109,8 @@ IOCTL reference >>> UVCIOC_CTRL_MAP - Map a UVC control to a V4L2 control >>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>> >>> +**This IOCTL is deprecated and will be eventually removed** >>> + >>> Argument: struct uvc_xu_control_mapping >>> >>> **Description**: >>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >>> index 9e4a251eca88085a1b4e0e854370015855be92ee..03c64b5698bf4331fed8437fa6e9c726a07450bd 100644 >>> --- a/drivers/media/usb/uvc/uvc_v4l2.c >>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >>> @@ -1044,6 +1044,8 @@ static long uvc_ioctl_default(struct file *file, void *priv, bool valid_prio, >>> switch (cmd) { >>> /* Dynamic controls. */ >>> case UVCIOC_CTRL_MAP: >>> + pr_warn_once("uvcvideo: " DEPRECATED >>> + "UVCIOC_CTRL_MAP ioctl will be eventually removed.\n"); >>> return uvc_ioctl_xu_ctrl_map(chain, arg); >>> >>> case UVCIOC_CTRL_QUERY: >> >> Deprecating and then removing this is going to be a long slow process. >> >> I was thinking that rather then remove it we would keep accepting the ioctl but instead >> of calling uvc_ioctl_xu_ctrl_map() we would simply return 0. E.g. change the above to: >> >> case UVCIOC_CTRL_MAP: >> pr_warn_once("uvcvideo: " DEPRECATED >> "UVCIOC_CTRL_MAP ioctl will eventually be ignored.\n"); >> return uvc_ioctl_xu_ctrl_map(chain, arg); >> >> And then say in one year after a kernel with the above is released change it to: >> >> case UVCIOC_CTRL_MAP: >> pr_warn_once("uvcvideo: " DEPRECATED >> "UVCIOC_CTRL_MAP ioctls are ignored.\n"); >> return 0; >> >> >> I think removing it in 1 year is too soon, but ignoring it is ok. This does mean >> that people will lose the custom v4l2-ctrls for which patch 2/6 is not adding >> mappings into the driver in 1 year after a kernel with the warning is released... >> >> I'm not 100% sure about this plan, so please let me know what you think. For >> outright deprecation warning + full removal I think we need to wait at least >> 2 years after shipping a kernel with the deprecation warning. > > Let me rephrase what you have written: > > today: > pr_warn_once("uvcvideo: " DEPRECATED "UVCIOC_CTRL_MAP ioctl will be > eventually ignored.\n"); > return uvc_ioctl_xu_ctrl_map(chain, arg); Ack for the above What I was trying to say for the 1 year / 2 year thing is not do "x after 1 year" and then "y after 2 years", but do either "x after 1 year" *or* "y after 2 years" So: > in 1 year: > pr_warn_once("uvcvideo: " DEPRECATED "UVCIOC_CTRL_MAP ioctl is ignored.\n"); > return 0; *or* > in 2 years: > return -ENOIOCTLCMD; The idea being that when we start doing warn-once + return 0 we can already remove all the code and keeping just the case label + pr_warn + return 0, which is not a lot of code to keep around. > Normally I would prefer not to lie to userspace (saying that the > mapping was done, but not doing it). > > But in this case, UVCIOC_CTRL_MAP does not seem to be very widely used > (check previous email), so I do not think it really matters if we skip > the "1 year step" and just return -ENOIOCTLCMD in 2 years. > > I leave it up to you to decide the deprecation steps. Laurent do you have any opinion on this ? Regards, Hans ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda ` (2 preceding siblings ...) 2025-11-19 19:37 ` [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP Ricardo Ribalda @ 2025-11-19 19:37 ` Ricardo Ribalda 2025-12-08 11:19 ` Hans de Goede 2025-12-22 0:56 ` Laurent Pinchart 2025-11-19 19:37 ` [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param Ricardo Ribalda 2025-11-19 19:37 ` [PATCH v2 6/6] media: uvcvideo: RFC: Convert allow_privacy_override into Kconfig Ricardo Ribalda 5 siblings, 2 replies; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw) To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda Manual formatting the GUIDs can lead to errors, document a programmatically way to format the GUIDs from lsusb into something tha the driver can use. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- include/linux/usb/uvc.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h index b939a01da11466747249c64c72a3ea40cd364a59..dea23aabbad48270c807463c1a7e1c0d1b79fc1f 100644 --- a/include/linux/usb/uvc.h +++ b/include/linux/usb/uvc.h @@ -10,6 +10,14 @@ /* ------------------------------------------------------------------------ * GUIDs + * + * The GUID returned by lsusb can be converted to this format with the + * following python snippet: + * + * import uuid + * id = "{01234567-89ab-cdef-0123-456789abcdef}" + * le = uuid.UUID(id).bytes_le + * print("{" + ", ".join([f"0x{b:02x}" for b in le]) + "}") */ #define UVC_GUID_UVC_CAMERA \ {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs 2025-11-19 19:37 ` [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs Ricardo Ribalda @ 2025-12-08 11:19 ` Hans de Goede 2025-12-22 0:56 ` Laurent Pinchart 1 sibling, 0 replies; 21+ messages in thread From: Hans de Goede @ 2025-12-08 11:19 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb Hi, On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > Manual formatting the GUIDs can lead to errors, document a > programmatically way to format the GUIDs from lsusb into something tha > the driver can use. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Thanks patch looks good to me: Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com> Since this is independent of the rest of the series I've merged this into my local uvc/for-next branch which I will push out later today. Regards, Hans > --- > include/linux/usb/uvc.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h > index b939a01da11466747249c64c72a3ea40cd364a59..dea23aabbad48270c807463c1a7e1c0d1b79fc1f 100644 > --- a/include/linux/usb/uvc.h > +++ b/include/linux/usb/uvc.h > @@ -10,6 +10,14 @@ > > /* ------------------------------------------------------------------------ > * GUIDs > + * > + * The GUID returned by lsusb can be converted to this format with the > + * following python snippet: > + * > + * import uuid > + * id = "{01234567-89ab-cdef-0123-456789abcdef}" > + * le = uuid.UUID(id).bytes_le > + * print("{" + ", ".join([f"0x{b:02x}" for b in le]) + "}") > */ > #define UVC_GUID_UVC_CAMERA \ > {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs 2025-11-19 19:37 ` [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs Ricardo Ribalda 2025-12-08 11:19 ` Hans de Goede @ 2025-12-22 0:56 ` Laurent Pinchart 1 sibling, 0 replies; 21+ messages in thread From: Laurent Pinchart @ 2025-12-22 0:56 UTC (permalink / raw) To: Ricardo Ribalda Cc: Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb Hi Ricardo, Thank you for the patch. On Wed, Nov 19, 2025 at 07:37:13PM +0000, Ricardo Ribalda wrote: > Manual formatting the GUIDs can lead to errors, document a > programmatically way to format the GUIDs from lsusb into something tha s/tha/that/ > the driver can use. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > include/linux/usb/uvc.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h > index b939a01da11466747249c64c72a3ea40cd364a59..dea23aabbad48270c807463c1a7e1c0d1b79fc1f 100644 > --- a/include/linux/usb/uvc.h > +++ b/include/linux/usb/uvc.h > @@ -10,6 +10,14 @@ > > /* ------------------------------------------------------------------------ > * GUIDs > + * > + * The GUID returned by lsusb can be converted to this format with the > + * following python snippet: > + * > + * import uuid > + * id = "{01234567-89ab-cdef-0123-456789abcdef}" > + * le = uuid.UUID(id).bytes_le > + * print("{" + ", ".join([f"0x{b:02x}" for b in le]) + "}") Note that lsusb has changed the endianness of the printed GUID at some point :-/ That was in 2017 though, so I think we can ignore the issue now. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > */ > #define UVC_GUID_UVC_CAMERA \ > {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda ` (3 preceding siblings ...) 2025-11-19 19:37 ` [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs Ricardo Ribalda @ 2025-11-19 19:37 ` Ricardo Ribalda 2025-11-19 21:54 ` Gergo Koteles 2025-12-08 11:58 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 6/6] media: uvcvideo: RFC: Convert allow_privacy_override into Kconfig Ricardo Ribalda 5 siblings, 2 replies; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw) To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda Some camera modules have XU controls that can configure the behaviour of the privacy LED. Block mapping of those controls, unless the module is configured with a new parameter: allow_privacy_override. This is just an interim solution. Based on the users feedback, we will either put the privacy controls behind a CONFIG option, or completely block them. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/uvc_ctrl.c | 38 ++++++++++++++++++++++++++++++++++++++ drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++++++++++++ drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++++ drivers/media/usb/uvc/uvcvideo.h | 2 ++ include/linux/usb/uvc.h | 4 ++++ 5 files changed, 71 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 57ce486f22bbc404a1f127539eb2d12373431631..d9cbb942f798dc7138608982a5d3e3ef9f8141f6 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -2951,6 +2951,35 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev, return ret; } +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector) +{ + /* + * This list is not exhaustive, it is a best effort to block access to + * non documented controls that can affect user's privacy. + */ + struct privacy_control { + u8 entity[16]; + u8 selector; + } privacy_control[] = { + { + .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, + .selector = 1, + }, + { + .entity = UVC_GUID_LOGITECH_PERIPHERAL, + .selector = 9, + }, + }; + int i; + + for (i = 0; i < ARRAY_SIZE(privacy_control); i++) + if (!memcmp(entity, privacy_control[i].entity, 16) && + selector == privacy_control[i].selector) + return true; + + return false; +} + int uvc_xu_ctrl_query(struct uvc_video_chain *chain, struct uvc_xu_control_query *xqry) { @@ -2995,6 +3024,15 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, return -ENOENT; } + if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) && + !uvc_allow_privacy_override_param) { + dev_warn_once(&chain->dev->intf->dev, + "Privacy related controls can only be accessed if param allow_privacy_override is true\n"); + uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n", + entity->guid, xqry->selector); + return -EACCES; + } + if (mutex_lock_interruptible(&chain->ctrl_mutex)) return -ERESTARTSYS; diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 71563d8f4bcf581694ccd4b665ff52b629caa0b6..c292bf8b6f57e9fdacee726285f5b46e638fd317 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -35,6 +35,7 @@ unsigned int uvc_hw_timestamps_param; static unsigned int uvc_quirks_param = -1; unsigned int uvc_dbg_param; unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; +bool uvc_allow_privacy_override_param; static struct usb_driver uvc_driver; @@ -2474,6 +2475,25 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); module_param_named(timeout, uvc_timeout_param, uint, 0644); MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); +static int param_set_privacy(const char *val, const struct kernel_param *kp) +{ + pr_warn_once("uvcvideo: " DEPRECATED + "allow_privacy_override parameter will be eventually removed.\n"); + return param_set_bool(val, kp); +} + +static const struct kernel_param_ops param_ops_privacy = { + .set = param_set_privacy, + .get = param_get_bool, +}; + +param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param); +module_param_cb(allow_privacy_override, ¶m_ops_privacy, + &uvc_allow_privacy_override_param, 0644); +__MODULE_PARM_TYPE(allow_privacy_override, "bool"); +MODULE_PARM_DESC(allow_privacy_override, + "Allow access to privacy related controls"); + /* ------------------------------------------------------------------------ * Driver initialization and cleanup */ diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 03c64b5698bf4331fed8437fa6e9c726a07450bd..510cf47c86a62ba7fe3c7fa51be82c996cf37f9f 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -133,6 +133,13 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, return -EINVAL; } + if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) && + !uvc_allow_privacy_override_param) { + dev_warn_once(&chain->dev->intf->dev, + "Privacy related controls can only be mapped if param allow_privacy_override is true\n"); + return -EACCES; + } + map = kzalloc(sizeof(*map), GFP_KERNEL); if (map == NULL) return -ENOMEM; diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 9a86d7f1f6ea022dace87614030bf0fde0d260f0..4b1a70e3100bbf2180411a865a89952a81d0f0a4 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -662,6 +662,7 @@ extern unsigned int uvc_clock_param; extern unsigned int uvc_dbg_param; extern unsigned int uvc_timeout_param; extern unsigned int uvc_hw_timestamps_param; +extern bool uvc_allow_privacy_override_param; #define uvc_dbg(_dev, flag, fmt, ...) \ do { \ @@ -792,6 +793,7 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, struct uvc_xu_control_query *xqry); void uvc_ctrl_cleanup_fh(struct uvc_fh *handle); +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector); /* Utility functions */ struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts, diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h index dea23aabbad48270c807463c1a7e1c0d1b79fc1f..70c2a7d2523611418aba363328f76667d379b571 100644 --- a/include/linux/usb/uvc.h +++ b/include/linux/usb/uvc.h @@ -49,6 +49,10 @@ #define UVC_GUID_LOGITECH_PERIPHERAL \ {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \ 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd } +#define UVC_GUID_LOGITECH_USER_HW_CONTROL_V1 \ + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \ + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x1f } + /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */ #define UVC_MSXU_CONTROL_FOCUS 0x01 -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param 2025-11-19 19:37 ` [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param Ricardo Ribalda @ 2025-11-19 21:54 ` Gergo Koteles 2025-11-19 22:09 ` Ricardo Ribalda 2025-12-08 11:58 ` Hans de Goede 1 sibling, 1 reply; 21+ messages in thread From: Gergo Koteles @ 2025-11-19 21:54 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb Hi Ricardo, On Wed, 2025-11-19 at 19:37 +0000, Ricardo Ribalda wrote: > Some camera modules have XU controls that can configure the behaviour of > the privacy LED. > > Block mapping of those controls, unless the module is configured with > a new parameter: allow_privacy_override. > > This is just an interim solution. Based on the users feedback, we will > either put the privacy controls behind a CONFIG option, or completely > block them. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 38 ++++++++++++++++++++++++++++++++++++++ > drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++++++++++++ > drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++++ > drivers/media/usb/uvc/uvcvideo.h | 2 ++ > include/linux/usb/uvc.h | 4 ++++ > 5 files changed, 71 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 57ce486f22bbc404a1f127539eb2d12373431631..d9cbb942f798dc7138608982a5d3e3ef9f8141f6 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -2951,6 +2951,35 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev, > return ret; > } > > +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector) > +{ > + /* > + * This list is not exhaustive, it is a best effort to block access to > + * non documented controls that can affect user's privacy. > + */ > + struct privacy_control { > + u8 entity[16]; > + u8 selector; > + } privacy_control[] = { > + { > + .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, > + .selector = 1, > + }, > + { > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 9, > + }, > + }; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(privacy_control); i++) > + if (!memcmp(entity, privacy_control[i].entity, 16) && > + selector == privacy_control[i].selector) > + return true; > + > + return false; > +} > + > int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > struct uvc_xu_control_query *xqry) > { > @@ -2995,6 +3024,15 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > return -ENOENT; > } > > + if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) && > + !uvc_allow_privacy_override_param) { > + dev_warn_once(&chain->dev->intf->dev, > + "Privacy related controls can only be accessed if param allow_privacy_override is true\n"); > + uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n", > + entity->guid, xqry->selector); > + return -EACCES; > + } > + What if this only applied to UVC_SET_CUR? > if (mutex_lock_interruptible(&chain->ctrl_mutex)) > return -ERESTARTSYS; > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 71563d8f4bcf581694ccd4b665ff52b629caa0b6..c292bf8b6f57e9fdacee726285f5b46e638fd317 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -35,6 +35,7 @@ unsigned int uvc_hw_timestamps_param; > static unsigned int uvc_quirks_param = -1; > unsigned int uvc_dbg_param; > unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; > +bool uvc_allow_privacy_override_param; > > static struct usb_driver uvc_driver; > > @@ -2474,6 +2475,25 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > module_param_named(timeout, uvc_timeout_param, uint, 0644); > MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); > > +static int param_set_privacy(const char *val, const struct kernel_param *kp) > +{ > + pr_warn_once("uvcvideo: " DEPRECATED > + "allow_privacy_override parameter will be eventually removed.\n"); > + return param_set_bool(val, kp); > +} > + > +static const struct kernel_param_ops param_ops_privacy = { > + .set = param_set_privacy, > + .get = param_get_bool, > +}; > + > +param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param); > +module_param_cb(allow_privacy_override, ¶m_ops_privacy, > + &uvc_allow_privacy_override_param, 0644); > +__MODULE_PARM_TYPE(allow_privacy_override, "bool"); > +MODULE_PARM_DESC(allow_privacy_override, > + "Allow access to privacy related controls"); > + > /* ------------------------------------------------------------------------ > * Driver initialization and cleanup > */ > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 03c64b5698bf4331fed8437fa6e9c726a07450bd..510cf47c86a62ba7fe3c7fa51be82c996cf37f9f 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -133,6 +133,13 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, > return -EINVAL; > } > > + if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) && > + !uvc_allow_privacy_override_param) { > + dev_warn_once(&chain->dev->intf->dev, > + "Privacy related controls can only be mapped if param allow_privacy_override is true\n"); > + return -EACCES; > + } > + Would a better solution be to be able to map and query, but not set? Regards, Gergo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param 2025-11-19 21:54 ` Gergo Koteles @ 2025-11-19 22:09 ` Ricardo Ribalda 0 siblings, 0 replies; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 22:09 UTC (permalink / raw) To: Gergo Koteles Cc: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb Hi Gergo Thanks for the review (and the discussion :) ) On Wed, 19 Nov 2025 at 22:54, Gergo Koteles <soyer@irl.hu> wrote: > > Hi Ricardo, > > On Wed, 2025-11-19 at 19:37 +0000, Ricardo Ribalda wrote: > > Some camera modules have XU controls that can configure the behaviour of > > the privacy LED. > > > > Block mapping of those controls, unless the module is configured with > > a new parameter: allow_privacy_override. > > > > This is just an interim solution. Based on the users feedback, we will > > either put the privacy controls behind a CONFIG option, or completely > > block them. > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > drivers/media/usb/uvc/uvc_ctrl.c | 38 ++++++++++++++++++++++++++++++++++++++ > > drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++++++++++++ > > drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++++ > > drivers/media/usb/uvc/uvcvideo.h | 2 ++ > > include/linux/usb/uvc.h | 4 ++++ > > 5 files changed, 71 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > > index 57ce486f22bbc404a1f127539eb2d12373431631..d9cbb942f798dc7138608982a5d3e3ef9f8141f6 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -2951,6 +2951,35 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev, > > return ret; > > } > > > > +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector) > > +{ > > + /* > > + * This list is not exhaustive, it is a best effort to block access to > > + * non documented controls that can affect user's privacy. > > + */ > > + struct privacy_control { > > + u8 entity[16]; > > + u8 selector; > > + } privacy_control[] = { > > + { > > + .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, > > + .selector = 1, > > + }, > > + { > > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > > + .selector = 9, > > + }, > > + }; > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(privacy_control); i++) > > + if (!memcmp(entity, privacy_control[i].entity, 16) && > > + selector == privacy_control[i].selector) > > + return true; > > + > > + return false; > > +} > > + > > int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > > struct uvc_xu_control_query *xqry) > > { > > @@ -2995,6 +3024,15 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > > return -ENOENT; > > } > > > > + if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) && > > + !uvc_allow_privacy_override_param) { > > + dev_warn_once(&chain->dev->intf->dev, > > + "Privacy related controls can only be accessed if param allow_privacy_override is true\n"); > > + uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n", > > + entity->guid, xqry->selector); > > + return -EACCES; > > + } > > + > > What if this only applied to UVC_SET_CUR? > > > if (mutex_lock_interruptible(&chain->ctrl_mutex)) > > return -ERESTARTSYS; > > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index 71563d8f4bcf581694ccd4b665ff52b629caa0b6..c292bf8b6f57e9fdacee726285f5b46e638fd317 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -35,6 +35,7 @@ unsigned int uvc_hw_timestamps_param; > > static unsigned int uvc_quirks_param = -1; > > unsigned int uvc_dbg_param; > > unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; > > +bool uvc_allow_privacy_override_param; > > > > static struct usb_driver uvc_driver; > > > > @@ -2474,6 +2475,25 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > > module_param_named(timeout, uvc_timeout_param, uint, 0644); > > MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); > > > > +static int param_set_privacy(const char *val, const struct kernel_param *kp) > > +{ > > + pr_warn_once("uvcvideo: " DEPRECATED > > + "allow_privacy_override parameter will be eventually removed.\n"); > > + return param_set_bool(val, kp); > > +} > > + > > +static const struct kernel_param_ops param_ops_privacy = { > > + .set = param_set_privacy, > > + .get = param_get_bool, > > +}; > > + > > +param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param); > > +module_param_cb(allow_privacy_override, ¶m_ops_privacy, > > + &uvc_allow_privacy_override_param, 0644); > > +__MODULE_PARM_TYPE(allow_privacy_override, "bool"); > > +MODULE_PARM_DESC(allow_privacy_override, > > + "Allow access to privacy related controls"); > > + > > /* ------------------------------------------------------------------------ > > * Driver initialization and cleanup > > */ > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index 03c64b5698bf4331fed8437fa6e9c726a07450bd..510cf47c86a62ba7fe3c7fa51be82c996cf37f9f 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -133,6 +133,13 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, > > return -EINVAL; > > } > > > > + if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) && > > + !uvc_allow_privacy_override_param) { > > + dev_warn_once(&chain->dev->intf->dev, > > + "Privacy related controls can only be mapped if param allow_privacy_override is true\n"); > > + return -EACCES; > > + } > > + > > Would a better solution be to be able to map and query, but not set? IMO it is less confusing if the controls are fully disabled. Maybe it is worth it to land the patches that we do not have any disagreement with (1-4) . And then make a new patchset thread with 5 and have a proper discussion there about usecases, mechanism to bypass block and future plans. Best regards > > > Regards, > Gergo -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param 2025-11-19 19:37 ` [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param Ricardo Ribalda 2025-11-19 21:54 ` Gergo Koteles @ 2025-12-08 11:58 ` Hans de Goede 2025-12-10 6:40 ` Ricardo Ribalda 1 sibling, 1 reply; 21+ messages in thread From: Hans de Goede @ 2025-12-08 11:58 UTC (permalink / raw) To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb Hi, On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > Some camera modules have XU controls that can configure the behaviour of > the privacy LED. > > Block mapping of those controls, unless the module is configured with > a new parameter: allow_privacy_override. > > This is just an interim solution. Based on the users feedback, we will > either put the privacy controls behind a CONFIG option, or completely > block them. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/usb/uvc/uvc_ctrl.c | 38 ++++++++++++++++++++++++++++++++++++++ > drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++++++++++++ > drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++++ > drivers/media/usb/uvc/uvcvideo.h | 2 ++ > include/linux/usb/uvc.h | 4 ++++ > 5 files changed, 71 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > index 57ce486f22bbc404a1f127539eb2d12373431631..d9cbb942f798dc7138608982a5d3e3ef9f8141f6 100644 > --- a/drivers/media/usb/uvc/uvc_ctrl.c > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > @@ -2951,6 +2951,35 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev, > return ret; > } > > +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector) > +{ > + /* > + * This list is not exhaustive, it is a best effort to block access to > + * non documented controls that can affect user's privacy. > + */ > + struct privacy_control { > + u8 entity[16]; > + u8 selector; > + } privacy_control[] = { > + { > + .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, > + .selector = 1, > + }, > + { > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > + .selector = 9, > + }, > + }; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(privacy_control); i++) > + if (!memcmp(entity, privacy_control[i].entity, 16) && > + selector == privacy_control[i].selector) > + return true; > + > + return false; > +} > + > int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > struct uvc_xu_control_query *xqry) > { > @@ -2995,6 +3024,15 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > return -ENOENT; > } > > + if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) && > + !uvc_allow_privacy_override_param) { > + dev_warn_once(&chain->dev->intf->dev, > + "Privacy related controls can only be accessed if param allow_privacy_override is true\n"); I would do /s/param/module parameter/ in the message here. I know it makes the message a bit long but I'm not sure all users will understand what kind of parameter is intended otherwise. > + uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n", > + entity->guid, xqry->selector); > + return -EACCES; > + } > + > if (mutex_lock_interruptible(&chain->ctrl_mutex)) > return -ERESTARTSYS; > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 71563d8f4bcf581694ccd4b665ff52b629caa0b6..c292bf8b6f57e9fdacee726285f5b46e638fd317 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -35,6 +35,7 @@ unsigned int uvc_hw_timestamps_param; > static unsigned int uvc_quirks_param = -1; > unsigned int uvc_dbg_param; > unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; > +bool uvc_allow_privacy_override_param; > > static struct usb_driver uvc_driver; > > @@ -2474,6 +2475,25 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > module_param_named(timeout, uvc_timeout_param, uint, 0644); > MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); > > +static int param_set_privacy(const char *val, const struct kernel_param *kp) > +{ > + pr_warn_once("uvcvideo: " DEPRECATED > + "allow_privacy_override parameter will be eventually removed.\n"); > + return param_set_bool(val, kp); > +} > + > +static const struct kernel_param_ops param_ops_privacy = { > + .set = param_set_privacy, > + .get = param_get_bool, > +}; > + > +param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param); > +module_param_cb(allow_privacy_override, ¶m_ops_privacy, > + &uvc_allow_privacy_override_param, 0644); > +__MODULE_PARM_TYPE(allow_privacy_override, "bool"); > +MODULE_PARM_DESC(allow_privacy_override, > + "Allow access to privacy related controls"); > + > /* ------------------------------------------------------------------------ > * Driver initialization and cleanup > */ > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 03c64b5698bf4331fed8437fa6e9c726a07450bd..510cf47c86a62ba7fe3c7fa51be82c996cf37f9f 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -133,6 +133,13 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, > return -EINVAL; > } > > + if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) && > + !uvc_allow_privacy_override_param) { > + dev_warn_once(&chain->dev->intf->dev, > + "Privacy related controls can only be mapped if param allow_privacy_override is true\n"); Same comment on the msg. > + return -EACCES; Did you test this with uvcdynctrl? will it log the error and continue with other controls, or will it abort mapping controls when it hits this ? I agree with the change and with the -EACCES error return I'm just wondering how known userspace consumers of the API will react to this change. I've been thinking a bit about this and although not ideal I agree that this approach is likely the best way to deal with the Logitech custom privacy LED controls. Regards, Hans > + } > + > map = kzalloc(sizeof(*map), GFP_KERNEL); > if (map == NULL) > return -ENOMEM; > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 9a86d7f1f6ea022dace87614030bf0fde0d260f0..4b1a70e3100bbf2180411a865a89952a81d0f0a4 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -662,6 +662,7 @@ extern unsigned int uvc_clock_param; > extern unsigned int uvc_dbg_param; > extern unsigned int uvc_timeout_param; > extern unsigned int uvc_hw_timestamps_param; > +extern bool uvc_allow_privacy_override_param; > > #define uvc_dbg(_dev, flag, fmt, ...) \ > do { \ > @@ -792,6 +793,7 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > struct uvc_xu_control_query *xqry); > > void uvc_ctrl_cleanup_fh(struct uvc_fh *handle); > +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector); > > /* Utility functions */ > struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts, > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h > index dea23aabbad48270c807463c1a7e1c0d1b79fc1f..70c2a7d2523611418aba363328f76667d379b571 100644 > --- a/include/linux/usb/uvc.h > +++ b/include/linux/usb/uvc.h > @@ -49,6 +49,10 @@ > #define UVC_GUID_LOGITECH_PERIPHERAL \ > {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \ > 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd } > +#define UVC_GUID_LOGITECH_USER_HW_CONTROL_V1 \ > + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \ > + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x1f } > + > > /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */ > #define UVC_MSXU_CONTROL_FOCUS 0x01 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param 2025-12-08 11:58 ` Hans de Goede @ 2025-12-10 6:40 ` Ricardo Ribalda 0 siblings, 0 replies; 21+ messages in thread From: Ricardo Ribalda @ 2025-12-10 6:40 UTC (permalink / raw) To: Hans de Goede Cc: Laurent Pinchart, Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-media, linux-kernel, linux-usb On Mon, 8 Dec 2025 at 20:58, Hans de Goede <hansg@kernel.org> wrote: > > Hi, > > On 19-Nov-25 8:37 PM, Ricardo Ribalda wrote: > > Some camera modules have XU controls that can configure the behaviour of > > the privacy LED. > > > > Block mapping of those controls, unless the module is configured with > > a new parameter: allow_privacy_override. > > > > This is just an interim solution. Based on the users feedback, we will > > either put the privacy controls behind a CONFIG option, or completely > > block them. > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > drivers/media/usb/uvc/uvc_ctrl.c | 38 ++++++++++++++++++++++++++++++++++++++ > > drivers/media/usb/uvc/uvc_driver.c | 20 ++++++++++++++++++++ > > drivers/media/usb/uvc/uvc_v4l2.c | 7 +++++++ > > drivers/media/usb/uvc/uvcvideo.h | 2 ++ > > include/linux/usb/uvc.h | 4 ++++ > > 5 files changed, 71 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c > > index 57ce486f22bbc404a1f127539eb2d12373431631..d9cbb942f798dc7138608982a5d3e3ef9f8141f6 100644 > > --- a/drivers/media/usb/uvc/uvc_ctrl.c > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c > > @@ -2951,6 +2951,35 @@ static int uvc_ctrl_init_xu_ctrl(struct uvc_device *dev, > > return ret; > > } > > > > +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector) > > +{ > > + /* > > + * This list is not exhaustive, it is a best effort to block access to > > + * non documented controls that can affect user's privacy. > > + */ > > + struct privacy_control { > > + u8 entity[16]; > > + u8 selector; > > + } privacy_control[] = { > > + { > > + .entity = UVC_GUID_LOGITECH_USER_HW_CONTROL_V1, > > + .selector = 1, > > + }, > > + { > > + .entity = UVC_GUID_LOGITECH_PERIPHERAL, > > + .selector = 9, > > + }, > > + }; > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(privacy_control); i++) > > + if (!memcmp(entity, privacy_control[i].entity, 16) && > > + selector == privacy_control[i].selector) > > + return true; > > + > > + return false; > > +} > > + > > int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > > struct uvc_xu_control_query *xqry) > > { > > @@ -2995,6 +3024,15 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > > return -ENOENT; > > } > > > > + if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) && > > + !uvc_allow_privacy_override_param) { > > + dev_warn_once(&chain->dev->intf->dev, > > + "Privacy related controls can only be accessed if param allow_privacy_override is true\n"); > > I would do /s/param/module parameter/ in the message here. I know it makes the message a bit > long but I'm not sure all users will understand what kind of parameter is intended otherwise. Ack > > > + uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n", > > + entity->guid, xqry->selector); > > + return -EACCES; > > + } > > + > > if (mutex_lock_interruptible(&chain->ctrl_mutex)) > > return -ERESTARTSYS; > > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index 71563d8f4bcf581694ccd4b665ff52b629caa0b6..c292bf8b6f57e9fdacee726285f5b46e638fd317 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -35,6 +35,7 @@ unsigned int uvc_hw_timestamps_param; > > static unsigned int uvc_quirks_param = -1; > > unsigned int uvc_dbg_param; > > unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; > > +bool uvc_allow_privacy_override_param; > > > > static struct usb_driver uvc_driver; > > > > @@ -2474,6 +2475,25 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); > > module_param_named(timeout, uvc_timeout_param, uint, 0644); > > MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); > > > > +static int param_set_privacy(const char *val, const struct kernel_param *kp) > > +{ > > + pr_warn_once("uvcvideo: " DEPRECATED > > + "allow_privacy_override parameter will be eventually removed.\n"); > > + return param_set_bool(val, kp); > > +} > > + > > +static const struct kernel_param_ops param_ops_privacy = { > > + .set = param_set_privacy, > > + .get = param_get_bool, > > +}; > > + > > +param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param); > > +module_param_cb(allow_privacy_override, ¶m_ops_privacy, > > + &uvc_allow_privacy_override_param, 0644); > > +__MODULE_PARM_TYPE(allow_privacy_override, "bool"); > > +MODULE_PARM_DESC(allow_privacy_override, > > + "Allow access to privacy related controls"); > > + > > /* ------------------------------------------------------------------------ > > * Driver initialization and cleanup > > */ > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index 03c64b5698bf4331fed8437fa6e9c726a07450bd..510cf47c86a62ba7fe3c7fa51be82c996cf37f9f 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -133,6 +133,13 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, > > return -EINVAL; > > } > > > > + if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) && > > + !uvc_allow_privacy_override_param) { > > + dev_warn_once(&chain->dev->intf->dev, > > + "Privacy related controls can only be mapped if param allow_privacy_override is true\n"); > > Same comment on the msg. > > > + return -EACCES; > > Did you test this with uvcdynctrl? will it log the error and continue with other controls, > or will it abort mapping controls when it hits this ? I tested with the logitech camera and virtme-ng: https://patchwork.linuxtv.org/project/linux-media/cover/20251119-uvcdynctrl-v2-0-0359ffb98c9e@chromium.org/ It is the first time that I use virtme for uvc... and it works like a charm! Also looking at uvcdynctrl it seems to log the errors and continue. If you have a specific test in mind I can try it next week. > > I agree with the change and with the -EACCES error return I'm just wondering how > known userspace consumers of the API will react to this change. > > I've been thinking a bit about this and although not ideal I agree that this approach > is likely the best way to deal with the Logitech custom privacy LED controls. +1 > > Regards, > > Hans > > > > + } > > + > > map = kzalloc(sizeof(*map), GFP_KERNEL); > > if (map == NULL) > > return -ENOMEM; > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > > index 9a86d7f1f6ea022dace87614030bf0fde0d260f0..4b1a70e3100bbf2180411a865a89952a81d0f0a4 100644 > > --- a/drivers/media/usb/uvc/uvcvideo.h > > +++ b/drivers/media/usb/uvc/uvcvideo.h > > @@ -662,6 +662,7 @@ extern unsigned int uvc_clock_param; > > extern unsigned int uvc_dbg_param; > > extern unsigned int uvc_timeout_param; > > extern unsigned int uvc_hw_timestamps_param; > > +extern bool uvc_allow_privacy_override_param; > > > > #define uvc_dbg(_dev, flag, fmt, ...) \ > > do { \ > > @@ -792,6 +793,7 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, > > struct uvc_xu_control_query *xqry); > > > > void uvc_ctrl_cleanup_fh(struct uvc_fh *handle); > > +bool uvc_ctrl_is_privacy_control(u8 entity[16], u8 selector); > > > > /* Utility functions */ > > struct usb_host_endpoint *uvc_find_endpoint(struct usb_host_interface *alts, > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h > > index dea23aabbad48270c807463c1a7e1c0d1b79fc1f..70c2a7d2523611418aba363328f76667d379b571 100644 > > --- a/include/linux/usb/uvc.h > > +++ b/include/linux/usb/uvc.h > > @@ -49,6 +49,10 @@ > > #define UVC_GUID_LOGITECH_PERIPHERAL \ > > {0x21, 0x2d, 0xe5, 0xff, 0x30, 0x80, 0x2c, 0x4e, \ > > 0x82, 0xd9, 0xf5, 0x87, 0xd0, 0x05, 0x40, 0xbd } > > +#define UVC_GUID_LOGITECH_USER_HW_CONTROL_V1 \ > > + {0x82, 0x06, 0x61, 0x63, 0x70, 0x50, 0xab, 0x49, \ > > + 0xb8, 0xcc, 0xb3, 0x85, 0x5e, 0x8d, 0x22, 0x1f } > > + > > > > /* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */ > > #define UVC_MSXU_CONTROL_FOCUS 0x01 > > > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 6/6] media: uvcvideo: RFC: Convert allow_privacy_override into Kconfig 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda ` (4 preceding siblings ...) 2025-11-19 19:37 ` [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param Ricardo Ribalda @ 2025-11-19 19:37 ` Ricardo Ribalda 5 siblings, 0 replies; 21+ messages in thread From: Ricardo Ribalda @ 2025-11-19 19:37 UTC (permalink / raw) To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab, Greg Kroah-Hartman Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda This patch is just shared for discussion purposes! Do not land. In a perfect world, after a deprecation process, we will be able to remove allow_privacy_override and block all privacy related controls. If there is any usecase out in the field that resists, we shall move it into a Kconfig. This patch shows how the transition to Kconfig can look. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/usb/uvc/Kconfig | 11 +++++++++++ drivers/media/usb/uvc/uvc_ctrl.c | 4 ++-- drivers/media/usb/uvc/uvc_driver.c | 20 -------------------- drivers/media/usb/uvc/uvc_v4l2.c | 4 ++-- drivers/media/usb/uvc/uvcvideo.h | 1 - 5 files changed, 15 insertions(+), 25 deletions(-) diff --git a/drivers/media/usb/uvc/Kconfig b/drivers/media/usb/uvc/Kconfig index 579532272fd6d7a8ef65c1a3a892b723f40e584e..7c0f2260d1357cc4f27fa63d90c42f61afd92da9 100644 --- a/drivers/media/usb/uvc/Kconfig +++ b/drivers/media/usb/uvc/Kconfig @@ -20,3 +20,14 @@ config USB_VIDEO_CLASS_INPUT_EVDEV to report button events. If you are in doubt, say Y. + +config USB_VIDEO_CLASS_ALLOW_PRIVACY_OVERRIDE + bool "Allow overriding the privacy controls" + default n + depends on USB_VIDEO_CLASS && BROKEN + help + If this option is enabled, the privacy related controls, such as + the ones controlling the privacy LED will be accessible from + userspace. + + If you are in doubt, say N. diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index d9cbb942f798dc7138608982a5d3e3ef9f8141f6..c41724a342e57f64f3c10af9752bd45209f80e36 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -3025,9 +3025,9 @@ int uvc_xu_ctrl_query(struct uvc_video_chain *chain, } if (uvc_ctrl_is_privacy_control(entity->guid, xqry->selector) && - !uvc_allow_privacy_override_param) { + !IS_ENABLED(CONFIG_USB_VIDEO_CLASS_ALLOW_PRIVACY_OVERRIDE)) { dev_warn_once(&chain->dev->intf->dev, - "Privacy related controls can only be accessed if param allow_privacy_override is true\n"); + "Privacy related controls can only be accessed if CONFIG_USB_VIDEO_CLASS_ALLOW_PRIVACY_OVERRIDE is true\n"); uvc_dbg(chain->dev, CONTROL, "Blocking access to privacy related Control %pUl/%u\n", entity->guid, xqry->selector); return -EACCES; diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index c292bf8b6f57e9fdacee726285f5b46e638fd317..71563d8f4bcf581694ccd4b665ff52b629caa0b6 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -35,7 +35,6 @@ unsigned int uvc_hw_timestamps_param; static unsigned int uvc_quirks_param = -1; unsigned int uvc_dbg_param; unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; -bool uvc_allow_privacy_override_param; static struct usb_driver uvc_driver; @@ -2475,25 +2474,6 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); module_param_named(timeout, uvc_timeout_param, uint, 0644); MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); -static int param_set_privacy(const char *val, const struct kernel_param *kp) -{ - pr_warn_once("uvcvideo: " DEPRECATED - "allow_privacy_override parameter will be eventually removed.\n"); - return param_set_bool(val, kp); -} - -static const struct kernel_param_ops param_ops_privacy = { - .set = param_set_privacy, - .get = param_get_bool, -}; - -param_check_bool(allow_privacy_override, &uvc_allow_privacy_override_param); -module_param_cb(allow_privacy_override, ¶m_ops_privacy, - &uvc_allow_privacy_override_param, 0644); -__MODULE_PARM_TYPE(allow_privacy_override, "bool"); -MODULE_PARM_DESC(allow_privacy_override, - "Allow access to privacy related controls"); - /* ------------------------------------------------------------------------ * Driver initialization and cleanup */ diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 510cf47c86a62ba7fe3c7fa51be82c996cf37f9f..d52497d68b910553f385b1b41ec5c4eecb915743 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -134,9 +134,9 @@ static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, } if (uvc_ctrl_is_privacy_control(xmap->entity, xmap->selector) && - !uvc_allow_privacy_override_param) { + !IS_ENABLED(CONFIG_USB_VIDEO_CLASS_ALLOW_PRIVACY_OVERRIDE)) { dev_warn_once(&chain->dev->intf->dev, - "Privacy related controls can only be mapped if param allow_privacy_override is true\n"); + "Privacy related controls can only be mapped if CONFIG_USB_VIDEO_CLASS_ALLOW_PRIVACY_OVERRIDE is true\n"); return -EACCES; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 4b1a70e3100bbf2180411a865a89952a81d0f0a4..5da219e1c6ac89e89a1658f1126bfa292876d55f 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -662,7 +662,6 @@ extern unsigned int uvc_clock_param; extern unsigned int uvc_dbg_param; extern unsigned int uvc_timeout_param; extern unsigned int uvc_hw_timestamps_param; -extern bool uvc_allow_privacy_override_param; #define uvc_dbg(_dev, flag, fmt, ...) \ do { \ -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-12-22 0:57 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-11-19 19:37 [PATCH v2 0/6] media: uvcvideo: Map known XU controls Ricardo Ribalda 2025-11-19 19:37 ` [PATCH v2 1/6] media: uvcvideo: Remove nodrop parameter Ricardo Ribalda 2025-12-08 10:54 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 2/6] media: uvcvideo: Import standard controls from uvcdynctrl Ricardo Ribalda 2025-12-08 11:02 ` Hans de Goede 2025-12-08 11:12 ` Hans de Goede 2025-12-09 6:28 ` Ricardo Ribalda 2025-12-09 16:22 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 3/6] media: uvcvideo: Announce deprecation intentions for UVCIOC_CTRL_MAP Ricardo Ribalda 2025-12-08 11:17 ` Hans de Goede 2025-12-09 6:41 ` Ricardo Ribalda 2025-12-09 16:27 ` Hans de Goede 2025-11-19 19:37 ` [PATCH v2 4/6] media: uvcvideo: Document how to format GUIDs Ricardo Ribalda 2025-12-08 11:19 ` Hans de Goede 2025-12-22 0:56 ` Laurent Pinchart 2025-11-19 19:37 ` [PATCH v2 5/6] media: uvcvideo: Introduce allow_privacy_override param Ricardo Ribalda 2025-11-19 21:54 ` Gergo Koteles 2025-11-19 22:09 ` Ricardo Ribalda 2025-12-08 11:58 ` Hans de Goede 2025-12-10 6:40 ` Ricardo Ribalda 2025-11-19 19:37 ` [PATCH v2 6/6] media: uvcvideo: RFC: Convert allow_privacy_override into Kconfig Ricardo Ribalda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®