From 52e4d9272b4bf8533e77194d06a789990753464e Mon Sep 17 00:00:00 2001 From: William Wu Date: Thu, 27 Mar 2025 09:00:21 +0800 Subject: [PATCH 01/20] Revert "usb: gadget: uvc: wait for req complete before free req" This reverts commit b37ab4ccd03095614b2395573f4dcbd07958be9d. Change-Id: I42a80e2d9c7c5691229c2d25c575ef246b6499dd Signed-off-by: William Wu --- drivers/usb/gadget/function/uvc.h | 3 -- drivers/usb/gadget/function/uvc_video.c | 38 ------------------------- 2 files changed, 41 deletions(-) diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 7ff10c9cea29..70e1d645fe7c 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -82,9 +82,6 @@ struct uvc_request { struct sg_table sgt; u8 header[UVCG_REQUEST_HEADER_LEN]; struct uvc_buffer *last_buf; -#if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI) - struct completion req_done; -#endif }; struct uvc_video { diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index e752a7b8937f..d7efaf595621 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -33,42 +33,11 @@ static bool uvc_using_zero_copy(struct uvc_video *video) else return false; } - -static void uvc_wait_req_complete(struct uvc_video *video, struct uvc_request *ureq) -{ - unsigned long flags; - struct usb_request *req; - int ret; - - spin_lock_irqsave(&video->req_lock, flags); - - list_for_each_entry(req, &video->req_free, list) { - if (req == ureq->req) - break; - } - - if (req != ureq->req) { - reinit_completion(&ureq->req_done); - - spin_unlock_irqrestore(&video->req_lock, flags); - ret = wait_for_completion_timeout(&ureq->req_done, - msecs_to_jiffies(500)); - if (ret == 0) - uvcg_warn(&video->uvc->func, - "timed out waiting for req done\n"); - return; - } - - spin_unlock_irqrestore(&video->req_lock, flags); -} #else static inline bool uvc_using_zero_copy(struct uvc_video *video) { return false; } - -static inline void uvc_wait_req_complete(struct uvc_video *video, struct uvc_request *ureq) -{ } #endif /* -------------------------------------------------------------------------- @@ -358,9 +327,6 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) spin_lock_irqsave(&video->req_lock, flags); list_add_tail(&req->list, &video->req_free); -#if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI) - complete(&ureq->req_done); -#endif spin_unlock_irqrestore(&video->req_lock, flags); if (uvc->state == UVC_STATE_STREAMING) @@ -377,7 +343,6 @@ uvc_video_free_requests(struct uvc_video *video) sg_free_table(&video->ureq[i].sgt); if (video->ureq[i].req) { - uvc_wait_req_complete(video, &video->ureq[i]); usb_ep_free_request(video->ep, video->ureq[i].req); video->ureq[i].req = NULL; } @@ -435,9 +400,6 @@ uvc_video_alloc_requests(struct uvc_video *video) video->ureq[i].video = video; video->ureq[i].last_buf = NULL; -#if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI) - init_completion(&video->ureq[i].req_done); -#endif list_add_tail(&video->ureq[i].req->list, &video->req_free); /* req_size/PAGE_SIZE + 1 for overruns and + 1 for header */ sg_alloc_table(&video->ureq[i].sgt, From fb9e41b3bfd5ec98aaf924d096c5482aeecb50bf Mon Sep 17 00:00:00 2001 From: Daniel Scally Date: Tue, 6 Dec 2022 16:12:03 +0000 Subject: [PATCH 02/20] UPSTREAM: usb: gadget: uvc: Rename bmInterfaceFlags -> bmInterlaceFlags In the specification documents for the Uncompressed and MJPEG USB Video Payloads, the field name is bmInterlaceFlags - it has been misnamed within the kernel. Although renaming the field does break the kernel's interface to userspace it should be low-risk in this instance. The field is read only and hardcoded to 0, so there was never any value in anyone reading it. A search of the uvc-gadget application and all the forks that I could find for it did not reveal any users either. Change-Id: I9d9903cebd796443387a40664a753498a10d6e6a Fixes: cdda479f15cd ("USB gadget: video class function driver") Reviewed-by: Laurent Pinchart Reviewed-by: Kieran Bingham Signed-off-by: Daniel Scally Link: https://lore.kernel.org/r/20221206161203.1562827-1-dan.scally@ideasonboard.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 81c25247a2a03a0f97e4805d7aff7541ccff6baa) --- Documentation/ABI/testing/configfs-usb-gadget-uvc | 4 ++-- drivers/usb/gadget/function/uvc_configfs.c | 12 ++++++------ drivers/usb/gadget/legacy/webcam.c | 4 ++-- include/uapi/linux/usb/video.h | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 7b5fe7c8361c..7fc36ab7f7c7 100644 --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -199,7 +199,7 @@ Description: Specific MJPEG format descriptors read-only bmaControls this format's data for bmaControls in the streaming header - bmInterfaceFlags specifies interlace information, + bmInterlaceFlags specifies interlace information, read-only bAspectRatioY the X dimension of the picture aspect ratio, read-only @@ -255,7 +255,7 @@ Description: Specific uncompressed format descriptors read-only bmaControls this format's data for bmaControls in the streaming header - bmInterfaceFlags specifies interlace information, + bmInterlaceFlags specifies interlace information, read-only bAspectRatioY the X dimension of the picture aspect ratio, read-only diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index 37c826e17252..51d7d942c82b 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -1653,7 +1653,7 @@ UVCG_UNCOMPRESSED_ATTR(b_bits_per_pixel, bBitsPerPixel, 8); UVCG_UNCOMPRESSED_ATTR(b_default_frame_index, bDefaultFrameIndex, 8); UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_x, bAspectRatioX, 8); UVCG_UNCOMPRESSED_ATTR_RO(b_aspect_ratio_y, bAspectRatioY, 8); -UVCG_UNCOMPRESSED_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8); +UVCG_UNCOMPRESSED_ATTR_RO(bm_interlace_flags, bmInterlaceFlags, 8); #undef UVCG_UNCOMPRESSED_ATTR #undef UVCG_UNCOMPRESSED_ATTR_RO @@ -1682,7 +1682,7 @@ static struct configfs_attribute *uvcg_uncompressed_attrs[] = { &uvcg_uncompressed_attr_b_default_frame_index, &uvcg_uncompressed_attr_b_aspect_ratio_x, &uvcg_uncompressed_attr_b_aspect_ratio_y, - &uvcg_uncompressed_attr_bm_interface_flags, + &uvcg_uncompressed_attr_bm_interlace_flags, &uvcg_uncompressed_attr_bma_controls, NULL, }; @@ -1715,7 +1715,7 @@ static struct config_group *uvcg_uncompressed_make(struct config_group *group, h->desc.bDefaultFrameIndex = 1; h->desc.bAspectRatioX = 0; h->desc.bAspectRatioY = 0; - h->desc.bmInterfaceFlags = 0; + h->desc.bmInterlaceFlags = 0; h->desc.bCopyProtect = 0; INIT_LIST_HEAD(&h->fmt.frames); @@ -1841,7 +1841,7 @@ UVCG_MJPEG_ATTR(b_default_frame_index, bDefaultFrameIndex, 8); UVCG_MJPEG_ATTR_RO(bm_flags, bmFlags, 8); UVCG_MJPEG_ATTR_RO(b_aspect_ratio_x, bAspectRatioX, 8); UVCG_MJPEG_ATTR_RO(b_aspect_ratio_y, bAspectRatioY, 8); -UVCG_MJPEG_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8); +UVCG_MJPEG_ATTR_RO(bm_interlace_flags, bmInterlaceFlags, 8); #undef UVCG_MJPEG_ATTR #undef UVCG_MJPEG_ATTR_RO @@ -1869,7 +1869,7 @@ static struct configfs_attribute *uvcg_mjpeg_attrs[] = { &uvcg_mjpeg_attr_bm_flags, &uvcg_mjpeg_attr_b_aspect_ratio_x, &uvcg_mjpeg_attr_b_aspect_ratio_y, - &uvcg_mjpeg_attr_bm_interface_flags, + &uvcg_mjpeg_attr_bm_interlace_flags, &uvcg_mjpeg_attr_bma_controls, NULL, }; @@ -1896,7 +1896,7 @@ static struct config_group *uvcg_mjpeg_make(struct config_group *group, h->desc.bDefaultFrameIndex = 1; h->desc.bAspectRatioX = 0; h->desc.bAspectRatioY = 0; - h->desc.bmInterfaceFlags = 0; + h->desc.bmInterlaceFlags = 0; h->desc.bCopyProtect = 0; INIT_LIST_HEAD(&h->fmt.frames); diff --git a/drivers/usb/gadget/legacy/webcam.c b/drivers/usb/gadget/legacy/webcam.c index 2e91b2a88929..6739830c60e4 100644 --- a/drivers/usb/gadget/legacy/webcam.c +++ b/drivers/usb/gadget/legacy/webcam.c @@ -171,7 +171,7 @@ static const struct uvc_format_uncompressed uvc_format_yuv = { .bDefaultFrameIndex = 1, .bAspectRatioX = 0, .bAspectRatioY = 0, - .bmInterfaceFlags = 0, + .bmInterlaceFlags = 0, .bCopyProtect = 0, }; @@ -222,7 +222,7 @@ static const struct uvc_format_mjpeg uvc_format_mjpg = { .bDefaultFrameIndex = 1, .bAspectRatioX = 0, .bAspectRatioY = 0, - .bmInterfaceFlags = 0, + .bmInterlaceFlags = 0, .bCopyProtect = 0, }; diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h index d160d5bdd730..ac97efaf6970 100644 --- a/include/uapi/linux/usb/video.h +++ b/include/uapi/linux/usb/video.h @@ -496,7 +496,7 @@ struct uvc_format_uncompressed { __u8 bDefaultFrameIndex; __u8 bAspectRatioX; __u8 bAspectRatioY; - __u8 bmInterfaceFlags; + __u8 bmInterlaceFlags; __u8 bCopyProtect; } __attribute__((__packed__)); @@ -552,7 +552,7 @@ struct uvc_format_mjpeg { __u8 bDefaultFrameIndex; __u8 bAspectRatioX; __u8 bAspectRatioY; - __u8 bmInterfaceFlags; + __u8 bmInterlaceFlags; __u8 bCopyProtect; } __attribute__((__packed__)); From 3237d532d5eadea603aef94787d8a6c50932ce40 Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Tue, 11 Oct 2022 09:53:48 +0200 Subject: [PATCH 03/20] UPSTREAM: usb: gadget: uvc: default the ctrl request interface offsets For the userspace it is needed to distinguish between requests for the control or streaming interface. The userspace would have to parse the configfs to know which interface index it has to compare the ctrl requests against. Since the interface numbers are not fixed, e.g. for composite gadgets, the interface offset depends on the setup. The kernel has this information when handing over the ctrl request to the userspace. This patch removes the offset from the interface numbers and expose the default interface defines in the uapi g_uvc.h. Change-Id: Idb6845c962d3da6d2a96c5d5e0083b39e5bba8af Signed-off-by: Michael Grzeschik Link: https://lore.kernel.org/r/20221011075348.1786897-1-m.grzeschik@pengutronix.de Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit d182bf156c4cb8b08ce4a75e82b3357b14a4382d) --- drivers/usb/gadget/function/f_uvc.c | 15 ++++++++++++--- include/uapi/linux/usb/g_uvc.h | 3 +++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 09076224b5ca..f4ef703709eb 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -39,9 +39,6 @@ MODULE_PARM_DESC(trace, "Trace level bitmask"); /* string IDs are assigned dynamically */ -#define UVC_STRING_CONTROL_IDX 0 -#define UVC_STRING_STREAMING_IDX 1 - static struct usb_string uvc_en_us_strings[] = { /* [UVC_STRING_CONTROL_IDX].s = DYNAMIC, */ [UVC_STRING_STREAMING_IDX].s = "Video Streaming", @@ -300,6 +297,8 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) struct uvc_device *uvc = to_uvc(f); struct v4l2_event v4l2_event; struct uvc_event *uvc_event = (void *)&v4l2_event.u.data; + unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff; + struct usb_ctrlrequest *mctrl; uvc_trace(UVC_TRACE_CONTROL, "setup request %02x %02x value %04x index %04x %04x\n", @@ -325,6 +324,16 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) memset(&v4l2_event, 0, sizeof(v4l2_event)); v4l2_event.type = UVC_EVENT_SETUP; memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req)); + + /* check for the interface number, fixup the interface number in + * the ctrl request so the userspace doesn't have to bother with + * offset and configfs parsing + */ + mctrl = &uvc_event->req; + mctrl->wIndex &= ~cpu_to_le16(0xff); + if (interface == uvc->streaming_intf) + mctrl->wIndex = cpu_to_le16(UVC_STRING_STREAMING_IDX); + v4l2_event_queue(&uvc->vdev, &v4l2_event); return 0; diff --git a/include/uapi/linux/usb/g_uvc.h b/include/uapi/linux/usb/g_uvc.h index 428926e35973..e5b7faaf200f 100644 --- a/include/uapi/linux/usb/g_uvc.h +++ b/include/uapi/linux/usb/g_uvc.h @@ -23,6 +23,9 @@ #define UVC_EVENT_RESUME (V4L2_EVENT_PRIVATE_START + 7) #define UVC_EVENT_LAST (V4L2_EVENT_PRIVATE_START + 7) +#define UVC_STRING_CONTROL_IDX 0 +#define UVC_STRING_STREAMING_IDX 1 + struct uvc_request_data { __s32 length; __u8 data[60]; From 013e3bbfbb4f794ca43f0e18f74f70c217e0a736 Mon Sep 17 00:00:00 2001 From: Linyu Yuan Date: Thu, 3 Aug 2023 17:10:50 +0800 Subject: [PATCH 04/20] BACKPORT: usb: gadget: unconditionally allocate hs/ss descriptor in bind operation Take f_midi_bind() for example, when composite layer call it, it will allocate hs descriptor by calling gadget_is_dualspeed() API to check gadget max support speed capability, but most other gadget function didn't do like this. To follow other function drivers, it is safe to remove the check which mean support all possible link speed by default in function driver. Similar change apply to midi2 and uvc. Also in midi and midi2, as there is no descriptor difference between super speed and super speed plus, follow other gadget function drivers, do not allocate descriptor for super speed plus, composite layer will handle it properly. Change-Id: Iabf23f6063767ad68e29dd13c05e2cceaf8d461e Signed-off-by: Linyu Yuan Link: https://lore.kernel.org/r/20230803091053.9714-5-quic_linyyuan@quicinc.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 46decc82ffd54212cc2c600031daec6e835a6503) --- drivers/usb/gadget/function/f_midi.c | 56 ++++++++++++---------------- drivers/usb/gadget/function/f_uvc.c | 26 ++++++------- 2 files changed, 35 insertions(+), 47 deletions(-) diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index fddf539008a9..2d02f25f9597 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -1023,40 +1023,30 @@ static int f_midi_bind(struct usb_configuration *c, struct usb_function *f) if (!f->fs_descriptors) goto fail_f_midi; - if (gadget_is_dualspeed(c->cdev->gadget)) { - bulk_in_desc.wMaxPacketSize = cpu_to_le16(512); - bulk_out_desc.wMaxPacketSize = cpu_to_le16(512); - f->hs_descriptors = usb_copy_descriptors(midi_function); - if (!f->hs_descriptors) - goto fail_f_midi; - } + bulk_in_desc.wMaxPacketSize = cpu_to_le16(512); + bulk_out_desc.wMaxPacketSize = cpu_to_le16(512); + f->hs_descriptors = usb_copy_descriptors(midi_function); + if (!f->hs_descriptors) + goto fail_f_midi; - if (gadget_is_superspeed(c->cdev->gadget)) { - bulk_in_desc.wMaxPacketSize = cpu_to_le16(1024); - bulk_out_desc.wMaxPacketSize = cpu_to_le16(1024); - i = endpoint_descriptor_index; - midi_function[i++] = (struct usb_descriptor_header *) - &bulk_out_desc; - midi_function[i++] = (struct usb_descriptor_header *) - &bulk_out_ss_comp_desc; - midi_function[i++] = (struct usb_descriptor_header *) - &ms_out_desc; - midi_function[i++] = (struct usb_descriptor_header *) - &bulk_in_desc; - midi_function[i++] = (struct usb_descriptor_header *) - &bulk_in_ss_comp_desc; - midi_function[i++] = (struct usb_descriptor_header *) - &ms_in_desc; - f->ss_descriptors = usb_copy_descriptors(midi_function); - if (!f->ss_descriptors) - goto fail_f_midi; - - if (gadget_is_superspeed_plus(c->cdev->gadget)) { - f->ssp_descriptors = usb_copy_descriptors(midi_function); - if (!f->ssp_descriptors) - goto fail_f_midi; - } - } + bulk_in_desc.wMaxPacketSize = cpu_to_le16(1024); + bulk_out_desc.wMaxPacketSize = cpu_to_le16(1024); + i = endpoint_descriptor_index; + midi_function[i++] = (struct usb_descriptor_header *) + &bulk_out_desc; + midi_function[i++] = (struct usb_descriptor_header *) + &bulk_out_ss_comp_desc; + midi_function[i++] = (struct usb_descriptor_header *) + &ms_out_desc; + midi_function[i++] = (struct usb_descriptor_header *) + &bulk_in_desc; + midi_function[i++] = (struct usb_descriptor_header *) + &bulk_in_ss_comp_desc; + midi_function[i++] = (struct usb_descriptor_header *) + &ms_in_desc; + f->ss_descriptors = usb_copy_descriptors(midi_function); + if (!f->ss_descriptors) + goto fail_f_midi; kfree(midi_function); diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index f4ef703709eb..f54641187de4 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -1004,21 +1004,19 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) f->fs_descriptors = NULL; goto error; } - if (gadget_is_dualspeed(cdev->gadget)) { - f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH); - if (IS_ERR(f->hs_descriptors)) { - ret = PTR_ERR(f->hs_descriptors); - f->hs_descriptors = NULL; - goto error; - } + + f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH); + if (IS_ERR(f->hs_descriptors)) { + ret = PTR_ERR(f->hs_descriptors); + f->hs_descriptors = NULL; + goto error; } - if (gadget_is_superspeed(c->cdev->gadget)) { - f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER); - if (IS_ERR(f->ss_descriptors)) { - ret = PTR_ERR(f->ss_descriptors); - f->ss_descriptors = NULL; - goto error; - } + + f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER); + if (IS_ERR(f->ss_descriptors)) { + ret = PTR_ERR(f->ss_descriptors); + f->ss_descriptors = NULL; + goto error; } /* Preallocate control endpoint request. */ From 32347a1a4d672846bb8eec84087f0891a6d5f136 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Fri, 27 Oct 2023 11:34:40 -0700 Subject: [PATCH 05/20] UPSTREAM: usb: gadget: uvc: Add missing initialization of ssp config descriptor In case the uvc gadget is super speed plus, the corresponding config descriptor wasn't initialized. As a result, the host will not recognize the devices when using super speed plus connection. This patch initializes them to super speed descriptors. Change-Id: I10aee17bc461f8d3d3b61f406d39113289ca951a Reviewed-by: Laurent Pinchart Signed-off-by: Shuzhen Wang Link: https://lore.kernel.org/r/20231027183440.1994315-1-shuzhenwang@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit c70793fb7632a153862ee9060e6d48131469a29c) --- drivers/usb/gadget/function/f_uvc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index f54641187de4..4bf9d9dee81c 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -668,6 +668,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed) opts = fi_to_f_uvc_opts(uvc->func.fi); switch (speed) { + case USB_SPEED_SUPER_PLUS: case USB_SPEED_SUPER: uvc_control_desc = uvc->desc.ss_control; uvc_streaming_cls = uvc->desc.ss_streaming; @@ -730,7 +731,8 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed) bytes += uvc_interrupt_ep.bLength + uvc_interrupt_cs_ep.bLength; n_desc += 2; - if (speed == USB_SPEED_SUPER) { + if (speed == USB_SPEED_SUPER || + speed == USB_SPEED_SUPER_PLUS) { bytes += uvc_ss_interrupt_comp.bLength; n_desc += 1; } @@ -774,7 +776,8 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed) if (uvc->enable_interrupt_ep) { UVC_COPY_DESCRIPTOR(mem, dst, &uvc_interrupt_ep); - if (speed == USB_SPEED_SUPER) + if (speed == USB_SPEED_SUPER || + speed == USB_SPEED_SUPER_PLUS) UVC_COPY_DESCRIPTOR(mem, dst, &uvc_ss_interrupt_comp); UVC_COPY_DESCRIPTOR(mem, dst, &uvc_interrupt_cs_ep); @@ -1019,6 +1022,13 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) goto error; } + f->ssp_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER_PLUS); + if (IS_ERR(f->ssp_descriptors)) { + ret = PTR_ERR(f->ssp_descriptors); + f->ssp_descriptors = NULL; + goto error; + } + /* Preallocate control endpoint request. */ uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL); uvc->control_buf = kmalloc(UVC_MAX_REQUEST_SIZE, GFP_KERNEL); From 465b830c991e0c9f8a79d457f91ff4eb8400b62e Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Fri, 2 Jun 2023 15:04:55 -0700 Subject: [PATCH 06/20] UPSTREAM: usb: gadget: uvc: clean up comments and styling in video_pump This patch elaborates on some of the edge cases handled by video_pump around setting no_interrupt flag, and brings the code style in line with rest of the file. Change-Id: I4824fb484e0901933e00218e7464d535acb609fc Link: https://lore.kernel.org/20230602151916.GH26944@pendragon.ideasonboard.com/ Signed-off-by: Avichal Rakesh Reviewed-by: Laurent Pinchart Message-ID: <20230602220455.313801-1-arakesh@google.com> Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 5ae8a35459e77fd9ddb1844baa8c736fc0223847) --- drivers/usb/gadget/function/uvc_video.c | 38 ++++++++++++++++--------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index d7efaf595621..57ae7d979ed8 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -430,13 +430,13 @@ static void uvcg_video_pump(struct work_struct *work) { struct uvc_video *video = container_of(work, struct uvc_video, pump); struct uvc_video_queue *queue = &video->queue; + /* video->max_payload_size is only set when using bulk transfer */ + bool is_bulk = video->max_payload_size; struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; + bool buf_done; int ret; - bool buf_int; - /* video->max_payload_size is only set when using bulk transfer */ - bool is_bulk = video->max_payload_size; while (video->ep->enabled) { /* @@ -462,20 +462,19 @@ static void uvcg_video_pump(struct work_struct *work) if (buf != NULL) { video->encode(req, video, buf); - /* Always interrupt for the last request of a video buffer */ - buf_int = buf->state == UVC_BUF_STATE_DONE; + buf_done = buf->state == UVC_BUF_STATE_DONE; } else if (!(queue->flags & UVC_QUEUE_DISCONNECTED) && !is_bulk) { /* * No video buffer available; the queue is still connected and - * we're traferring over ISOC. Queue a 0 length request to + * we're transferring over ISOC. Queue a 0 length request to * prevent missed ISOC transfers. */ req->length = 0; - buf_int = false; + buf_done = false; } else { /* - * Either queue has been disconnected or no video buffer - * available to bulk transfer. Either way, stop processing + * Either the queue has been disconnected or no video buffer + * available for bulk transfer. Either way, stop processing * further. */ spin_unlock_irqrestore(&queue->irqlock, flags); @@ -483,11 +482,24 @@ static void uvcg_video_pump(struct work_struct *work) } /* - * With usb3 we have more requests. This will decrease the - * interrupt load to a quarter but also catches the corner - * cases, which needs to be handled. + * With USB3 handling more requests at a higher speed, we can't + * afford to generate an interrupt for every request. Decide to + * interrupt: + * + * - When no more requests are available in the free queue, as + * this may be our last chance to refill the endpoint's + * request queue. + * + * - When this is request is the last request for the video + * buffer, as we want to start sending the next video buffer + * ASAP in case it doesn't get started already in the next + * iteration of this loop. + * + * - Four times over the length of the requests queue (as + * indicated by video->uvc_num_requests), as a trade-off + * between latency and interrupt load. */ - if (list_empty(&video->req_free) || buf_int || + if (list_empty(&video->req_free) || buf_done || !(video->req_int_count % DIV_ROUND_UP(video->uvc_num_requests, 4))) { video->req_int_count = 0; From 92b18f78851fc7ef1fb0620152e5fe3c6382290c Mon Sep 17 00:00:00 2001 From: Yue Haibing Date: Fri, 18 Aug 2023 20:40:25 +0800 Subject: [PATCH 07/20] UPSTREAM: usb: gadget: function: Remove unused declarations These declarations are not implemented anymore, remove them. Change-Id: Ie4149af1aa41b5b7aac98e304334b44fb9449be6 Signed-off-by: Yue Haibing Link: https://lore.kernel.org/r/20230818124025.51576-1-yuehaibing@huawei.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit ae257611573cde279d31be3961a59e255f567fb0) --- drivers/usb/gadget/function/u_phonet.h | 1 - drivers/usb/gadget/function/u_serial.h | 4 ---- drivers/usb/gadget/function/uvc.h | 2 -- 3 files changed, 7 deletions(-) diff --git a/drivers/usb/gadget/function/u_phonet.h b/drivers/usb/gadget/function/u_phonet.h index c53233b37192..ff62ca22c40d 100644 --- a/drivers/usb/gadget/function/u_phonet.h +++ b/drivers/usb/gadget/function/u_phonet.h @@ -20,7 +20,6 @@ struct f_phonet_opts { struct net_device *gphonet_setup_default(void); void gphonet_set_gadget(struct net_device *net, struct usb_gadget *g); int gphonet_register_netdev(struct net_device *net); -int phonet_bind_config(struct usb_configuration *c, struct net_device *dev); void gphonet_cleanup(struct net_device *dev); #endif /* __U_PHONET_H */ diff --git a/drivers/usb/gadget/function/u_serial.h b/drivers/usb/gadget/function/u_serial.h index 102a7323a1fd..901d99310bc4 100644 --- a/drivers/usb/gadget/function/u_serial.h +++ b/drivers/usb/gadget/function/u_serial.h @@ -71,8 +71,4 @@ void gserial_disconnect(struct gserial *); void gserial_suspend(struct gserial *p); void gserial_resume(struct gserial *p); -/* functions are bound to configurations by a config or gadget driver */ -int gser_bind_config(struct usb_configuration *c, u8 port_num); -int obex_bind_config(struct usb_configuration *c, u8 port_num); - #endif /* __U_SERIAL_H */ diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 70e1d645fe7c..d7f7bf26bbd1 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -181,8 +181,6 @@ struct uvc_file_handle { */ extern void uvc_function_setup_continue(struct uvc_device *uvc); -extern void uvc_endpoint_stream(struct uvc_device *dev); - extern void uvc_function_connect(struct uvc_device *uvc); extern void uvc_function_disconnect(struct uvc_device *uvc); From cdea73055c907789dc4613d7bb29c966c958d5fe Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Wed, 8 Nov 2023 16:41:01 -0800 Subject: [PATCH 08/20] BACKPORT: usb: gadget: uvc: prevent use of disabled endpoint Currently the set_alt callback immediately disables the endpoint and queues the v4l2 streamoff event. However, as the streamoff event is processed asynchronously, it is possible that the video_pump thread attempts to queue requests to an already disabled endpoint. This change moves disabling usb endpoint to the end of streamoff event callback. As the endpoint's state can no longer be used, video_pump is now guarded by uvc->state as well. To be consistent with the actual streaming state, uvc->state is now toggled between CONNECTED and STREAMING from the v4l2 event callback only. Change-Id: I64c1a72c9797858b96f5f1bb828258935c0318be Link: https://lore.kernel.org/20230615171558.GK741@pendragon.ideasonboard.com/ Link: https://lore.kernel.org/20230531085544.253363-1-dan.scally@ideasonboard.com/ Reviewed-by: Daniel Scally Reviewed-by: Michael Grzeschik Tested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh Link: https://lore.kernel.org/r/20231109004104.3467968-1-arakesh@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 991544dc579b636e69defa3eec486fd6f6191e59) --- drivers/usb/gadget/function/f_uvc.c | 15 +++++++-------- drivers/usb/gadget/function/f_uvc.h | 2 +- drivers/usb/gadget/function/uvc.h | 2 +- drivers/usb/gadget/function/uvc_v4l2.c | 20 +++++++++++++++++--- drivers/usb/gadget/function/uvc_video.c | 3 ++- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 4bf9d9dee81c..91f10eb54191 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -339,10 +339,13 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) return 0; } -void uvc_function_setup_continue(struct uvc_device *uvc) +void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep) { struct usb_composite_dev *cdev = uvc->func.config->cdev; + if (disable_ep && uvc->video.ep) + usb_ep_disable(uvc->video.ep); + usb_composite_setup_continue(cdev); } @@ -432,15 +435,11 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt) if (uvc->state != UVC_STATE_STREAMING) return 0; - if (uvc->video.ep) - usb_ep_disable(uvc->video.ep); - memset(&v4l2_event, 0, sizeof(v4l2_event)); v4l2_event.type = UVC_EVENT_STREAMOFF; v4l2_event_queue(&uvc->vdev, &v4l2_event); - uvc->state = UVC_STATE_CONNECTED; - return 0; + return USB_GADGET_DELAYED_STATUS; case 1: if (uvc->state != UVC_STATE_CONNECTED) @@ -508,8 +507,8 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt) memset(&v4l2_event, 0, sizeof(v4l2_event)); v4l2_event.type = UVC_EVENT_STREAMOFF; v4l2_event_queue(&uvc->vdev, &v4l2_event); - uvc->state = UVC_STATE_CONNECTED; - return 0; + + return USB_GADGET_DELAYED_STATUS; default: return -EINVAL; diff --git a/drivers/usb/gadget/function/f_uvc.h b/drivers/usb/gadget/function/f_uvc.h index 1db972d4beeb..083aef0c65c6 100644 --- a/drivers/usb/gadget/function/f_uvc.h +++ b/drivers/usb/gadget/function/f_uvc.h @@ -11,7 +11,7 @@ struct uvc_device; -void uvc_function_setup_continue(struct uvc_device *uvc); +void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep); void uvc_function_connect(struct uvc_device *uvc); diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index d7f7bf26bbd1..38b202b7ef13 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -180,7 +180,7 @@ struct uvc_file_handle { * Functions */ -extern void uvc_function_setup_continue(struct uvc_device *uvc); +extern void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep); extern void uvc_function_connect(struct uvc_device *uvc); extern void uvc_function_disconnect(struct uvc_device *uvc); diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 7bb68a0a37ac..dc36d55a8623 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -469,7 +469,7 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type) * to provide video frames. */ if (!usb_endpoint_xfer_bulk(video->ep->desc)) - uvc_function_setup_continue(uvc); + uvc_function_setup_continue(uvc, 0); uvc->state = UVC_STATE_STREAMING; @@ -482,11 +482,18 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) struct video_device *vdev = video_devdata(file); struct uvc_device *uvc = video_get_drvdata(vdev); struct uvc_video *video = &uvc->video; + int ret = 0; if (type != video->queue.queue.type) return -EINVAL; - return uvcg_video_enable(video, 0); + uvc->state = UVC_STATE_CONNECTED; + ret = uvcg_video_enable(video, 0); + if (ret < 0) + return ret; + + uvc_function_setup_continue(uvc, 1); + return 0; } static int @@ -519,6 +526,14 @@ uvc_v4l2_subscribe_event(struct v4l2_fh *fh, static void uvc_v4l2_disable(struct uvc_device *uvc) { uvc_function_disconnect(uvc); + /* + * Drop uvc->state to CONNECTED if it was streaming before. + * This ensures that the usb_requests are no longer queued + * to the controller. + */ + if (uvc->state == UVC_STATE_STREAMING) + uvc->state = UVC_STATE_CONNECTED; + uvcg_video_enable(&uvc->video, 0); uvcg_free_buffers(&uvc->video.queue); uvc->func_connected = false; @@ -669,4 +684,3 @@ const struct v4l2_file_operations uvc_v4l2_fops = { .get_unmapped_area = uvcg_v4l2_get_unmapped_area, #endif }; - diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 57ae7d979ed8..b4145a246034 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -432,13 +432,14 @@ static void uvcg_video_pump(struct work_struct *work) struct uvc_video_queue *queue = &video->queue; /* video->max_payload_size is only set when using bulk transfer */ bool is_bulk = video->max_payload_size; + struct uvc_device *uvc = video->uvc; struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; bool buf_done; int ret; - while (video->ep->enabled) { + while (uvc->state == UVC_STATE_STREAMING && video->ep->enabled) { /* * Retrieve the first available USB request, protected by the * request lock. From 5762defe3b13f80c9438ad405303a563262e1bc7 Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Wed, 8 Nov 2023 16:41:02 -0800 Subject: [PATCH 09/20] UPSTREAM: usb: gadget: uvc: Allocate uvc_requests one at a time Currently, the uvc gadget driver allocates all uvc_requests as one array and deallocates them all when the video stream stops. This includes de-allocating all the usb_requests associated with those uvc_requests. This can lead to use-after-free issues if any of those de-allocated usb_requests were still owned by the usb controller. This patch is 1 of 2 patches addressing the use-after-free issue. Instead of bulk allocating all uvc_requests as an array, this patch allocates uvc_requests one at a time, which should allows for similar granularity when deallocating the uvc_requests. This patch has no functional changes other than allocating each uvc_request separately, and similarly freeing each of them separately. Change-Id: Ifc4c21cb46d5c947b6be03c5423ddfcbd5d9f23f Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@google.com Reviewed-by: Daniel Scally Reviewed-by: Michael Grzeschik Suggested-by: Michael Grzeschik Tested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh Link: https://lore.kernel.org/r/20231109004104.3467968-2-arakesh@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit aeb686a98a9e9743c4c0338957e59643a2708146) --- drivers/usb/gadget/function/uvc.h | 3 +- drivers/usb/gadget/function/uvc_video.c | 90 ++++++++++++++----------- 2 files changed, 52 insertions(+), 41 deletions(-) diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 38b202b7ef13..4b6c5fe9bb6f 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -82,6 +82,7 @@ struct uvc_request { struct sg_table sgt; u8 header[UVCG_REQUEST_HEADER_LEN]; struct uvc_buffer *last_buf; + struct list_head list; }; struct uvc_video { @@ -103,7 +104,7 @@ struct uvc_video { /* Requests */ unsigned int req_size; - struct uvc_request *ureq; + struct list_head ureqs; /* all uvc_requests allocated by uvc_video */ struct list_head req_free; spinlock_t req_lock; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index b4145a246034..8da170ccc916 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -270,6 +270,24 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video, * Request handling */ +static void +uvc_video_free_request(struct uvc_request *ureq, struct usb_ep *ep) +{ + sg_free_table(&ureq->sgt); + if (ureq->req && ep) { + usb_ep_free_request(ep, ureq->req); + ureq->req = NULL; + } + + kfree(ureq->req_buffer); + ureq->req_buffer = NULL; + + if (!list_empty(&ureq->list)) + list_del_init(&ureq->list); + + kfree(ureq); +} + static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req) { int ret; @@ -336,27 +354,12 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) static int uvc_video_free_requests(struct uvc_video *video) { - unsigned int i; + struct uvc_request *ureq, *temp; - if (video->ureq) { - for (i = 0; i < video->uvc_num_requests; ++i) { - sg_free_table(&video->ureq[i].sgt); - - if (video->ureq[i].req) { - usb_ep_free_request(video->ep, video->ureq[i].req); - video->ureq[i].req = NULL; - } - - if (video->ureq[i].req_buffer) { - kfree(video->ureq[i].req_buffer); - video->ureq[i].req_buffer = NULL; - } - } - - kfree(video->ureq); - video->ureq = NULL; - } + list_for_each_entry_safe(ureq, temp, &video->ureqs, list) + uvc_video_free_request(ureq, video->ep); + INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); video->req_size = 0; return 0; @@ -365,6 +368,7 @@ uvc_video_free_requests(struct uvc_video *video) static int uvc_video_alloc_requests(struct uvc_video *video) { + struct uvc_request *ureq; unsigned int req_size; unsigned int i; int ret = -ENOMEM; @@ -380,29 +384,33 @@ uvc_video_alloc_requests(struct uvc_video *video) * max_t(unsigned int, video->ep->maxburst, 1); } - video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL); - if (video->ureq == NULL) - return -ENOMEM; - - for (i = 0; i < video->uvc_num_requests; ++i) { - video->ureq[i].req_buffer = kmalloc(req_size, GFP_KERNEL); - if (video->ureq[i].req_buffer == NULL) + for (i = 0; i < video->uvc_num_requests; i++) { + ureq = kzalloc(sizeof(struct uvc_request), GFP_KERNEL); + if (ureq == NULL) goto error; - video->ureq[i].req = usb_ep_alloc_request(video->ep, GFP_KERNEL); - if (video->ureq[i].req == NULL) + INIT_LIST_HEAD(&ureq->list); + + list_add_tail(&ureq->list, &video->ureqs); + + ureq->req_buffer = kmalloc(req_size, GFP_KERNEL); + if (ureq->req_buffer == NULL) goto error; - video->ureq[i].req->buf = video->ureq[i].req_buffer; - video->ureq[i].req->length = 0; - video->ureq[i].req->complete = uvc_video_complete; - video->ureq[i].req->context = &video->ureq[i]; - video->ureq[i].video = video; - video->ureq[i].last_buf = NULL; + ureq->req = usb_ep_alloc_request(video->ep, GFP_KERNEL); + if (ureq->req == NULL) + goto error; - list_add_tail(&video->ureq[i].req->list, &video->req_free); + ureq->req->buf = ureq->req_buffer; + ureq->req->length = 0; + ureq->req->complete = uvc_video_complete; + ureq->req->context = ureq; + ureq->video = video; + ureq->last_buf = NULL; + + list_add_tail(&ureq->req->list, &video->req_free); /* req_size/PAGE_SIZE + 1 for overruns and + 1 for header */ - sg_alloc_table(&video->ureq[i].sgt, + sg_alloc_table(&ureq->sgt, DIV_ROUND_UP(req_size - UVCG_REQUEST_HEADER_LEN, PAGE_SIZE) + 2, GFP_KERNEL); } @@ -537,8 +545,8 @@ static void uvcg_video_pump(struct work_struct *work) */ int uvcg_video_enable(struct uvc_video *video, int enable) { - unsigned int i; int ret; + struct uvc_request *ureq; struct uvc_device *uvc; struct f_uvc_opts *opts; @@ -555,9 +563,10 @@ int uvcg_video_enable(struct uvc_video *video, int enable) cancel_work_sync(&video->pump); uvcg_queue_cancel(&video->queue, 0); - for (i = 0; i < video->uvc_num_requests; ++i) - if (video->ureq && video->ureq[i].req) - usb_ep_dequeue(video->ep, video->ureq[i].req); + list_for_each_entry(ureq, &video->ureqs, list) { + if (ureq->req) + usb_ep_dequeue(video->ep, ureq->req); + } uvc_video_free_requests(video); uvcg_queue_enable(&video->queue, 0); @@ -592,6 +601,7 @@ int uvcg_video_enable(struct uvc_video *video, int enable) */ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc) { + INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); spin_lock_init(&video->req_lock); INIT_WORK(&video->pump, uvcg_video_pump); From c5a3634277771c2e01f933ff77b4bd7f9d5ba3ee Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Wed, 8 Nov 2023 16:41:03 -0800 Subject: [PATCH 10/20] UPSTREAM: usb: gadget: uvc: move video disable logic to its own function This patch refactors the video disable logic in uvcg_video_enable into its own separate function 'uvcg_video_disable'. This function is now used anywhere uvcg_video_enable(video, 0) was used. Change-Id: Ifba331485eee8b671126d92f5af7a2f71d79ecc2 Reviewed-by: Daniel Scally Suggested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh Link: https://lore.kernel.org/r/20231109004104.3467968-3-arakesh@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 2079b60bda3257146a4e8ed7525513865f7e6b3e) --- drivers/usb/gadget/function/uvc_v4l2.c | 6 +-- drivers/usb/gadget/function/uvc_video.c | 57 ++++++++++++++++--------- drivers/usb/gadget/function/uvc_video.h | 3 +- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index dc36d55a8623..e27875c73e78 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -453,7 +453,7 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type) return -ENODEV; /* Enable UVC video. */ - ret = uvcg_video_enable(video, 1); + ret = uvcg_video_enable(video); if (ret < 0) return ret; @@ -488,7 +488,7 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) return -EINVAL; uvc->state = UVC_STATE_CONNECTED; - ret = uvcg_video_enable(video, 0); + ret = uvcg_video_disable(video); if (ret < 0) return ret; @@ -534,7 +534,7 @@ static void uvc_v4l2_disable(struct uvc_device *uvc) if (uvc->state == UVC_STATE_STREAMING) uvc->state = UVC_STATE_CONNECTED; - uvcg_video_enable(&uvc->video, 0); + uvcg_video_disable(&uvc->video); uvcg_free_buffers(&uvc->video.queue); uvc->func_connected = false; wake_up_interruptible(&uvc->func_connected_queue); diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 8da170ccc916..f4eb17452643 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -541,15 +541,48 @@ static void uvcg_video_pump(struct work_struct *work) } /* - * Enable or disable the video stream. + * Disable the video stream */ -int uvcg_video_enable(struct uvc_video *video, int enable) +int +uvcg_video_disable(struct uvc_video *video) { - int ret; struct uvc_request *ureq; struct uvc_device *uvc; struct f_uvc_opts *opts; + if (video->ep == NULL) { + uvcg_info(&video->uvc->func, + "Video disable failed, device is uninitialized.\n"); + return -ENODEV; + } + + uvc = container_of(video, struct uvc_device, video); + opts = fi_to_f_uvc_opts(uvc->func.fi); + if (cpu_latency_qos_request_active(&uvc->pm_qos)) + cpu_latency_qos_remove_request(&uvc->pm_qos); + + cancel_work_sync(&video->pump); + uvcg_queue_cancel(&video->queue, 0); + + list_for_each_entry(ureq, &video->ureqs, list) { + if (ureq->req) + usb_ep_dequeue(video->ep, ureq->req); + } + + uvc_video_free_requests(video); + uvcg_queue_enable(&video->queue, 0); + return 0; +} + +/* + * Enable the video stream. + */ +int uvcg_video_enable(struct uvc_video *video) +{ + struct uvc_device *uvc; + struct f_uvc_opts *opts; + int ret; + if (video->ep == NULL) { uvcg_info(&video->uvc->func, "Video enable failed, device is uninitialized.\n"); @@ -558,24 +591,8 @@ int uvcg_video_enable(struct uvc_video *video, int enable) uvc = container_of(video, struct uvc_device, video); opts = fi_to_f_uvc_opts(uvc->func.fi); - - if (!enable) { - cancel_work_sync(&video->pump); - uvcg_queue_cancel(&video->queue, 0); - - list_for_each_entry(ureq, &video->ureqs, list) { - if (ureq->req) - usb_ep_dequeue(video->ep, ureq->req); - } - - uvc_video_free_requests(video); - uvcg_queue_enable(&video->queue, 0); - if (cpu_latency_qos_request_active(&uvc->pm_qos)) - cpu_latency_qos_remove_request(&uvc->pm_qos); - return 0; - } - cpu_latency_qos_add_request(&uvc->pm_qos, opts->pm_qos_latency); + if ((ret = uvcg_queue_enable(&video->queue, 1)) < 0) return ret; diff --git a/drivers/usb/gadget/function/uvc_video.h b/drivers/usb/gadget/function/uvc_video.h index 03adeefa343b..8ef6259741f1 100644 --- a/drivers/usb/gadget/function/uvc_video.h +++ b/drivers/usb/gadget/function/uvc_video.h @@ -14,7 +14,8 @@ struct uvc_video; -int uvcg_video_enable(struct uvc_video *video, int enable); +int uvcg_video_enable(struct uvc_video *video); +int uvcg_video_disable(struct uvc_video *video); int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc); From 5f00f1b5a7e1c63ee92ce1eef3ace092e698e87a Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Wed, 8 Nov 2023 16:41:04 -0800 Subject: [PATCH 11/20] UPSTREAM: usb: gadget: uvc: Fix use-after-free for inflight usb_requests Currently, the uvc gadget driver allocates all uvc_requests as one array and deallocates them all when the video stream stops. This includes de-allocating all the usb_requests associated with those uvc_requests. This can lead to use-after-free issues if any of those de-allocated usb_requests were still owned by the usb controller. This is patch 2 of 2 in fixing the use-after-free issue. It adds a new flag to uvc_video to track when frames and requests should be flowing. When disabling the video stream, the flag is tripped and, instead of de-allocating all uvc_requests and usb_requests, the gadget driver only de-allocates those usb_requests that are currently owned by it (as present in req_free). Other usb_requests are left untouched until their completion handler is called which takes care of freeing the usb_request and its corresponding uvc_request. Now that uvc_video does not depends on uvc->state, this patch removes unnecessary upates to uvc->state that were made to accommodate uvc_video logic. This should ensure that uvc gadget driver never accidentally de-allocates a usb_request that it doesn't own. Change-Id: Ie1cc134c191e087bcca114832ad99f5d3119e682 Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@google.com Reviewed-by: Daniel Scally Reviewed-by: Michael Grzeschik Suggested-by: Michael Grzeschik Tested-by: Michael Grzeschik Signed-off-by: Avichal Rakesh Link: https://lore.kernel.org/r/20231109004104.3467968-4-arakesh@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit da324ffce34c521b239f319d4051260444a3eb4a) --- drivers/usb/gadget/function/uvc.h | 1 + drivers/usb/gadget/function/uvc_v4l2.c | 10 +- drivers/usb/gadget/function/uvc_video.c | 129 ++++++++++++++++++++---- 3 files changed, 111 insertions(+), 29 deletions(-) diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 4b6c5fe9bb6f..ffe12a44a115 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -103,6 +103,7 @@ struct uvc_video { unsigned int uvc_num_requests; /* Requests */ + bool is_enabled; /* tracks whether video stream is enabled */ unsigned int req_size; struct list_head ureqs; /* all uvc_requests allocated by uvc_video */ struct list_head req_free; diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index e27875c73e78..2d5616240803 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -487,11 +487,11 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) if (type != video->queue.queue.type) return -EINVAL; - uvc->state = UVC_STATE_CONNECTED; ret = uvcg_video_disable(video); if (ret < 0) return ret; + uvc->state = UVC_STATE_CONNECTED; uvc_function_setup_continue(uvc, 1); return 0; } @@ -526,14 +526,6 @@ uvc_v4l2_subscribe_event(struct v4l2_fh *fh, static void uvc_v4l2_disable(struct uvc_device *uvc) { uvc_function_disconnect(uvc); - /* - * Drop uvc->state to CONNECTED if it was streaming before. - * This ensures that the usb_requests are no longer queued - * to the controller. - */ - if (uvc->state == UVC_STATE_STREAMING) - uvc->state = UVC_STATE_CONNECTED; - uvcg_video_disable(&uvc->video); uvcg_free_buffers(&uvc->video.queue); uvc->func_connected = false; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index f4eb17452643..bc2b65fc8ebb 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -270,6 +270,10 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video, * Request handling */ +/* + * Callers must take care to hold req_lock when this function may be called + * from multiple threads. For example, when frames are streaming to the host. + */ static void uvc_video_free_request(struct uvc_request *ureq, struct usb_ep *ep) { @@ -314,9 +318,26 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) struct uvc_request *ureq = req->context; struct uvc_video *video = ureq->video; struct uvc_video_queue *queue = &video->queue; - struct uvc_device *uvc = video->uvc; + struct uvc_buffer *last_buf; unsigned long flags; + spin_lock_irqsave(&video->req_lock, flags); + if (!video->is_enabled) { + /* + * When is_enabled is false, uvcg_video_disable() ensures + * that in-flight uvc_buffers are returned, so we can + * safely call free_request without worrying about + * last_buf. + */ + uvc_video_free_request(ureq, ep); + spin_unlock_irqrestore(&video->req_lock, flags); + return; + } + + last_buf = ureq->last_buf; + ureq->last_buf = NULL; + spin_unlock_irqrestore(&video->req_lock, flags); + switch (req->status) { case 0: break; @@ -338,17 +359,26 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) uvcg_queue_cancel(queue, 0); } - if (ureq->last_buf) { - uvcg_complete_buffer(&video->queue, ureq->last_buf); - ureq->last_buf = NULL; + if (last_buf) { + spin_lock_irqsave(&queue->irqlock, flags); + uvcg_complete_buffer(queue, last_buf); + spin_unlock_irqrestore(&queue->irqlock, flags); } spin_lock_irqsave(&video->req_lock, flags); - list_add_tail(&req->list, &video->req_free); - spin_unlock_irqrestore(&video->req_lock, flags); - - if (uvc->state == UVC_STATE_STREAMING) + /* + * Video stream might have been disabled while we were + * processing the current usb_request. So make sure + * we're still streaming before queueing the usb_request + * back to req_free + */ + if (video->is_enabled) { + list_add_tail(&req->list, &video->req_free); queue_work(video->async_wq, &video->pump); + } else { + uvc_video_free_request(ureq, ep); + } + spin_unlock_irqrestore(&video->req_lock, flags); } static int @@ -440,20 +470,22 @@ static void uvcg_video_pump(struct work_struct *work) struct uvc_video_queue *queue = &video->queue; /* video->max_payload_size is only set when using bulk transfer */ bool is_bulk = video->max_payload_size; - struct uvc_device *uvc = video->uvc; struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; bool buf_done; int ret; - while (uvc->state == UVC_STATE_STREAMING && video->ep->enabled) { + while (true) { + if (!video->ep->enabled) + return; + /* - * Retrieve the first available USB request, protected by the - * request lock. + * Check is_enabled and retrieve the first available USB + * request, protected by the request lock. */ spin_lock_irqsave(&video->req_lock, flags); - if (list_empty(&video->req_free)) { + if (!video->is_enabled || list_empty(&video->req_free)) { spin_unlock_irqrestore(&video->req_lock, flags); return; } @@ -535,9 +567,11 @@ static void uvcg_video_pump(struct work_struct *work) return; spin_lock_irqsave(&video->req_lock, flags); - list_add_tail(&req->list, &video->req_free); + if (video->is_enabled) + list_add_tail(&req->list, &video->req_free); + else + uvc_video_free_request(req->context, video->ep); spin_unlock_irqrestore(&video->req_lock, flags); - return; } /* @@ -546,7 +580,11 @@ static void uvcg_video_pump(struct work_struct *work) int uvcg_video_disable(struct uvc_video *video) { - struct uvc_request *ureq; + unsigned long flags; + struct list_head inflight_bufs; + struct usb_request *req, *temp; + struct uvc_buffer *buf, *btemp; + struct uvc_request *ureq, *utemp; struct uvc_device *uvc; struct f_uvc_opts *opts; @@ -561,15 +599,58 @@ uvcg_video_disable(struct uvc_video *video) if (cpu_latency_qos_request_active(&uvc->pm_qos)) cpu_latency_qos_remove_request(&uvc->pm_qos); + INIT_LIST_HEAD(&inflight_bufs); + spin_lock_irqsave(&video->req_lock, flags); + video->is_enabled = false; + + /* + * Remove any in-flight buffers from the uvc_requests + * because we want to return them before cancelling the + * queue. This ensures that we aren't stuck waiting for + * all complete callbacks to come through before disabling + * vb2 queue. + */ + list_for_each_entry(ureq, &video->ureqs, list) { + if (ureq->last_buf) { + list_add_tail(&ureq->last_buf->queue, &inflight_bufs); + ureq->last_buf = NULL; + } + } + spin_unlock_irqrestore(&video->req_lock, flags); + cancel_work_sync(&video->pump); uvcg_queue_cancel(&video->queue, 0); - list_for_each_entry(ureq, &video->ureqs, list) { - if (ureq->req) - usb_ep_dequeue(video->ep, ureq->req); + spin_lock_irqsave(&video->req_lock, flags); + /* + * Remove all uvc_requests from ureqs with list_del_init + * This lets uvc_video_free_request correctly identify + * if the uvc_request is attached to a list or not when freeing + * memory. + */ + list_for_each_entry_safe(ureq, utemp, &video->ureqs, list) + list_del_init(&ureq->list); + + list_for_each_entry_safe(req, temp, &video->req_free, list) { + list_del(&req->list); + uvc_video_free_request(req->context, video->ep); } - uvc_video_free_requests(video); + INIT_LIST_HEAD(&video->ureqs); + INIT_LIST_HEAD(&video->req_free); + video->req_size = 0; + spin_unlock_irqrestore(&video->req_lock, flags); + + /* + * Return all the video buffers before disabling the queue. + */ + spin_lock_irqsave(&video->queue.irqlock, flags); + list_for_each_entry_safe(buf, btemp, &inflight_bufs, queue) { + list_del(&buf->queue); + uvcg_complete_buffer(&video->queue, buf); + } + spin_unlock_irqrestore(&video->queue.irqlock, flags); + uvcg_queue_enable(&video->queue, 0); return 0; } @@ -592,6 +673,13 @@ int uvcg_video_enable(struct uvc_video *video) uvc = container_of(video, struct uvc_device, video); opts = fi_to_f_uvc_opts(uvc->func.fi); cpu_latency_qos_add_request(&uvc->pm_qos, opts->pm_qos_latency); + /* + * Safe to access request related fields without req_lock because + * this is the only thread currently active, and no other + * request handling thread will become active until this function + * returns. + */ + video->is_enabled = true; if ((ret = uvcg_queue_enable(&video->queue, 1)) < 0) return ret; @@ -618,6 +706,7 @@ int uvcg_video_enable(struct uvc_video *video) */ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc) { + video->is_enabled = false; INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); spin_lock_init(&video->req_lock); From c9407f06fcea412760927e705fe0d43c71d35536 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Mon, 20 Nov 2023 06:20:25 +0000 Subject: [PATCH 12/20] UPSTREAM: usb:gadget:uvc Do not use worker thread to pump isoc usb requests When we use an async work queue to perform the function of pumping usb requests to the usb controller, it is possible that amongst other factors, thread scheduling affects at what cadence we're able to pump requests. This could mean isoc usb requests miss their uframes - resulting in video stream flickers on the host device. To avoid this, we make the async_wq thread only produce isoc usb_requests with uvc buffers encoded into them. The process of queueing to the endpoint is done by the uvc_video_complete() handler. In case no usb_requests are ready with encoded information, we just queue a zero length request to the endpoint from the complete handler. For bulk endpoints the async_wq thread still queues usb requests to the endpoint. Change-Id: Idc1000fbd32f46455f3084101c15f00b05c20f80 Signed-off-by: Michael Grzeschik Signed-off-by: Jayant Chowdhary Suggested-by: Avichal Rakesh Suggested-by: Alan Stern Link: https://lore.kernel.org/r/20231120062026.3759463-1-jchowdhary@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 6acba0345b68772830582ca1ca369a2f45631275) --- drivers/usb/gadget/function/uvc.h | 8 + drivers/usb/gadget/function/uvc_video.c | 202 ++++++++++++++++++------ 2 files changed, 165 insertions(+), 45 deletions(-) diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index ffe12a44a115..0f128c0d09c4 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -106,7 +106,15 @@ struct uvc_video { bool is_enabled; /* tracks whether video stream is enabled */ unsigned int req_size; struct list_head ureqs; /* all uvc_requests allocated by uvc_video */ + + /* USB requests that the video pump thread can encode into */ struct list_head req_free; + + /* + * USB requests video pump thread has already encoded into. These are + * ready to be queued to the endpoint. + */ + struct list_head req_ready; spinlock_t req_lock; unsigned int req_int_count; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index bc2b65fc8ebb..6771515c5488 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -312,6 +312,101 @@ static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req) return ret; } +/* This function must be called with video->req_lock held. */ +static int uvcg_video_usb_req_queue(struct uvc_video *video, + struct usb_request *req, bool queue_to_ep) +{ + bool is_bulk = video->max_payload_size; + struct list_head *list = NULL; + + if (!video->is_enabled) { + uvc_video_free_request(req->context, video->ep); + return -ENODEV; + } + if (queue_to_ep) { + struct uvc_request *ureq = req->context; + /* + * With USB3 handling more requests at a higher speed, we can't + * afford to generate an interrupt for every request. Decide to + * interrupt: + * + * - When no more requests are available in the free queue, as + * this may be our last chance to refill the endpoint's + * request queue. + * + * - When this is request is the last request for the video + * buffer, as we want to start sending the next video buffer + * ASAP in case it doesn't get started already in the next + * iteration of this loop. + * + * - Four times over the length of the requests queue (as + * indicated by video->uvc_num_requests), as a trade-off + * between latency and interrupt load. + */ + if (list_empty(&video->req_free) || ureq->last_buf || + !(video->req_int_count % + DIV_ROUND_UP(video->uvc_num_requests, 4))) { + video->req_int_count = 0; + req->no_interrupt = 0; + } else { + req->no_interrupt = 1; + } + video->req_int_count++; + return uvcg_video_ep_queue(video, req); + } + /* + * If we're not queuing to the ep, for isoc we're queuing + * to the req_ready list, otherwise req_free. + */ + list = is_bulk ? &video->req_free : &video->req_ready; + list_add_tail(&req->list, list); + return 0; +} + +/* + * Must only be called from uvcg_video_enable - since after that we only want to + * queue requests to the endpoint from the uvc_video_complete complete handler. + * This function is needed in order to 'kick start' the flow of requests from + * gadget driver to the usb controller. + */ +static void uvc_video_ep_queue_initial_requests(struct uvc_video *video) +{ + struct usb_request *req = NULL; + unsigned long flags = 0; + unsigned int count = 0; + int ret = 0; + + /* + * We only queue half of the free list since we still want to have + * some free usb_requests in the free list for the video_pump async_wq + * thread to encode uvc buffers into. Otherwise we could get into a + * situation where the free list does not have any usb requests to + * encode into - we always end up queueing 0 length requests to the + * end point. + */ + unsigned int half_list_size = video->uvc_num_requests / 2; + + spin_lock_irqsave(&video->req_lock, flags); + /* + * Take these requests off the free list and queue them all to the + * endpoint. Since we queue 0 length requests with the req_lock held, + * there isn't any 'data' race involved here with the complete handler. + */ + while (count < half_list_size) { + req = list_first_entry(&video->req_free, struct usb_request, + list); + list_del(&req->list); + req->length = 0; + ret = uvcg_video_ep_queue(video, req); + if (ret < 0) { + uvcg_queue_cancel(&video->queue, 0); + break; + } + count++; + } + spin_unlock_irqrestore(&video->req_lock, flags); +} + static void uvc_video_complete(struct usb_ep *ep, struct usb_request *req) { @@ -320,6 +415,8 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) struct uvc_video_queue *queue = &video->queue; struct uvc_buffer *last_buf; unsigned long flags; + bool is_bulk = video->max_payload_size; + int ret = 0; spin_lock_irqsave(&video->req_lock, flags); if (!video->is_enabled) { @@ -373,8 +470,45 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) * back to req_free */ if (video->is_enabled) { - list_add_tail(&req->list, &video->req_free); - queue_work(video->async_wq, &video->pump); + /* + * Here we check whether any request is available in the ready + * list. If it is, queue it to the ep and add the current + * usb_request to the req_free list - for video_pump to fill in. + * Otherwise, just use the current usb_request to queue a 0 + * length request to the ep. Since we always add to the req_free + * list if we dequeue from the ready list, there will never + * be a situation where the req_free list is completely out of + * requests and cannot recover. + */ + struct usb_request *to_queue = req; + + to_queue->length = 0; + if (!list_empty(&video->req_ready)) { + to_queue = list_first_entry(&video->req_ready, + struct usb_request, list); + list_del(&to_queue->list); + list_add_tail(&req->list, &video->req_free); + /* + * Queue work to the wq as well since it is possible that a + * buffer may not have been completely encoded with the set of + * in-flight usb requests for whih the complete callbacks are + * firing. + * In that case, if we do not queue work to the worker thread, + * the buffer will never be marked as complete - and therefore + * not be returned to userpsace. As a result, + * dequeue -> queue -> dequeue flow of uvc buffers will not + * happen. + */ + queue_work(video->async_wq, &video->pump); + } + /* + * Queue to the endpoint. The actual queueing to ep will + * only happen on one thread - the async_wq for bulk endpoints + * and this thread for isoc endpoints. + */ + ret = uvcg_video_usb_req_queue(video, to_queue, !is_bulk); + if (ret < 0) + uvcg_queue_cancel(queue, 0); } else { uvc_video_free_request(ureq, ep); } @@ -391,6 +525,7 @@ uvc_video_free_requests(struct uvc_video *video) INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); + INIT_LIST_HEAD(&video->req_ready); video->req_size = 0; return 0; } @@ -473,8 +608,7 @@ static void uvcg_video_pump(struct work_struct *work) struct usb_request *req = NULL; struct uvc_buffer *buf; unsigned long flags; - bool buf_done; - int ret; + int ret = 0; while (true) { if (!video->ep->enabled) @@ -503,15 +637,6 @@ static void uvcg_video_pump(struct work_struct *work) if (buf != NULL) { video->encode(req, video, buf); - buf_done = buf->state == UVC_BUF_STATE_DONE; - } else if (!(queue->flags & UVC_QUEUE_DISCONNECTED) && !is_bulk) { - /* - * No video buffer available; the queue is still connected and - * we're transferring over ISOC. Queue a 0 length request to - * prevent missed ISOC transfers. - */ - req->length = 0; - buf_done = false; } else { /* * Either the queue has been disconnected or no video buffer @@ -522,45 +647,25 @@ static void uvcg_video_pump(struct work_struct *work) break; } - /* - * With USB3 handling more requests at a higher speed, we can't - * afford to generate an interrupt for every request. Decide to - * interrupt: - * - * - When no more requests are available in the free queue, as - * this may be our last chance to refill the endpoint's - * request queue. - * - * - When this is request is the last request for the video - * buffer, as we want to start sending the next video buffer - * ASAP in case it doesn't get started already in the next - * iteration of this loop. - * - * - Four times over the length of the requests queue (as - * indicated by video->uvc_num_requests), as a trade-off - * between latency and interrupt load. - */ - if (list_empty(&video->req_free) || buf_done || - !(video->req_int_count % - DIV_ROUND_UP(video->uvc_num_requests, 4))) { - video->req_int_count = 0; - req->no_interrupt = 0; - } else { - req->no_interrupt = 1; - } - - /* Queue the USB request */ - ret = uvcg_video_ep_queue(video, req); spin_unlock_irqrestore(&queue->irqlock, flags); + spin_lock_irqsave(&video->req_lock, flags); + /* For bulk end points we queue from the worker thread + * since we would preferably not want to wait on requests + * to be ready, in the uvcg_video_complete() handler. + * For isoc endpoints we add the request to the ready list + * and only queue it to the endpoint from the complete handler. + */ + ret = uvcg_video_usb_req_queue(video, req, is_bulk); + spin_unlock_irqrestore(&video->req_lock, flags); + if (ret < 0) { uvcg_queue_cancel(queue, 0); break; } - /* Endpoint now owns the request */ + /* The request is owned by the endpoint / ready list. */ req = NULL; - video->req_int_count++; } if (!req) @@ -636,8 +741,14 @@ uvcg_video_disable(struct uvc_video *video) uvc_video_free_request(req->context, video->ep); } + list_for_each_entry_safe(req, temp, &video->req_ready, list) { + list_del(&req->list); + uvc_video_free_request(req->context, video->ep); + } + INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); + INIT_LIST_HEAD(&video->req_ready); video->req_size = 0; spin_unlock_irqrestore(&video->req_lock, flags); @@ -696,7 +807,7 @@ int uvcg_video_enable(struct uvc_video *video) video->req_int_count = 0; - queue_work(video->async_wq, &video->pump); + uvc_video_ep_queue_initial_requests(video); return ret; } @@ -709,6 +820,7 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc) video->is_enabled = false; INIT_LIST_HEAD(&video->ureqs); INIT_LIST_HEAD(&video->req_free); + INIT_LIST_HEAD(&video->req_ready); spin_lock_init(&video->req_lock); INIT_WORK(&video->pump, uvcg_video_pump); From 4348336da3f399940c5ee515c0a254fb31f7478b Mon Sep 17 00:00:00 2001 From: Frank Li Date: Sun, 24 Dec 2023 10:38:16 -0500 Subject: [PATCH 13/20] BACKPORT: Revert "usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()" This reverts commit 3c5b006f3ee800b4bd9ed37b3a8f271b8560126e. gadget_is_{super|dual}speed() API check UDC controller capitblity. It should pass down highest speed endpoint descriptor to UDC controller. So UDC controller driver can reserve enough resource at check_config(), especially mult and maxburst. So UDC driver (such as cdns3) can know need at least (mult + 1) * (maxburst + 1) * wMaxPacketSize internal memory for this uvc functions. Change-Id: Iae006b68943aac4ec9958e08a11f19dec270f954 Cc: Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20231224153816.1664687-5-Frank.Li@nxp.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 895ee5aefb7e24203de5dffae7ce9a02d78fa3d1) --- drivers/usb/gadget/function/f_uvc.c | 55 ++++++++++++++--------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 91f10eb54191..7cb0df08a010 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -807,7 +807,6 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) struct usb_ep *ep; struct f_uvc_opts *opts; int ret = -EINVAL; - u8 address; uvcg_info(f, "%s()\n", __func__); @@ -904,20 +903,20 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) } uvc->enable_interrupt_ep = opts->enable_interrupt_ep; - if (gadget_is_superspeed(c->cdev->gadget)) { - if (!opts->streaming_bulk) - ep = usb_ep_autoconfig_ss(cdev->gadget, - &uvc_ss_streaming_ep, - &uvc_ss_streaming_comp); - else + /* + * gadget_is_{super|dual}speed() API check UDC controller capitblity. It should pass down + * highest speed endpoint descriptor to UDC controller. So UDC controller driver can reserve + * enough resource at check_config(), especially mult and maxburst. So UDC driver (such as + * cdns3) can know need at least (mult + 1) * (maxburst + 1) * wMaxPacketSize internal + * memory for this uvc functions. This is the only straightforward method to resolve the UDC + * resource allocation issue in the current gadget framework. + */ + if (opts->streaming_bulk) { + if (gadget_is_superspeed(c->cdev->gadget)) { ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_bulk_streaming_ep, &uvc_ss_bulk_streaming_comp); - } else if (gadget_is_dualspeed(cdev->gadget)) { - if (!opts->streaming_bulk) { - ep = usb_ep_autoconfig(cdev->gadget, - &uvc_hs_streaming_ep); - } else { + } else if (gadget_is_dualspeed(cdev->gadget)) { ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_bulk_streaming_ep); /* @@ -929,14 +928,17 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) uvc_hs_bulk_streaming_ep.wMaxPacketSize = cpu_to_le16(min(opts->streaming_maxpacket, 512U)); - } - } else { - if (!opts->streaming_bulk) - ep = usb_ep_autoconfig(cdev->gadget, - &uvc_fs_streaming_ep); - else + } else { ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_bulk_streaming_ep); + } + } else if (gadget_is_superspeed(c->cdev->gadget)) { + ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep, + &uvc_ss_streaming_comp); + } else if (gadget_is_dualspeed(cdev->gadget)) { + ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep); + } else { + ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep); } if (!ep) { @@ -944,17 +946,14 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) goto error; } uvc->video.ep = ep; - address = uvc->video.ep->address; - if (!opts->streaming_bulk) { - uvc_fs_streaming_ep.bEndpointAddress = address; - uvc_hs_streaming_ep.bEndpointAddress = address; - uvc_ss_streaming_ep.bEndpointAddress = address; - } else { - uvc_fs_bulk_streaming_ep.bEndpointAddress = address; - uvc_hs_bulk_streaming_ep.bEndpointAddress = address; - uvc_ss_bulk_streaming_ep.bEndpointAddress = address; - } + uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address; + uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address; + uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address; + + uvc_fs_bulk_streaming_ep.bEndpointAddress = uvc->video.ep->address; + uvc_hs_bulk_streaming_ep.bEndpointAddress = uvc->video.ep->address; + uvc_ss_bulk_streaming_ep.bEndpointAddress = uvc->video.ep->address; #if defined(CONFIG_ARCH_ROCKCHIP) && defined(CONFIG_NO_GKI) if (opts->device_name) From 4548f07edf5bf391d895273fb506f4b775868ad5 Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Thu, 4 Jan 2024 13:50:08 -0800 Subject: [PATCH 14/20] UPSTREAM: usb: gadget: uvc: Fix use are free during STREAMOFF There is a path that may lead to freed memory being referenced, causing kernel panics. The kernel panic has the following stack trace: Workqueue: uvcgadget uvcg_video_pump.c51fb85fece46625450f86adbf92c56c.cfi_jt pstate: 60c00085 (nZCv daIf +PAN +UAO -TCO BTYPE=--) pc : __list_del_entry_valid+0xc0/0xd4 lr : __list_del_entry_valid+0xc0/0xd4 Call trace: __list_del_entry_valid+0xc0/0xd4 uvc_video_free_request+0x60/0x98 uvcg_video_pump+0x1cc/0x204 process_one_work+0x21c/0x4b8 worker_thread+0x29c/0x574 kthread+0x158/0x1b0 ret_from_fork+0x10/0x30 The root cause is that uvcg_video_usb_req_queue frees the uvc_request if is_enabled is false and returns an error status. video_pump also frees the associated request if uvcg_video_usb_req_queue returns an error status, leading to double free and accessing garbage memory. To fix the issue, this patch removes freeing logic from uvcg_video_usb_req_queue, and lets the callers to the function handle queueing errors as they see fit. Change-Id: I8d8e644de5b5f26601e7764fd675f775535f63d3 Fixes: 6acba0345b68 ("usb:gadget:uvc Do not use worker thread to pump isoc usb requests") Tested-by: Avichal Rakesh Signed-off-by: Avichal Rakesh Link: https://lore.kernel.org/r/20240104215009.2252452-1-arakesh@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit fe814b5b0f3042f1a583734497e726ee53783cc1) --- drivers/usb/gadget/function/uvc_video.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 6771515c5488..be0282ae1eaf 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -319,10 +319,9 @@ static int uvcg_video_usb_req_queue(struct uvc_video *video, bool is_bulk = video->max_payload_size; struct list_head *list = NULL; - if (!video->is_enabled) { - uvc_video_free_request(req->context, video->ep); + if (!video->is_enabled) return -ENODEV; - } + if (queue_to_ep) { struct uvc_request *ureq = req->context; /* @@ -507,8 +506,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) * and this thread for isoc endpoints. */ ret = uvcg_video_usb_req_queue(video, to_queue, !is_bulk); - if (ret < 0) + if (ret < 0) { + /* + * Endpoint error, but the stream is still enabled. + * Put request back in req_free for it to be cleaned + * up later. + */ uvcg_queue_cancel(queue, 0); + list_add_tail(&to_queue->list, &video->req_free); + } } else { uvc_video_free_request(ureq, ep); } From c1a2d0928713c0eab80a56824504a4203ab243eb Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Thu, 4 Jan 2024 13:50:09 -0800 Subject: [PATCH 15/20] UPSTREAM: usb: gadget: uvc: Remove nested locking When handling error status from uvcg_video_usb_req_queue, uvc_video_complete currently calls uvcg_queue_cancel with video->req_lock held. uvcg_queue_cancel internally locks queue->irqlock, which nests queue->irqlock inside video->req_lock. This isn't a functional bug at the moment, but does open up possibilities for ABBA deadlocks in the future. This patch fixes the accidental nesting by dropping video->req_lock before calling uvcg_queue_cancel. Change-Id: Ic96546cff545b397fdf52b4f361bca42702cb940 Fixes: 6acba0345b68 ("usb:gadget:uvc Do not use worker thread to pump isoc usb requests") Signed-off-by: Avichal Rakesh Link: https://lore.kernel.org/r/20240104215009.2252452-2-arakesh@google.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit 9866dc4314c6c858e451933f965d64532aec00a9) --- drivers/usb/gadget/function/uvc_video.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index be0282ae1eaf..93fcb0ec7d6d 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -512,13 +512,15 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) * Put request back in req_free for it to be cleaned * up later. */ - uvcg_queue_cancel(queue, 0); list_add_tail(&to_queue->list, &video->req_free); } } else { uvc_video_free_request(ureq, ep); + ret = 0; } spin_unlock_irqrestore(&video->req_lock, flags); + if (ret < 0) + uvcg_queue_cancel(queue, 0); } static int From 2c38326f65dbca6c09e9a715932ccb28abfca4cb Mon Sep 17 00:00:00 2001 From: Xu Yang Date: Wed, 14 Aug 2024 19:25:37 +0800 Subject: [PATCH 16/20] UPSTREAM: usb: gadget: uvc: queue pump work in uvcg_video_enable() Since commit "6acba0345b68 usb:gadget:uvc Do not use worker thread to pump isoc usb requests", pump work could only be queued in uvc_video_complete() and uvc_v4l2_qbuf(). If VIDIOC_QBUF is executed before VIDIOC_STREAMON, we can only depend on uvc_video_complete() to queue pump work. However, this requires some free requests in req_ready list. If req_ready list is empty all the time, pump work will never be queued and video datas will never be pumped to usb controller. Actually, this situation could happen when run uvc-gadget with static image: $ ./uvc-gadget -i 1080p.jpg uvc.0 When capture image from this device, the user app will always block there. The issue is uvc driver has queued video buffer before streamon, but the req_ready list is empty all the time after streamon. This will queue pump work in uvcg_video_enable() to fill some request to req_ready list so the uvc device could work properly. Change-Id: Iedfba9335ea5f1a61dc1157f8d20c647803c84cd Fixes: 6acba0345b68 ("usb:gadget:uvc Do not use worker thread to pump isoc usb requests") Cc: stable@vger.kernel.org Signed-off-by: Xu Yang Link: https://lore.kernel.org/r/20240814112537.2608949-1-xu.yang_2@nxp.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit b52a07e07dead777517af3cbda851bb2cc157c9d) --- drivers/usb/gadget/function/uvc_video.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 93fcb0ec7d6d..f4da17a94007 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -816,6 +816,7 @@ int uvcg_video_enable(struct uvc_video *video) video->req_int_count = 0; uvc_video_ep_queue_initial_requests(video); + queue_work(video->async_wq, &video->pump); return ret; } From 8c339b276f9c6e349ff78ba7b299582143472a64 Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Fri, 27 Sep 2024 20:51:38 +0530 Subject: [PATCH 17/20] BACKPORT: usb: gadget: uvc: configfs: Add frame-based frame format support Add support for frame-based frame format, which can be used to support multiple formats like H264 or H265, in addition to MJPEG and YUV frames. The frame-based format is set to H264 by default, but it can be updated to other formats by modifying the GUID through the guid configfs attribute. Different structures are used for all three formats, as H264 has a different structure compared to MJPEG and uncompressed formats. These structures will be passed to the frame make function based on the active format, using a common frame structure with additional parameters needed only for frame-based formats. These parameters are handled at runtime in the UVC driver. Signed-off-by: Akash Kumar Link: https://lore.kernel.org/r/20240927152138.31416-1-quic_akakum@quicinc.com Signed-off-by: Greg Kroah-Hartman Change-Id: I8be496e30e2f3f0e5756c0789c73f5c1746c9303 Signed-off-by: William Wu (cherry picked from commit 7b5a58952fc3b51905c2963647485565df1e5e26) --- .../ABI/testing/configfs-usb-gadget-uvc | 64 ++++++++++ drivers/usb/gadget/function/uvc_configfs.c | 111 +++++++++--------- drivers/usb/gadget/function/uvc_configfs.h | 17 ++- drivers/usb/gadget/function/uvc_v4l2.c | 11 +- 4 files changed, 142 insertions(+), 61 deletions(-) diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 7fc36ab7f7c7..2ea24893b956 100644 --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -297,6 +297,70 @@ Description: Specific uncompressed frame descriptors support ========================= ===================================== +What: /config/usb-gadget/gadget/functions/uvc.name/streaming/framebased +Date: Sept 2024 +KernelVersion: 5.15 +Description: Framebased format descriptors + +What: /config/usb-gadget/gadget/functions/uvc.name/streaming/framebased/name +Date: Sept 2024 +KernelVersion: 5.15 +Description: Specific framebased format descriptors + + ================== ======================================= + bFormatIndex unique id for this format descriptor; + only defined after parent header is + linked into the streaming class; + read-only + bmaControls this format's data for bmaControls in + the streaming header + bmInterlaceFlags specifies interlace information, + read-only + bAspectRatioY the X dimension of the picture aspect + ratio, read-only + bAspectRatioX the Y dimension of the picture aspect + ratio, read-only + bDefaultFrameIndex optimum frame index for this stream + bBitsPerPixel number of bits per pixel used to + specify color in the decoded video + frame + guidFormat globally unique id used to identify + stream-encoding format + ================== ======================================= + +What: /config/usb-gadget/gadget/functions/uvc.name/streaming/framebased/name/name +Date: Sept 2024 +KernelVersion: 5.15 +Description: Specific framebased frame descriptors + + ========================= ===================================== + bFrameIndex unique id for this framedescriptor; + only defined after parent format is + linked into the streaming header; + read-only + dwFrameInterval indicates how frame interval can be + programmed; a number of values + separated by newline can be specified + dwDefaultFrameInterval the frame interval the device would + like to use as default + dwBytesPerLine Specifies the number of bytes per line + of video for packed fixed frame size + formats, allowing the receiver to + perform stride alignment of the video. + If the bVariableSize value (above) is + TRUE (1), or if the format does not + permit such alignment, this value shall + be set to zero (0). + dwMaxBitRate the maximum bit rate at the shortest + frame interval in bps + dwMinBitRate the minimum bit rate at the longest + frame interval in bps + wHeight height of decoded bitmap frame in px + wWidth width of decoded bitmam frame in px + bmCapabilities still image support, fixed frame-rate + support + ========================= ===================================== + What: /config/usb-gadget/gadget/functions/uvc.name/streaming/header Date: Dec 2014 KernelVersion: 4.0 diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index 51d7d942c82b..e824b6de35ba 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -852,6 +852,7 @@ static const struct uvcg_config_group_type uvcg_control_grp_type = { /* ----------------------------------------------------------------------------- * streaming/uncompressed * streaming/mjpeg + * streaming/framebased */ static const char * const uvcg_format_names[] = { @@ -970,6 +971,9 @@ static int uvcg_streaming_header_allow_link(struct config_item *src, target_fmt = container_of(to_config_group(target), struct uvcg_format, group); + if (!target_fmt) + goto out; + uvcg_format_set_indices(to_config_group(target)); format_ptr = kzalloc(sizeof(*format_ptr), GFP_KERNEL); @@ -1009,6 +1013,9 @@ static void uvcg_streaming_header_drop_link(struct config_item *src, target_fmt = container_of(to_config_group(target), struct uvcg_format, group); + if (!target_fmt) + goto out; + list_for_each_entry_safe(format_ptr, tmp, &src_hdr->formats, entry) if (format_ptr->fmt == target_fmt) { list_del(&format_ptr->entry); @@ -1019,6 +1026,7 @@ static void uvcg_streaming_header_drop_link(struct config_item *src, --target_fmt->linked; +out: mutex_unlock(&opts->lock); mutex_unlock(su_mutex); } @@ -1229,7 +1237,7 @@ static ssize_t uvcg_frame_dw_frame_interval_show(struct config_item *item, int result, i; char *pg = page; - mutex_lock(su_mutex); /* for navigating configfs hierarchy */ + mutex_lock(su_mutex); /* for navigating configfs hierarchy */ opts_item = frm->item.ci_parent->ci_parent->ci_parent->ci_parent; opts = to_f_uvc_opts(opts_item); @@ -1409,7 +1417,7 @@ static struct config_item *uvcg_frame_make(struct config_group *group, h->frame.dw_max_bit_rate = 55296000; h->frame.dw_max_video_frame_buffer_size = 460800; h->frame.dw_default_frame_interval = 666666; - h->frame.dw_bytes_perline = 0; + h->frame.dw_bytes_perline = 0; opts_item = group->cg_item.ci_parent->ci_parent->ci_parent; opts = to_f_uvc_opts(opts_item); @@ -1486,10 +1494,6 @@ static void uvcg_format_set_indices(struct config_group *fmt) list_for_each_entry(ci, &fmt->cg_children, ci_entry) { struct uvcg_frame *frm; - if (ci->ci_type != &uvcg_frame_type1 && - ci->ci_type != &uvcg_frame_type2) - continue; - frm = to_uvcg_frame(ci); frm->frame.b_frame_index = i++; } @@ -1924,26 +1928,14 @@ static const struct uvcg_config_group_type uvcg_mjpeg_grp_type = { * streaming/framebased/ */ -struct uvcg_framebased { - struct uvcg_format fmt; - struct uvc_format_framebased desc; -}; - -static struct uvcg_framebased *to_uvcg_framebased(struct config_item *item) -{ - return container_of( - container_of(to_config_group(item), struct uvcg_format, group), - struct uvcg_framebased, fmt); -} - static struct configfs_group_operations uvcg_framebased_group_ops = { - .make_item = uvcg_frame_make, - .drop_item = uvcg_frame_drop, + .make_item = uvcg_frame_make, + .drop_item = uvcg_frame_drop, }; #define UVCG_FRAMEBASED_ATTR_RO(cname, aname, bits) \ -static ssize_t uvcg_framebased_##cname##_show(struct config_item *item,\ - char *page) \ +static ssize_t uvcg_framebased_##cname##_show(struct config_item *item, \ + char *page) \ { \ struct uvcg_framebased *u = to_uvcg_framebased(item); \ struct f_uvc_opts *opts; \ @@ -1967,8 +1959,8 @@ static ssize_t uvcg_framebased_##cname##_show(struct config_item *item,\ UVC_ATTR_RO(uvcg_framebased_, cname, aname) #define UVCG_FRAMEBASED_ATTR(cname, aname, bits) \ -static ssize_t uvcg_framebased_##cname##_show(struct config_item *item,\ - char *page)\ +static ssize_t uvcg_framebased_##cname##_show(struct config_item *item, \ + char *page) \ { \ struct uvcg_framebased *u = to_uvcg_framebased(item); \ struct f_uvc_opts *opts; \ @@ -1991,7 +1983,7 @@ static ssize_t uvcg_framebased_##cname##_show(struct config_item *item,\ \ static ssize_t \ uvcg_framebased_##cname##_store(struct config_item *item, \ - const char *page, size_t len) \ + const char *page, size_t len) \ { \ struct uvcg_framebased *u = to_uvcg_framebased(item); \ struct f_uvc_opts *opts; \ @@ -2040,7 +2032,7 @@ UVCG_FRAMEBASED_ATTR_RO(bm_interface_flags, bmInterfaceFlags, 8); #undef UVCG_FRAMEBASED_ATTR_RO static ssize_t uvcg_framebased_guid_format_show(struct config_item *item, - char *page) + char *page) { struct uvcg_framebased *ch = to_uvcg_framebased(item); struct f_uvc_opts *opts; @@ -2062,7 +2054,7 @@ static ssize_t uvcg_framebased_guid_format_show(struct config_item *item, } static ssize_t uvcg_framebased_guid_format_store(struct config_item *item, - const char *page, size_t len) + const char *page, size_t len) { struct uvcg_framebased *ch = to_uvcg_framebased(item); struct f_uvc_opts *opts; @@ -2103,7 +2095,7 @@ uvcg_framebased_bma_controls_show(struct config_item *item, char *page) static inline ssize_t uvcg_framebased_bma_controls_store(struct config_item *item, - const char *page, size_t len) + const char *page, size_t len) { struct uvcg_framebased *u = to_uvcg_framebased(item); @@ -2125,53 +2117,54 @@ static struct configfs_attribute *uvcg_framebased_attrs[] = { }; static const struct config_item_type uvcg_framebased_type = { - .ct_item_ops = &uvcg_config_item_ops, - .ct_group_ops = &uvcg_framebased_group_ops, - .ct_attrs = uvcg_framebased_attrs, - .ct_owner = THIS_MODULE, + .ct_item_ops = &uvcg_config_item_ops, + .ct_group_ops = &uvcg_framebased_group_ops, + .ct_attrs = uvcg_framebased_attrs, + .ct_owner = THIS_MODULE, }; static struct config_group *uvcg_framebased_make(struct config_group *group, - const char *name) + const char *name) { - static char guid[] = { /*Declear frame frame based as H264*/ + static char guid[] = { /*Declear frame based as H264 format*/ 'H', '2', '6', '4', 0x00, 0x00, 0x10, 0x00, 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }; - struct uvcg_framebased *f; + struct uvcg_framebased *h; - f = kzalloc(sizeof(*f), GFP_KERNEL); - if (!f) + h = kzalloc(sizeof(*h), GFP_KERNEL); + if (!h) return ERR_PTR(-ENOMEM); - f->desc.bLength = UVC_DT_FORMAT_FRAMEBASED_SIZE; - f->desc.bDescriptorType = USB_DT_CS_INTERFACE; - f->desc.bDescriptorSubType = UVC_VS_FORMAT_FRAME_BASED; - memcpy(f->desc.guidFormat, guid, sizeof(guid)); - f->desc.bBitsPerPixel = 16; - f->desc.bDefaultFrameIndex = 1; - f->desc.bAspectRatioX = 0; - f->desc.bAspectRatioY = 0; - f->desc.bmInterfaceFlags = 0; - f->desc.bCopyProtect = 0; - f->desc.bVariableSize = 1; + h->desc.bLength = UVC_DT_FORMAT_FRAMEBASED_SIZE; + h->desc.bDescriptorType = USB_DT_CS_INTERFACE; + h->desc.bDescriptorSubType = UVC_VS_FORMAT_FRAME_BASED; + memcpy(h->desc.guidFormat, guid, sizeof(guid)); + h->desc.bBitsPerPixel = 16; + h->desc.bDefaultFrameIndex = 1; + h->desc.bAspectRatioX = 0; + h->desc.bAspectRatioY = 0; + h->desc.bmInterfaceFlags = 0; + h->desc.bCopyProtect = 0; + h->desc.bVariableSize = 1; - INIT_LIST_HEAD(&f->fmt.frames); - f->fmt.type = UVCG_FRAMEBASED; - config_group_init_type_name(&f->fmt.group, name, + INIT_LIST_HEAD(&h->fmt.frames); + h->fmt.type = UVCG_FRAMEBASED; + config_group_init_type_name(&h->fmt.group, name, &uvcg_framebased_type); - return &f->fmt.group; + return &h->fmt.group; } static struct configfs_group_operations uvcg_framebased_grp_ops = { - .make_group = uvcg_framebased_make, + .make_group = uvcg_framebased_make, }; + static const struct uvcg_config_group_type uvcg_framebased_grp_type = { .type = { - .ct_item_ops = &uvcg_config_item_ops, - .ct_group_ops = &uvcg_framebased_grp_ops, - .ct_owner = THIS_MODULE, + .ct_item_ops = &uvcg_config_item_ops, + .ct_group_ops = &uvcg_framebased_grp_ops, + .ct_owner = THIS_MODULE, }, .name = "framebased", }; @@ -2321,6 +2314,7 @@ static int __uvcg_iter_strm_cls(struct uvcg_streaming_header *h, if (ret) return ret; grp = &f->fmt->group; + j = 0; list_for_each_entry(item, &grp->cg_children, ci_entry) { frm = to_uvcg_frame(item); ret = fun(frm, priv2, priv3, j++, UVCG_FRAME); @@ -2372,6 +2366,7 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n, } else if (fmt->type == UVCG_FRAMEBASED) { struct uvcg_framebased *f = container_of(fmt, struct uvcg_framebased, fmt); + *size += sizeof(f->desc); } else { return -EINVAL; @@ -2506,10 +2501,10 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n, frm->frame.b_frame_interval_type); else if (frm->fmt_type == UVCG_MJPEG) h->bLength = UVC_DT_FRAME_MJPEG_SIZE( - frm->frame.b_frame_interval_type); + frm->frame.b_frame_interval_type); else if (frm->fmt_type == UVCG_FRAMEBASED) h->bLength = UVC_DT_FRAME_FRAMEBASED_SIZE( - frm->frame.b_frame_interval_type); + frm->frame.b_frame_interval_type); } break; } diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h index 5e8ed87ccc71..7f7fd2e3eee0 100644 --- a/drivers/usb/gadget/function/uvc_configfs.h +++ b/drivers/usb/gadget/function/uvc_configfs.h @@ -100,8 +100,7 @@ struct uvcg_frame { u32 dw_max_video_frame_buffer_size; u32 dw_default_frame_interval; u8 b_frame_interval_type; - /* dw_bytes_perline is only for framebased format */ - u32 dw_bytes_perline; + u32 dw_bytes_perline; } __attribute__((packed)) frame; u32 *dw_frame_interval; }; @@ -139,6 +138,20 @@ static inline struct uvcg_mjpeg *to_uvcg_mjpeg(struct config_item *item) return container_of(to_uvcg_format(item), struct uvcg_mjpeg, fmt); } +/* ----------------------------------------------------------------------------- + * streaming/framebased/ + */ + +struct uvcg_framebased { + struct uvcg_format fmt; + struct uvc_format_framebased desc; +}; + +static inline struct uvcg_framebased *to_uvcg_framebased(struct config_item *item) +{ + return container_of(to_uvcg_format(item), struct uvcg_framebased, fmt); +} + int uvcg_attach_configfs(struct f_uvc_opts *opts); #endif /* UVC_CONFIGFS_H */ diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 2d5616240803..549f93593acd 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -31,13 +31,22 @@ static struct uvc_format_desc *to_uvc_format(struct uvcg_format *uformat) { char guid[16] = UVC_GUID_FORMAT_MJPEG; struct uvc_format_desc *format; - struct uvcg_uncompressed *unc; if (uformat->type == UVCG_UNCOMPRESSED) { + struct uvcg_uncompressed *unc; + unc = to_uvcg_uncompressed(&uformat->group.cg_item); if (!unc) return ERR_PTR(-EINVAL); + memcpy(guid, unc->desc.guidFormat, sizeof(guid)); + } else if (uformat->type == UVCG_FRAMEBASED) { + struct uvcg_framebased *unc; + + unc = to_uvcg_framebased(&uformat->group.cg_item); + if (!unc) + return ERR_PTR(-EINVAL); + memcpy(guid, unc->desc.guidFormat, sizeof(guid)); } From de97b80566dd8b65f4fe0a20dedfd9f7f6c129e3 Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 16 Oct 2024 15:58:06 +0200 Subject: [PATCH 18/20] BACKPORT: usb: gadget: uvc: wake pump everytime we update the free list Since the req_free list will updated if enqueuing one request was not possible it will be added back to the free list. With every available free request in the queue it is a valid case for the pump worker to use it and continue the pending bufferdata into requests for the req_ready list. Change-Id: Ib52f29e23a938b469782f4b25221e127c01cc8a3 Fixes: 6acba0345b68 ("usb:gadget:uvc Do not use worker thread to pump isoc usb requests") Signed-off-by: Michael Grzeschik Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-1-e224bb1035f0@pengutronix.de Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit adc292d54de9db2e6b8ecb7f81f278bbbaf713e9) --- drivers/usb/gadget/function/uvc_video.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index f4da17a94007..6ddd491c1013 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -513,6 +513,10 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) * up later. */ list_add_tail(&to_queue->list, &video->req_free); + /* + * There is a new free request - wake up the pump. + */ + queue_work(video->async_wq, &video->pump); } } else { uvc_video_free_request(ureq, ep); From b0914fdaabcab33a524dd1368c2a0c99676dea8f Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 16 Oct 2024 15:58:14 +0200 Subject: [PATCH 19/20] UPSTREAM: usb: gadget: uvc: dont call usb_composite_setup_continue when not streaming If the streamoff call was triggered by some previous disconnect or userspace application shutdown the uvc_function_setup_continue should not be called and the state should not be overwritten. For this situation the set_alt(0) was never called and the streaming ep has no USB_GADGET_DELAYED_STATUS pending. Since the state then was already updated before we also omit the state update. Change-Id: I65382e0ed663138ea694c27276fe813c30863f89 Signed-off-by: Michael Grzeschik Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-9-e224bb1035f0@pengutronix.de Signed-off-by: Greg Kroah-Hartman Signed-off-by: William Wu (cherry picked from commit e723ebc3a9aa172ab8042382afcae310c953104d) --- drivers/usb/gadget/function/uvc_v4l2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 549f93593acd..a3ec066419eb 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -500,6 +500,9 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type) if (ret < 0) return ret; + if (uvc->state != UVC_STATE_STREAMING) + return 0; + uvc->state = UVC_STATE_CONNECTED; uvc_function_setup_continue(uvc, 1); return 0; From 92f11a005627a76ba464841a9981b7e0df7c2d8d Mon Sep 17 00:00:00 2001 From: William Wu Date: Mon, 24 Mar 2025 19:58:01 +0800 Subject: [PATCH 20/20] usb: gadget: uvc: Fix frame size for framebase format The commit e219a712bc06 ("usb: gadget: uvc: add v4l2 try_format api call") has introduced a bug that limit the max frame size of framebase format (H264/H265) to dw_max_video_frame_buffer_size which is a fixed value 460800 bytes. This patch calculates the bytesperline value based on the actual bBitsPerPixel and frame.w_width of framebase format. Fixes: e219a712bc06 ("usb: gadget: uvc: add v4l2 try_format api call") Signed-off-by: William Wu Change-Id: I40c5b72a9c3707bdcc13244d5ae912339626071b --- drivers/usb/gadget/function/uvc_v4l2.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index a3ec066419eb..7f257d18c2d2 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -78,6 +78,16 @@ static int uvc_get_frame_size(struct uvcg_format *uformat, { unsigned int bpl = uvc_v4l2_get_bytesperline(uformat, uframe); + if (uformat->type == UVCG_FRAMEBASED && !bpl) { + struct uvcg_framebased *u; + + u = to_uvcg_framebased(&uformat->group.cg_item); + if (u) { + bpl = u->desc.bBitsPerPixel * uframe->frame.w_width / 8; + pr_info("%s: set bpl to %d for framebased format\n", __func__, bpl); + } + } + return bpl ? bpl * uframe->frame.w_height : uframe->frame.dw_max_video_frame_buffer_size; }