Re: [PATCH] media: uvcvideo: preserve status URB interval on resubmit

Ricardo Ribalda <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media
Message-ID <CANiDSCsguGSgvEkWTCWVEctHFYN3teRXS07OczJfumWzVqv4Qw@mail.gmail.com>
Hi Xu

Good catch :)

On Thu, 13 Aug 2026 at 09:48, raoxu <[email protected]> wrote:
>
> From: Xu Rao <[email protected]>
>
> usb_fill_int_urb() takes an endpoint interval value and stores the
> period expected by usb_submit_urb() in urb->interval. For high-speed
> and SuperSpeed interrupt endpoints, it converts the logarithmic
> descriptor encoding to the corresponding microframe period.
>
> The UVC status URB is initialized once in uvc_status_init(), including
> the UVC_QUIRK_STATUS_INTERVAL adjustment required by high-speed devices
> that report bInterval using the old full-speed convention. The first
> submission therefore carries the intended interval.
>
> Both status resubmit paths, however, overwrite urb->interval with the
> raw endpoint bInterval before submitting the same URB again. On
> high-speed and SuperSpeed devices this mixes the descriptor encoding
> with the already converted URB interval representation. For devices
> using UVC_QUIRK_STATUS_INTERVAL it also deterministically discards the
> quirk adjustment after the first status completion, making the
> workaround effective only for the initial submission.
>
> This has been easy to miss because the initial status submission is
> correct, full-speed interrupt endpoints do not have the same
> logarithmic encoding mismatch, and the status endpoint carries
> relatively infrequent control and streaming notifications rather than
> the video payload itself. The problem only appears after the status URB
> has completed and is resubmitted, and the externally visible effect
> also depends on how the host controller handles the resulting period.
>
> Do not restore bInterval when resubmitting the status URB. Reuse the
> interval established by usb_fill_int_urb() and usb_submit_urb(), which
> also preserves the quirk-adjusted value. No additional state or
> interval conversion is needed because the same URB is being reused.
>
> Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
> Cc: [email protected]
Reviewed-by: Ricardo Ribalda <[email protected]>
> Signed-off-by: Xu Rao <[email protected]>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c   | 1 -
>  drivers/media/usb/uvc/uvc_status.c | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 3ca108b83f1d..6552f541aa0d 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2196,7 +2196,6 @@ static void uvc_ctrl_status_event_work(struct work_struct *work)
>                 return;
>
>         /* Resubmit the URB. */
> -       w->urb->interval = dev->int_ep->desc.bInterval;
>         ret = usb_submit_urb(w->urb, GFP_KERNEL);
>         if (ret < 0)
>                 dev_err(&dev->intf->dev,
> diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
> index b632cf5e3fe9..680c6c7f4ad7 100644
> --- a/drivers/media/usb/uvc/uvc_status.c
> +++ b/drivers/media/usb/uvc/uvc_status.c
> @@ -245,7 +245,6 @@ static void uvc_status_complete(struct urb *urb)
>         }
>
>         /* Resubmit the URB. */
> -       urb->interval = dev->int_ep->desc.bInterval;
>         ret = usb_submit_urb(urb, GFP_ATOMIC);
>         if (ret < 0)
>                 dev_err(&dev->intf->dev,
> --
> 2.50.1
>
>


-- 
Ricardo Ribalda
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.