Index | Thread | Search

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

Download raw body.

Thread
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?

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