Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels

[Takashi Iwai <tiwai@suse.de>]

On Thu, 02 Jan 2020 12:20:45 +0100,
Johan Hovold wrote:
> 
> 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@rowland.harvard.edu>
> > > > 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@rowland.harvard.edu>
> > > > 
> > > >     Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.1910281050420.1485-100000@iolanthe.rowland.org
> > > >     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > 
> > > > 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.

FWIW, Roger confirmed that reverting the commit d482c7bb0541 does
indeed fix the issue (with the latest 5.4.y kernel).


thanks,

Takashi