From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: sys/uvideo: support pre UVC 1.5 devices
To: Marcus Glocker <marcus@nazgul.ch>
Cc: tech@openbsd.org
Date: Tue, 12 Aug 2025 23:17:43 +0200

On Tue, 12 Aug 2025 22:48:32 +0200,
Marcus Glocker <marcus@nazgul.ch> wrote:
> 
> On Tue, Aug 12, 2025 at 06:30:50PM +0200, Kirill A. Korinsky wrote:
> 
> > From antoher hand clean it up, to avoid garbage from some devices seems not
> > that bad idea.
> 
> Well, fine for me.  Just wanted to understand if there is a specific
> reason for it.  But could you please tweak it a bit:
> 
> > > > +	/* Different UVC version defines different length of a probe */
> 
> A bit more clear comment would be nice, so we still understand this
> when reading it in two years.  Something like:
> 
> /*
>  * Different UVC version defines different length of a probe;
>  * Make sure that the unused porition of probe_data remains zero.
>  */
> 
> > > > +	bzero(probe_data + actlen,
> > > > +	    sizeof(struct usb_video_probe_commit ) - actlen);
> 
> Can you please remove the space here?           ^


Thanks for tweaks.

Like this?

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.260 uvideo.c
--- sys/dev/usb/uvideo.c	3 Aug 2025 20:00:11 -0000	1.260
+++ sys/dev/usb/uvideo.c	12 Aug 2025 21:15:53 -0000
@@ -2032,6 +2032,7 @@ uvideo_vs_get_probe(struct uvideo_softc 
 	usb_device_request_t req;
 	usbd_status error;
 	uint16_t tmp;
+	int actlen;
 	struct usb_video_probe_commit *pc;
 
 	req.bmRequestType = UVIDEO_GET_IF;
@@ -2044,13 +2045,21 @@ uvideo_vs_get_probe(struct uvideo_softc 
 
 	pc = (struct usb_video_probe_commit *)probe_data;
 
-	error = usbd_do_request(sc->sc_udev, &req, probe_data);
-	if (error) {
+	error = usbd_do_request_flags(sc->sc_udev, &req, probe_data,
+	    USBD_SHORT_XFER_OK, &actlen, USBD_DEFAULT_TIMEOUT);
+	if (error != USBD_NORMAL_COMPLETION) {
 		printf("%s: could not GET probe request: %s\n",
 		    DEVNAME(sc), usbd_errstr(error));
 		return (USBD_INVAL);
 	}
 
+	/*
+	 * Different UVC version defines different length of a probe;
+	 * Make sure that the unused porition of probe_data remains zero.
+	 */
+	bzero(probe_data + actlen,
+	    sizeof(struct usb_video_probe_commit) - actlen);
+
 	if (sc->sc_quirk &&
 	    sc->sc_quirk->flags & UVIDEO_FLAG_FORMAT_INDEX_IN_BMHINT &&
 	    UGETW(pc->bmHint) > 255) {
@@ -2065,7 +2074,8 @@ uvideo_vs_get_probe(struct uvideo_softc 
 		USETW(pc->bmHint, 1);
 	}
 
-	DPRINTF(1, "%s: GET probe request successfully\n", DEVNAME(sc));
+	DPRINTF(1, "%s: GET probe request successfully, length: %d\n",
+	    DEVNAME(sc), actlen);
 
 	DPRINTF(1, "bmHint=0x%02x\n", UGETW(pc->bmHint));
 	DPRINTF(1, "bFormatIndex=0x%02x\n", pc->bFormatIndex);
@@ -2081,6 +2091,26 @@ uvideo_vs_get_probe(struct uvideo_softc 
 	    UGETDW(pc->dwMaxVideoFrameSize));
 	DPRINTF(1, "dwMaxPayloadTransferSize=%d (bytes)\n",
 	    UGETDW(pc->dwMaxPayloadTransferSize));
+	DPRINTF(1, "dwClockFrequency=%d (Hz)\n",
+	    UGETDW(pc->dwClockFrequency));
+	DPRINTF(1, "bmFramingInfo=0x%02x\n", pc->bmFramingInfo);
+	DPRINTF(1, "bPreferedVersion=%d\n", pc->bPreferedVersion);
+	DPRINTF(1, "bMinVersion=%d\n", pc->bMinVersion);
+	DPRINTF(1, "bMaxVersion=%d\n", pc->bMaxVersion);
+	DPRINTF(1, "bUsage=%d\n", pc->bUsage);
+	DPRINTF(1, "bBitDepthLuma=%d\n", pc->bBitDepthLuma);
+	DPRINTF(1, "bmSettings=0x%02x\n", pc->bmSettings);
+	DPRINTF(1, "bMaxNumberOfRefFramesPlus1=%d\n",
+	    pc->bMaxNumberOfRefFramesPlus1);
+	DPRINTF(1, "bmRateControlModes=%d\n",
+	    UGETW(pc->bmRateControlModes));
+#ifdef UVIDEO_DEBUG
+	if (1 <= uvideo_debug) {
+		printf("bmLayoutPerStream=0x");
+		uvideo_hexdump(pc->bmLayoutPerStream,
+		    sizeof(pc->bmLayoutPerStream), 1);
+	}
+#endif
 
 	return (USBD_NORMAL_COMPLETION);
 }
Index: sys/dev/usb/uvideo.h
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.h,v
diff -u -p -r1.67 uvideo.h
--- sys/dev/usb/uvideo.h	3 Aug 2025 20:00:11 -0000	1.67
+++ sys/dev/usb/uvideo.h	8 Aug 2025 12:18:43 -0000
@@ -275,7 +275,7 @@ struct usb_video_color_matching_descr {
 	uByte	bMatrixCoefficients;
 } __packed;
 
-/* Table 4-47: Video Probe and Commit Controls */
+/* Table 4-75: Video Probe and Commit Controls */
 struct usb_video_probe_commit {
 	uWord	bmHint;
 	uByte	bFormatIndex;
@@ -293,6 +293,12 @@ struct usb_video_probe_commit {
 	uByte	bPreferedVersion;
 	uByte	bMinVersion;
 	uByte	bMaxVersion;
+	uByte	bUsage;
+	uByte	bBitDepthLuma;
+	uByte	bmSettings;
+	uByte	bMaxNumberOfRefFramesPlus1;
+	uWord	bmRateControlModes;
+	uByte	bmLayoutPerStream[8];
 } __packed;
 
 /*