media: uvcvideo: Pass allocation size directly to uvc_alloc_urb_buffer

The uvc_alloc_urb_buffer() function implicitly depended on the
stream->urb_size field, which was set by its caller,
uvc_alloc_urb_buffers(). This implicit data flow makes the code harder
to follow.

More importantly, stream->urb_size was updated within the allocation
loop before the allocation was confirmed to be successful. If the
allocation failed, the stream object would be left with a urb_size that
doesn't correspond to valid, allocated URB buffers.

Refactor uvc_alloc_urb_buffer() to accept the buffer size as an explicit
argument. This makes the function's dependencies clear and improves the
robustness of the error handling path. The stream->urb_size is now set only
after a complete and successful allocation.

This is a pure refactoring and introduces no functional changes.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Itay Chamiel <itay.chamiel@q.ai>
Link: https://patch.msgid.link/20260114-uvc-alloc-urb-v1-2-cedf3fb66711@chromium.org
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index ec76595f3c4be0f49b798ec663d6855d78ab21c4..59eb95a4b70c05b1a12986e908b7e9979b064fd0 100644 (file)
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1771,12 +1771,13 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream)
 }
 
 static bool uvc_alloc_urb_buffer(struct uvc_streaming *stream,
-                                struct uvc_urb *uvc_urb, gfp_t gfp_flags)
+                                struct uvc_urb *uvc_urb, unsigned int size,
+                                gfp_t gfp_flags)
 {
        struct usb_device *udev = stream->dev->udev;
 
-       uvc_urb->buffer = usb_alloc_noncoherent(udev, stream->urb_size,
-                                               gfp_flags, &uvc_urb->dma,
+       uvc_urb->buffer = usb_alloc_noncoherent(udev, size, gfp_flags,
+                                               &uvc_urb->dma,
                                                uvc_stream_dir(stream),
                                                &uvc_urb->sgt);
        return !!uvc_urb->buffer;
@@ -1813,12 +1814,13 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
 
        /* Retry allocations until one succeed. */
        for (; npackets > 0; npackets /= 2) {
-               stream->urb_size = psize * npackets;
+               unsigned int urb_size = psize * npackets;
 
                for (i = 0; i < UVC_URBS; ++i) {
                        struct uvc_urb *uvc_urb = &stream->uvc_urb[i];
 
-                       if (!uvc_alloc_urb_buffer(stream, uvc_urb, gfp_flags)) {
+                       if (!uvc_alloc_urb_buffer(stream, uvc_urb, urb_size,
+                                                 gfp_flags)) {
                                uvc_free_urb_buffers(stream);
                                break;
                        }
@@ -1830,6 +1832,7 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
                        uvc_dbg(stream->dev, VIDEO,
                                "Allocated %u URB buffers of %ux%u bytes each\n",
                                UVC_URBS, npackets, psize);
+                       stream->urb_size = urb_size;
                        return npackets;
                }
        }
@@ -1837,7 +1840,6 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
        uvc_dbg(stream->dev, VIDEO,
                "Failed to allocate URB buffers (%u bytes per packet)\n",
                psize);
-       stream->urb_size = 0;
        return 0;
 }