Re: uvideo.c Logitech QuickCam Pro5000 does not attach uvideo only uadio (7.9-current)

Marcus Glocker <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <h6iqoemig4kcnwb3llr5l2jz2nliaogxybxg32ykbrqk7tlamf@kgotfey7hcxb>
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 <[email protected]> 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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.