On Wed, Jan 01, 2020 at 03:09:35PM -0500, Alan Stern wrote: > On Wed, 1 Jan 2020, Greg KH wrote: > > On Wed, Jan 01, 2020 at 08:35:59PM +0200, Laurent Pinchart wrote: > > > [ 470.351700] usb 1-1.4.3.1: config 1 interface 2 altsetting 0 endpoint 0x82 has wMaxPacketSize 0, skipping > > > > > > This seems to be the culprit, and it points to the USB core. One > > > interface is ignored due to its wMaxPacketSize value, and the uvcvideo > > > driver then fails to find it. > > > > > > The wMaxPacketSize check was added in > > > > > > commit d482c7bb0541d19dea8bff437a9f3c5563b5b2d2 > > > Author: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > > Date: Mon Oct 28 10:52:35 2019 -0400 > > > > > > USB: Skip endpoints with 0 maxpacket length > > > > > > Endpoints with a maxpacket length of 0 are probably useless. They > > > can't transfer any data, and it's not at all unlikely that an HCD will > > > crash or hang when trying to handle an URB for such an endpoint. > > > > > > Currently the USB core does not check for endpoints having a maxpacket > > > value of 0. This patch adds a check, printing a warning and skipping > > > over any endpoints it catches. > > > > > > Now, the USB spec does not rule out endpoints having maxpacket = 0. > > > But since they wouldn't have any practical use, there doesn't seem to > > > be any good reason for us to accept them. > > > > > > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > > > > > Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.1910281050420.1485-100000@xxxxxxxxxxxxxxxxxxxx > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > > > > The commit was merged in v5.4 and backported to v5.3.11 in > > > 47aaab6377204cdbcd16f52a23c584f994fd0d15. > > > > > > For reference for Alan and linux-usb, the issue being discussed is > > > described in https://bugzilla.suse.com/show_bug.cgi?id=1159811. The > > > above commit seems to cause a regression with several cameras. I've > > > attached to this e-mail the lsusb output provided by Roger. > > > > How can a device work with an endpoint of 0 length? > > > > What does the driver expect to do with those endpoints? Does it expect > > it to be present but just ignore it? > > I see what's going on. The endpoint in question is isochronous, and > the bAlternateSetting value is 0, which makes this the default > altsetting for that interface. The USB spec says (at the end of > section 5.6.3): > > All device default interface settings must not include any > isochronous endpoints with non-zero data payload sizes > (specified via wMaxPacketSize in the endpoint descriptor). > Alternate interface settings may specify non-zero data payload > sizes for isochronous endpoints. > > That explains why the maxpacket size is set to 0. > > So it looks like the endpoint-descriptor parsing code might want to > make a special case to accept isochronous endpoints with maxpacket 0 if > bAlternateSetting is 0. But whether we do this or not, I would expect > the uvcvideo driver to look for isochronous endpoints in the alternate > settings it will actually use, not in altsetting 0. Then the presence > or absence of that endpoint descriptor would make no difference to > uvcvideo. > > (Unless the UVC spec _requires_ these endpoint descriptors to be > present. If it does then we should simply change the core parsing code > and leave uvcvideo alone.) Note that we also have this little gem in the ftdi usb-serial driver (since 2009) overriding a zero max packet size for devices with broken descriptors: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size calculation") Note sure how common those are but they will no longer work after the new sanity check in core. I guess we could add quirks for them (to core) in case we get any reports, but perhaps reverting the check should be considered. There doesn't seem to be any other drivers accepting and explicitly handling zero max packet sizes like this in mainline however. Johan
