From: Marcus Glocker <marcus@nazgul.ch>
Subject: Re: uvideo.c Logitech QuickCam Pro5000 does not attach uvideo only uadio (7.9-current)
To: "Kirill A. Korinsky" <kirill@korins.ky>
Cc: OpenBSD tech <tech@openbsd.org>
Date: Tue, 2 Jun 2026 20:07:20 +0200

On Tue, Jun 02, 2026 at 06:26:36PM +0200, Kirill A. Korinsky wrote:

> UVIDEO_FLAG_VENDOR_CLASS has been broken since r1.147: attach selected
> only UICLASS_VIDEO interfaces, even after match accepted a device whose
> video control interface incorrectly reports UICLASS_VENDOR.
> 
> Restore the vendor class path in attach, and add the remaining Logitech
> vendor class UVC devices listed by Linux for which usbdevs already has
> product IDs.
> 
> Ok?

One observation;  Now we call uvideo_lookup() twice in uvideo_attach(),
and we introduce another 'quirk' variable, although we already have
sc->sc_quirk.  Can't we just move the existing uvideo_lookup() call in
uvideo_attach() up, as it is, and then just use sc->sc_quirk also for
this vendor class quirk check?

Limits the diff to still one uvideo_lookup() call in uvideo_attach(),
and no need to introduce a new variable, unless I am missing something
obvious ...
 
> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
> diff -u -p -r1.265 uvideo.c
> --- sys/dev/usb/uvideo.c	6 Sep 2025 13:45:41 -0000	1.265
> +++ sys/dev/usb/uvideo.c	2 Jun 2026 16:09:29 -0000
> @@ -387,11 +387,41 @@ const struct uvideo_devs {
>  	    UVIDEO_FLAG_ISIGHT_STREAM_HEADER
>  	},
>  	{   /* Incorrectly reports as bInterfaceClass=UICLASS_VENDOR */
> +	    { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMFUSION_1 },
> +	    NULL,
> +	    NULL,
> +	    UVIDEO_FLAG_VENDOR_CLASS
> +	},
> +	{   /* Incorrectly reports as bInterfaceClass=UICLASS_VENDOR */
> +	    { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMORBITMP_1 },
> +	    NULL,
> +	    NULL,
> +	    UVIDEO_FLAG_VENDOR_CLASS
> +	},
> +	{   /* Incorrectly reports as bInterfaceClass=UICLASS_VENDOR */
> +	    { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMNBPRO },
> +	    NULL,
> +	    NULL,
> +	    UVIDEO_FLAG_VENDOR_CLASS
> +	},
> +	{   /* Incorrectly reports as bInterfaceClass=UICLASS_VENDOR */
> +	    { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMPRO5K_1 },
> +	    NULL,
> +	    NULL,
> +	    UVIDEO_FLAG_VENDOR_CLASS
> +	},
> +	{   /* Incorrectly reports as bInterfaceClass=UICLASS_VENDOR */
>  	    { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMOEM_1 },
>  	    NULL,
>  	    NULL,
>  	    UVIDEO_FLAG_VENDOR_CLASS
>  	},
> +	{   /* Incorrectly reports as bInterfaceClass=UICLASS_VENDOR */
> +	    { USB_VENDOR_LOGITECH, USB_PRODUCT_LOGITECH_QUICKCAMOEM_2 },
> +	    NULL,
> +	    NULL,
> +	    UVIDEO_FLAG_VENDOR_CLASS
> +	},
>  	{   /* Infrared camera not supported */
>  	    { USB_VENDOR_CHICONY, USB_PRODUCT_CHICONY_IRCAMERA },
>  	    NULL,
> @@ -556,10 +586,13 @@ uvideo_attach(struct device *parent, str
>  	usb_interface_descriptor_t *id;
>  	const usb_descriptor_t *desc;
>  	struct usbd_desc_iter iter;
> +	const struct uvideo_devs *quirk;
>  	int i;
>  
>  	sc->sc_udev = uaa->device;
>  
> +	quirk = uvideo_lookup(uaa->vendor, uaa->product);
> +
>  	/* Find the first unclaimed video interface. */
>  	for (i = 0; i < uaa->nifaces; i++) {
>  		if (usbd_iface_claimed(sc->sc_udev, i))
> @@ -568,6 +601,10 @@ uvideo_attach(struct device *parent, str
>  		if (id == NULL)
>  			continue;
>  		if (id->bInterfaceClass == UICLASS_VIDEO)
> +			break;
> +		if (quirk != NULL &&
> +		    quirk->flags & UVIDEO_FLAG_VENDOR_CLASS &&
> +		    id->bInterfaceClass == UICLASS_VENDOR)
>  			break;
>  	}
>  	if (i == uaa->nifaces) {
> 
> -- 
> wbr, Kirill
>