* Re: [PATCH] media: uvcvideo: preserve status URB interval on resubmit
2026-08-13 7:46 [PATCH] media: uvcvideo: preserve status URB interval on resubmit raoxu
@ 2026-08-13 20:52 ` Ricardo Ribalda
2026-09-29 14:48 ` Hans de Goede
2026-10-01 23:18 ` Laurent Pinchart
2 siblings, 0 replies; 5+ messages in thread
From: Ricardo Ribalda @ 2026-08-13 20:52 UTC (permalink / raw)
To: raoxu; +Cc: laurent.pinchart, hansg, mchehab, linux-media, linux-kernel, stable
Hi Xu
Good catch :)
On Thu, 13 Aug 2026 at 09:48, raoxu <raoxu@uniontech.com> wrote:
>
> From: Xu Rao <raoxu@uniontech.com>
>
> 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: stable@vger.kernel.org
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: uvcvideo: preserve status URB interval on resubmit
2026-08-13 7:46 [PATCH] media: uvcvideo: preserve status URB interval on resubmit raoxu
2026-08-13 20:52 ` Ricardo Ribalda
@ 2026-09-29 14:48 ` Hans de Goede
2026-10-01 23:18 ` Laurent Pinchart
2 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2026-09-29 14:48 UTC (permalink / raw)
To: raoxu, laurent.pinchart; +Cc: mchehab, linux-media, linux-kernel, stable
Hi,
On 13-Aug-26 09:46, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
>
> 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: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
Thank you for your patch.
I have merged this into:
https://gitlab.freedesktop.org/linux-media/users/uvc/-/commits/for-next/
Regards,
Hans
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: uvcvideo: preserve status URB interval on resubmit
2026-08-13 7:46 [PATCH] media: uvcvideo: preserve status URB interval on resubmit raoxu
2026-08-13 20:52 ` Ricardo Ribalda
2026-09-29 14:48 ` Hans de Goede
@ 2026-10-01 23:18 ` Laurent Pinchart
2026-10-02 1:18 ` Laurent Pinchart
2 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2026-10-01 23:18 UTC (permalink / raw)
To: raoxu; +Cc: hansg, mchehab, linux-media, linux-kernel, stable
On Thu, Aug 13, 2026 at 03:46:32PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
>
> 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.
>
This commit message is also very long and has a large amount of
irrelevant, when not borderline incorrect, information. Is this missing
an Assisted-by tag ?
> Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
> 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,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] media: uvcvideo: preserve status URB interval on resubmit
2026-10-01 23:18 ` Laurent Pinchart
@ 2026-10-02 1:18 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2026-10-02 1:18 UTC (permalink / raw)
To: raoxu; +Cc: hansg, mchehab, linux-media, linux-kernel, stable
On Fri, Oct 02, 2026 at 02:18:56AM +0300, Laurent Pinchart wrote:
> On Thu, Aug 13, 2026 at 03:46:32PM +0800, raoxu wrote:
> > From: Xu Rao <raoxu@uniontech.com>
> >
> > 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.
> >
>
> This commit message is also very long and has a large amount of
> irrelevant, when not borderline incorrect, information. Is this missing
> an Assisted-by tag ?
>
> > Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> > Fixes: e5225c820c05 ("media: uvcvideo: Send a control event when a Control Change interrupt arrives")
Also, how is stable supposed to make sense of this ?
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xu Rao <raoxu@uniontech.com>
> > ---
> > 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,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread