Something is really broken for Logitech webcams

Something is really broken for Logitech webcams

[Oliver Neukum]

Hi,

looking at bd21f0222adab
("USB: Fix chipmunk-like voice when using Logitech C270 for recording audio.")
we see that it introduced RESET_RESUME for the C270 webcam

For that camera I asked a user to provide lsusb, which got this
(a bit shortened)

Bus 001 Device 004: ID 046d:0825 Logitech, Inc. Webcam C270
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               2.00
   bDeviceClass          239 Miscellaneous Device
   bDeviceSubClass         2
   bDeviceProtocol         1 Interface Association
   bMaxPacketSize0        64
   idVendor           0x046d Logitech, Inc.
   idProduct          0x0825 Webcam C270

     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           1
       bInterfaceClass        14 Video
       bInterfaceSubClass      1 Video Control
       bInterfaceProtocol      0
       iInterface              0

So this thing says that it is UVC. That means that it should trigger
e387ef5c47dde ("usb: Add USB_QUIRK_RESET_RESUME for all Logitech UVC webcams")
adding the following declaration:

static const struct usb_device_id usb_interface_quirk_list[] = {
         /* Logitech UVC Cameras */
         { USB_VENDOR_AND_INTERFACE_INFO(0x046d, USB_CLASS_VIDEO, 1, 0),
           .driver_info = USB_QUIRK_RESET_RESUME },

wich AFAICT matches the device.

On the face of it, this makes no sense. What is going on here?
Do we really need to assume that Logitech has been selling a whole slew
of cameras in both UVC and something else, which we have no driver for,
but nobody complains?

Could somebody please look at this or am I somehow not right in the head?

And Marco specifically, did you make your patch based on a bisection?

	Regards
		Oliver

Re: Something is really broken for Logitech webcams

[marco]

July 27, 2023 at 1:48 PM, "Oliver Neukum" <oneukum@suse.com> wrote:


> 
> And Marco specifically, did you make your patch based on a bisection?
>

No, I just reverted an old commit that removed that webcam from the quirks list.

Best regards,
Marco

Re: Something is really broken for Logitech webcams

[Oliver Neukum]

On 27.07.23 19:40, marco@zatta.me wrote:
> July 27, 2023 at 1:48 PM, "Oliver Neukum" <oneukum@suse.com> wrote:
> 
> 
>>
>> And Marco specifically, did you make your patch based on a bisection?
>>
> 
> No, I just reverted an old commit that removed that webcam from the quirks list.

But you have good evidence that this has fixed the issue?
That is the problem was not so rare they extreme luck may have
been the explanation?

I am asking because something here really does not make sense.

	Regards
		Oliver

Re: Something is really broken for Logitech webcams

[Laurent Pinchart]

Hi Oliver,

On Thu, Jul 27, 2023 at 01:48:28PM +0200, Oliver Neukum wrote:
> Hi,
> 
> looking at bd21f0222adab
> ("USB: Fix chipmunk-like voice when using Logitech C270 for recording audio.")
> we see that it introduced RESET_RESUME for the C270 webcam
> 
> For that camera I asked a user to provide lsusb, which got this
> (a bit shortened)
> 
> Bus 001 Device 004: ID 046d:0825 Logitech, Inc. Webcam C270
> Device Descriptor:
>    bLength                18
>    bDescriptorType         1
>    bcdUSB               2.00
>    bDeviceClass          239 Miscellaneous Device
>    bDeviceSubClass         2
>    bDeviceProtocol         1 Interface Association
>    bMaxPacketSize0        64
>    idVendor           0x046d Logitech, Inc.
>    idProduct          0x0825 Webcam C270
> 
>      Interface Descriptor:
>        bLength                 9
>        bDescriptorType         4
>        bInterfaceNumber        0
>        bAlternateSetting       0
>        bNumEndpoints           1
>        bInterfaceClass        14 Video
>        bInterfaceSubClass      1 Video Control
>        bInterfaceProtocol      0
>        iInterface              0
> 
> So this thing says that it is UVC. That means that it should trigger
> e387ef5c47dde ("usb: Add USB_QUIRK_RESET_RESUME for all Logitech UVC webcams")
> adding the following declaration:
> 
> static const struct usb_device_id usb_interface_quirk_list[] = {
>          /* Logitech UVC Cameras */
>          { USB_VENDOR_AND_INTERFACE_INFO(0x046d, USB_CLASS_VIDEO, 1, 0),
>            .driver_info = USB_QUIRK_RESET_RESUME },
> 
> wich AFAICT matches the device.
> 
> On the face of it, this makes no sense. What is going on here?
> Do we really need to assume that Logitech has been selling a whole slew
> of cameras in both UVC and something else, which we have no driver for,
> but nobody complains?

The first Logitech webcams that were developed against the UVC
specification were not fully UVC-compatible. They were thus advertised
with a vendor-specific class. The uvcvideo driver supports them, you can
find the list in uvc_driver.c:

Logitech Quickcam Fusion			0x08c1
Logitech Quickcam Orbit MP			0x08c2
Logitech Quickcam Pro for Notebook		0x08c3
Logitech Quickcam Pro 5000			0x08c5
Logitech Quickcam OEM Dell Notebook		0x08c6
Logitech Quickcam OEM Cisco VT Camera II	0x08c7

This explains why usb_quirk_list has 6 corresponding entries.

Why Marco's patch is needed, I'm not sure. It seems that the device and
interface quirks are handled at different points of time, maybe that
plays a role ?

> Could somebody please look at this or am I somehow not right in the head?
> 
> And Marco specifically, did you make your patch based on a bisection?

-- 
Regards,

Laurent Pinchart