On Wed, 20 Jun 2018, Sebastian Andrzej Siewior wrote:
> uvc_init_video_isoc() assigns
> urb->interval = p->desc.bInterval;
>
> for the interval. This is correct for FS/LS.
That's a strange thing to say. For one thing, LS devices don't support
isochronous transfers at all. And while this assignment would be
correct for FS interrupt URBs, it is wrong for FS isochronous URBs.
> For HS/SS the bInterval
> value is using a logarithmic encoding. The usb_fill_int_urb() function
> takes this into account while settings the ->interval member.
> ->start_frame is set to -1 on init and should be filled by the HC on
> completion of the URB.
>
> Use usb_fill_int_urb() to fill the members of the struct urb.
Please don't do this. If you instead on using an inline routine to
save on source code, create an explicit usb_fill_isoc_urb() function.
Alan Stern
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_video.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index a88b2e51a666..79e7a827ed44 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1619,21 +1619,19 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
> return -ENOMEM;
> }
>
> - urb->dev = stream->dev->udev;
> - urb->context = stream;
> - urb->pipe = usb_rcvisocpipe(stream->dev->udev,
> - ep->desc.bEndpointAddress);
> + usb_fill_int_urb(urb, stream->dev->udev,
> + usb_rcvisocpipe(stream->dev->udev,
> + ep->desc.bEndpointAddress),
> + stream->urb_buffer[i], size,
> + uvc_video_complete, stream,
> + ep->desc.bInterval);
> #ifndef CONFIG_DMA_NONCOHERENT
> urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
> urb->transfer_dma = stream->urb_dma[i];
> #else
> urb->transfer_flags = URB_ISO_ASAP;
> #endif
> - urb->interval = ep->desc.bInterval;
> - urb->transfer_buffer = stream->urb_buffer[i];
> - urb->complete = uvc_video_complete;
> urb->number_of_packets = npackets;
> - urb->transfer_buffer_length = size;
>
> for (j = 0; j < npackets; ++j) {
> urb->iso_frame_desc[j].offset = j * psize;
>
