Re: [PATCH 4/4] Revert "usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()"

[Frank Li]

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@xxxxxxx>
> > ---
> >  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;
> >  
>