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 22:06:50 +0200

On Tue, Jun 02, 2026 at 09:45:20PM +0200, Kirill A. Korinsky wrote:

> On Tue, 02 Jun 2026 20:07:20 +0200,
> Marcus Glocker <marcus@nazgul.ch> wrote:
> > 
> > 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 ...
> >  
> 
> Thanks! What's happened when you make a diff in 2 am when you discovered an
> email in bed instead of sleeping :-)
> 
> Ok?

ok mglocker@
 
> 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 19:43:43 -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,
> @@ -560,6 +590,9 @@ uvideo_attach(struct device *parent, str
>  
>  	sc->sc_udev = uaa->device;
>  
> +	/* maybe the device has quirks */
> +	sc->sc_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))
> @@ -569,6 +602,10 @@ uvideo_attach(struct device *parent, str
>  			continue;
>  		if (id->bInterfaceClass == UICLASS_VIDEO)
>  			break;
> +		if (sc->sc_quirk != NULL &&
> +		    sc->sc_quirk->flags & UVIDEO_FLAG_VENDOR_CLASS &&
> +		    id->bInterfaceClass == UICLASS_VENDOR)
> +			break;
>  	}
>  	if (i == uaa->nifaces) {
>  		printf("%s: can't find video interface\n", DEVNAME(sc));
> @@ -612,9 +649,6 @@ uvideo_attach(struct device *parent, str
>  	/* Remember our association by saving the first interface. */
>  	sc->sc_iface = iad->bFirstInterface;
>  	sc->sc_nifaces = iad->bInterfaceCount;
> -
> -	/* maybe the device has quirks */
> -	sc->sc_quirk = uvideo_lookup(uaa->vendor, uaa->product);
>  
>  	if (sc->sc_quirk && sc->sc_quirk->flags & UVIDEO_FLAG_NOATTACH) {
>  		printf("%s: device not supported\n", DEVNAME(sc));
> 
> 
> -- 
> wbr, Kirill
>