[PATCH 1/4] usb: gadget: uvc: fix req_payload_size calculation

Xu Yang posted 4 patches 1 month ago
There is a newer version of this series
[PATCH 1/4] usb: gadget: uvc: fix req_payload_size calculation
Posted by Xu Yang 1 month ago
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@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.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
Re: [PATCH 1/4] usb: gadget: uvc: fix req_payload_size calculation
Posted by Frank Li 1 month ago
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@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Reviewed-by: Frank Li <Frank.Li@nxp.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
>