From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: sys/uvideo: compute max_packet_size the same way in all cases
To: Marcus Glocker <marcus@nazgul.ch>
Cc: tech@openbsd.org
Date: Sun, 23 Feb 2025 14:51:51 +0100

On Sun, 23 Feb 2025 14:46:05 +0100,
Marcus Glocker <marcus@nazgul.ch> wrote:
> 
> > Index: sys/dev/usb/uvideo.c
> > ===================================================================
> > RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
> > diff -u -p -r1.238 uvideo.c
> > --- sys/dev/usb/uvideo.c	15 Feb 2025 09:05:15 -0000	1.238
> > +++ sys/dev/usb/uvideo.c	15 Feb 2025 14:10:50 -0000
> > @@ -114,6 +114,8 @@ void		uvideo_attach(struct device *, str
> >  void		uvideo_attach_hook(struct device *);
> >  int		uvideo_detach(struct device *, int);
> > 
> > +uint32_t	uvideo_vc_parse_max_packet_size(usb_endpoint_descriptor_t *);
> > +
> 
> Can you please sort the function prototype to where you have inserted
> the new function to?
>

Make sense. Like this?

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.239 uvideo.c
--- sys/dev/usb/uvideo.c	23 Feb 2025 08:28:57 -0000	1.239
+++ sys/dev/usb/uvideo.c	23 Feb 2025 13:50:24 -0000
@@ -138,6 +138,7 @@ usbd_status	uvideo_vs_parse_desc_format_
 usbd_status	uvideo_vs_parse_desc_frame(struct uvideo_softc *);
 usbd_status	uvideo_vs_parse_desc_frame_sub(struct uvideo_softc *,
 		    const usb_descriptor_t *);
+uint32_t	uvideo_vc_parse_max_packet_size(usb_endpoint_descriptor_t *);
 usbd_status	uvideo_vs_parse_desc_alt(struct uvideo_softc *, int, int, int);
 usbd_status	uvideo_vs_set_alt(struct uvideo_softc *,
 		    struct usbd_interface *, int);
@@ -1235,6 +1236,19 @@ uvideo_vs_parse_desc_frame_sub(struct uv
 	return (USBD_NORMAL_COMPLETION);
 }
 
+uint32_t
+uvideo_vc_parse_max_packet_size(usb_endpoint_descriptor_t *ed)
+{
+	uint32_t psize;
+
+	/* XXX: get USB 3.0 speed from wBytesPerInterval */
+
+	psize = UGETW(ed->wMaxPacketSize);
+	psize = UE_GET_SIZE(psize) * (1 + UE_GET_TRANS(psize));
+
+	return psize;
+}
+
 usbd_status
 uvideo_vs_parse_desc_alt(struct uvideo_softc *sc, int vs_nr, int iface, int numalts)
 {
@@ -1245,6 +1259,7 @@ uvideo_vs_parse_desc_alt(struct uvideo_s
 	usb_endpoint_descriptor_t *ed;
 	uint8_t ep_dir, ep_type;
 	int bulk_endpoint;
+	uint32_t psize;
 
 	vs = &sc->sc_vs_coll[vs_nr];
 
@@ -1306,13 +1321,14 @@ uvideo_vs_parse_desc_alt(struct uvideo_s
 		if (bulk_endpoint && !vs->bulk_endpoint)
 			goto next;
 
+		psize = uvideo_vc_parse_max_packet_size(ed);
 		/* save endpoint with largest bandwidth */
-		if (UGETW(ed->wMaxPacketSize) > vs->psize) {
+		if (psize > vs->psize) {
 			vs->ifaceh = &sc->sc_udev->ifaces[iface];
 			vs->endpoint = ed->bEndpointAddress;
 			vs->numalts = numalts;
 			vs->curalt = id->bAlternateSetting;
-			vs->psize = UGETW(ed->wMaxPacketSize);
+			vs->psize = psize;
 			vs->iface = iface;
 			vs->bulk_endpoint = bulk_endpoint;
 		}
@@ -1383,8 +1399,7 @@ uvideo_vs_set_alt(struct uvideo_softc *s
 			goto next;
 
 		/* save endpoint with requested bandwidth */
-		psize = UGETW(ed->wMaxPacketSize);
-		psize = UE_GET_SIZE(psize) * (1 + UE_GET_TRANS(psize));
+		psize = uvideo_vc_parse_max_packet_size(ed);
 		if (psize >= max_packet_size)
 			diff = psize - max_packet_size;
 		else