From 50459f103edfe47c9a599d766a850ef6014936c5 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 15 Nov 2016 18:44:29 +0200 Subject: [PATCH 01/27] media: uvcvideo: Remove format descriptions The V4L2 core overwrites format descriptions in v4l_fill_fmtdesc(), there's no need to manually set the descriptions in the driver. This prepares for removal of the format descriptions from the uvc_fmts table. Unlike V4L2, UVC makes a distinction between the SD-DV, SDL-DV and HD-DV formats. It also indicates whether the DV format uses 50Hz or 60Hz. This information is parsed by the driver to construct a format name string that is printed in a debug message, but serves no other purpose as V4L2 has a single V4L2_PIX_FMT_DV pixel format that covers all those cases. As the information is available in the UVC descriptors, and thus accessible to users with lsusb if they really care, don't log it in a debug message and drop the format name string to simplify the code. Signed-off-by: Laurent Pinchart Reviewed-by: Ricardo Ribalda Reviewed-by: Michael Grzeschik --- drivers/media/usb/uvc/uvc_driver.c | 24 ++---------------------- drivers/media/usb/uvc/uvc_v4l2.c | 2 -- drivers/media/usb/uvc/uvcvideo.h | 2 -- 3 files changed, 2 insertions(+), 26 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index e4bcb5011360..0a35d7c0d0a0 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -251,14 +251,10 @@ static int uvc_parse_format(struct uvc_device *dev, fmtdesc = uvc_format_by_guid(&buffer[5]); if (fmtdesc != NULL) { - strscpy(format->name, fmtdesc->name, - sizeof(format->name)); format->fcc = fmtdesc->fcc; } else { dev_info(&streaming->intf->dev, "Unknown video format %pUl\n", &buffer[5]); - snprintf(format->name, sizeof(format->name), "%pUl\n", - &buffer[5]); format->fcc = 0; } @@ -270,8 +266,6 @@ static int uvc_parse_format(struct uvc_device *dev, */ if (dev->quirks & UVC_QUIRK_FORCE_Y8) { if (format->fcc == V4L2_PIX_FMT_YUYV) { - strscpy(format->name, "Greyscale 8-bit (Y8 )", - sizeof(format->name)); format->fcc = V4L2_PIX_FMT_GREY; format->bpp = 8; width_multiplier = 2; @@ -312,7 +306,6 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - strscpy(format->name, "MJPEG", sizeof(format->name)); format->fcc = V4L2_PIX_FMT_MJPEG; format->flags = UVC_FMT_FLAG_COMPRESSED; format->bpp = 0; @@ -328,17 +321,7 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - switch (buffer[8] & 0x7f) { - case 0: - strscpy(format->name, "SD-DV", sizeof(format->name)); - break; - case 1: - strscpy(format->name, "SDL-DV", sizeof(format->name)); - break; - case 2: - strscpy(format->name, "HD-DV", sizeof(format->name)); - break; - default: + if ((buffer[8] & 0x7f) > 2) { uvc_dbg(dev, DESCR, "device %d videostreaming interface %d: unknown DV format %u\n", dev->udev->devnum, @@ -346,9 +329,6 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz", - sizeof(format->name)); - format->fcc = V4L2_PIX_FMT_DV; format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM; format->bpp = 0; @@ -375,7 +355,7 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - uvc_dbg(dev, DESCR, "Found format %s\n", format->name); + uvc_dbg(dev, DESCR, "Found format %p4cc", &format->fcc); buflen -= buffer[0]; buffer += buffer[0]; diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index f4d4c33b6dfb..dcd178d249b6 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -660,8 +660,6 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, fmt->flags = 0; if (format->flags & UVC_FMT_FLAG_COMPRESSED) fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; - strscpy(fmt->description, format->name, sizeof(fmt->description)); - fmt->description[sizeof(fmt->description) - 1] = 0; fmt->pixelformat = format->fcc; return 0; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index df93db259312..b60e4ae95e81 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -264,8 +264,6 @@ struct uvc_format { u32 fcc; u32 flags; - char name[32]; - unsigned int nframes; struct uvc_frame *frame; }; From 41ddb251c68ac75c101d3a50a68c4629c9055e4c Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 20 Sep 2022 16:04:55 +0200 Subject: [PATCH 02/27] media: uvcvideo: Handle cameras with invalid descriptors If the source entity does not contain any pads, do not create a link. Reported-by: syzbot Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_entity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_entity.c b/drivers/media/usb/uvc/uvc_entity.c index 7c4d2f93d351..cc68dd24eb42 100644 --- a/drivers/media/usb/uvc/uvc_entity.c +++ b/drivers/media/usb/uvc/uvc_entity.c @@ -37,7 +37,7 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain, continue; remote = uvc_entity_by_id(chain->dev, entity->baSourceID[i]); - if (remote == NULL) + if (remote == NULL || remote->num_pads == 0) return -EINVAL; source = (UVC_ENTITY_TYPE(remote) == UVC_TT_STREAMING) From 3bc22dc66a4f386393119118169ed0b78c389898 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 20 Sep 2022 16:09:50 +0200 Subject: [PATCH 03/27] media: uvcvideo: Only create input devs if hw supports it Examine the stream headers to figure out if the device has a button and can be used as an input. Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_status.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c index 7518ffce22ed..cb90aff344bc 100644 --- a/drivers/media/usb/uvc/uvc_status.c +++ b/drivers/media/usb/uvc/uvc_status.c @@ -18,11 +18,34 @@ * Input device */ #ifdef CONFIG_USB_VIDEO_CLASS_INPUT_EVDEV + +static bool uvc_input_has_button(struct uvc_device *dev) +{ + struct uvc_streaming *stream; + + /* + * The device has button events if both bTriggerSupport and + * bTriggerUsage are one. Otherwise the camera button does not + * exist or is handled automatically by the camera without host + * driver or client application intervention. + */ + list_for_each_entry(stream, &dev->streams, list) { + if (stream->header.bTriggerSupport == 1 && + stream->header.bTriggerUsage == 1) + return true; + } + + return false; +} + static int uvc_input_init(struct uvc_device *dev) { struct input_dev *input; int ret; + if (!uvc_input_has_button(dev)) + return 0; + input = input_allocate_device(); if (input == NULL) return -ENOMEM; From 4867bb590ae445bcfaa711a86b603c97e94574b3 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Tue, 25 Oct 2022 16:41:01 +0200 Subject: [PATCH 04/27] media: uvcvideo: Handle errors from calls to usb_string On a Webcam from Quanta, we see the following error. usb 3-5: New USB device found, idVendor=0408, idProduct=30d2, bcdDevice= 0.03 usb 3-5: New USB device strings: Mfr=3, Product=1, SerialNumber=2 usb 3-5: Product: USB2.0 HD UVC WebCam usb 3-5: Manufacturer: Quanta usb 3-5: SerialNumber: 0x0001 ... uvcvideo: Found UVC 1.10 device USB2.0 HD UVC WebCam (0408:30d2) uvcvideo: Failed to initialize entity for entity 5 uvcvideo: Failed to register entities (-22). The Webcam reports an entity of type UVC_VC_EXTENSION_UNIT. It reports a string index of '7' associated with that entity. The attempt to read that string from the camera fails with error -32 (-EPIPE). usb_string() returns that error, but it is ignored. As result, the entity name is empty. This later causes v4l2_device_register_subdev() to return -EINVAL, and no entities are registered as result. While this appears to be a firmware problem with the camera, the kernel should still handle the situation gracefully. To do that, check the return value from usb_string(). If it reports an error, assign the entity's default name. Signed-off-by: Guenter Roeck Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 48 ++++++++++++------------------ 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 0a35d7c0d0a0..4c372965517d 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -859,10 +859,8 @@ static int uvc_parse_vendor_control(struct uvc_device *dev, + n; memcpy(unit->extension.bmControls, &buffer[23+p], 2*n); - if (buffer[24+p+2*n] != 0) - usb_string(udev, buffer[24+p+2*n], unit->name, - sizeof(unit->name)); - else + if (buffer[24+p+2*n] == 0 || + usb_string(udev, buffer[24+p+2*n], unit->name, sizeof(unit->name)) < 0) sprintf(unit->name, "Extension %u", buffer[3]); list_add_tail(&unit->list, &dev->entities); @@ -986,15 +984,15 @@ static int uvc_parse_standard_control(struct uvc_device *dev, memcpy(term->media.bmTransportModes, &buffer[10+n], p); } - if (buffer[7] != 0) - usb_string(udev, buffer[7], term->name, - sizeof(term->name)); - else if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA) - sprintf(term->name, "Camera %u", buffer[3]); - else if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT) - sprintf(term->name, "Media %u", buffer[3]); - else - sprintf(term->name, "Input %u", buffer[3]); + if (buffer[7] == 0 || + usb_string(udev, buffer[7], term->name, sizeof(term->name)) < 0) { + if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA) + sprintf(term->name, "Camera %u", buffer[3]); + if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT) + sprintf(term->name, "Media %u", buffer[3]); + else + sprintf(term->name, "Input %u", buffer[3]); + } list_add_tail(&term->list, &dev->entities); break; @@ -1027,10 +1025,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev, memcpy(term->baSourceID, &buffer[7], 1); - if (buffer[8] != 0) - usb_string(udev, buffer[8], term->name, - sizeof(term->name)); - else + if (buffer[8] == 0 || + usb_string(udev, buffer[8], term->name, sizeof(term->name)) < 0) sprintf(term->name, "Output %u", buffer[3]); list_add_tail(&term->list, &dev->entities); @@ -1052,10 +1048,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev, memcpy(unit->baSourceID, &buffer[5], p); - if (buffer[5+p] != 0) - usb_string(udev, buffer[5+p], unit->name, - sizeof(unit->name)); - else + if (buffer[5+p] == 0 || + usb_string(udev, buffer[5+p], unit->name, sizeof(unit->name)) < 0) sprintf(unit->name, "Selector %u", buffer[3]); list_add_tail(&unit->list, &dev->entities); @@ -1085,10 +1079,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev, if (dev->uvc_version >= 0x0110) unit->processing.bmVideoStandards = buffer[9+n]; - if (buffer[8+n] != 0) - usb_string(udev, buffer[8+n], unit->name, - sizeof(unit->name)); - else + if (buffer[8+n] == 0 || + usb_string(udev, buffer[8+n], unit->name, sizeof(unit->name)) < 0) sprintf(unit->name, "Processing %u", buffer[3]); list_add_tail(&unit->list, &dev->entities); @@ -1116,10 +1108,8 @@ static int uvc_parse_standard_control(struct uvc_device *dev, unit->extension.bmControls = (u8 *)unit + sizeof(*unit); memcpy(unit->extension.bmControls, &buffer[23+p], n); - if (buffer[23+p+n] != 0) - usb_string(udev, buffer[23+p+n], unit->name, - sizeof(unit->name)); - else + if (buffer[23+p+n] == 0 || + usb_string(udev, buffer[23+p+n], unit->name, sizeof(unit->name)) < 0) sprintf(unit->name, "Extension %u", buffer[3]); list_add_tail(&unit->list, &dev->entities); From 2a8c1952ed4bc85f64174def8c00c71c264dc3a2 Mon Sep 17 00:00:00 2001 From: Pedro Guilherme Siqueira Moreira Date: Tue, 25 Oct 2022 02:04:48 -0300 Subject: [PATCH 05/27] media: uvcvideo: Fix missing newline after declarations Fixes 'Missing a blank line after declarations' warning issued by scripts/checkpatch.pl on drivers/media/usb/uvc/uvc_driver.c Signed-off-by: Pedro Guilherme Siqueira Moreira Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 4c372965517d..8f649fa9ab22 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -712,6 +712,7 @@ static int uvc_parse_streaming(struct uvc_device *dev, /* Parse the alternate settings to find the maximum bandwidth. */ for (i = 0; i < intf->num_altsetting; ++i) { struct usb_host_endpoint *ep; + alts = &intf->altsetting[i]; ep = uvc_find_endpoint(alts, streaming->header.bEndpointAddress); @@ -1826,12 +1827,14 @@ static void uvc_delete(struct kref *kref) list_for_each_safe(p, n, &dev->chains) { struct uvc_video_chain *chain; + chain = list_entry(p, struct uvc_video_chain, list); kfree(chain); } list_for_each_safe(p, n, &dev->entities) { struct uvc_entity *entity; + entity = list_entry(p, struct uvc_entity, list); #ifdef CONFIG_MEDIA_CONTROLLER uvc_mc_cleanup_entity(entity); @@ -1841,6 +1844,7 @@ static void uvc_delete(struct kref *kref) list_for_each_safe(p, n, &dev->streams) { struct uvc_streaming *streaming; + streaming = list_entry(p, struct uvc_streaming, list); usb_driver_release_interface(&uvc_driver.driver, streaming->intf); From 7b78a8464d5701796a4e146b3973422350e56977 Mon Sep 17 00:00:00 2001 From: Pedro Guilherme Siqueira Moreira Date: Tue, 25 Oct 2022 02:04:49 -0300 Subject: [PATCH 06/27] media: uvcvideo: Fix assignment inside if condition Fixes 'do not use assignment in if condition' errors issued by scripts/checkpatch.pl on drivers/media/usb/uvc/uvc_driver.c Signed-off-by: Pedro Guilherme Siqueira Moreira Reviewed-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 8f649fa9ab22..d84e46fdb1b9 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1144,7 +1144,8 @@ static int uvc_parse_control(struct uvc_device *dev) buffer[1] != USB_DT_CS_INTERFACE) goto next_descriptor; - if ((ret = uvc_parse_standard_control(dev, buffer, buflen)) < 0) + ret = uvc_parse_standard_control(dev, buffer, buflen); + if (ret < 0) return ret; next_descriptor: @@ -2180,7 +2181,8 @@ static int uvc_probe(struct usb_interface *intf, usb_set_intfdata(intf, dev); /* Initialize the interrupt URB. */ - if ((ret = uvc_status_init(dev)) < 0) { + ret = uvc_status_init(dev); + if (ret < 0) { dev_info(&dev->udev->dev, "Unable to initialize the status endpoint (%d), status interrupt will not be supported.\n", ret); From 0ce75d5ecd9edaa027c0c2a7df0edcdf59ea7738 Mon Sep 17 00:00:00 2001 From: Pedro Guilherme Siqueira Moreira Date: Tue, 25 Oct 2022 02:04:50 -0300 Subject: [PATCH 07/27] media: uvcvideo: Fix usage of symbolic permissions to octal Change symbolic permissions to octal equivalents as recommended by scripts/checkpatch.pl on drivers/media/usb/uvc/uvc_driver.c. Signed-off-by: Pedro Guilherme Siqueira Moreira Reviewed-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index d84e46fdb1b9..4f59583bf516 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2329,17 +2329,17 @@ static int uvc_clock_param_set(const char *val, const struct kernel_param *kp) } module_param_call(clock, uvc_clock_param_set, uvc_clock_param_get, - &uvc_clock_param, S_IRUGO|S_IWUSR); + &uvc_clock_param, 0644); MODULE_PARM_DESC(clock, "Video buffers timestamp clock"); -module_param_named(hwtimestamps, uvc_hw_timestamps_param, uint, S_IRUGO|S_IWUSR); +module_param_named(hwtimestamps, uvc_hw_timestamps_param, uint, 0644); MODULE_PARM_DESC(hwtimestamps, "Use hardware timestamps"); -module_param_named(nodrop, uvc_no_drop_param, uint, S_IRUGO|S_IWUSR); +module_param_named(nodrop, uvc_no_drop_param, uint, 0644); MODULE_PARM_DESC(nodrop, "Don't drop incomplete frames"); -module_param_named(quirks, uvc_quirks_param, uint, S_IRUGO|S_IWUSR); +module_param_named(quirks, uvc_quirks_param, uint, 0644); MODULE_PARM_DESC(quirks, "Forced device quirks"); -module_param_named(trace, uvc_dbg_param, uint, S_IRUGO|S_IWUSR); +module_param_named(trace, uvc_dbg_param, uint, 0644); MODULE_PARM_DESC(trace, "Trace level bitmask"); -module_param_named(timeout, uvc_timeout_param, uint, S_IRUGO|S_IWUSR); +module_param_named(timeout, uvc_timeout_param, uint, 0644); MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); /* ------------------------------------------------------------------------ From adfd3910c27fbc0959ae5f1dafaec563f7d0bdd2 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 20 Dec 2022 23:56:44 +0100 Subject: [PATCH 08/27] media: uvcvideo: Remove void casting for the status endpoint Make the code more resilient, by replacing the castings with proper structure definitions and using offsetof() instead of open coding the location of the data. Suggested-by: Sergey Senozhatsky Signed-off-by: Ricardo Ribalda Reviewed-by: Andrzej Pietrasiewicz Reviewed-by: Laurent Pinchart Reviewed-by: Sergey Senozhatsky Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_status.c | 65 ++++++++++-------------------- drivers/media/usb/uvc/uvcvideo.h | 25 ++++++++++-- 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c index cb90aff344bc..602830a8023e 100644 --- a/drivers/media/usb/uvc/uvc_status.c +++ b/drivers/media/usb/uvc/uvc_status.c @@ -96,38 +96,23 @@ static void uvc_input_report_key(struct uvc_device *dev, unsigned int code, /* -------------------------------------------------------------------------- * Status interrupt endpoint */ -struct uvc_streaming_status { - u8 bStatusType; - u8 bOriginator; - u8 bEvent; - u8 bValue[]; -} __packed; - -struct uvc_control_status { - u8 bStatusType; - u8 bOriginator; - u8 bEvent; - u8 bSelector; - u8 bAttribute; - u8 bValue[]; -} __packed; - static void uvc_event_streaming(struct uvc_device *dev, - struct uvc_streaming_status *status, int len) + struct uvc_status *status, int len) { - if (len < 3) { + if (len <= offsetof(struct uvc_status, bEvent)) { uvc_dbg(dev, STATUS, "Invalid streaming status event received\n"); return; } if (status->bEvent == 0) { - if (len < 4) + if (len <= offsetof(struct uvc_status, streaming)) return; + uvc_dbg(dev, STATUS, "Button (intf %u) %s len %d\n", status->bOriginator, - status->bValue[0] ? "pressed" : "released", len); - uvc_input_report_key(dev, KEY_CAMERA, status->bValue[0]); + status->streaming.button ? "pressed" : "released", len); + uvc_input_report_key(dev, KEY_CAMERA, status->streaming.button); } else { uvc_dbg(dev, STATUS, "Stream %u error event %02x len %d\n", status->bOriginator, status->bEvent, len); @@ -154,7 +139,7 @@ static struct uvc_control *uvc_event_entity_find_ctrl(struct uvc_entity *entity, } static struct uvc_control *uvc_event_find_ctrl(struct uvc_device *dev, - const struct uvc_control_status *status, + const struct uvc_status *status, struct uvc_video_chain **chain) { list_for_each_entry((*chain), &dev->chains, list) { @@ -166,7 +151,7 @@ static struct uvc_control *uvc_event_find_ctrl(struct uvc_device *dev, continue; ctrl = uvc_event_entity_find_ctrl(entity, - status->bSelector); + status->control.bSelector); if (ctrl) return ctrl; } @@ -176,7 +161,7 @@ static struct uvc_control *uvc_event_find_ctrl(struct uvc_device *dev, } static bool uvc_event_control(struct urb *urb, - const struct uvc_control_status *status, int len) + const struct uvc_status *status, int len) { static const char *attrs[] = { "value", "info", "failure", "min", "max" }; struct uvc_device *dev = urb->context; @@ -184,24 +169,24 @@ static bool uvc_event_control(struct urb *urb, struct uvc_control *ctrl; if (len < 6 || status->bEvent != 0 || - status->bAttribute >= ARRAY_SIZE(attrs)) { + status->control.bAttribute >= ARRAY_SIZE(attrs)) { uvc_dbg(dev, STATUS, "Invalid control status event received\n"); return false; } uvc_dbg(dev, STATUS, "Control %u/%u %s change len %d\n", - status->bOriginator, status->bSelector, - attrs[status->bAttribute], len); + status->bOriginator, status->control.bSelector, + attrs[status->control.bAttribute], len); /* Find the control. */ ctrl = uvc_event_find_ctrl(dev, status, &chain); if (!ctrl) return false; - switch (status->bAttribute) { + switch (status->control.bAttribute) { case UVC_CTRL_VALUE_CHANGE: return uvc_ctrl_status_event_async(urb, chain, ctrl, - status->bValue); + status->control.bValue); case UVC_CTRL_INFO_CHANGE: case UVC_CTRL_FAILURE_CHANGE: @@ -237,28 +222,22 @@ static void uvc_status_complete(struct urb *urb) len = urb->actual_length; if (len > 0) { - switch (dev->status[0] & 0x0f) { + switch (dev->status->bStatusType & 0x0f) { case UVC_STATUS_TYPE_CONTROL: { - struct uvc_control_status *status = - (struct uvc_control_status *)dev->status; - - if (uvc_event_control(urb, status, len)) + if (uvc_event_control(urb, dev->status, len)) /* The URB will be resubmitted in work context. */ return; break; } case UVC_STATUS_TYPE_STREAMING: { - struct uvc_streaming_status *status = - (struct uvc_streaming_status *)dev->status; - - uvc_event_streaming(dev, status, len); + uvc_event_streaming(dev, dev->status, len); break; } default: uvc_dbg(dev, STATUS, "Unknown status event type %u\n", - dev->status[0]); + dev->status->bStatusType); break; } } @@ -282,12 +261,12 @@ int uvc_status_init(struct uvc_device *dev) uvc_input_init(dev); - dev->status = kzalloc(UVC_MAX_STATUS_SIZE, GFP_KERNEL); - if (dev->status == NULL) + dev->status = kzalloc(sizeof(*dev->status), GFP_KERNEL); + if (!dev->status) return -ENOMEM; dev->int_urb = usb_alloc_urb(0, GFP_KERNEL); - if (dev->int_urb == NULL) { + if (!dev->int_urb) { kfree(dev->status); return -ENOMEM; } @@ -304,7 +283,7 @@ int uvc_status_init(struct uvc_device *dev) interval = fls(interval) - 1; usb_fill_int_urb(dev->int_urb, dev->udev, pipe, - dev->status, UVC_MAX_STATUS_SIZE, uvc_status_complete, + dev->status, sizeof(*dev->status), uvc_status_complete, dev, interval); return 0; diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index b60e4ae95e81..cd5861cae3b0 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -51,8 +51,6 @@ #define UVC_URBS 5 /* Maximum number of packets per URB. */ #define UVC_MAX_PACKETS 32 -/* Maximum status buffer size in bytes of interrupt URB. */ -#define UVC_MAX_STATUS_SIZE 16 #define UVC_CTRL_CONTROL_TIMEOUT 5000 #define UVC_CTRL_STREAMING_TIMEOUT 5000 @@ -525,6 +523,26 @@ struct uvc_device_info { const struct uvc_control_mapping **mappings; }; +struct uvc_status_streaming { + u8 button; +} __packed; + +struct uvc_status_control { + u8 bSelector; + u8 bAttribute; + u8 bValue[11]; +} __packed; + +struct uvc_status { + u8 bStatusType; + u8 bOriginator; + u8 bEvent; + union { + struct uvc_status_control control; + struct uvc_status_streaming streaming; + }; +} __packed; + struct uvc_device { struct usb_device *udev; struct usb_interface *intf; @@ -557,7 +575,8 @@ struct uvc_device { /* Status Interrupt Endpoint */ struct usb_host_endpoint *int_ep; struct urb *int_urb; - u8 *status; + struct uvc_status *status; + struct input_dev *input; char input_phys[64]; From 16045708a3ff0266b1b8cca6198ea50eb80fff6a Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 6 Dec 2022 00:15:04 +0100 Subject: [PATCH 09/27] media: uvcvideo: Recover stalled ElGato devices Elgato Cam Link 4k can be in a stalled state if the resolution of the external source has changed while the firmware initializes. Once in this state, the device is useless until it receives a USB reset. It has even been observed that the stalled state will continue even after unplugging the device. lsusb -v Bus 002 Device 002: ID 0fd9:0066 Elgato Systems GmbH Cam Link 4K Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 bDeviceProtocol 1 Interface Association bMaxPacketSize0 9 idVendor 0x0fd9 Elgato Systems GmbH idProduct 0x0066 bcdDevice 0.00 iManufacturer 1 Elgato iProduct 2 Cam Link 4K iSerial 4 0005AC52FE000 bNumConfigurations 1 Reviewed-by: Sergey Senozhatsky Reviewed-by: Laurent Pinchart Signed-off-by: Ricardo Ribalda Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_video.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index d2eb9066e4dc..503ef29211ca 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -129,12 +129,13 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit, return -EPIPE; } +static const struct usb_device_id elgato_cam_link_4k = { + USB_DEVICE(0x0fd9, 0x0066) +}; + static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, struct uvc_streaming_control *ctrl) { - static const struct usb_device_id elgato_cam_link_4k = { - USB_DEVICE(0x0fd9, 0x0066) - }; struct uvc_format *format = NULL; struct uvc_frame *frame = NULL; unsigned int i; @@ -297,7 +298,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream, dev_err(&stream->intf->dev, "Failed to query (%u) UVC %s control : %d (exp. %u).\n", query, probe ? "probe" : "commit", ret, size); - ret = -EIO; + ret = (ret == -EPROTO) ? -EPROTO : -EIO; goto out; } @@ -2121,6 +2122,21 @@ int uvc_video_init(struct uvc_streaming *stream) * request on the probe control, as required by the UVC specification. */ ret = uvc_get_video_ctrl(stream, probe, 1, UVC_GET_CUR); + + /* + * Elgato Cam Link 4k can be in a stalled state if the resolution of + * the external source has changed while the firmware initializes. + * Once in this state, the device is useless until it receives a + * USB reset. It has even been observed that the stalled state will + * continue even after unplugging the device. + */ + if (ret == -EPROTO && + usb_match_one_id(stream->dev->intf, &elgato_cam_link_4k)) { + dev_err(&stream->intf->dev, "Elgato Cam Link 4K firmware crash detected\n"); + dev_err(&stream->intf->dev, "Resetting the device, unplug and replug to recover\n"); + usb_reset_device(stream->dev->udev); + } + if (ret < 0) return ret; From 81e78a6fc320693c269990fb1af37b8c2c382cf2 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Fri, 2 Dec 2022 17:45:06 +0100 Subject: [PATCH 10/27] media: uvcvideo: Limit power line control for Acer EasyCamera The device does not implement the power line control correctly. Add a corresponding control mapping override. Bus 003 Device 002: ID 5986:1180 Acer, Inc EasyCamera Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 bDeviceProtocol 1 Interface Association bMaxPacketSize0 64 idVendor 0x5986 Acer, Inc idProduct 0x1180 bcdDevice 56.04 iManufacturer 3 Bison iProduct 1 EasyCamera iSerial 2 bNumConfigurations 1 Signed-off-by: Ricardo Ribalda Reviewed-by: Sergey Senozhatsky Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 4f59583bf516..1de17db3356b 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2967,6 +2967,15 @@ static const struct usb_device_id uvc_ids[] = { .bInterfaceSubClass = 1, .bInterfaceProtocol = 0, .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited }, + /* Acer EasyCamera */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x5986, + .idProduct = 0x1180, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 0, + .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited }, /* Intel RealSense D4M */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO, From 5f0e659d2a2ac6172d495915f2775fa592c7ab17 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 25 Oct 2022 21:42:21 +0300 Subject: [PATCH 11/27] media: uvcvideo: Factor out usb_string() calls When parsing UVC descriptors to instantiate entities, the driver calls usb_string() to retrieve the entity name from the device, and falls back to a default name if the string can't be retrieved. This code pattern occurs multiple times. Factor it out to a separate helper function. Signed-off-by: Laurent Pinchart Reviewed-by: Guenter Roeck --- drivers/media/usb/uvc/uvc_driver.c | 59 ++++++++++++++++++------------ 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 1de17db3356b..5f6e0da0ef90 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -794,6 +794,27 @@ static struct uvc_entity *uvc_alloc_entity(u16 type, u16 id, return entity; } +static void uvc_entity_set_name(struct uvc_device *dev, struct uvc_entity *entity, + const char *type_name, u8 string_id) +{ + int ret; + + /* + * First attempt to read the entity name from the device. If the entity + * has no associated string, or if reading the string fails (most + * likely due to a buggy firmware), fall back to default names based on + * the entity type. + */ + if (string_id) { + ret = usb_string(dev->udev, string_id, entity->name, + sizeof(entity->name)); + if (!ret) + return; + } + + sprintf(entity->name, "%s %u", type_name, entity->id); +} + /* Parse vendor-specific extensions. */ static int uvc_parse_vendor_control(struct uvc_device *dev, const unsigned char *buffer, int buflen) @@ -860,9 +881,7 @@ static int uvc_parse_vendor_control(struct uvc_device *dev, + n; memcpy(unit->extension.bmControls, &buffer[23+p], 2*n); - if (buffer[24+p+2*n] == 0 || - usb_string(udev, buffer[24+p+2*n], unit->name, sizeof(unit->name)) < 0) - sprintf(unit->name, "Extension %u", buffer[3]); + uvc_entity_set_name(dev, unit, "Extension", buffer[24+p+2*n]); list_add_tail(&unit->list, &dev->entities); handled = 1; @@ -880,6 +899,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev, struct usb_interface *intf; struct usb_host_interface *alts = dev->intf->cur_altsetting; unsigned int i, n, p, len; + const char *type_name; u16 type; switch (buffer[2]) { @@ -985,15 +1005,14 @@ static int uvc_parse_standard_control(struct uvc_device *dev, memcpy(term->media.bmTransportModes, &buffer[10+n], p); } - if (buffer[7] == 0 || - usb_string(udev, buffer[7], term->name, sizeof(term->name)) < 0) { - if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA) - sprintf(term->name, "Camera %u", buffer[3]); - if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT) - sprintf(term->name, "Media %u", buffer[3]); - else - sprintf(term->name, "Input %u", buffer[3]); - } + if (UVC_ENTITY_TYPE(term) == UVC_ITT_CAMERA) + type_name = "Camera"; + else if (UVC_ENTITY_TYPE(term) == UVC_ITT_MEDIA_TRANSPORT_INPUT) + type_name = "Media"; + else + type_name = "Input"; + + uvc_entity_set_name(dev, term, type_name, buffer[7]); list_add_tail(&term->list, &dev->entities); break; @@ -1026,9 +1045,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev, memcpy(term->baSourceID, &buffer[7], 1); - if (buffer[8] == 0 || - usb_string(udev, buffer[8], term->name, sizeof(term->name)) < 0) - sprintf(term->name, "Output %u", buffer[3]); + uvc_entity_set_name(dev, term, "Output", buffer[8]); list_add_tail(&term->list, &dev->entities); break; @@ -1049,9 +1066,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev, memcpy(unit->baSourceID, &buffer[5], p); - if (buffer[5+p] == 0 || - usb_string(udev, buffer[5+p], unit->name, sizeof(unit->name)) < 0) - sprintf(unit->name, "Selector %u", buffer[3]); + uvc_entity_set_name(dev, unit, "Selector", buffer[5+p]); list_add_tail(&unit->list, &dev->entities); break; @@ -1080,9 +1095,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev, if (dev->uvc_version >= 0x0110) unit->processing.bmVideoStandards = buffer[9+n]; - if (buffer[8+n] == 0 || - usb_string(udev, buffer[8+n], unit->name, sizeof(unit->name)) < 0) - sprintf(unit->name, "Processing %u", buffer[3]); + uvc_entity_set_name(dev, unit, "Processing", buffer[8+n]); list_add_tail(&unit->list, &dev->entities); break; @@ -1109,9 +1122,7 @@ static int uvc_parse_standard_control(struct uvc_device *dev, unit->extension.bmControls = (u8 *)unit + sizeof(*unit); memcpy(unit->extension.bmControls, &buffer[23+p], n); - if (buffer[23+p+n] == 0 || - usb_string(udev, buffer[23+p+n], unit->name, sizeof(unit->name)) < 0) - sprintf(unit->name, "Extension %u", buffer[3]); + uvc_entity_set_name(dev, unit, "Extension", buffer[23+p+n]); list_add_tail(&unit->list, &dev->entities); break; From 9f582f0418ed1c18f92c9e4628075d6ec9a7d9fb Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Tue, 3 Jan 2023 15:36:19 +0100 Subject: [PATCH 12/27] media: uvcvideo: Check for INACTIVE in uvc_ctrl_is_accessible() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check for inactive controls in uvc_ctrl_is_accessible(). Use the new value for the master_id controls if present, otherwise use the existing value to determine if it is OK to set the control. Doing this here avoids attempting to set an inactive control, which will return an error from the USB device, which returns an invalid errorcode. This fixes: warn: v4l2-test-controls.cpp(483): s_ctrl returned EIO   warn: v4l2-test-controls.cpp(483): s_ctrl returned EIO test VIDIOC_G/S_CTRL: OK   warn: v4l2-test-controls.cpp(739): s_ext_ctrls returned EIO   warn: v4l2-test-controls.cpp(739): s_ext_ctrls returned EIO   warn: v4l2-test-controls.cpp(816): s_ext_ctrls returned EIO test VIDIOC_G/S/TRY_EXT_CTRLS: OK Tested with: v4l2-ctl -c auto_exposure=1 OK v4l2-ctl -c exposure_time_absolute=251 OK v4l2-ctl -c auto_exposure=3 OK v4l2-ctl -c exposure_time_absolute=251 VIDIOC_S_EXT_CTRLS: failed: Input/output error exposure_time_absolute: Input/output error ERROR v4l2-ctl -c auto_exposure=3,exposure_time_absolute=251,auto_exposure=1 v4l2-ctl -C auto_exposure,exposure_time_absolute   auto_exposure: 1 exposure_time_absolute: 251 Reviewed-by: Laurent Pinchart Reviewed-by: Ricardo Ribalda Signed-off-by: Hans Verkuil Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 42 +++++++++++++++++++++++++++++++- drivers/media/usb/uvc/uvc_v4l2.c | 3 +-- drivers/media/usb/uvc/uvcvideo.h | 3 ++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index c95a2229f4fa..6f5aaaf09ee0 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1085,11 +1085,28 @@ static int uvc_query_v4l2_class(struct uvc_video_chain *chain, u32 req_id, return 0; } +/* + * Check if control @v4l2_id can be accessed by the given control @ioctl + * (VIDIOC_G_EXT_CTRLS, VIDIOC_TRY_EXT_CTRLS or VIDIOC_S_EXT_CTRLS). + * + * For set operations on slave controls, check if the master's value is set to + * manual, either in the others controls set in the same ioctl call, or from + * the master's current value. This catches VIDIOC_S_EXT_CTRLS calls that set + * both the master and slave control, such as for instance setting + * auto_exposure=1, exposure_time_absolute=251. + */ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id, - bool read) + const struct v4l2_ext_controls *ctrls, + unsigned long ioctl) { + struct uvc_control_mapping *master_map = NULL; + struct uvc_control *master_ctrl = NULL; struct uvc_control_mapping *mapping; struct uvc_control *ctrl; + bool read = ioctl == VIDIOC_G_EXT_CTRLS; + s32 val; + int ret; + int i; if (__uvc_query_v4l2_class(chain, v4l2_id, 0) >= 0) return -EACCES; @@ -1104,6 +1121,29 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id, if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR) && !read) return -EACCES; + if (ioctl != VIDIOC_S_EXT_CTRLS || !mapping->master_id) + return 0; + + /* + * Iterate backwards in cases where the master control is accessed + * multiple times in the same ioctl. We want the last value. + */ + for (i = ctrls->count - 1; i >= 0; i--) { + if (ctrls->controls[i].id == mapping->master_id) + return ctrls->controls[i].value == + mapping->master_manual ? 0 : -EACCES; + } + + __uvc_find_control(ctrl->entity, mapping->master_id, &master_map, + &master_ctrl, 0); + + if (!master_ctrl || !(master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR)) + return 0; + + ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val); + if (ret >= 0 && val != mapping->master_manual) + return -EACCES; + return 0; } diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index dcd178d249b6..4187e59680bb 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -1018,8 +1018,7 @@ static int uvc_ctrl_check_access(struct uvc_video_chain *chain, int ret = 0; for (i = 0; i < ctrls->count; ++ctrl, ++i) { - ret = uvc_ctrl_is_accessible(chain, ctrl->id, - ioctl == VIDIOC_G_EXT_CTRLS); + ret = uvc_ctrl_is_accessible(chain, ctrl->id, ctrls, ioctl); if (ret) break; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index cd5861cae3b0..8c0df94374c9 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -778,7 +778,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle) int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl); int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl); int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id, - bool read); + const struct v4l2_ext_controls *ctrls, + unsigned long ioctl); int uvc_xu_ctrl_query(struct uvc_video_chain *chain, struct uvc_xu_control_query *xqry); From 67a655be5748426a9bda7845fc264ccf71a76ea3 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Tue, 3 Jan 2023 15:36:20 +0100 Subject: [PATCH 13/27] media: uvcvideo: Improve error logging in uvc_query_ctrl() Standard use of the driver may result in error messages on the kernel log. This can hide other more important messages, and alert unnecessarily the user. Let's keep dev_err() for the important occasions. If __uvc_query_ctrl() failed with a non -EPIPE error, then report that with dev_err. If an error code is obtained, then report that with dev_dbg. Reviewed-by: Ricardo Ribalda Signed-off-by: Hans Verkuil Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_video.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 503ef29211ca..f35674fb8912 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -79,13 +79,14 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit, if (likely(ret == size)) return 0; - dev_err(&dev->udev->dev, - "Failed to query (%s) UVC control %u on unit %u: %d (exp. %u).\n", - uvc_query_name(query), cs, unit, ret, size); - - if (ret != -EPIPE) + if (ret != -EPIPE) { + dev_err(&dev->udev->dev, + "Failed to query (%s) UVC control %u on unit %u: %d (exp. %u).\n", + uvc_query_name(query), cs, unit, ret, size); return ret; + } + /* Reuse data[0] to request the error code. */ tmp = *(u8 *)data; ret = __uvc_query_ctrl(dev, UVC_GET_CUR, 0, intfnum, From 44cdba4130ecdb59ff94c617b6cd8dc22b4f3c97 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 3 Jan 2023 15:36:21 +0100 Subject: [PATCH 14/27] media: uvcvideo: Return -EACCES for Wrong state error Error 2 is defined by UVC as: Wrong State: The device is in a state that disallows the specific request. The device will remain in this state until a specific action from the host or the user is completed. This is documented as happening when attempting to set the value of a manual control when the device is in auto mode. While V4L2 allows this, the closest error code defined by VIDIOC_S_CTRL is EACCES: EACCES: Attempt to set a read-only control or to get a write-only control. Or if there is an attempt to set an inactive control and the driver is not capable of caching the new value until the control is active again. Replace EILSEQ with EACCES. Reviewed-by: Laurent Pinchart Suggested-by: Hans Verkuil Signed-off-by: Ricardo Ribalda Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index f35674fb8912..3d3753a4c1f8 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -108,7 +108,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit, case 1: /* Not ready */ return -EBUSY; case 2: /* Wrong state */ - return -EILSEQ; + return -EACCES; case 3: /* Power */ return -EREMOTE; case 4: /* Out of range */ From a763b9fb58be869e252a7d33acb0a6390b01c801 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 3 Jan 2023 15:36:22 +0100 Subject: [PATCH 15/27] media: uvcvideo: Do not return positive errors in uvc_query_ctrl() If the returned size of the query does not match the expected size or it is zero, return -EPIPE instead of 0 or a positive value. This will avoid confusing the caller (and ultimately userspace) that doesn't expect a positive or zero value. Reviewed-by: Laurent Pinchart Suggested-by: Laurent Pinchart Signed-off-by: Ricardo Ribalda Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 3d3753a4c1f8..e4b71df08216 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -83,7 +83,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit, dev_err(&dev->udev->dev, "Failed to query (%s) UVC control %u on unit %u: %d (exp. %u).\n", uvc_query_name(query), cs, unit, ret, size); - return ret; + return ret < 0 ? ret : -EPIPE; } /* Reuse data[0] to request the error code. */ From 7faf8ae4277156da32eada72c07f5edeb82cd9e4 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 3 Jan 2023 15:36:23 +0100 Subject: [PATCH 16/27] media: uvcvideo: Fix handling on Bitmask controls Minimum and step values for V4L2_CTRL_TYPE_BITMASK controls should be 0. There is no need to query the camera firmware about this and maybe get invalid results. Also value should be masked to the max value advertised by the hardware. Finally, handle UVC 1.5 mask controls that use MAX instead of RES to describe the valid bits. Fixes v4l2-compliane: Control ioctls (Input 0): fail: v4l2-test-controls.cpp(97): minimum must be 0 for a bitmask control test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: FAIL Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 52 ++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 6f5aaaf09ee0..65d41946ef1a 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -1161,6 +1161,25 @@ static const char *uvc_map_get_name(const struct uvc_control_mapping *map) return "Unknown Control"; } +static u32 uvc_get_ctrl_bitmap(struct uvc_control *ctrl, + struct uvc_control_mapping *mapping) +{ + /* + * Some controls, like CT_AE_MODE_CONTROL, use GET_RES to represent + * the number of bits supported. Those controls do not list GET_MAX + * as supported. + */ + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_RES) + return mapping->get(mapping, UVC_GET_RES, + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); + + if (ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) + return mapping->get(mapping, UVC_GET_MAX, + uvc_ctrl_data(ctrl, UVC_CTRL_DATA_MAX)); + + return ~0; +} + static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, struct uvc_control *ctrl, struct uvc_control_mapping *mapping, @@ -1235,6 +1254,12 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, v4l2_ctrl->step = 0; return 0; + case V4L2_CTRL_TYPE_BITMASK: + v4l2_ctrl->minimum = 0; + v4l2_ctrl->maximum = uvc_get_ctrl_bitmap(ctrl, mapping); + v4l2_ctrl->step = 0; + return 0; + default: break; } @@ -1336,19 +1361,14 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain, menu_info = &mapping->menu_info[query_menu->index]; - if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK && - (ctrl->info.flags & UVC_CTRL_FLAG_GET_RES)) { - s32 bitmap; - + if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) { if (!ctrl->cached) { ret = uvc_ctrl_populate_cache(chain, ctrl); if (ret < 0) goto done; } - bitmap = mapping->get(mapping, UVC_GET_RES, - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); - if (!(bitmap & menu_info->value)) { + if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & menu_info->value)) { ret = -EINVAL; goto done; } @@ -1831,6 +1851,17 @@ int uvc_ctrl_set(struct uvc_fh *handle, value = xctrl->value; break; + case V4L2_CTRL_TYPE_BITMASK: + if (!ctrl->cached) { + ret = uvc_ctrl_populate_cache(chain, ctrl); + if (ret < 0) + return ret; + } + + xctrl->value &= uvc_get_ctrl_bitmap(ctrl, mapping); + value = xctrl->value; + break; + case V4L2_CTRL_TYPE_BOOLEAN: xctrl->value = clamp(xctrl->value, 0, 1); value = xctrl->value; @@ -1845,17 +1876,14 @@ int uvc_ctrl_set(struct uvc_fh *handle, * Valid menu indices are reported by the GET_RES request for * UVC controls that support it. */ - if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK && - (ctrl->info.flags & UVC_CTRL_FLAG_GET_RES)) { + if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) { if (!ctrl->cached) { ret = uvc_ctrl_populate_cache(chain, ctrl); if (ret < 0) return ret; } - step = mapping->get(mapping, UVC_GET_RES, - uvc_ctrl_data(ctrl, UVC_CTRL_DATA_RES)); - if (!(step & value)) + if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & value)) return -EINVAL; } From 252d50da337ba97019574b1e830879d5dd5121d2 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 3 Jan 2023 15:36:25 +0100 Subject: [PATCH 17/27] media: uvcvideo: Refactor __uvc_ctrl_add_mapping Simplify the exit code with a common error tag freeing all the memory. Suggested-by: Laurent Pinchart Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 65d41946ef1a..ffa0e2654264 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -2286,32 +2286,30 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain, unsigned int i; /* - * Most mappings come from static kernel data and need to be duplicated. + * Most mappings come from static kernel data, and need to be duplicated. * Mappings that come from userspace will be unnecessarily duplicated, * this could be optimized. */ map = kmemdup(mapping, sizeof(*mapping), GFP_KERNEL); - if (map == NULL) + if (!map) return -ENOMEM; + map->name = NULL; + map->menu_info = NULL; + /* For UVCIOC_CTRL_MAP custom control */ if (mapping->name) { map->name = kstrdup(mapping->name, GFP_KERNEL); - if (!map->name) { - kfree(map); - return -ENOMEM; - } + if (!map->name) + goto err_nomem; } INIT_LIST_HEAD(&map->ev_subs); size = sizeof(*mapping->menu_info) * mapping->menu_count; map->menu_info = kmemdup(mapping->menu_info, size, GFP_KERNEL); - if (map->menu_info == NULL) { - kfree(map->name); - kfree(map); - return -ENOMEM; - } + if (!map->menu_info) + goto err_nomem; if (map->get == NULL) map->get = uvc_get_le_value; @@ -2332,6 +2330,12 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain, ctrl->info.selector); return 0; + +err_nomem: + kfree(map->menu_info); + kfree(map->name); + kfree(map); + return -ENOMEM; } int uvc_ctrl_add_mapping(struct uvc_video_chain *chain, From 5dd0eab84ae9a4b292baf1ad02e1a273c475cd04 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Tue, 3 Jan 2023 12:01:21 +0100 Subject: [PATCH 18/27] media: uvcvideo: Limit power line control for Acer EasyCamera The device does not implement the power line control correctly. Add a corresponding control mapping override. Bus 003 Device 002: ID 5986:1180 Acer, Inc EasyCamera Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 bDeviceProtocol 1 Interface Association bMaxPacketSize0 64 idVendor 0x5986 Acer, Inc idProduct 0x1180 bcdDevice 56.04 iManufacturer 3 Bison iProduct 1 EasyCamera iSerial 2 bNumConfigurations 1 Reviewed-by: Sergey Senozhatsky Reviewed-by: Laurent Pinchart Signed-off-by: Ricardo Ribalda Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 5f6e0da0ef90..80ccb42d8f4a 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2979,6 +2979,15 @@ static const struct usb_device_id uvc_ids[] = { .bInterfaceProtocol = 0, .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited }, /* Acer EasyCamera */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x5986, + .idProduct = 0x1180, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 0, + .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_limited }, + /* Acer EasyCamera */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO, .idVendor = 0x5986, From 70fcaf92a39e6cb7542de0fc0ad4562f42f7f54a Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Wed, 4 Jan 2023 11:45:19 +0100 Subject: [PATCH 19/27] media: uvcvideo: Extend documentation of uvc_video_clock_decode() Make an explicit reference to UVC 1.5, explaining how the algorithm supports the different behaviour of UVC 1.1 and 1.5. Reviewed-by: Laurent Pinchart Tested-by: HungNien Chen Signed-off-by: Ricardo Ribalda Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_video.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index e4b71df08216..b1f7416858b7 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -518,7 +518,9 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf, /* * To limit the amount of data, drop SCRs with an SOF identical to the - * previous one. + * previous one. This filtering is also needed to support UVC 1.5, where + * all the data packets of the same frame contains the same SOF. In that + * case only the first one will match the host_sof. */ dev_sof = get_unaligned_le16(&data[header_size - 2]); if (dev_sof == stream->clock.last_sof) From 40140eda661ea4be219ef194a4f43b7b5e3bbd27 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Thu, 5 Jan 2023 14:52:54 +0100 Subject: [PATCH 20/27] media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU Replace the count with a mask field that lets us choose not only the max value, but also the minimum value and what values are valid in between. Suggested-by: Laurent Pinchart Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 33 +++++++++++++++++++++--------- drivers/media/usb/uvc/uvc_driver.c | 4 +++- drivers/media/usb/uvc/uvc_v4l2.c | 3 ++- drivers/media/usb/uvc/uvcvideo.h | 2 +- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index ffa0e2654264..305cce26f567 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -6,6 +6,7 @@ * Laurent Pinchart (laurent.pinchart@ideasonboard.com) */ +#include #include #include #include @@ -525,7 +526,8 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = { .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_BITMASK, .menu_info = exposure_auto_controls, - .menu_count = ARRAY_SIZE(exposure_auto_controls), + .menu_mask = GENMASK(V4L2_EXPOSURE_APERTURE_PRIORITY, + V4L2_EXPOSURE_AUTO), .slave_ids = { V4L2_CID_EXPOSURE_ABSOLUTE, }, }, { @@ -731,7 +733,8 @@ static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = { .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_ENUM, .menu_info = power_line_frequency_controls, - .menu_count = ARRAY_SIZE(power_line_frequency_controls) - 1, + .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ, + V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), }, }; @@ -745,7 +748,8 @@ static const struct uvc_control_mapping uvc_ctrl_mappings_uvc15[] = { .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_ENUM, .menu_info = power_line_frequency_controls, - .menu_count = ARRAY_SIZE(power_line_frequency_controls), + .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO, + V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), }, }; @@ -975,7 +979,9 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping, const struct uvc_menu_info *menu = mapping->menu_info; unsigned int i; - for (i = 0; i < mapping->menu_count; ++i, ++menu) { + for (i = 0; BIT(i) <= mapping->menu_mask; ++i, ++menu) { + if (!test_bit(i, &mapping->menu_mask)) + continue; if (menu->value == value) { value = i; break; @@ -1228,12 +1234,14 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, switch (mapping->v4l2_type) { case V4L2_CTRL_TYPE_MENU: - v4l2_ctrl->minimum = 0; - v4l2_ctrl->maximum = mapping->menu_count - 1; + v4l2_ctrl->minimum = ffs(mapping->menu_mask) - 1; + v4l2_ctrl->maximum = fls(mapping->menu_mask) - 1; v4l2_ctrl->step = 1; menu = mapping->menu_info; - for (i = 0; i < mapping->menu_count; ++i, ++menu) { + for (i = 0; BIT(i) <= mapping->menu_mask; ++i, ++menu) { + if (!test_bit(i, &mapping->menu_mask)) + continue; if (menu->value == v4l2_ctrl->default_value) { v4l2_ctrl->default_value = i; break; @@ -1354,7 +1362,7 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain, goto done; } - if (query_menu->index >= mapping->menu_count) { + if (!test_bit(query_menu->index, &mapping->menu_mask)) { ret = -EINVAL; goto done; } @@ -1868,8 +1876,13 @@ int uvc_ctrl_set(struct uvc_fh *handle, break; case V4L2_CTRL_TYPE_MENU: - if (xctrl->value < 0 || xctrl->value >= mapping->menu_count) + if (xctrl->value < (ffs(mapping->menu_mask) - 1) || + xctrl->value > (fls(mapping->menu_mask) - 1)) return -ERANGE; + + if (!test_bit(xctrl->value, &mapping->menu_mask)) + return -EINVAL; + value = mapping->menu_info[xctrl->value].value; /* @@ -2306,7 +2319,7 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain, INIT_LIST_HEAD(&map->ev_subs); - size = sizeof(*mapping->menu_info) * mapping->menu_count; + size = sizeof(*mapping->menu_info) * fls(mapping->menu_mask); map->menu_info = kmemdup(mapping->menu_info, size, GFP_KERNEL); if (!map->menu_info) goto err_nomem; diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 80ccb42d8f4a..b4710e1aa0a5 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -2371,7 +2372,8 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = { .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_ENUM, .menu_info = power_line_frequency_controls_limited, - .menu_count = ARRAY_SIZE(power_line_frequency_controls_limited), + .menu_mask = + GENMASK(ARRAY_SIZE(power_line_frequency_controls_limited) - 1, 0), }; static const struct uvc_device_info uvc_ctrl_power_line_limited = { diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 4187e59680bb..950b42d78a10 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -6,6 +6,7 @@ * Laurent Pinchart (laurent.pinchart@ideasonboard.com) */ +#include #include #include #include @@ -80,7 +81,7 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain, goto free_map; } - map->menu_count = xmap->menu_count; + map->menu_mask = GENMASK(xmap->menu_count - 1, 0); break; default: diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 8c0df94374c9..e8f277e380a5 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -115,7 +115,7 @@ struct uvc_control_mapping { u32 data_type; const struct uvc_menu_info *menu_info; - u32 menu_count; + unsigned long menu_mask; u32 master_id; s32 master_manual; From 96a160b068e09b4ed5a32e2179e5219fc3545eca Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Thu, 5 Jan 2023 14:52:55 +0100 Subject: [PATCH 21/27] media: uvcvideo: Refactor uvc_ctrl_mappings_uvcXX Convert the array of structs into an array of pointers, that way the mappings can be reused. Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 74 ++++++++++++++++---------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 305cce26f567..af765d4cae90 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -723,34 +723,40 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = { }, }; -static const struct uvc_control_mapping uvc_ctrl_mappings_uvc11[] = { - { - .id = V4L2_CID_POWER_LINE_FREQUENCY, - .entity = UVC_GUID_UVC_PROCESSING, - .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, - .size = 2, - .offset = 0, - .v4l2_type = V4L2_CTRL_TYPE_MENU, - .data_type = UVC_CTRL_DATA_TYPE_ENUM, - .menu_info = power_line_frequency_controls, - .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ, - V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), - }, +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = { + .id = V4L2_CID_POWER_LINE_FREQUENCY, + .entity = UVC_GUID_UVC_PROCESSING, + .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, + .size = 2, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_MENU, + .data_type = UVC_CTRL_DATA_TYPE_ENUM, + .menu_info = power_line_frequency_controls, + .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ, + V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), }; -static const struct uvc_control_mapping uvc_ctrl_mappings_uvc15[] = { - { - .id = V4L2_CID_POWER_LINE_FREQUENCY, - .entity = UVC_GUID_UVC_PROCESSING, - .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, - .size = 2, - .offset = 0, - .v4l2_type = V4L2_CTRL_TYPE_MENU, - .data_type = UVC_CTRL_DATA_TYPE_ENUM, - .menu_info = power_line_frequency_controls, - .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO, - V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), - }, +static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc11[] = { + &uvc_ctrl_power_line_mapping_uvc11, + NULL, /* Sentinel */ +}; + +static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = { + .id = V4L2_CID_POWER_LINE_FREQUENCY, + .entity = UVC_GUID_UVC_PROCESSING, + .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, + .size = 2, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_MENU, + .data_type = UVC_CTRL_DATA_TYPE_ENUM, + .menu_info = power_line_frequency_controls, + .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO, + V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), +}; + +static const struct uvc_control_mapping *uvc_ctrl_mappings_uvc15[] = { + &uvc_ctrl_power_line_mapping_uvc15, + NULL, /* Sentinel */ }; /* ------------------------------------------------------------------------ @@ -2506,8 +2512,7 @@ static void uvc_ctrl_prune_entity(struct uvc_device *dev, static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, struct uvc_control *ctrl) { - const struct uvc_control_mapping *mappings; - unsigned int num_mappings; + const struct uvc_control_mapping **mappings; unsigned int i; /* @@ -2574,16 +2579,11 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain, } /* Finally process version-specific mappings. */ - if (chain->dev->uvc_version < 0x0150) { - mappings = uvc_ctrl_mappings_uvc11; - num_mappings = ARRAY_SIZE(uvc_ctrl_mappings_uvc11); - } else { - mappings = uvc_ctrl_mappings_uvc15; - num_mappings = ARRAY_SIZE(uvc_ctrl_mappings_uvc15); - } + mappings = chain->dev->uvc_version < 0x0150 + ? uvc_ctrl_mappings_uvc11 : uvc_ctrl_mappings_uvc15; - for (i = 0; i < num_mappings; ++i) { - const struct uvc_control_mapping *mapping = &mappings[i]; + for (i = 0; mappings[i]; ++i) { + const struct uvc_control_mapping *mapping = mappings[i]; if (uvc_entity_match_guid(ctrl->entity, mapping->entity) && ctrl->info.selector == mapping->selector) From 3aa8628eb78a63d0bf93e159846e9c92bccc8f69 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Thu, 5 Jan 2023 14:52:56 +0100 Subject: [PATCH 22/27] media: uvcvideo: Refactor power_line_frequency_controls_limited Move the control mapping to uvc_ctrl.c. This way we do not have references to UVC controls or V4L2 controls in uvc_driver.c. This also fixes a bug introduced in commit 382075604a68 ("media: uvcvideo: Limit power line control for Quanta UVC Webcam"). The offending commit caused the power line control menu entries to have incorrect indices compared to the V4L2_CID_POWER_LINE_FREQUENCY_* enumeration. Now that the limited mapping reuses the correct menu_info array, the indices correctly map to the V4L2 control specification. Fixes: 382075604a68 ("media: uvcvideo: Limit power line control for Quanta UVC Webcam") Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 13 +++++++++++++ drivers/media/usb/uvc/uvc_driver.c | 18 ------------------ drivers/media/usb/uvc/uvcvideo.h | 1 + 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index af765d4cae90..c15c38427cc3 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -723,6 +723,19 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = { }, }; +const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = { + .id = V4L2_CID_POWER_LINE_FREQUENCY, + .entity = UVC_GUID_UVC_PROCESSING, + .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, + .size = 2, + .offset = 0, + .v4l2_type = V4L2_CTRL_TYPE_MENU, + .data_type = UVC_CTRL_DATA_TYPE_ENUM, + .menu_info = power_line_frequency_controls, + .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ, + V4L2_CID_POWER_LINE_FREQUENCY_50HZ), +}; + static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = { .id = V4L2_CID_POWER_LINE_FREQUENCY, .entity = UVC_GUID_UVC_PROCESSING, diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index b4710e1aa0a5..370b46c98108 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2358,24 +2358,6 @@ MODULE_PARM_DESC(timeout, "Streaming control requests timeout"); * Driver initialization and cleanup */ -static const struct uvc_menu_info power_line_frequency_controls_limited[] = { - { 1, "50 Hz" }, - { 2, "60 Hz" }, -}; - -static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = { - .id = V4L2_CID_POWER_LINE_FREQUENCY, - .entity = UVC_GUID_UVC_PROCESSING, - .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, - .size = 2, - .offset = 0, - .v4l2_type = V4L2_CTRL_TYPE_MENU, - .data_type = UVC_CTRL_DATA_TYPE_ENUM, - .menu_info = power_line_frequency_controls_limited, - .menu_mask = - GENMASK(ARRAY_SIZE(power_line_frequency_controls_limited) - 1, 0), -}; - static const struct uvc_device_info uvc_ctrl_power_line_limited = { .mappings = (const struct uvc_control_mapping *[]) { &uvc_ctrl_power_line_mapping_limited, diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index e8f277e380a5..6573df1e0a47 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -745,6 +745,7 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags); void uvc_status_stop(struct uvc_device *dev); /* Controls */ +extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited; extern const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops; int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, From a7c28150af42798263a30bedbe46f201b46fb486 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Thu, 5 Jan 2023 14:52:57 +0100 Subject: [PATCH 23/27] media: uvcvideo: Fix power line control for Lenovo Integrated Camera The device does not implement the power line frequenyc control correctly. It is a UVC 1.5 device, but implements the control as a UVC 1.1 device. Add the corresponding control mapping override. Bus 003 Device 002: ID 30c9:0093 Lenovo Integrated Camera Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.01 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 bDeviceProtocol 1 Interface Association bMaxPacketSize0 64 idVendor 0x30c9 idProduct 0x0093 bcdDevice 0.07 iManufacturer 3 Lenovo iProduct 1 Integrated Camera iSerial 2 8SSC21J75356V1SR2830069 bNumConfigurations 1 Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 2 +- drivers/media/usb/uvc/uvc_driver.c | 16 ++++++++++++++++ drivers/media/usb/uvc/uvcvideo.h | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index c15c38427cc3..6354efffe9b0 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -736,7 +736,7 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = { V4L2_CID_POWER_LINE_FREQUENCY_50HZ), }; -static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = { +const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = { .id = V4L2_CID_POWER_LINE_FREQUENCY, .entity = UVC_GUID_UVC_PROCESSING, .selector = UVC_PU_POWER_LINE_FREQUENCY_CONTROL, diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 370b46c98108..6005e3c7eb96 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2365,6 +2365,13 @@ static const struct uvc_device_info uvc_ctrl_power_line_limited = { }, }; +static const struct uvc_device_info uvc_ctrl_power_line_uvc11 = { + .mappings = (const struct uvc_control_mapping *[]) { + &uvc_ctrl_power_line_mapping_uvc11, + NULL, /* Sentinel */ + }, +}; + static const struct uvc_device_info uvc_quirk_probe_minmax = { .quirks = UVC_QUIRK_PROBE_MINMAX, }; @@ -2944,6 +2951,15 @@ static const struct usb_device_id uvc_ids[] = { .bInterfaceSubClass = 1, .bInterfaceProtocol = 0, .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_FORCE_BPP) }, + /* Lenovo Integrated Camera */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x30c9, + .idProduct = 0x0093, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = UVC_PC_PROTOCOL_15, + .driver_info = (kernel_ulong_t)&uvc_ctrl_power_line_uvc11 }, /* Sonix Technology USB 2.0 Camera */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO, diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 6573df1e0a47..178729467967 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -746,6 +746,7 @@ void uvc_status_stop(struct uvc_device *dev); /* Controls */ extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited; +extern const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11; extern const struct v4l2_subscribed_event_ops uvc_ctrl_sub_ev_ops; int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, From 716c330433e3ad6074d057092b98c09a989e17d8 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Thu, 5 Jan 2023 14:52:58 +0100 Subject: [PATCH 24/27] media: uvcvideo: Use standard names for menus Instead of duplicating the menu info, use the one from the core. Also, do not use extra memory for 1:1 mappings. Suggested-by: Laurent Pinchart Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 129 ++++++++++++++++++++++--------- drivers/media/usb/uvc/uvc_v4l2.c | 105 ++++++++++++++++++------- drivers/media/usb/uvc/uvcvideo.h | 3 +- include/uapi/linux/uvcvideo.h | 4 +- 4 files changed, 176 insertions(+), 65 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index 6354efffe9b0..ee58f0db2763 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -364,19 +364,45 @@ static const u32 uvc_control_classes[] = { V4L2_CID_USER_CLASS, }; -static const struct uvc_menu_info power_line_frequency_controls[] = { - { 0, "Disabled" }, - { 1, "50 Hz" }, - { 2, "60 Hz" }, - { 3, "Auto" }, -}; +static const int exposure_auto_mapping[] = { 2, 1, 4, 8 }; -static const struct uvc_menu_info exposure_auto_controls[] = { - { 2, "Auto Mode" }, - { 1, "Manual Mode" }, - { 4, "Shutter Priority Mode" }, - { 8, "Aperture Priority Mode" }, -}; +/* + * This function translates the V4L2 menu index @idx, as exposed to userspace as + * the V4L2 control value, to the corresponding UVC control value used by the + * device. The custom menu_mapping in the control @mapping is used when + * available, otherwise the function assumes that the V4L2 and UVC values are + * identical. + * + * For controls of type UVC_CTRL_DATA_TYPE_BITMASK, the UVC control value is + * expressed as a bitmask and is thus guaranteed to have a single bit set. + * + * The function returns -EINVAL if the V4L2 menu index @idx isn't valid for the + * control, which includes all controls whose type isn't UVC_CTRL_DATA_TYPE_ENUM + * or UVC_CTRL_DATA_TYPE_BITMASK. + */ +static int uvc_mapping_get_menu_value(const struct uvc_control_mapping *mapping, + u32 idx) +{ + if (!test_bit(idx, &mapping->menu_mask)) + return -EINVAL; + + if (mapping->menu_mapping) + return mapping->menu_mapping[idx]; + + return idx; +} + +static const char * +uvc_mapping_get_menu_name(const struct uvc_control_mapping *mapping, u32 idx) +{ + if (!test_bit(idx, &mapping->menu_mask)) + return NULL; + + if (mapping->menu_names) + return mapping->menu_names[idx]; + + return v4l2_ctrl_get_menu(mapping->id)[idx]; +} static s32 uvc_ctrl_get_zoom(struct uvc_control_mapping *mapping, u8 query, const u8 *data) @@ -525,7 +551,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = { .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_BITMASK, - .menu_info = exposure_auto_controls, + .menu_mapping = exposure_auto_mapping, .menu_mask = GENMASK(V4L2_EXPOSURE_APERTURE_PRIORITY, V4L2_EXPOSURE_AUTO), .slave_ids = { V4L2_CID_EXPOSURE_ABSOLUTE, }, @@ -731,7 +757,6 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_limited = { .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_ENUM, - .menu_info = power_line_frequency_controls, .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ, V4L2_CID_POWER_LINE_FREQUENCY_50HZ), }; @@ -744,7 +769,6 @@ const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc11 = { .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_ENUM, - .menu_info = power_line_frequency_controls, .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_60HZ, V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), }; @@ -762,7 +786,6 @@ static const struct uvc_control_mapping uvc_ctrl_power_line_mapping_uvc15 = { .offset = 0, .v4l2_type = V4L2_CTRL_TYPE_MENU, .data_type = UVC_CTRL_DATA_TYPE_ENUM, - .menu_info = power_line_frequency_controls, .menu_mask = GENMASK(V4L2_CID_POWER_LINE_FREQUENCY_AUTO, V4L2_CID_POWER_LINE_FREQUENCY_DISABLED), }; @@ -995,13 +1018,17 @@ static s32 __uvc_ctrl_get_value(struct uvc_control_mapping *mapping, s32 value = mapping->get(mapping, UVC_GET_CUR, data); if (mapping->v4l2_type == V4L2_CTRL_TYPE_MENU) { - const struct uvc_menu_info *menu = mapping->menu_info; unsigned int i; - for (i = 0; BIT(i) <= mapping->menu_mask; ++i, ++menu) { + for (i = 0; BIT(i) <= mapping->menu_mask; ++i) { + u32 menu_value; + if (!test_bit(i, &mapping->menu_mask)) continue; - if (menu->value == value) { + + menu_value = uvc_mapping_get_menu_value(mapping, i); + + if (menu_value == value) { value = i; break; } @@ -1212,7 +1239,6 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, { struct uvc_control_mapping *master_map = NULL; struct uvc_control *master_ctrl = NULL; - const struct uvc_menu_info *menu; unsigned int i; memset(v4l2_ctrl, 0, sizeof(*v4l2_ctrl)); @@ -1257,11 +1283,15 @@ static int __uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, v4l2_ctrl->maximum = fls(mapping->menu_mask) - 1; v4l2_ctrl->step = 1; - menu = mapping->menu_info; - for (i = 0; BIT(i) <= mapping->menu_mask; ++i, ++menu) { + for (i = 0; BIT(i) <= mapping->menu_mask; ++i) { + u32 menu_value; + if (!test_bit(i, &mapping->menu_mask)) continue; - if (menu->value == v4l2_ctrl->default_value) { + + menu_value = uvc_mapping_get_menu_value(mapping, i); + + if (menu_value == v4l2_ctrl->default_value) { v4l2_ctrl->default_value = i; break; } @@ -1360,11 +1390,11 @@ int uvc_query_v4l2_ctrl(struct uvc_video_chain *chain, int uvc_query_v4l2_menu(struct uvc_video_chain *chain, struct v4l2_querymenu *query_menu) { - const struct uvc_menu_info *menu_info; struct uvc_control_mapping *mapping; struct uvc_control *ctrl; u32 index = query_menu->index; u32 id = query_menu->id; + const char *name; int ret; memset(query_menu, 0, sizeof(*query_menu)); @@ -1386,22 +1416,34 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain, goto done; } - menu_info = &mapping->menu_info[query_menu->index]; - if (mapping->data_type == UVC_CTRL_DATA_TYPE_BITMASK) { + int mask; + if (!ctrl->cached) { ret = uvc_ctrl_populate_cache(chain, ctrl); if (ret < 0) goto done; } - if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & menu_info->value)) { + mask = uvc_mapping_get_menu_value(mapping, query_menu->index); + if (mask < 0) { + ret = mask; + goto done; + } + + if (!(uvc_get_ctrl_bitmap(ctrl, mapping) & mask)) { ret = -EINVAL; goto done; } } - strscpy(query_menu->name, menu_info->name, sizeof(query_menu->name)); + name = uvc_mapping_get_menu_name(mapping, query_menu->index); + if (!name) { + ret = -EINVAL; + goto done; + } + + strscpy(query_menu->name, name, sizeof(query_menu->name)); done: mutex_unlock(&chain->ctrl_mutex); @@ -1902,7 +1944,7 @@ int uvc_ctrl_set(struct uvc_fh *handle, if (!test_bit(xctrl->value, &mapping->menu_mask)) return -EINVAL; - value = mapping->menu_info[xctrl->value].value; + value = uvc_mapping_get_menu_value(mapping, xctrl->value); /* * Valid menu indices are reported by the GET_RES request for @@ -2327,7 +2369,8 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain, return -ENOMEM; map->name = NULL; - map->menu_info = NULL; + map->menu_names = NULL; + map->menu_mapping = NULL; /* For UVCIOC_CTRL_MAP custom control */ if (mapping->name) { @@ -2338,10 +2381,22 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain, INIT_LIST_HEAD(&map->ev_subs); - size = sizeof(*mapping->menu_info) * fls(mapping->menu_mask); - map->menu_info = kmemdup(mapping->menu_info, size, GFP_KERNEL); - if (!map->menu_info) - goto err_nomem; + if (mapping->menu_mapping && mapping->menu_mask) { + size = sizeof(mapping->menu_mapping[0]) + * fls(mapping->menu_mask); + map->menu_mapping = kmemdup(mapping->menu_mapping, size, + GFP_KERNEL); + if (!map->menu_mapping) + goto err_nomem; + } + if (mapping->menu_names && mapping->menu_mask) { + size = sizeof(mapping->menu_names[0]) + * fls(mapping->menu_mask); + map->menu_names = kmemdup(mapping->menu_names, size, + GFP_KERNEL); + if (!map->menu_names) + goto err_nomem; + } if (map->get == NULL) map->get = uvc_get_le_value; @@ -2364,7 +2419,8 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain, return 0; err_nomem: - kfree(map->menu_info); + kfree(map->menu_names); + kfree(map->menu_mapping); kfree(map->name); kfree(map); return -ENOMEM; @@ -2689,7 +2745,8 @@ static void uvc_ctrl_cleanup_mappings(struct uvc_device *dev, list_for_each_entry_safe(mapping, nm, &ctrl->info.mappings, list) { list_del(&mapping->list); - kfree(mapping->menu_info); + kfree(mapping->menu_names); + kfree(mapping->menu_mapping); kfree(mapping->name); kfree(mapping); } diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 950b42d78a10..35453f81c1d9 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -26,14 +26,84 @@ #include "uvcvideo.h" +static int uvc_control_add_xu_mapping(struct uvc_video_chain *chain, + struct uvc_control_mapping *map, + const struct uvc_xu_control_mapping *xmap) +{ + unsigned int i; + size_t size; + int ret; + + /* + * Prevent excessive memory consumption, as well as integer + * overflows. + */ + if (xmap->menu_count == 0 || + xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) + return -EINVAL; + + map->menu_names = NULL; + map->menu_mapping = NULL; + + map->menu_mask = BIT_MASK(xmap->menu_count); + + size = xmap->menu_count * sizeof(*map->menu_mapping); + map->menu_mapping = kzalloc(size, GFP_KERNEL); + if (!map->menu_mapping) { + ret = -ENOMEM; + goto done; + } + + for (i = 0; i < xmap->menu_count ; i++) { + if (copy_from_user((u32 *)&map->menu_mapping[i], + &xmap->menu_info[i].value, + sizeof(map->menu_mapping[i]))) { + ret = -EACCES; + goto done; + } + } + + /* + * Always use the standard naming if available, otherwise copy the + * names supplied by userspace. + */ + if (!v4l2_ctrl_get_menu(map->id)) { + size = xmap->menu_count * sizeof(map->menu_names[0]); + map->menu_names = kzalloc(size, GFP_KERNEL); + if (!map->menu_names) { + ret = -ENOMEM; + goto done; + } + + for (i = 0; i < xmap->menu_count ; i++) { + /* sizeof(names[i]) - 1: to take care of \0 */ + if (copy_from_user((char *)map->menu_names[i], + xmap->menu_info[i].name, + sizeof(map->menu_names[i]) - 1)) { + ret = -EACCES; + goto done; + } + } + } + + ret = uvc_ctrl_add_mapping(chain, map); + +done: + kfree(map->menu_names); + map->menu_names = NULL; + kfree(map->menu_mapping); + map->menu_mapping = NULL; + + return ret; +} + /* ------------------------------------------------------------------------ * UVC ioctls */ -static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain, - struct uvc_xu_control_mapping *xmap) +static int uvc_ioctl_xu_ctrl_map(struct uvc_video_chain *chain, + struct uvc_xu_control_mapping *xmap) { struct uvc_control_mapping *map; - unsigned int size; int ret; map = kzalloc(sizeof(*map), GFP_KERNEL); @@ -61,39 +131,20 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain, case V4L2_CTRL_TYPE_INTEGER: case V4L2_CTRL_TYPE_BOOLEAN: case V4L2_CTRL_TYPE_BUTTON: + ret = uvc_ctrl_add_mapping(chain, map); break; case V4L2_CTRL_TYPE_MENU: - /* - * Prevent excessive memory consumption, as well as integer - * overflows. - */ - if (xmap->menu_count == 0 || - xmap->menu_count > UVC_MAX_CONTROL_MENU_ENTRIES) { - ret = -EINVAL; - goto free_map; - } - - size = xmap->menu_count * sizeof(*map->menu_info); - map->menu_info = memdup_user(xmap->menu_info, size); - if (IS_ERR(map->menu_info)) { - ret = PTR_ERR(map->menu_info); - goto free_map; - } - - map->menu_mask = GENMASK(xmap->menu_count - 1, 0); + ret = uvc_control_add_xu_mapping(chain, map, xmap); break; default: uvc_dbg(chain->dev, CONTROL, "Unsupported V4L2 control type %u\n", xmap->v4l2_type); ret = -ENOTTY; - goto free_map; + break; } - ret = uvc_ctrl_add_mapping(chain, map); - - kfree(map->menu_info); free_map: kfree(map); @@ -1314,7 +1365,7 @@ static long uvc_ioctl_default(struct file *file, void *fh, bool valid_prio, switch (cmd) { /* Dynamic controls. */ case UVCIOC_CTRL_MAP: - return uvc_ioctl_ctrl_map(chain, arg); + return uvc_ioctl_xu_ctrl_map(chain, arg); case UVCIOC_CTRL_QUERY: return uvc_xu_ctrl_query(chain, arg); @@ -1427,7 +1478,7 @@ static long uvc_v4l2_compat_ioctl32(struct file *file, ret = uvc_v4l2_get_xu_mapping(&karg.xmap, up); if (ret) return ret; - ret = uvc_ioctl_ctrl_map(handle->chain, &karg.xmap); + ret = uvc_ioctl_xu_ctrl_map(handle->chain, &karg.xmap); if (ret) return ret; ret = uvc_v4l2_put_xu_mapping(&karg.xmap, up); diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 178729467967..e85df8deb965 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -114,7 +114,8 @@ struct uvc_control_mapping { enum v4l2_ctrl_type v4l2_type; u32 data_type; - const struct uvc_menu_info *menu_info; + const u32 *menu_mapping; + const char (*menu_names)[UVC_MENU_NAME_LEN]; unsigned long menu_mask; u32 master_id; diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h index 8288137387c0..d45d0c2ea252 100644 --- a/include/uapi/linux/uvcvideo.h +++ b/include/uapi/linux/uvcvideo.h @@ -36,9 +36,11 @@ UVC_CTRL_FLAG_GET_MAX | UVC_CTRL_FLAG_GET_RES | \ UVC_CTRL_FLAG_GET_DEF) +#define UVC_MENU_NAME_LEN 32 + struct uvc_menu_info { __u32 value; - __u8 name[32]; + __u8 name[UVC_MENU_NAME_LEN]; }; struct uvc_xu_control_mapping { From 619d9b710cf06f7a00a17120ca92333684ac45a8 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Thu, 5 Jan 2023 15:31:29 +0100 Subject: [PATCH 25/27] media: uvcvideo: Fix race condition with usb_kill_urb usb_kill_urb warranties that all the handlers are finished when it returns, but does not protect against threads that might be handling asynchronously the urb. For UVC, the function uvc_ctrl_status_event_async() takes care of control changes asynchronously. If the code is executed in the following order: CPU 0 CPU 1 ===== ===== uvc_status_complete() uvc_status_stop() uvc_ctrl_status_event_work() uvc_status_start() -> FAIL Then uvc_status_start will keep failing and this error will be shown: <4>[ 5.540139] URB 0000000000000000 submitted while active drivers/usb/core/urb.c:378 usb_submit_urb+0x4c3/0x528 Let's improve the current situation, by not re-submiting the urb if we are stopping the status event. Also process the queued work (if any) during stop. CPU 0 CPU 1 ===== ===== uvc_status_complete() uvc_status_stop() uvc_status_start() uvc_ctrl_status_event_work() -> FAIL Hopefully, with the usb layer protection this should be enough to cover all the cases. Cc: stable@vger.kernel.org Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives") Reviewed-by: Yunke Cao Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_ctrl.c | 5 ++++ drivers/media/usb/uvc/uvc_status.c | 37 ++++++++++++++++++++++++++++++ drivers/media/usb/uvc/uvcvideo.h | 1 + 3 files changed, 43 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c index ee58f0db2763..c3aeba3fe31b 100644 --- a/drivers/media/usb/uvc/uvc_ctrl.c +++ b/drivers/media/usb/uvc/uvc_ctrl.c @@ -6,6 +6,7 @@ * Laurent Pinchart (laurent.pinchart@ideasonboard.com) */ +#include #include #include #include @@ -1571,6 +1572,10 @@ static void uvc_ctrl_status_event_work(struct work_struct *work) uvc_ctrl_status_event(w->chain, w->ctrl, w->data); + /* The barrier is needed to synchronize with uvc_status_stop(). */ + if (smp_load_acquire(&dev->flush_status)) + return; + /* Resubmit the URB. */ w->urb->interval = dev->int_ep->desc.bInterval; ret = usb_submit_urb(w->urb, GFP_KERNEL); diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c index 602830a8023e..a78a88c710e2 100644 --- a/drivers/media/usb/uvc/uvc_status.c +++ b/drivers/media/usb/uvc/uvc_status.c @@ -6,6 +6,7 @@ * Laurent Pinchart (laurent.pinchart@ideasonboard.com) */ +#include #include #include #include @@ -311,5 +312,41 @@ int uvc_status_start(struct uvc_device *dev, gfp_t flags) void uvc_status_stop(struct uvc_device *dev) { + struct uvc_ctrl_work *w = &dev->async_ctrl; + + /* + * Prevent the asynchronous control handler from requeing the URB. The + * barrier is needed so the flush_status change is visible to other + * CPUs running the asynchronous handler before usb_kill_urb() is + * called below. + */ + smp_store_release(&dev->flush_status, true); + + /* + * Cancel any pending asynchronous work. If any status event was queued, + * process it synchronously. + */ + if (cancel_work_sync(&w->work)) + uvc_ctrl_status_event(w->chain, w->ctrl, w->data); + + /* Kill the urb. */ usb_kill_urb(dev->int_urb); + + /* + * The URB completion handler may have queued asynchronous work. This + * won't resubmit the URB as flush_status is set, but it needs to be + * cancelled before returning or it could then race with a future + * uvc_status_start() call. + */ + if (cancel_work_sync(&w->work)) + uvc_ctrl_status_event(w->chain, w->ctrl, w->data); + + /* + * From this point, there are no events on the queue and the status URB + * is dead. No events will be queued until uvc_status_start() is called. + * The barrier is needed to make sure that flush_status is visible to + * uvc_ctrl_status_event_work() when uvc_status_start() will be called + * again. + */ + smp_store_release(&dev->flush_status, false); } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index e85df8deb965..c0e706fcd2cb 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -577,6 +577,7 @@ struct uvc_device { struct usb_host_endpoint *int_ep; struct urb *int_urb; struct uvc_status *status; + bool flush_status; struct input_dev *input; char input_phys[64]; From 136effa754b57632f99574fc4a3433e0cfc031d9 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Wed, 4 Jan 2023 11:45:23 +0100 Subject: [PATCH 26/27] media: uvcvideo: Quirk for autosuspend in Logitech B910 and C910 Logitech B910 and C910 firmware are unable to recover from a USB autosuspend. When it resumes, the device is in a state where it only produces invalid frames. Eg: $ echo 0xFFFF > /sys/module/uvcvideo/parameters/trace # enable verbose log $ yavta -c1 -n1 --file='frame#.jpg' --format MJPEG --size=1920x1080 /dev/video1 [350438.435219] uvcvideo: uvc_v4l2_open [350438.529794] uvcvideo: Resuming interface 2 [350438.529801] uvcvideo: Resuming interface 3 [350438.529991] uvcvideo: Trying format 0x47504a4d (MJPG): 1920x1080. [350438.529996] uvcvideo: Using default frame interval 33333.3 us (30.0 fps). [350438.551496] uvcvideo: uvc_v4l2_mmap [350438.555890] uvcvideo: Device requested 3060 B/frame bandwidth. [350438.555896] uvcvideo: Selecting alternate setting 11 (3060 B/frame bandwidth). [350438.556362] uvcvideo: Allocated 5 URB buffers of 32x3060 bytes each. [350439.316468] uvcvideo: Marking buffer as bad (error bit set). [350439.316475] uvcvideo: Frame complete (EOF found). [350439.316477] uvcvideo: EOF in empty payload. [350439.316484] uvcvideo: frame 1 stats: 149/261/417 packets, 1/149/417 pts (early initial), 416/417 scr, last pts/stc/sof 2976325734/2978107243/249 [350439.384510] uvcvideo: Marking buffer as bad (error bit set). [350439.384516] uvcvideo: Frame complete (EOF found). [350439.384518] uvcvideo: EOF in empty payload. [350439.384525] uvcvideo: frame 2 stats: 265/379/533 packets, 1/265/533 pts (early initial), 532/533 scr, last pts/stc/sof 2979524454/2981305193/316 [350439.448472] uvcvideo: Marking buffer as bad (error bit set). [350439.448478] uvcvideo: Frame complete (EOF found). [350439.448480] uvcvideo: EOF in empty payload. [350439.448487] uvcvideo: frame 3 stats: 265/377/533 packets, 1/265/533 pts (early initial), 532/533 scr, last pts/stc/sof 2982723174/2984503144/382 ...(loop)... The devices can leave this invalid state if the alternate setting of the streaming interface is toggled. This patch adds a quirk for this device so it can be autosuspended properly. lsusb -v: Bus 001 Device 049: ID 046d:0821 Logitech, Inc. HD Webcam C910 Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 bDeviceProtocol 1 Interface Association bMaxPacketSize0 64 idVendor 0x046d Logitech, Inc. idProduct 0x0821 HD Webcam C910 bcdDevice 0.10 iManufacturer 0 iProduct 0 iSerial 1 390022B0 bNumConfigurations 1 Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_driver.c | 18 ++++++++++++++++++ drivers/media/usb/uvc/uvc_video.c | 11 +++++++++++ drivers/media/usb/uvc/uvcvideo.h | 1 + 3 files changed, 30 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 6005e3c7eb96..11f3d716b5bf 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -2474,6 +2474,24 @@ static const struct usb_device_id uvc_ids[] = { .bInterfaceSubClass = 1, .bInterfaceProtocol = 0, .driver_info = (kernel_ulong_t)&uvc_quirk_probe_minmax }, + /* Logitech, Webcam C910 */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x046d, + .idProduct = 0x0821, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 0, + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_WAKE_AUTOSUSPEND)}, + /* Logitech, Webcam B910 */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x046d, + .idProduct = 0x0823, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 0, + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_WAKE_AUTOSUSPEND)}, /* Logitech Quickcam Fusion */ { .match_flags = USB_DEVICE_ID_MATCH_DEVICE | USB_DEVICE_ID_MATCH_INT_INFO, diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index b1f7416858b7..98d34d20afe3 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1969,6 +1969,17 @@ static int uvc_video_start_transfer(struct uvc_streaming *stream, "Selecting alternate setting %u (%u B/frame bandwidth)\n", altsetting, best_psize); + /* + * Some devices, namely the Logitech C910 and B910, are unable + * to recover from a USB autosuspend, unless the alternate + * setting of the streaming interface is toggled. + */ + if (stream->dev->quirks & UVC_QUIRK_WAKE_AUTOSUSPEND) { + usb_set_interface(stream->dev->udev, intfnum, + altsetting); + usb_set_interface(stream->dev->udev, intfnum, 0); + } + ret = usb_set_interface(stream->dev->udev, intfnum, altsetting); if (ret < 0) return ret; diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index c0e706fcd2cb..9a596c8d894a 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -72,6 +72,7 @@ #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x00000400 #define UVC_QUIRK_FORCE_Y8 0x00000800 #define UVC_QUIRK_FORCE_BPP 0x00001000 +#define UVC_QUIRK_WAKE_AUTOSUSPEND 0x00002000 /* Format flags */ #define UVC_FMT_FLAG_COMPRESSED 0x00000001 From b839212988575c701aab4d3d9ca15e44c87e383c Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 5 Jan 2023 22:17:04 -0800 Subject: [PATCH 27/27] media: uvcvideo: Silence memcpy() run-time false positive warnings The memcpy() in uvc_video_decode_meta() intentionally copies across the length and flags members and into the trailing buf flexible array. Split the copy so that the compiler can better reason about (the lack of) buffer overflows here. Avoid the run-time false positive warning: memcpy: detected field-spanning write (size 12) of single field "&meta->length" at drivers/media/usb/uvc/uvc_video.c:1355 (size 1) Additionally fix a typo in the documentation for struct uvc_meta_buf. Reported-by: ionut_n2001@yahoo.com Link: https://bugzilla.kernel.org/show_bug.cgi?id=216810 Signed-off-by: Kees Cook Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- drivers/media/usb/uvc/uvc_video.c | 4 +++- include/uapi/linux/uvcvideo.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 98d34d20afe3..d4b023d4de7c 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1356,7 +1356,9 @@ static void uvc_video_decode_meta(struct uvc_streaming *stream, if (has_scr) memcpy(stream->clock.last_scr, scr, 6); - memcpy(&meta->length, mem, length); + meta->length = mem[0]; + meta->flags = mem[1]; + memcpy(meta->buf, &mem[2], length - 2); meta_buf->bytesused += length + sizeof(meta->ns) + sizeof(meta->sof); uvc_dbg(stream->dev, FRAME, diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h index d45d0c2ea252..f86185456dc5 100644 --- a/include/uapi/linux/uvcvideo.h +++ b/include/uapi/linux/uvcvideo.h @@ -88,7 +88,7 @@ struct uvc_xu_control_query { * struct. The first two fields are added by the driver, they can be used for * clock synchronisation. The rest is an exact copy of a UVC payload header. * Only complete objects with complete buffers are included. Therefore it's - * always sizeof(meta->ts) + sizeof(meta->sof) + meta->length bytes large. + * always sizeof(meta->ns) + sizeof(meta->sof) + meta->length bytes large. */ struct uvc_meta_buf { __u64 ns;