On Wed, Dec 20, 2023 at 10:33:17PM +0800, yuan linyu wrote:
>
> On 2023/12/20 00:17, Frank Li wrote:
> > On Thu, Aug 03, 2023 at 05:10:49PM +0800, Linyu Yuan wrote:
> >> 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@xxxxxxxxxxx>
> >> ---
> >> v2: no change
> >>
> >> drivers/usb/gadget/function/f_uvc.c | 10 +---------
> >> 1 file changed, 1 insertion(+), 9 deletions(-)
> >>
> >> 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);
> > 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.
/**
* 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.
>
> with change before?
>
>
> >From my understanding, according to configfs gadget driver design, when find a endpoint, there is no
>
> working speed, this means each hardware endpoint should support all possible speeds.
> >
> > UDC will allocate too much internal memory to each EP. It may failure when
> > use ss config. Generally, ss config have bigger max package size.
> is there another way to solve your issue in your driver ?
> >
> > Frank
> >
> >> 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;
> >>
> >> --
> >> 2.17.1
> >>
>
