linux-kernel - Re: [PATCH 4/4] Revert "usb: gadget: f_uvc: change endpoint allocation in uvc_function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Fri, 22 Dec 2023 22:42:14 -0500
From: Frank Li <Frank.li@....com>
To: yuan linyu <cugyly@....com>
Cc: peter.chen@...nel.org, a-govindraju@...com, gregkh@...uxfoundation.org,
	imx@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-usb@...r.kernel.org, pawell@...ence.com, rogerq@...nel.org
Subject: Re: [PATCH 4/4] Revert "usb: gadget: f_uvc: change endpoint
 allocation in uvc_function_bind()"

On Fri, Dec 22, 2023 at 08:26:13PM +0800, yuan linyu wrote:
> 
> On 2023/12/22 00:54, Frank Li wrote:
> > This reverts commit 3c5b006f3ee800b4bd9ed37b3a8f271b8560126e.
> >
> > gadget_is_{super|dual}speed() API check UDC controller capitblity. It
> > should pass down highest speed endpoint descriptor to UDC controller. So
> > UDC controller driver can reserve enough resource at check_config(),
> > especially mult and maxburst. So UDC driver (such as cdns3) can know need
> > at least (mult + 1) * (maxburst + 1) * wMaxPacketSize internal memory for
> > this uvc functions.
> >
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> >  drivers/usb/gadget/function/f_uvc.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> > index faa398109431f..cc4e08c8169b4 100644
> > --- a/drivers/usb/gadget/function/f_uvc.c
> > +++ b/drivers/usb/gadget/function/f_uvc.c
> > @@ -719,13 +719,21 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
> >  	}
> >  	uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
> >  
> > -	ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
> 
> how about all none-0 endpoint used for all uvc ?

Unit of CDNS UDC is package size, how many package size associated to EP.
Interrupt EP only change package size according to speed.

May have issue with other controller. 

Idealy, usb function should pass down all speeds setting by a API, the
composite driver check UDC again for difference connect speeds.

But it is quite big change. 

> 
> please add some comment if currently this is only way to fix your issue.

I can add commens. 

> 
> need it for stable ?

Suppose yes.

> 
> > +	if (gadget_is_superspeed(c->cdev->gadget))
> > +		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
> > +					  &uvc_ss_streaming_comp);
> > +	else if (gadget_is_dualspeed(cdev->gadget))
> > +		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
> > +	else
> > +		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
> > +
> >  	if (!ep) {
> >  		uvcg_info(f, "Unable to allocate streaming EP\n");
> >  		goto error;
> >  	}
> >  	uvc->video.ep = ep;
> >  
> > +	uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
> >  	uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
> >  	uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
> >  
>