linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
       [not found]   ` <20200101175220.GA18140@suse.com>
@ 2020-01-01 18:35     ` Laurent Pinchart
  2020-01-01 18:47       ` Greg KH
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-01-01 18:35 UTC (permalink / raw)
  To: Roger Whittaker; +Cc: Takashi Iwai, Alan Stern, linux-usb

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

Hi Roger,

(CCin'g Alan Stern and linux-usb)

On Wed, Jan 01, 2020 at 05:52:27PM +0000, Roger Whittaker wrote:
> On Wed, Jan 01, 2020 at 07:24:49PM +0200, Laurent Pinchart wrote:
> 
> > The last message is worse. Could you send me the output of lsusb -v (you
> > can restrict it to the camera with -d), if possible running as root, for
> > both the working and non-working kernels ?
> 
> Thanks very much for your reply.
> 
> The lsusb outputs are attached - they are in fact identical to each
> other.
> 
> Also attached, the dmesg lines when replugging the camera on both
> kernels.

Thank you for the information.

I had missed the following message:

[  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.

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: lsusb-with-5.3.9-1-default --]
[-- Type: text/plain, Size: 9342 bytes --]


Bus 001 Device 010: ID 1778:0214  
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass          239 Miscellaneous Device
  bDeviceSubClass         2 
  bDeviceProtocol         1 Interface Association
  bMaxPacketSize0        64
  idVendor           0x1778 
  idProduct          0x0214 
  bcdDevice            7.07
  iManufacturer           1 IPEVO Inc.
  iProduct                2 IPEVO Point 2 View
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0299
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.10
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      64
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x85  EP 5 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval              10
    Interface Association:
      bLength                 8
      bDescriptorType        11
      bFirstInterface         1
      bInterfaceCount         2
      bFunctionClass         14 Video
      bFunctionSubClass       3 Video Interface Collection
      bFunctionProtocol       0 
      iFunction               2 IPEVO Point 2 View
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass        14 Video
      bInterfaceSubClass      1 Video Control
      bInterfaceProtocol      0 
      iInterface              2 IPEVO Point 2 View
      VideoControl Interface Descriptor:
        bLength                13
        bDescriptorType        36
        bDescriptorSubtype      1 (HEADER)
        bcdUVC               1.00
        wTotalLength       0x0050
        dwClockFrequency        6.000000MHz
        bInCollection           1
        baInterfaceNr( 0)       2
      VideoControl Interface Descriptor:
        bLength                18
        bDescriptorType        36
        bDescriptorSubtype      2 (INPUT_TERMINAL)
        bTerminalID             1
        wTerminalType      0x0201 Camera Sensor
        bAssocTerminal          0
        iTerminal               0 
        wObjectiveFocalLengthMin      0
        wObjectiveFocalLengthMax      0
        wOcularFocalLength            0
        bControlSize                  3
        bmControls           0x000200a2
          Auto-Exposure Mode
          Focus (Absolute)
          Iris (Absolute)
          Focus, Auto
      VideoControl Interface Descriptor:
        bLength                11
        bDescriptorType        36
        bDescriptorSubtype      5 (PROCESSING_UNIT)
      Warning: Descriptor too short
        bUnitID                 3
        bSourceID               1
        wMaxMultiplier          0
        bControlSize            2
        bmControls     0x0000147f
          Brightness
          Contrast
          Hue
          Saturation
          Sharpness
          Gamma
          White Balance Temperature
          Power Line Frequency
          White Balance Temperature, Auto
        iProcessing             0 
        bmVideoStandards     0x1d
          None
          PAL - 625/50
          SECAM - 625/50
          NTSC - 625/50
      VideoControl Interface Descriptor:
        bLength                29
        bDescriptorType        36
        bDescriptorSubtype      6 (EXTENSION_UNIT)
        bUnitID                 4
        guidExtensionCode         {5a215226-3289-4156-894a-5c557cdf9664}
        bNumControl             4
        bNrPins                 1
        baSourceID( 0)          3
        bControlSize            4
        bmControls( 0)       0xff
        bmControls( 1)       0xff
        bmControls( 2)       0xff
        bmControls( 3)       0xff
        iExtension              0 
      VideoControl Interface Descriptor:
        bLength                 9
        bDescriptorType        36
        bDescriptorSubtype      3 (OUTPUT_TERMINAL)
        bTerminalID             2
        wTerminalType      0x0101 USB Streaming
        bAssocTerminal          0
        bSourceID               4
        iTerminal               0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               9
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass        14 Video
      bInterfaceSubClass      2 Video Streaming
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            5
          Transfer Type            Isochronous
          Synch Type               Asynchronous
          Usage Type               Data
        wMaxPacketSize     0x0000  1x 0 bytes
        bInterval               1
        INTERFACE CLASS:  0f 24 01 02 ea 01 82 00 02 02 01 00 01 00 00
        INTERFACE CLASS:  1b 24 04 01 07 59 55 59 32 00 00 10 00 80 00 00 aa 00 38 9b 71 10 01 00 00 00 00
        INTERFACE CLASS:  1e 24 05 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  1e 24 05 02 00 40 01 f0 00 c0 00 03 4b c0 00 03 4b 00 58 02 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  1e 24 05 03 00 20 03 58 02 70 00 14 99 70 00 14 99 00 a6 0e 00 9a 5b 06 00 01 9a 5b 06 00
        INTERFACE CLASS:  1e 24 05 04 00 00 04 00 03 00 00 16 80 00 00 16 80 00 00 18 00 2a 2c 0a 00 01 2a 2c 0a 00
        INTERFACE CLASS:  1e 24 05 05 00 00 05 00 04 00 00 12 c0 00 00 12 c0 00 00 28 00 d0 12 13 00 01 d0 12 13 00
        INTERFACE CLASS:  1e 24 05 06 00 40 06 b0 04 00 00 0e a6 00 00 0e a6 00 98 3a 00 a0 25 26 00 01 a0 25 26 00
        INTERFACE CLASS:  1e 24 05 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  0b 24 03 00 01 80 02 e0 01 01 00
        INTERFACE CLASS:  0b 24 06 02 07 00 01 00 00 00 00
        INTERFACE CLASS:  1e 24 07 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  1e 24 07 02 00 40 01 f0 00 c0 00 03 4b c0 00 03 4b 00 58 02 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  1e 24 07 03 00 20 03 58 02 70 00 14 99 70 00 14 99 00 a6 0e 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  1e 24 07 04 00 00 04 00 03 00 00 16 80 00 00 16 80 00 00 18 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  1e 24 07 05 00 00 05 00 04 00 00 12 c0 00 00 12 c0 00 00 28 00 2a 2c 0a 00 01 2a 2c 0a 00
        INTERFACE CLASS:  1e 24 07 06 00 40 06 b0 04 00 00 0e a6 00 00 0e a6 00 98 3a 00 d0 12 13 00 01 d0 12 13 00
        INTERFACE CLASS:  1e 24 07 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  06 24 0d 00 00 00
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       1
      bNumEndpoints           1
      bInterfaceClass        14 Video
      bInterfaceSubClass      2 Video Streaming
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            5
          Transfer Type            Isochronous
          Synch Type               Asynchronous
          Usage Type               Data
        wMaxPacketSize     0x13fc  3x 1020 bytes
        bInterval               1
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass          239 Miscellaneous Device
  bDeviceSubClass         2 
  bDeviceProtocol         1 Interface Association
  bMaxPacketSize0        64
  bNumConfigurations      1
Device Status:     0x0000
  (Bus Powered)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-01 18:35     ` Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels Laurent Pinchart
@ 2020-01-01 18:47       ` Greg KH
  2020-01-01 20:09         ` Alan Stern
  0 siblings, 1 reply; 23+ messages in thread
From: Greg KH @ 2020-01-01 18:47 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Roger Whittaker, Takashi Iwai, Alan Stern, linux-usb

On Wed, Jan 01, 2020 at 08:35:59PM +0200, Laurent Pinchart wrote:
> Hi Roger,
> 
> (CCin'g Alan Stern and linux-usb)
> 
> On Wed, Jan 01, 2020 at 05:52:27PM +0000, Roger Whittaker wrote:
> > On Wed, Jan 01, 2020 at 07:24:49PM +0200, Laurent Pinchart wrote:
> > 
> > > The last message is worse. Could you send me the output of lsusb -v (you
> > > can restrict it to the camera with -d), if possible running as root, for
> > > both the working and non-working kernels ?
> > 
> > Thanks very much for your reply.
> > 
> > The lsusb outputs are attached - they are in fact identical to each
> > other.
> > 
> > Also attached, the dmesg lines when replugging the camera on both
> > kernels.
> 
> Thank you for the information.
> 
> I had missed the following message:
> 
> [  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?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-01 18:47       ` Greg KH
@ 2020-01-01 20:09         ` Alan Stern
  2020-01-02 11:20           ` Johan Hovold
  0 siblings, 1 reply; 23+ messages in thread
From: Alan Stern @ 2020-01-01 20:09 UTC (permalink / raw)
  To: Greg KH; +Cc: Laurent Pinchart, Roger Whittaker, Takashi Iwai, linux-usb

On Wed, 1 Jan 2020, Greg KH wrote:

> On Wed, Jan 01, 2020 at 08:35:59PM +0200, Laurent Pinchart wrote:
> > Hi Roger,
> > 
> > (CCin'g Alan Stern and linux-usb)
> > 
> > On Wed, Jan 01, 2020 at 05:52:27PM +0000, Roger Whittaker wrote:
> > > On Wed, Jan 01, 2020 at 07:24:49PM +0200, Laurent Pinchart wrote:
> > > 
> > > > The last message is worse. Could you send me the output of lsusb -v (you
> > > > can restrict it to the camera with -d), if possible running as root, for
> > > > both the working and non-working kernels ?
> > > 
> > > Thanks very much for your reply.
> > > 
> > > The lsusb outputs are attached - they are in fact identical to each
> > > other.
> > > 
> > > Also attached, the dmesg lines when replugging the camera on both
> > > kernels.
> > 
> > Thank you for the information.
> > 
> > I had missed the following message:
> > 
> > [  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.)

Alan Stern


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-01 20:09         ` Alan Stern
@ 2020-01-02 11:20           ` Johan Hovold
  2020-01-02 13:11             ` Takashi Iwai
  0 siblings, 1 reply; 23+ messages in thread
From: Johan Hovold @ 2020-01-02 11:20 UTC (permalink / raw)
  To: Alan Stern
  Cc: Greg KH, Laurent Pinchart, Roger Whittaker, Takashi Iwai, linux-usb

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.

There doesn't seem to be any other drivers accepting and explicitly
handling zero max packet sizes like this in mainline however.

Johan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 11:20           ` Johan Hovold
@ 2020-01-02 13:11             ` Takashi Iwai
  2020-01-02 15:06               ` Alan Stern
  0 siblings, 1 reply; 23+ messages in thread
From: Takashi Iwai @ 2020-01-02 13:11 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Alan Stern, Greg KH, Laurent Pinchart, Roger Whittaker,
	Takashi Iwai, linux-usb

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 13:11             ` Takashi Iwai
@ 2020-01-02 15:06               ` Alan Stern
  2020-01-02 15:32                 ` Johan Hovold
  2020-01-02 16:38                 ` Laurent Pinchart
  0 siblings, 2 replies; 23+ messages in thread
From: Alan Stern @ 2020-01-02 15:06 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Johan Hovold, Greg KH, Laurent Pinchart, Roger Whittaker,
	Takashi Iwai, linux-usb

On Thu, 2 Jan 2020, Takashi Iwai wrote:

> 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).

All right.  Suppose instead of reverting that commit, I change the code
so that it only logs a warning when it finds an endpoint descriptor
with maxpacket = 0 (and it skips the warning for isochronous endpoints
in altsetting 0).  At the same time, we can add a check to
usb_submit_urb() to refuse URBs if the endpoint's maxpacket is 0.

Sounds good?

Alan Stern


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 15:06               ` Alan Stern
@ 2020-01-02 15:32                 ` Johan Hovold
  2020-01-02 18:24                   ` Alan Stern
  2020-01-02 16:38                 ` Laurent Pinchart
  1 sibling, 1 reply; 23+ messages in thread
From: Johan Hovold @ 2020-01-02 15:32 UTC (permalink / raw)
  To: Alan Stern
  Cc: Takashi Iwai, Johan Hovold, Greg KH, Laurent Pinchart,
	Roger Whittaker, Takashi Iwai, linux-usb

On Thu, Jan 02, 2020 at 10:06:33AM -0500, Alan Stern wrote:
> On Thu, 2 Jan 2020, Takashi Iwai wrote:
> > On Thu, 02 Jan 2020 12:20:45 +0100, Johan Hovold wrote:

> > > 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).
> 
> All right.  Suppose instead of reverting that commit, I change the code
> so that it only logs a warning when it finds an endpoint descriptor
> with maxpacket = 0 (and it skips the warning for isochronous endpoints
> in altsetting 0).  At the same time, we can add a check to
> usb_submit_urb() to refuse URBs if the endpoint's maxpacket is 0.
> 
> Sounds good?

Sounds good to me.

Just make sure not to add a WARN() in usb_submit_urb() so that we end up
having to add maxpacket checks to every USB driver when syzbot starts
hitting this (only driver's doing maxpacket divisions or similar should
need that).

Johan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 15:06               ` Alan Stern
  2020-01-02 15:32                 ` Johan Hovold
@ 2020-01-02 16:38                 ` Laurent Pinchart
  2020-01-02 16:57                   ` Roger Whittaker
  1 sibling, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-01-02 16:38 UTC (permalink / raw)
  To: Alan Stern
  Cc: Takashi Iwai, Johan Hovold, Greg KH, Roger Whittaker,
	Takashi Iwai, linux-usb

Hi Alan,

On Thu, Jan 02, 2020 at 10:06:33AM -0500, Alan Stern wrote:
> On Thu, 2 Jan 2020, Takashi Iwai wrote:
> > 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).
> 
> All right.  Suppose instead of reverting that commit, I change the code
> so that it only logs a warning when it finds an endpoint descriptor
> with maxpacket = 0 (and it skips the warning for isochronous endpoints
> in altsetting 0).  At the same time, we can add a check to
> usb_submit_urb() to refuse URBs if the endpoint's maxpacket is 0.
> 
> Sounds good?

It makes it easier for me as I won't have to adapt the uvcvideo driver,
so it certainly sounds good :-)

The USB specification allows different endpoint settings in different
alternate settings, and unless I'm mistaken, it includes a different
number of endpoints and different endpoint numbers. The UVC
specification isn't very clear on this topic, but in practice most
isochronous devices don't include any isochronous endpoint in altsetting
0. Only 5 devices in my database of ~400 has an isochronous endpoint in
altsetting 0, all of them with a wMaxPacketSize value set to 0. The
above commit, if it only ignored the endpoint, should thus be fine, as
the uvcvideo driver checks for endpoints in other alternate settings
(the related code is the loop at the end of uvc_parse_streaming() in
drivers/media/usb/uvc/uvc_driver.c). I thus wonder what's going wrong,
and if the commit couldn't cause the interface to be mistakenly ignored
as a whole ?

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 ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 16:38                 ` Laurent Pinchart
@ 2020-01-02 16:57                   ` Roger Whittaker
  2020-01-02 17:03                     ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Roger Whittaker @ 2020-01-02 16:57 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Alan Stern, Takashi Iwai, Johan Hovold, Greg KH, Takashi Iwai, linux-usb

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

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.
[   73.807728] uvcvideo: Found a Status endpoint (addr 81).
[   73.807730] uvcvideo: Found UVC 1.00 device IPEVO Point 2 View (1778:0214)
[   73.807759] uvcvideo: Failed to query (GET_INFO) UVC control 2 on unit 1: -32 (exp. 1).
[   73.807832] uvcvideo: Control error 6
[   73.807834] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/2 to device 1.4.3.1 entity 1
[   73.807835] uvcvideo: Adding mapping 'Exposure, Auto' to control 00000000-0000-0000-0000-000000000001/2.
[   73.807876] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/6 to device 1.4.3.1 entity 1
[   73.807877] uvcvideo: Adding mapping 'Focus (absolute)' to control 00000000-0000-0000-0000-000000000001/6.
[   73.807918] uvcvideo: Failed to query (GET_INFO) UVC control 9 on unit 1: -32 (exp. 1).
[   73.807996] uvcvideo: Control error 6
[   73.807997] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/9 to device 1.4.3.1 entity 1
[   73.807998] uvcvideo: Adding mapping 'Iris, Absolute' to control 00000000-0000-0000-0000-000000000001/9.
[   73.808037] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/8 to device 1.4.3.1 entity 1
[   73.808038] uvcvideo: Adding mapping 'Focus, Auto' to control 00000000-0000-0000-0000-000000000001/8.
[   73.808079] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/2 to device 1.4.3.1 entity 3
[   73.808080] uvcvideo: Adding mapping 'Brightness' to control 00000000-0000-0000-0000-000000000101/2.
[   73.808119] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/3 to device 1.4.3.1 entity 3
[   73.808120] uvcvideo: Adding mapping 'Contrast' to control 00000000-0000-0000-0000-000000000101/3.
[   73.808159] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/6 to device 1.4.3.1 entity 3
[   73.808160] uvcvideo: Adding mapping 'Hue' to control 00000000-0000-0000-0000-000000000101/6.
[   73.808205] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/7 to device 1.4.3.1 entity 3
[   73.808206] uvcvideo: Adding mapping 'Saturation' to control 00000000-0000-0000-0000-000000000101/7.
[   73.808255] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/8 to device 1.4.3.1 entity 3
[   73.808257] uvcvideo: Adding mapping 'Sharpness' to control 00000000-0000-0000-0000-000000000101/8.
[   73.808308] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/9 to device 1.4.3.1 entity 3
[   73.808309] uvcvideo: Adding mapping 'Gamma' to control 00000000-0000-0000-0000-000000000101/9.
[   73.808349] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/10 to device 1.4.3.1 entity 3
[   73.808350] uvcvideo: Adding mapping 'White Balance Temperature' to control 00000000-0000-0000-0000-000000000101/10.
[   73.808389] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/5 to device 1.4.3.1 entity 3
[   73.808390] uvcvideo: Adding mapping 'Power Line Frequency' to control 00000000-0000-0000-0000-000000000101/5.
[   73.808431] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/11 to device 1.4.3.1 entity 3
[   73.808432] uvcvideo: Adding mapping 'White Balance Temperature, Auto' to control 00000000-0000-0000-0000-000000000101/11.
[   73.808434] uvcvideo: Scanning UVC chain: OT 2 <- XU 4 <- PU 3 <- IT 1
[   73.808437] uvcvideo: Found a valid video chain (1 -> 2).
[   73.808438] uvcvideo: No streaming interface found for terminal 2.
[   73.808442] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Extension 4 was not initialized!
[   73.808444] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Processing 3 was not initialized!
[   73.808446] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Camera 1 was not initialized!
[   73.808542] input: IPEVO Point 2 View: IPEVO Point as /devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1.4/1-1.4.3/1-1.4.3.1/1-1.4.3.1:1.1/input/input21
[   73.808590] uvcvideo: UVC device initialized.
[   73.808636] usbcore: registered new interface driver uvcvideo
[   73.808637] USB Video Class driver (1.1.1)
[   75.899721] uvcvideo: Suspending interface 1


------------------------------------------------------------------------

With 5.4.7-1.g43720a7-default (the kernel Takashi built with commit
d482c7bb0541 reverted), loading uvcvideo with

options uvcvideo trace=0xffff

On plugging:

[  267.765563] usb 1-1.4.3.1: new high-speed USB device number 13 using xhci_hcd
[  267.879567] usb 1-1.4.3.1: New USB device found, idVendor=1778, idProduct=0214, bcdDevice= 7.07
[  267.879573] usb 1-1.4.3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[  267.879577] usb 1-1.4.3.1: Product: IPEVO Point 2 View
[  267.879580] usb 1-1.4.3.1: Manufacturer: IPEVO Inc.
[  267.882718] hid-generic 0003:1778:0214.000A: hiddev98,hidraw7: USB HID v1.10 Device [IPEVO Inc. IPEVO Point 2 View] on usb-0000:00:14.0-1.4.3.1/input0
[  267.883135] uvcvideo: Probing generic UVC device 1.4.3.1
[  267.883260] uvcvideo: trying extra data from endpoint 0.
[  267.883265] uvcvideo: Found format YUV 4:2:2 (YUYV).
[  267.883268] uvcvideo: - 640x480 (30.0 fps)
[  267.883277] uvcvideo: - 320x240 (30.0 fps)
[  267.883278] uvcvideo: - 800x600 (24.0 fps)
[  267.883280] uvcvideo: - 1024x768 (15.0 fps)
[  267.883282] uvcvideo: - 1280x1024 (8.0 fps)
[  267.883284] uvcvideo: - 1600x1200 (4.0 fps)
[  267.883286] uvcvideo: - 640x480 (30.0 fps)
[  267.883288] uvcvideo: Found format MJPEG.
[  267.883290] uvcvideo: - 640x480 (45.0 fps)
[  267.883292] uvcvideo: - 320x240 (45.0 fps)
[  267.883293] uvcvideo: - 800x600 (45.0 fps)
[  267.883295] uvcvideo: - 1024x768 (30.0 fps)
[  267.883297] uvcvideo: - 1280x1024 (15.0 fps)
[  267.883299] uvcvideo: - 1600x1200 (8.0 fps)
[  267.883301] uvcvideo: - 640x480 (45.0 fps)
[  267.883310] uvcvideo: Found a Status endpoint (addr 81).
[  267.883314] uvcvideo: Found UVC 1.00 device IPEVO Point 2 View (1778:0214)
[  267.883380] uvcvideo: Failed to query (GET_INFO) UVC control 2 on unit 1: -32 (exp. 1).
[  267.883411] uvcvideo: Control error 6
[  267.883416] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/2 to device 1.4.3.1 entity 1
[  267.883419] uvcvideo: Adding mapping 'Exposure, Auto' to control 00000000-0000-0000-0000-000000000001/2.
[  267.883468] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/6 to device 1.4.3.1 entity 1
[  267.883471] uvcvideo: Adding mapping 'Focus (absolute)' to control 00000000-0000-0000-0000-000000000001/6.
[  267.883512] uvcvideo: Failed to query (GET_INFO) UVC control 9 on unit 1: -32 (exp. 1).
[  267.883588] uvcvideo: Control error 6
[  267.883590] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/9 to device 1.4.3.1 entity 1
[  267.883593] uvcvideo: Adding mapping 'Iris, Absolute' to control 00000000-0000-0000-0000-000000000001/9.
[  267.883642] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/8 to device 1.4.3.1 entity 1
[  267.883645] uvcvideo: Adding mapping 'Focus, Auto' to control 00000000-0000-0000-0000-000000000001/8.
[  267.883694] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/2 to device 1.4.3.1 entity 3
[  267.883696] uvcvideo: Adding mapping 'Brightness' to control 00000000-0000-0000-0000-000000000101/2.
[  267.883745] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/3 to device 1.4.3.1 entity 3
[  267.883747] uvcvideo: Adding mapping 'Contrast' to control 00000000-0000-0000-0000-000000000101/3.
[  267.883795] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/6 to device 1.4.3.1 entity 3
[  267.883797] uvcvideo: Adding mapping 'Hue' to control 00000000-0000-0000-0000-000000000101/6.
[  267.883846] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/7 to device 1.4.3.1 entity 3
[  267.883848] uvcvideo: Adding mapping 'Saturation' to control 00000000-0000-0000-0000-000000000101/7.
[  267.883895] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/8 to device 1.4.3.1 entity 3
[  267.883898] uvcvideo: Adding mapping 'Sharpness' to control 00000000-0000-0000-0000-000000000101/8.
[  267.883947] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/9 to device 1.4.3.1 entity 3
[  267.883949] uvcvideo: Adding mapping 'Gamma' to control 00000000-0000-0000-0000-000000000101/9.
[  267.883999] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/10 to device 1.4.3.1 entity 3
[  267.884002] uvcvideo: Adding mapping 'White Balance Temperature' to control 00000000-0000-0000-0000-000000000101/10.
[  267.884050] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/5 to device 1.4.3.1 entity 3
[  267.884053] uvcvideo: Adding mapping 'Power Line Frequency' to control 00000000-0000-0000-0000-000000000101/5.
[  267.884101] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/11 to device 1.4.3.1 entity 3
[  267.884104] uvcvideo: Adding mapping 'White Balance Temperature, Auto' to control 00000000-0000-0000-0000-000000000101/11.
[  267.884108] uvcvideo: Scanning UVC chain: OT 2 <- XU 4 <- PU 3 <- IT 1
[  267.884117] uvcvideo: Found a valid video chain (1 -> 2).
[  267.885020] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Extension 4 was not initialized!
[  267.885025] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Processing 3 was not initialized!
[  267.885028] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Camera 1 was not initialized!
[  267.885188] input: IPEVO Point 2 View: IPEVO Point as /devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1.4/1-1.4.3/1-1.4.3.1/1-1.4.3.1:1.1/input/input22
[  267.885266] uvcvideo: UVC device initialized.
[  267.919845] uvcvideo: uvc_v4l2_open
[  267.919884] uvcvideo: uvc_v4l2_release
[  270.387236] uvcvideo: Suspending interface 2
[  270.387241] uvcvideo: Suspending interface 1




-- 
============================================
Roger Whittaker
SUSE Linux Premium Support Engineer
 
roger.whittaker@suse.com
+44 7802 357081
 
SUSE Linux
One Station Square
Bracknell
RG12 1QB
United Kingdom
============================================

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 16:57                   ` Roger Whittaker
@ 2020-01-02 17:03                     ` Laurent Pinchart
  2020-01-02 17:49                       ` Alan Stern
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-01-02 17:03 UTC (permalink / raw)
  To: Roger Whittaker
  Cc: Alan Stern, Takashi Iwai, Johan Hovold, Greg KH, Takashi Iwai, linux-usb

Hi Roger,

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.

> [   73.807728] uvcvideo: Found a Status endpoint (addr 81).
> [   73.807730] uvcvideo: Found UVC 1.00 device IPEVO Point 2 View (1778:0214)
> [   73.807759] uvcvideo: Failed to query (GET_INFO) UVC control 2 on unit 1: -32 (exp. 1).
> [   73.807832] uvcvideo: Control error 6
> [   73.807834] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/2 to device 1.4.3.1 entity 1
> [   73.807835] uvcvideo: Adding mapping 'Exposure, Auto' to control 00000000-0000-0000-0000-000000000001/2.
> [   73.807876] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/6 to device 1.4.3.1 entity 1
> [   73.807877] uvcvideo: Adding mapping 'Focus (absolute)' to control 00000000-0000-0000-0000-000000000001/6.
> [   73.807918] uvcvideo: Failed to query (GET_INFO) UVC control 9 on unit 1: -32 (exp. 1).
> [   73.807996] uvcvideo: Control error 6
> [   73.807997] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/9 to device 1.4.3.1 entity 1
> [   73.807998] uvcvideo: Adding mapping 'Iris, Absolute' to control 00000000-0000-0000-0000-000000000001/9.
> [   73.808037] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/8 to device 1.4.3.1 entity 1
> [   73.808038] uvcvideo: Adding mapping 'Focus, Auto' to control 00000000-0000-0000-0000-000000000001/8.
> [   73.808079] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/2 to device 1.4.3.1 entity 3
> [   73.808080] uvcvideo: Adding mapping 'Brightness' to control 00000000-0000-0000-0000-000000000101/2.
> [   73.808119] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/3 to device 1.4.3.1 entity 3
> [   73.808120] uvcvideo: Adding mapping 'Contrast' to control 00000000-0000-0000-0000-000000000101/3.
> [   73.808159] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/6 to device 1.4.3.1 entity 3
> [   73.808160] uvcvideo: Adding mapping 'Hue' to control 00000000-0000-0000-0000-000000000101/6.
> [   73.808205] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/7 to device 1.4.3.1 entity 3
> [   73.808206] uvcvideo: Adding mapping 'Saturation' to control 00000000-0000-0000-0000-000000000101/7.
> [   73.808255] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/8 to device 1.4.3.1 entity 3
> [   73.808257] uvcvideo: Adding mapping 'Sharpness' to control 00000000-0000-0000-0000-000000000101/8.
> [   73.808308] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/9 to device 1.4.3.1 entity 3
> [   73.808309] uvcvideo: Adding mapping 'Gamma' to control 00000000-0000-0000-0000-000000000101/9.
> [   73.808349] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/10 to device 1.4.3.1 entity 3
> [   73.808350] uvcvideo: Adding mapping 'White Balance Temperature' to control 00000000-0000-0000-0000-000000000101/10.
> [   73.808389] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/5 to device 1.4.3.1 entity 3
> [   73.808390] uvcvideo: Adding mapping 'Power Line Frequency' to control 00000000-0000-0000-0000-000000000101/5.
> [   73.808431] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/11 to device 1.4.3.1 entity 3
> [   73.808432] uvcvideo: Adding mapping 'White Balance Temperature, Auto' to control 00000000-0000-0000-0000-000000000101/11.
> [   73.808434] uvcvideo: Scanning UVC chain: OT 2 <- XU 4 <- PU 3 <- IT 1
> [   73.808437] uvcvideo: Found a valid video chain (1 -> 2).
> [   73.808438] uvcvideo: No streaming interface found for terminal 2.
> [   73.808442] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Extension 4 was not initialized!
> [   73.808444] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Processing 3 was not initialized!
> [   73.808446] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Camera 1 was not initialized!
> [   73.808542] input: IPEVO Point 2 View: IPEVO Point as /devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1.4/1-1.4.3/1-1.4.3.1/1-1.4.3.1:1.1/input/input21
> [   73.808590] uvcvideo: UVC device initialized.
> [   73.808636] usbcore: registered new interface driver uvcvideo
> [   73.808637] USB Video Class driver (1.1.1)
> [   75.899721] uvcvideo: Suspending interface 1
> 
> 
> ------------------------------------------------------------------------
> 
> With 5.4.7-1.g43720a7-default (the kernel Takashi built with commit
> d482c7bb0541 reverted), loading uvcvideo with
> 
> options uvcvideo trace=0xffff
> 
> On plugging:
> 
> [  267.765563] usb 1-1.4.3.1: new high-speed USB device number 13 using xhci_hcd
> [  267.879567] usb 1-1.4.3.1: New USB device found, idVendor=1778, idProduct=0214, bcdDevice= 7.07
> [  267.879573] usb 1-1.4.3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> [  267.879577] usb 1-1.4.3.1: Product: IPEVO Point 2 View
> [  267.879580] usb 1-1.4.3.1: Manufacturer: IPEVO Inc.
> [  267.882718] hid-generic 0003:1778:0214.000A: hiddev98,hidraw7: USB HID v1.10 Device [IPEVO Inc. IPEVO Point 2 View] on usb-0000:00:14.0-1.4.3.1/input0
> [  267.883135] uvcvideo: Probing generic UVC device 1.4.3.1
> [  267.883260] uvcvideo: trying extra data from endpoint 0.
> [  267.883265] uvcvideo: Found format YUV 4:2:2 (YUYV).
> [  267.883268] uvcvideo: - 640x480 (30.0 fps)
> [  267.883277] uvcvideo: - 320x240 (30.0 fps)
> [  267.883278] uvcvideo: - 800x600 (24.0 fps)
> [  267.883280] uvcvideo: - 1024x768 (15.0 fps)
> [  267.883282] uvcvideo: - 1280x1024 (8.0 fps)
> [  267.883284] uvcvideo: - 1600x1200 (4.0 fps)
> [  267.883286] uvcvideo: - 640x480 (30.0 fps)
> [  267.883288] uvcvideo: Found format MJPEG.
> [  267.883290] uvcvideo: - 640x480 (45.0 fps)
> [  267.883292] uvcvideo: - 320x240 (45.0 fps)
> [  267.883293] uvcvideo: - 800x600 (45.0 fps)
> [  267.883295] uvcvideo: - 1024x768 (30.0 fps)
> [  267.883297] uvcvideo: - 1280x1024 (15.0 fps)
> [  267.883299] uvcvideo: - 1600x1200 (8.0 fps)
> [  267.883301] uvcvideo: - 640x480 (45.0 fps)
> [  267.883310] uvcvideo: Found a Status endpoint (addr 81).
> [  267.883314] uvcvideo: Found UVC 1.00 device IPEVO Point 2 View (1778:0214)
> [  267.883380] uvcvideo: Failed to query (GET_INFO) UVC control 2 on unit 1: -32 (exp. 1).
> [  267.883411] uvcvideo: Control error 6
> [  267.883416] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/2 to device 1.4.3.1 entity 1
> [  267.883419] uvcvideo: Adding mapping 'Exposure, Auto' to control 00000000-0000-0000-0000-000000000001/2.
> [  267.883468] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/6 to device 1.4.3.1 entity 1
> [  267.883471] uvcvideo: Adding mapping 'Focus (absolute)' to control 00000000-0000-0000-0000-000000000001/6.
> [  267.883512] uvcvideo: Failed to query (GET_INFO) UVC control 9 on unit 1: -32 (exp. 1).
> [  267.883588] uvcvideo: Control error 6
> [  267.883590] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/9 to device 1.4.3.1 entity 1
> [  267.883593] uvcvideo: Adding mapping 'Iris, Absolute' to control 00000000-0000-0000-0000-000000000001/9.
> [  267.883642] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/8 to device 1.4.3.1 entity 1
> [  267.883645] uvcvideo: Adding mapping 'Focus, Auto' to control 00000000-0000-0000-0000-000000000001/8.
> [  267.883694] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/2 to device 1.4.3.1 entity 3
> [  267.883696] uvcvideo: Adding mapping 'Brightness' to control 00000000-0000-0000-0000-000000000101/2.
> [  267.883745] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/3 to device 1.4.3.1 entity 3
> [  267.883747] uvcvideo: Adding mapping 'Contrast' to control 00000000-0000-0000-0000-000000000101/3.
> [  267.883795] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/6 to device 1.4.3.1 entity 3
> [  267.883797] uvcvideo: Adding mapping 'Hue' to control 00000000-0000-0000-0000-000000000101/6.
> [  267.883846] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/7 to device 1.4.3.1 entity 3
> [  267.883848] uvcvideo: Adding mapping 'Saturation' to control 00000000-0000-0000-0000-000000000101/7.
> [  267.883895] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/8 to device 1.4.3.1 entity 3
> [  267.883898] uvcvideo: Adding mapping 'Sharpness' to control 00000000-0000-0000-0000-000000000101/8.
> [  267.883947] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/9 to device 1.4.3.1 entity 3
> [  267.883949] uvcvideo: Adding mapping 'Gamma' to control 00000000-0000-0000-0000-000000000101/9.
> [  267.883999] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/10 to device 1.4.3.1 entity 3
> [  267.884002] uvcvideo: Adding mapping 'White Balance Temperature' to control 00000000-0000-0000-0000-000000000101/10.
> [  267.884050] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/5 to device 1.4.3.1 entity 3
> [  267.884053] uvcvideo: Adding mapping 'Power Line Frequency' to control 00000000-0000-0000-0000-000000000101/5.
> [  267.884101] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/11 to device 1.4.3.1 entity 3
> [  267.884104] uvcvideo: Adding mapping 'White Balance Temperature, Auto' to control 00000000-0000-0000-0000-000000000101/11.
> [  267.884108] uvcvideo: Scanning UVC chain: OT 2 <- XU 4 <- PU 3 <- IT 1
> [  267.884117] uvcvideo: Found a valid video chain (1 -> 2).
> [  267.885020] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Extension 4 was not initialized!
> [  267.885025] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Processing 3 was not initialized!
> [  267.885028] uvcvideo 1-1.4.3.1:1.1: Entity type for entity Camera 1 was not initialized!
> [  267.885188] input: IPEVO Point 2 View: IPEVO Point as /devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1.4/1-1.4.3/1-1.4.3.1/1-1.4.3.1:1.1/input/input22
> [  267.885266] uvcvideo: UVC device initialized.
> [  267.919845] uvcvideo: uvc_v4l2_open
> [  267.919884] uvcvideo: uvc_v4l2_release
> [  270.387236] uvcvideo: Suspending interface 2
> [  270.387241] uvcvideo: Suspending interface 1

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 17:03                     ` Laurent Pinchart
@ 2020-01-02 17:49                       ` Alan Stern
  2020-01-02 21:51                         ` Roger Whittaker
  2020-01-02 23:11                         ` Laurent Pinchart
  0 siblings, 2 replies; 23+ messages in thread
From: Alan Stern @ 2020-01-02 17:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Roger Whittaker, Takashi Iwai, Johan Hovold, Greg KH,
	Takashi Iwai, linux-usb

On Thu, 2 Jan 2020, Laurent Pinchart wrote:

> Hi Roger,
> 
> 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.

Alan Stern


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 15:32                 ` Johan Hovold
@ 2020-01-02 18:24                   ` Alan Stern
  0 siblings, 0 replies; 23+ messages in thread
From: Alan Stern @ 2020-01-02 18:24 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Takashi Iwai, Greg KH, Laurent Pinchart, Roger Whittaker,
	Takashi Iwai, linux-usb

On Thu, 2 Jan 2020, Johan Hovold wrote:

> > > FWIW, Roger confirmed that reverting the commit d482c7bb0541 does
> > > indeed fix the issue (with the latest 5.4.y kernel).
> > 
> > All right.  Suppose instead of reverting that commit, I change the code
> > so that it only logs a warning when it finds an endpoint descriptor
> > with maxpacket = 0 (and it skips the warning for isochronous endpoints
> > in altsetting 0).  At the same time, we can add a check to
> > usb_submit_urb() to refuse URBs if the endpoint's maxpacket is 0.
> > 
> > Sounds good?
> 
> Sounds good to me.
> 
> Just make sure not to add a WARN() in usb_submit_urb() so that we end up
> having to add maxpacket checks to every USB driver when syzbot starts
> hitting this (only driver's doing maxpacket divisions or similar should
> need that).

Heh.  Yes, syzbot interprets WARN() very differently from dev_warn().

Okay, here's a patch for testing.  This is meant to go on top of the 
existing commit; instead of rejecting bogus endpoints entirely it just 
issues a warning.

And it turns out that usb_submit_urb() already checks for maxpacket = 
0, so no changes are needed there.

Alan Stern



Index: usb-devel/drivers/usb/core/config.c
===================================================================
--- usb-devel.orig/drivers/usb/core/config.c
+++ usb-devel/drivers/usb/core/config.c
@@ -346,12 +346,16 @@ static int usb_parse_endpoint(struct dev
 			endpoint->desc.wMaxPacketSize = cpu_to_le16(8);
 	}
 
-	/* Validate the wMaxPacketSize field */
+	/*
+	 * Validate the wMaxPacketSize field.
+	 * Some devices have isochronous endpoints in altsetting 0;
+	 * the USB-2 spec requires such endpoints to have wMaxPacketSize = 0
+	 * (see the end of section 5.6.3), so don't warn about them.
+	 */
 	maxp = usb_endpoint_maxp(&endpoint->desc);
-	if (maxp == 0) {
-		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has wMaxPacketSize 0, skipping\n",
+	if (maxp == 0 && !(usb_endpoint_xfer_isoc(d) && asnum == 0)) {
+		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has invalid wMaxPacketSize 0\n",
 		    cfgno, inum, asnum, d->bEndpointAddress);
-		goto skip_to_next_endpoint_or_interface_descriptor;
 	}
 
 	/* Find the highest legal maxpacket size for this endpoint */


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 17:49                       ` Alan Stern
@ 2020-01-02 21:51                         ` Roger Whittaker
  2020-01-02 23:11                         ` Laurent Pinchart
  1 sibling, 0 replies; 23+ messages in thread
From: Roger Whittaker @ 2020-01-02 21:51 UTC (permalink / raw)
  To: Alan Stern
  Cc: Laurent Pinchart, Takashi Iwai, Johan Hovold, Greg KH,
	Takashi Iwai, linux-usb

On Thu, Jan 02, 2020 at 12:49:00PM -0500, Alan Stern wrote:

> 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.

5.3.12-2-default (commit present)

T:  Bus=01 Lev=04 Prnt=08 Port=00 Cnt=01 Dev#= 10 Spd=480  MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=1778 ProdID=0214 Rev= 7.07
S:  Manufacturer=IPEVO Inc.
S:  Product=IPEVO Point 2 View
C:* #Ifs= 3 Cfg#= 1 Atr=80 MxPwr=500mA
A:  FirstIf#= 1 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=85(I) Atr=03(Int.) MxPS=   8 Ivl=64ms
I:* If#= 1 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=32ms
I:* If#= 2 Alt= 0 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=(none)
E:  Ad=82(I) Atr=05(Isoc) MxPS=   0 Ivl=125us
I:  If#= 2 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=(none)
E:  Ad=82(I) Atr=05(Isoc) MxPS=3060 Ivl=125us

----------------

5.4.7-1.g43720a7-default (commit reverted)

T:  Bus=01 Lev=04 Prnt=08 Port=00 Cnt=01 Dev#= 13 Spd=480  MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=1778 ProdID=0214 Rev= 7.07
S:  Manufacturer=IPEVO Inc.
S:  Product=IPEVO Point 2 View
C:* #Ifs= 3 Cfg#= 1 Atr=80 MxPwr=500mA
A:  FirstIf#= 1 IfCount= 2 Cls=0e(video) Sub=03 Prot=00
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=85(I) Atr=03(Int.) MxPS=   8 Ivl=64ms
I:* If#= 1 Alt= 0 #EPs= 1 Cls=0e(video) Sub=01 Prot=00 Driver=uvcvideo
E:  Ad=81(I) Atr=03(Int.) MxPS=   8 Ivl=32ms
I:* If#= 2 Alt= 0 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=82(I) Atr=05(Isoc) MxPS=   0 Ivl=125us
I:  If#= 2 Alt= 1 #EPs= 1 Cls=0e(video) Sub=02 Prot=00 Driver=uvcvideo
E:  Ad=82(I) Atr=05(Isoc) MxPS=3060 Ivl=125us




-- 
============================================
Roger Whittaker
SUSE Linux Premium Support Engineer
 
roger.whittaker@suse.com
+44 7802 357081
 
SUSE Linux
One Station Square
Bracknell
RG12 1QB
United Kingdom
============================================

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 17:49                       ` Alan Stern
  2020-01-02 21:51                         ` Roger Whittaker
@ 2020-01-02 23:11                         ` Laurent Pinchart
  2020-01-03 15:13                           ` Alan Stern
  1 sibling, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-01-02 23:11 UTC (permalink / raw)
  To: Alan Stern
  Cc: Roger Whittaker, Takashi Iwai, Johan Hovold, Greg KH,
	Takashi Iwai, linux-usb

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-02 23:11                         ` Laurent Pinchart
@ 2020-01-03 15:13                           ` Alan Stern
  2020-01-04 18:22                             ` Laurent Pinchart
  0 siblings, 1 reply; 23+ messages in thread
From: Alan Stern @ 2020-01-03 15:13 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Roger Whittaker, Takashi Iwai, Johan Hovold, Greg KH,
	Takashi Iwai, linux-usb

On Fri, 3 Jan 2020, Laurent Pinchart wrote:

> 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.

Ah, a very tricky and unexpected interaction!

> 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).

With the patch I posted yesterday, everything should go back to working 
the way it used to.  Have you had a chance to test it?

Alan Stern


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-03 15:13                           ` Alan Stern
@ 2020-01-04 18:22                             ` Laurent Pinchart
  2020-01-05 12:28                               ` Roger Whittaker
  0 siblings, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-01-04 18:22 UTC (permalink / raw)
  To: Alan Stern
  Cc: Roger Whittaker, Takashi Iwai, Johan Hovold, Greg KH,
	Takashi Iwai, linux-usb

Hi Alan,

On Fri, Jan 03, 2020 at 10:13:29AM -0500, Alan Stern wrote:
> On Fri, 3 Jan 2020, Laurent Pinchart wrote:
> 
> > 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.
> 
> Ah, a very tricky and unexpected interaction!
> 
> > 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).
> 
> With the patch I posted yesterday, everything should go back to working 
> the way it used to.  Have you had a chance to test it?

I don't have any camera affected by this issue, so I can't test it I'm
afraid. Roger, would you be able to give it a try ?

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels
  2020-01-04 18:22                             ` Laurent Pinchart
@ 2020-01-05 12:28                               ` Roger Whittaker
  2020-01-06 15:43                                 ` [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0 Alan Stern
  0 siblings, 1 reply; 23+ messages in thread
From: Roger Whittaker @ 2020-01-05 12:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Alan Stern, Takashi Iwai, Johan Hovold, Greg KH, Takashi Iwai, linux-usb

On Sat, Jan 04, 2020 at 08:22:05PM +0200, Laurent Pinchart wrote:

> [...]

> > With the patch I posted yesterday, everything should go back to working 
> > the way it used to.  Have you had a chance to test it?
> 
> I don't have any camera affected by this issue, so I can't test it I'm
> afraid. Roger, would you be able to give it a try ?

With 5.4.7-1.g8211231-default that Takashi built with the patch
mentioned
(http://download.opensuse.org/repositories/home:/tiwai:/bsc1159811-fix2/standard/x86_64/)

output of lsusb -v -d 1778:0214

Bus 001 Device 013: ID 1778:0214  
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass          239 Miscellaneous Device
  bDeviceSubClass         2 
  bDeviceProtocol         1 Interface Association
  bMaxPacketSize0        64
  idVendor           0x1778 
  idProduct          0x0214 
  bcdDevice            7.07
  iManufacturer           1 IPEVO Inc.
  iProduct                2 IPEVO Point 2 View
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0299
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.10
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      64
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x85  EP 5 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval              10
    Interface Association:
      bLength                 8
      bDescriptorType        11
      bFirstInterface         1
      bInterfaceCount         2
      bFunctionClass         14 Video
      bFunctionSubClass       3 Video Interface Collection
      bFunctionProtocol       0 
      iFunction               2 IPEVO Point 2 View
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass        14 Video
      bInterfaceSubClass      1 Video Control
      bInterfaceProtocol      0 
      iInterface              2 IPEVO Point 2 View
      VideoControl Interface Descriptor:
        bLength                13
        bDescriptorType        36
        bDescriptorSubtype      1 (HEADER)
        bcdUVC               1.00
        wTotalLength       0x0050
        dwClockFrequency        6.000000MHz
        bInCollection           1
        baInterfaceNr( 0)       2
      VideoControl Interface Descriptor:
        bLength                18
        bDescriptorType        36
        bDescriptorSubtype      2 (INPUT_TERMINAL)
        bTerminalID             1
        wTerminalType      0x0201 Camera Sensor
        bAssocTerminal          0
        iTerminal               0 
        wObjectiveFocalLengthMin      0
        wObjectiveFocalLengthMax      0
        wOcularFocalLength            0
        bControlSize                  3
        bmControls           0x000200a2
          Auto-Exposure Mode
          Focus (Absolute)
          Iris (Absolute)
          Focus, Auto
      VideoControl Interface Descriptor:
        bLength                11
        bDescriptorType        36
        bDescriptorSubtype      5 (PROCESSING_UNIT)
      Warning: Descriptor too short
        bUnitID                 3
        bSourceID               1
        wMaxMultiplier          0
        bControlSize            2
        bmControls     0x0000147f
          Brightness
          Contrast
          Hue
          Saturation
          Sharpness
          Gamma
          White Balance Temperature
          Power Line Frequency
          White Balance Temperature, Auto
        iProcessing             0 
        bmVideoStandards     0x1d
          None
          PAL - 625/50
          SECAM - 625/50
          NTSC - 625/50
      VideoControl Interface Descriptor:
        bLength                29
        bDescriptorType        36
        bDescriptorSubtype      6 (EXTENSION_UNIT)
        bUnitID                 4
        guidExtensionCode         {5a215226-3289-4156-894a-5c557cdf9664}
        bNumControl             4
        bNrPins                 1
        baSourceID( 0)          3
        bControlSize            4
        bmControls( 0)       0xff
        bmControls( 1)       0xff
        bmControls( 2)       0xff
        bmControls( 3)       0xff
        iExtension              0 
      VideoControl Interface Descriptor:
        bLength                 9
        bDescriptorType        36
        bDescriptorSubtype      3 (OUTPUT_TERMINAL)
        bTerminalID             2
        wTerminalType      0x0101 USB Streaming
        bAssocTerminal          0
        bSourceID               4
        iTerminal               0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               9
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass        14 Video
      bInterfaceSubClass      2 Video Streaming
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            5
          Transfer Type            Isochronous
          Synch Type               Asynchronous
          Usage Type               Data
        wMaxPacketSize     0x0000  1x 0 bytes
        bInterval               1
        INTERFACE CLASS:  0f 24 01 02 ea 01 82 00 02 02 01 00 01 00 00
        INTERFACE CLASS:  1b 24 04 01 07 59 55 59 32 00 00 10 00 80 00 00 aa 00 38 9b 71 10 01 00 00 00 00
        INTERFACE CLASS:  1e 24 05 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  1e 24 05 02 00 40 01 f0 00 c0 00 03 4b c0 00 03 4b 00 58 02 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  1e 24 05 03 00 20 03 58 02 70 00 14 99 70 00 14 99 00 a6 0e 00 9a 5b 06 00 01 9a 5b 06 00
        INTERFACE CLASS:  1e 24 05 04 00 00 04 00 03 00 00 16 80 00 00 16 80 00 00 18 00 2a 2c 0a 00 01 2a 2c 0a 00
        INTERFACE CLASS:  1e 24 05 05 00 00 05 00 04 00 00 12 c0 00 00 12 c0 00 00 28 00 d0 12 13 00 01 d0 12 13 00
        INTERFACE CLASS:  1e 24 05 06 00 40 06 b0 04 00 00 0e a6 00 00 0e a6 00 98 3a 00 a0 25 26 00 01 a0 25 26 00
        INTERFACE CLASS:  1e 24 05 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  0b 24 03 00 01 80 02 e0 01 01 00
        INTERFACE CLASS:  0b 24 06 02 07 00 01 00 00 00 00
        INTERFACE CLASS:  1e 24 07 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  1e 24 07 02 00 40 01 f0 00 c0 00 03 4b c0 00 03 4b 00 58 02 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  1e 24 07 03 00 20 03 58 02 70 00 14 99 70 00 14 99 00 a6 0e 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  1e 24 07 04 00 00 04 00 03 00 00 16 80 00 00 16 80 00 00 18 00 15 16 05 00 01 15 16 05 00
        INTERFACE CLASS:  1e 24 07 05 00 00 05 00 04 00 00 12 c0 00 00 12 c0 00 00 28 00 2a 2c 0a 00 01 2a 2c 0a 00
        INTERFACE CLASS:  1e 24 07 06 00 40 06 b0 04 00 00 0e a6 00 00 0e a6 00 98 3a 00 d0 12 13 00 01 d0 12 13 00
        INTERFACE CLASS:  1e 24 07 01 00 80 02 e0 01 00 00 0d 2f 00 00 0d 2f 00 60 09 00 0e 64 03 00 01 0e 64 03 00
        INTERFACE CLASS:  06 24 0d 00 00 00
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       1
      bNumEndpoints           1
      bInterfaceClass        14 Video
      bInterfaceSubClass      2 Video Streaming
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            5
          Transfer Type            Isochronous
          Synch Type               Asynchronous
          Usage Type               Data
        wMaxPacketSize     0x13fc  3x 1020 bytes
        bInterval               1
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass          239 Miscellaneous Device
  bDeviceSubClass         2 
  bDeviceProtocol         1 Interface Association
  bMaxPacketSize0        64
  bNumConfigurations      1
Device Status:     0x0000
  (Bus Powered)


dmesg lines on plugging:

[   95.016139] usb 1-1.4.3.3: new high-speed USB device number 13 using xhci_hcd
[   95.130236] usb 1-1.4.3.3: New USB device found, idVendor=1778, idProduct=0214, bcdDevice= 7.07
[   95.130241] usb 1-1.4.3.3: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[   95.130244] usb 1-1.4.3.3: Product: IPEVO Point 2 View
[   95.130246] usb 1-1.4.3.3: Manufacturer: IPEVO Inc.
[   95.133103] hid-generic 0003:1778:0214.000A: hiddev97,hidraw6: USB HID v1.10 Device [IPEVO Inc. IPEVO Point 2 View] on usb-0000:00:14.0-1.4.3.3/input0
[   95.133500] uvcvideo: Probing generic UVC device 1.4.3.3
[   95.133618] uvcvideo: trying extra data from endpoint 0.
[   95.133623] uvcvideo: Found format YUV 4:2:2 (YUYV).
[   95.133626] uvcvideo: - 640x480 (30.0 fps)
[   95.133629] uvcvideo: - 320x240 (30.0 fps)
[   95.133631] uvcvideo: - 800x600 (24.0 fps)
[   95.133633] uvcvideo: - 1024x768 (15.0 fps)
[   95.133635] uvcvideo: - 1280x1024 (8.0 fps)
[   95.133636] uvcvideo: - 1600x1200 (4.0 fps)
[   95.133638] uvcvideo: - 640x480 (30.0 fps)
[   95.133639] uvcvideo: Found format MJPEG.
[   95.133641] uvcvideo: - 640x480 (45.0 fps)
[   95.133642] uvcvideo: - 320x240 (45.0 fps)
[   95.133644] uvcvideo: - 800x600 (45.0 fps)
[   95.133645] uvcvideo: - 1024x768 (30.0 fps)
[   95.133647] uvcvideo: - 1280x1024 (15.0 fps)
[   95.133648] uvcvideo: - 1600x1200 (8.0 fps)
[   95.133649] uvcvideo: - 640x480 (45.0 fps)
[   95.133656] uvcvideo: Found a Status endpoint (addr 81).
[   95.133658] uvcvideo: Found UVC 1.00 device IPEVO Point 2 View (1778:0214)
[   95.133698] uvcvideo: Failed to query (GET_INFO) UVC control 2 on unit 1: -32 (exp. 1).
[   95.133763] uvcvideo: Control error 6
[   95.133766] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/2 to device 1.4.3.3 entity 1
[   95.133769] uvcvideo: Adding mapping 'Exposure, Auto' to control 00000000-0000-0000-0000-000000000001/2.
[   95.133815] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/6 to device 1.4.3.3 entity 1
[   95.133818] uvcvideo: Adding mapping 'Focus (absolute)' to control 00000000-0000-0000-0000-000000000001/6.
[   95.133860] uvcvideo: Failed to query (GET_INFO) UVC control 9 on unit 1: -32 (exp. 1).
[   95.133934] uvcvideo: Control error 6
[   95.133936] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/9 to device 1.4.3.3 entity 1
[   95.133939] uvcvideo: Adding mapping 'Iris, Absolute' to control 00000000-0000-0000-0000-000000000001/9.
[   95.133984] uvcvideo: Added control 00000000-0000-0000-0000-000000000001/8 to device 1.4.3.3 entity 1
[   95.133986] uvcvideo: Adding mapping 'Focus, Auto' to control 00000000-0000-0000-0000-000000000001/8.
[   95.134031] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/2 to device 1.4.3.3 entity 3
[   95.134033] uvcvideo: Adding mapping 'Brightness' to control 00000000-0000-0000-0000-000000000101/2.
[   95.134078] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/3 to device 1.4.3.3 entity 3
[   95.134080] uvcvideo: Adding mapping 'Contrast' to control 00000000-0000-0000-0000-000000000101/3.
[   95.134130] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/6 to device 1.4.3.3 entity 3
[   95.134132] uvcvideo: Adding mapping 'Hue' to control 00000000-0000-0000-0000-000000000101/6.
[   95.134178] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/7 to device 1.4.3.3 entity 3
[   95.134180] uvcvideo: Adding mapping 'Saturation' to control 00000000-0000-0000-0000-000000000101/7.
[   95.134225] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/8 to device 1.4.3.3 entity 3
[   95.134227] uvcvideo: Adding mapping 'Sharpness' to control 00000000-0000-0000-0000-000000000101/8.
[   95.134266] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/9 to device 1.4.3.3 entity 3
[   95.134269] uvcvideo: Adding mapping 'Gamma' to control 00000000-0000-0000-0000-000000000101/9.
[   95.134313] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/10 to device 1.4.3.3 entity 3
[   95.134316] uvcvideo: Adding mapping 'White Balance Temperature' to control 00000000-0000-0000-0000-000000000101/10.
[   95.134361] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/5 to device 1.4.3.3 entity 3
[   95.134364] uvcvideo: Adding mapping 'Power Line Frequency' to control 00000000-0000-0000-0000-000000000101/5.
[   95.134409] uvcvideo: Added control 00000000-0000-0000-0000-000000000101/11 to device 1.4.3.3 entity 3
[   95.134411] uvcvideo: Adding mapping 'White Balance Temperature, Auto' to control 00000000-0000-0000-0000-000000000101/11.
[   95.134415] uvcvideo: Scanning UVC chain: OT 2 <- XU 4 <- PU 3 <- IT 1
[   95.134422] uvcvideo: Found a valid video chain (1 -> 2).
[   95.135278] uvcvideo 1-1.4.3.3:1.1: Entity type for entity Extension 4 was not initialized!
[   95.135284] uvcvideo 1-1.4.3.3:1.1: Entity type for entity Processing 3 was not initialized!
[   95.135289] uvcvideo 1-1.4.3.3:1.1: Entity type for entity Camera 1 was not initialized!
[   95.135468] input: IPEVO Point 2 View: IPEVO Point as /devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1.4/1-1.4.3/1-1.4.3.3/1-1.4.3.3:1.1/input/input22
[   95.135632] uvcvideo: UVC device initialized.
[   95.173185] uvcvideo: uvc_v4l2_open
[   95.173224] uvcvideo: uvc_v4l2_release
[   97.532205] uvcvideo: Suspending interface 2
[   97.532210] uvcvideo: Suspending interface 1


# ls -l /dev/video*
crw-rw----+ 1 root video 81, 0 Jan  5 12:20 /dev/video0
crw-rw----+ 1 root video 81, 1 Jan  5 12:20 /dev/video1

Camera works.


-- 
============================================
Roger Whittaker
SUSE Linux Premium Support Engineer
 
roger.whittaker@suse.com
+44 7802 357081
 
SUSE Linux
One Station Square
Bracknell
RG12 1QB
United Kingdom
============================================

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0
  2020-01-05 12:28                               ` Roger Whittaker
@ 2020-01-06 15:43                                 ` Alan Stern
  2020-01-06 16:03                                   ` Johan Hovold
  2020-01-06 16:13                                   ` Laurent Pinchart
  0 siblings, 2 replies; 23+ messages in thread
From: Alan Stern @ 2020-01-06 15:43 UTC (permalink / raw)
  To: Greg KH
  Cc: Roger Whittaker, Laurent Pinchart, Takashi Iwai, Johan Hovold, USB list

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@suse.com>
Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2

---


[as1928]


 drivers/usb/core/config.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: usb-devel/drivers/usb/core/config.c
===================================================================
--- usb-devel.orig/drivers/usb/core/config.c
+++ usb-devel/drivers/usb/core/config.c
@@ -346,12 +346,16 @@ static int usb_parse_endpoint(struct dev
 			endpoint->desc.wMaxPacketSize = cpu_to_le16(8);
 	}
 
-	/* Validate the wMaxPacketSize field */
+	/*
+	 * Validate the wMaxPacketSize field.
+	 * Some devices have isochronous endpoints in altsetting 0;
+	 * the USB-2 spec requires such endpoints to have wMaxPacketSize = 0
+	 * (see the end of section 5.6.3), so don't warn about them.
+	 */
 	maxp = usb_endpoint_maxp(&endpoint->desc);
-	if (maxp == 0) {
-		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has wMaxPacketSize 0, skipping\n",
+	if (maxp == 0 && !(usb_endpoint_xfer_isoc(d) && asnum == 0)) {
+		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has invalid wMaxPacketSize 0\n",
 		    cfgno, inum, asnum, d->bEndpointAddress);
-		goto skip_to_next_endpoint_or_interface_descriptor;
 	}
 
 	/* Find the highest legal maxpacket size for this endpoint */


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0
  2020-01-06 15:43                                 ` [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0 Alan Stern
@ 2020-01-06 16:03                                   ` Johan Hovold
  2020-01-06 16:17                                     ` Alan Stern
  2020-01-06 16:13                                   ` Laurent Pinchart
  1 sibling, 1 reply; 23+ messages in thread
From: Johan Hovold @ 2020-01-06 16:03 UTC (permalink / raw)
  To: Alan Stern
  Cc: Greg KH, Roger Whittaker, Laurent Pinchart, Takashi Iwai,
	Johan Hovold, USB list

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@suse.com>
> Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2

Acked-by: Johan Hovold <johan@kernel.org>

We also need

Cc: stable <stable@vger.kernel.org>

as d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length") ended up
being (auto- ?) selected for stable.

Johan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0
  2020-01-06 15:43                                 ` [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0 Alan Stern
  2020-01-06 16:03                                   ` Johan Hovold
@ 2020-01-06 16:13                                   ` Laurent Pinchart
  2020-01-06 16:21                                     ` Alan Stern
  1 sibling, 1 reply; 23+ messages in thread
From: Laurent Pinchart @ 2020-01-06 16:13 UTC (permalink / raw)
  To: Alan Stern; +Cc: Greg KH, Roger Whittaker, Takashi Iwai, Johan Hovold, USB list

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@suse.com>
> Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2

The patch looks good to me, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

But shouldn't we also warn if maxp != 0 && usb_endpoint_xfer_isoc(d) &&
asnum == 0 ?

> ---
> 
> 
> [as1928]
> 
> 
>  drivers/usb/core/config.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> Index: usb-devel/drivers/usb/core/config.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/config.c
> +++ usb-devel/drivers/usb/core/config.c
> @@ -346,12 +346,16 @@ static int usb_parse_endpoint(struct dev
>  			endpoint->desc.wMaxPacketSize = cpu_to_le16(8);
>  	}
>  
> -	/* Validate the wMaxPacketSize field */
> +	/*
> +	 * Validate the wMaxPacketSize field.
> +	 * Some devices have isochronous endpoints in altsetting 0;
> +	 * the USB-2 spec requires such endpoints to have wMaxPacketSize = 0
> +	 * (see the end of section 5.6.3), so don't warn about them.
> +	 */
>  	maxp = usb_endpoint_maxp(&endpoint->desc);
> -	if (maxp == 0) {
> -		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has wMaxPacketSize 0, skipping\n",
> +	if (maxp == 0 && !(usb_endpoint_xfer_isoc(d) && asnum == 0)) {
> +		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has invalid wMaxPacketSize 0\n",
>  		    cfgno, inum, asnum, d->bEndpointAddress);
> -		goto skip_to_next_endpoint_or_interface_descriptor;
>  	}
>  
>  	/* Find the highest legal maxpacket size for this endpoint */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0
  2020-01-06 16:03                                   ` Johan Hovold
@ 2020-01-06 16:17                                     ` Alan Stern
  2020-01-06 19:12                                       ` Greg KH
  0 siblings, 1 reply; 23+ messages in thread
From: Alan Stern @ 2020-01-06 16:17 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg KH, Roger Whittaker, Laurent Pinchart, Takashi Iwai, USB list

On Mon, 6 Jan 2020, Johan Hovold wrote:

> 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@suse.com>
> > Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2
> 
> Acked-by: Johan Hovold <johan@kernel.org>
> 
> We also need
> 
> Cc: stable <stable@vger.kernel.org>
> 
> as d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length") ended up
> being (auto- ?) selected for stable.

Absolutely -- I had intended to add that CC: but it slipped my mind 
when the email was being prepared.

Alan Stern


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0
  2020-01-06 16:13                                   ` Laurent Pinchart
@ 2020-01-06 16:21                                     ` Alan Stern
  0 siblings, 0 replies; 23+ messages in thread
From: Alan Stern @ 2020-01-06 16:21 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Greg KH, Roger Whittaker, Takashi Iwai, Johan Hovold, USB list

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@suse.com>
> > Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2
> 
> The patch looks good to me, so
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> 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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0
  2020-01-06 16:17                                     ` Alan Stern
@ 2020-01-06 19:12                                       ` Greg KH
  0 siblings, 0 replies; 23+ messages in thread
From: Greg KH @ 2020-01-06 19:12 UTC (permalink / raw)
  To: Alan Stern
  Cc: Johan Hovold, Roger Whittaker, Laurent Pinchart, Takashi Iwai, USB list

On Mon, Jan 06, 2020 at 11:17:12AM -0500, Alan Stern wrote:
> On Mon, 6 Jan 2020, Johan Hovold wrote:
> 
> > 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@suse.com>
> > > Fixes: d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length")
> > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > > CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Link: https://marc.info/?l=linux-usb&m=157790377329882&w=2
> > 
> > Acked-by: Johan Hovold <johan@kernel.org>
> > 
> > We also need
> > 
> > Cc: stable <stable@vger.kernel.org>
> > 
> > as d482c7bb0541 ("USB: Skip endpoints with 0 maxpacket length") ended up
> > being (auto- ?) selected for stable.
> 
> Absolutely -- I had intended to add that CC: but it slipped my mind 
> when the email was being prepared.

I'll catch this when it hits Linus's tree.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-01-06 19:13 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200101144709.GA8389@suse.com>
     [not found] ` <20200101172449.GF6226@pendragon.ideasonboard.com>
     [not found]   ` <20200101175220.GA18140@suse.com>
2020-01-01 18:35     ` Certain cameras no longer working with uvcvideo on recent (openSUSE) kernels Laurent Pinchart
2020-01-01 18:47       ` Greg KH
2020-01-01 20:09         ` Alan Stern
2020-01-02 11:20           ` Johan Hovold
2020-01-02 13:11             ` Takashi Iwai
2020-01-02 15:06               ` Alan Stern
2020-01-02 15:32                 ` Johan Hovold
2020-01-02 18:24                   ` Alan Stern
2020-01-02 16:38                 ` Laurent Pinchart
2020-01-02 16:57                   ` Roger Whittaker
2020-01-02 17:03                     ` Laurent Pinchart
2020-01-02 17:49                       ` Alan Stern
2020-01-02 21:51                         ` Roger Whittaker
2020-01-02 23:11                         ` Laurent Pinchart
2020-01-03 15:13                           ` Alan Stern
2020-01-04 18:22                             ` Laurent Pinchart
2020-01-05 12:28                               ` Roger Whittaker
2020-01-06 15:43                                 ` [PATCH] USB: Fix: Don't skip endpoint descriptors with maxpacket=0 Alan Stern
2020-01-06 16:03                                   ` Johan Hovold
2020-01-06 16:17                                     ` Alan Stern
2020-01-06 19:12                                       ` Greg KH
2020-01-06 16:13                                   ` Laurent Pinchart
2020-01-06 16:21                                     ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).