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