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

On Sun, Feb 23, 2025 at 02:51:51PM GMT, Kirill A. Korinsky wrote:

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

ok mglocker@
 
> 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
>