On Mon, 6 Jan 2020, Laurent Pinchart wrote:
> Hi Alan,
>
> Thank you for the patch.
>
> On Mon, Jan 06, 2020 at 10:43:42AM -0500, Alan Stern wrote:
> > It turns out that even though endpoints with a maxpacket length of 0
> > aren't useful for data transfer, the descriptors do serve other
> > purposes. In particular, skipping them will also skip over other
> > class-specific descriptors for classes such as UVC. This unexpected
> > side effect has caused some UVC cameras to stop working.
> >
> > In addition, the USB spec requires that when isochronous endpoint
> > descriptors are present in an interface's altsetting 0 (which is true
> > on some devices), the maxpacket size _must_ be set to 0. Warning
> > about such things seems like a bad idea.
> >
> > This patch updates an earlier commit which would log a warning and
> > skip these endpoint descriptors. Now we only log a warning, and we
> > don't even do that for isochronous endpoints in altsetting 0.
> >
> > We don't need to worry about preventing endpoints with maxpacket = 0
> > from ever being used for data transfers; usb_submit_urb() already
> > checks for this.
> >
> > Reported-and-tested-by: Roger Whittaker <Roger.Whittaker@xxxxxxxx>
> > Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
> > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > CC: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2
>
> The patch looks good to me, so
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> But shouldn't we also warn if maxp != 0 && usb_endpoint_xfer_isoc(d) &&
> asnum == 0 ?
We could, but that's a different kind of issue. That would be more a
question of not adhering to the standard than of possibly failing to
work.
In theory the USBCV tests already check for this. Manufacturers that
don't run those tests probably also won't care if the Linux kernel
complains. But as far as I know, none of our drivers will malfunction
if there's an isochronous endpoint in altsetting 0 with maxpacket > 0.
Alan Stern
