Hi Alan,
On Thu, Jan 02, 2020 at 12:49:00PM -0500, Alan Stern wrote:
> On Thu, 2 Jan 2020, Laurent Pinchart wrote:
> > On Thu, Jan 02, 2020 at 04:57:42PM +0000, Roger Whittaker wrote:
> > > On Thu, Jan 02, 2020 at 06:38:07PM +0200, Laurent Pinchart wrote:
> > >
> > > > Roger, would you be able to set the uvcvideo trace module parameter to
> > > > 0xffff before plugging the device, and provide the messages printed by
> > > > the driver to the kernel log both with and without the above commit ?
> > >
> > > With 5.3.12-2-default, loading uvcvideo with
> > >
> > > options uvcvideo trace=0xffff
> >
> > Thank you.
> >
> > > On plugging:
> > >
> > > [ 73.571566] usb 1-1.4.3.1: new high-speed USB device number 12 using xhci_hcd
> > > [ 73.729180] usb 1-1.4.3.1: config 1 interface 2 altsetting 0 endpoint 0x82 has wMaxPacketSize 0, skipping
> > > [ 73.729552] usb 1-1.4.3.1: New USB device found, idVendor=1778, idProduct=0214, bcdDevice= 7.07
> > > [ 73.729558] usb 1-1.4.3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> > > [ 73.729561] usb 1-1.4.3.1: Product: IPEVO Point 2 View
> > > [ 73.729564] usb 1-1.4.3.1: Manufacturer: IPEVO Inc.
> > > [ 73.732670] hid-generic 0003:1778:0214.0009: hiddev98,hidraw8: USB HID v1.10 Device [IPEVO Inc. IPEVO Point 2 View] on usb-0000:00:14.0-1.4.3.1/input0
> > > [ 73.781765] videodev: Linux video capture interface: v2.00
> > > [ 73.807553] uvcvideo: Probing generic UVC device 1.4.3.1
> > > [ 73.807693] uvcvideo: no class-specific streaming interface descriptors found.
> >
> > It seems that Alan's patch causes more than the endpoint to be ignored.
>
> Roger, you can get more information by plugging in the device and then
> posting the contents of /sys/kernel/debug/usb/devices (or just the
> portion that refers to the camera). It would be interesting to compare
> the values from the kernel with the commit present and the kernel with
> the commit reverted.
I've investigated this a bit further.
UVC defines class-specific interface descriptors that are usually
located right after the standard interface descriptor in altsetting 0.
The uvcvideo driver accesses those descriptor through
intf->altsetting[0].extra. However, some devices insert an isochronous
endpoint descriptor with wMaxPAcketSize set to 0 between the standard
interface descriptor and the UVC class-specific interface descriptors.
Before your patch, these descriptors were recorded in the extra field of
the endpoint, as they're located right after it:
static int usb_parse_endpoint(struct device *ddev, int cfgno, int inum,
int asnum, struct usb_host_interface *ifp, int num_ep,
unsigned char *buffer, int size)
{
...
/* Skip over any Class Specific or Vendor Specific descriptors;
* find the next endpoint or interface descriptor */
endpoint->extra = buffer;
i = find_next_descriptor(buffer, size, USB_DT_ENDPOINT,
USB_DT_INTERFACE, &n);
endpoint->extralen = i;
...
}
The uvcvideo driver looks at endpoint->extra when altsetting[0] has no
extra data.
With your patch, the endpoint is skipped, and the class-specific
interface descriptors are dropped with it. The uvcvideo driver can't
access them and fails.
One solution may be to add to altsetting[0].extra all the extra
class-specific descriptors, regardless of whether they're located before
or after endpoints. I however fear we may not always be able to identify
those descriptors properly, especially with the CS_INTERFACE descriptor
type being defined in class specifications, not in the USB core
specification. There's also a risk of breaking working devices if we do
so (the uvcvideo driver should be able to cope, but other drivers may
always look for descriptors in the endpoint).
--
Regards,
Laurent Pinchart
