[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aV/TUFLMcT2lng5m@lizhi-Precision-Tower-5810>
Date: Thu, 8 Jan 2026 10:54:56 -0500
From: Frank Li <Frank.li@....com>
To: Xu Yang <xu.yang_2@....com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Michael Grzeschik <m.grzeschik@...gutronix.de>, jun.li@....com,
imx@...ts.linux.dev, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/4] usb: gadget: uvc: fix req_payload_size calculation
On Thu, Jan 08, 2026 at 03:43:02PM +0800, Xu Yang wrote:
> Current req_payload_size calculation has 2 issue:
>
> (1) When the first time calculate req_payload_size for all the buffers,
> reqs_per_frame = 0 will be the divisor of DIV_ROUND_UP(). So
> the result is undefined.
> This happens because VIDIOC_STREAMON is always executed after
> VIDIOC_QBUF. So video->reqs_per_frame will be 0 until VIDIOC_STREAMON
> is run.
>
> (2) The buf->req_payload_size may be bigger than max_req_size.
>
> Take YUYV pixel format as example:
> If bInterval = 1, video->interval = 666666, high-speed:
> video->reqs_per_frame = 666666 / 1250 = 534
> 720p: buf->req_payload_size = 1843200 / 534 = 3452
> 1080p: buf->req_payload_size = 4147200 / 534 = 7766
>
> Based on such req_payload_size, the controller can't run normally.
>
> To fix above issue, assign max_req_size to buf->req_payload_size when
> video->reqs_per_frame = 0. And limit buf->req_payload_size to
> video->req_size if it's large than video->req_size. Since max_req_size
> is used at many place, add it to struct uvc_video and set the value once
> endpoint is enabled.
>
> Fixes: 98ad03291560 ("usb: gadget: uvc: set req_length based on payload by nreqs instead of req_size")
> Cc: stable@...r.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@....com>
Reviewed-by: Frank Li <Frank.Li@....com>
> ---
> drivers/usb/gadget/function/f_uvc.c | 4 ++++
> drivers/usb/gadget/function/uvc.h | 1 +
> drivers/usb/gadget/function/uvc_queue.c | 15 +++++++++++----
> drivers/usb/gadget/function/uvc_video.c | 4 +---
> 4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index aa6ab666741a9518690995ccdc04e742b4359a0e..a96476507d2fdf4eb0817f3aac09b7ee08df593a 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -362,6 +362,10 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
> return ret;
> usb_ep_enable(uvc->video.ep);
>
> + uvc->video.max_req_size = uvc->video.ep->maxpacket
> + * max_t(unsigned int, uvc->video.ep->maxburst, 1)
> + * (uvc->video.ep->mult);
> +
> memset(&v4l2_event, 0, sizeof(v4l2_event));
> v4l2_event.type = UVC_EVENT_STREAMON;
> v4l2_event_queue(&uvc->vdev, &v4l2_event);
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> index 9e79cbe50715791a7f7ddd3bc20e9a28d221db61..b3f88670bff801a43d084646974602e5995bb192 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -117,6 +117,7 @@ struct uvc_video {
> /* Requests */
> bool is_enabled; /* tracks whether video stream is enabled */
> unsigned int req_size;
> + unsigned int max_req_size;
> struct list_head ureqs; /* all uvc_requests allocated by uvc_video */
>
> /* USB requests that the video pump thread can encode into */
> diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
> index 9a1bbd79ff5af945bdd5dcf0c1cb1b6dbdc12a9c..21d80322cb6148ed87eb77f453a1f1644e4923ae 100644
> --- a/drivers/usb/gadget/function/uvc_queue.c
> +++ b/drivers/usb/gadget/function/uvc_queue.c
> @@ -86,10 +86,17 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
> buf->bytesused = 0;
> } else {
> buf->bytesused = vb2_get_plane_payload(vb, 0);
> - buf->req_payload_size =
> - DIV_ROUND_UP(buf->bytesused +
> - (video->reqs_per_frame * UVCG_REQUEST_HEADER_LEN),
> - video->reqs_per_frame);
> +
> + if (video->reqs_per_frame != 0) {
> + buf->req_payload_size =
> + DIV_ROUND_UP(buf->bytesused +
> + (video->reqs_per_frame * UVCG_REQUEST_HEADER_LEN),
> + video->reqs_per_frame);
> + if (buf->req_payload_size > video->req_size)
> + buf->req_payload_size = video->req_size;
> + } else {
> + buf->req_payload_size = video->max_req_size;
> + }
> }
>
> return 0;
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index fb77b0b21790178751d36a23f07d5b1efff5c25f..1c0672f707e4e5f29c937a1868f0400aad62e5cb 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -503,9 +503,7 @@ uvc_video_prep_requests(struct uvc_video *video)
> unsigned int max_req_size, req_size, header_size;
> unsigned int nreq;
>
> - max_req_size = video->ep->maxpacket
> - * max_t(unsigned int, video->ep->maxburst, 1)
> - * (video->ep->mult);
> + max_req_size = video->max_req_size;
>
> if (!usb_endpoint_xfer_isoc(video->ep->desc)) {
> video->req_size = max_req_size;
>
> --
> 2.34.1
>
Powered by blists - more mailing lists