diff mbox series

[v2,3/7] usb: gadget: f_uvc: change endpoint allocation in uvc_function_bind()

Message ID 20230803091053.9714-4-quic_linyyuan@quicinc.com
State Superseded
Headers show
Series remove some usage of gadget_is_{*}speed() API | expand

Commit Message

Linyu Yuan Aug. 3, 2023, 9:10 a.m. UTC
when call uvc_function_bind(), gadget still have no connection speed,
just follow other gadget function, use fs endpoint descriptor to allocate
a video endpoint, remove gadget_is_{super|dual}speed() API call.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v2: no change

 drivers/usb/gadget/function/f_uvc.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

Comments

yuan linyu Dec. 21, 2023, 2:14 p.m. UTC | #1
On 2023/12/21 00:02, Frank Li wrote:
>>> Some UDC driver use gadget_check_config() and match_ep() to allocate EP
>>> internal fifo memory resource, if only pass download full speed EP.
>> Could you share  the detail of problem ? do you mean find another different endpoint compared
> The problem is little bit complex. I try to use simple words.
>
> The background:
>
> Generally, UDC have some EP<0..15> and have some internal memory as FIFO.
> for example 16K.  You can simple assign EP<n> to 1K memory, which can hold
> whole package.
>
> But for UVC, some controller required internal FIFO hold whole frame data
>
> (mult+1) * (MaxBurst +1) * wPackageSize.
>
> For most case,  not every gadget use all 16 EPs. So you can assgin more
> memory into one EP, so it will reduce bus 'ping' package number and reduce
> NACK to improve transfer speed.
>
> The problem:
> pass fs_stream to udc driver, udc driver's check_config function will see
> mult and maxburst is 0. so only reserve 1K for ISO EP, but when try to 
> enable EP,  mult is 2, so there are not enough internal memory for it
> because more memory already assign to other EPs.
>
> Ideally, when gadget frame work can call check_config again when know
> usb speed, but it is not easy to fix it.
>
> Simple method use ss_stream_ep here and other function drviers. Super
> speed's package size is bigger than high/full speed. If resource can
> support super speed, it can support high/full speed.


I don't find any difference of uvc_ss_streaming_ep, uvc_hs_streaming_ep, uvc_fs_streaming_ep

descriptors. how difference happen in UDC ?


>
>
> /**
>  * gadget_is_superspeed() - return true if the hardware handles superspeed
>  * @g: controller that might support superspeed
>  */
>
> @max_speed: Highest speed the driver handles
>
> And according to gadget_is_superspeed() define, it indicate if udc
> controller support supersped, not link speed. 
>
> Orignial code is correct. If UDC support superspeed, then use ss_stream_ep.
>
> becasue superspeed is worse case compared as high and full speed.
>
> So I think original is correct.
>
> Frank.
>
>
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 5e919fb65833..c8e149f8315f 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -719,21 +719,13 @@  uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	}
 	uvc->enable_interrupt_ep = opts->enable_interrupt_ep;
 
-	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);
-
+	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;