Hi Laurent, > -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx] > Sent: Friday, February 08, 2013 4:22 AM > To: Bhupesh SHARMA > Cc: linux-usb@xxxxxxxxxxxxxxx; balbi@xxxxxx > Subject: Re: [PATCH V3 2/5] usb: gadget/uvc: Make video streaming buffer > size compliant to USB3.0 SS specs > > Hi Bhupesh, > > Thank you for the patch. > > On Thursday 17 January 2013 16:23:50 Bhupesh Sharma wrote: > > As per the USB3.0 specs, the bandwidth requirements of a UVC's video > > streaming endpoint will change to support super-speed. These changes > > will be dependent on whether the UVC video streaming endpoint is Bulk > > or > > Isochronous: > > > > - If video streaming endpoint is Isochronous: > > As per Section 4.4.8.2 (Isochronous Transfer Bandwidth Requirements) > > of the USB3.0 specs: > > A SuperSpeed isochronous endpoint can move up to three burst > > transactions of up to 16 maximum sized packets (3 * 16 * 1024 bytes) > > per service interval. > > > > - If video streaming endpoint is Bulk: > > As per 4.4.6.1 (Bulk Transfer Data Packet Size) of the USB3.0 specs: > > An endpoint for bulk transfers shall set the maximum data packet > > payload size in its endpoint descriptor to 1024 bytes. > > It also specifies the burst size that the endpoint can accept from or > > transmit on the SuperSpeed bus. The allowable burst size for a > > bulk endpoint shall be in the range of 1 to 16. > > > > So, in the Isochronous case, we can define the USB request's buffer to > > be equal to = (Maximum packet size) * (bMaxBurst + 1) * (Mult + 1), so > > that the UDC driver can try to send out this buffer in one Isochronous > > service interval. > > > > The same computation will hold good for the Bulk case as the Mult > > value is 0 here and we can have a USB request buffer of maximum > > 16 * 1024 bytes size, which can be sent out by the UDC driver as per > > the Bulk bandwidth allocation on the USB3 bus. > > > > This patch adds the above-mentioned support and is also USB2.0 > > backward compliant. > > This looks good to me, except... > > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxx> > > --- > > drivers/usb/gadget/uvc_video.c | 9 +++++++-- > > 1 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/gadget/uvc_video.c > > b/drivers/usb/gadget/uvc_video.c index b0e53a8..f7d1913 100644 > > --- a/drivers/usb/gadget/uvc_video.c > > +++ b/drivers/usb/gadget/uvc_video.c > > @@ -235,7 +235,10 @@ uvc_video_alloc_requests(struct uvc_video > *video) > > BUG_ON(video->req_size); > > > > for (i = 0; i < UVC_NUM_REQUESTS; ++i) { > > - video->req_buffer[i] = kmalloc(video->ep->maxpacket, > GFP_KERNEL); > > + video->req_buffer[i] = kmalloc(video->ep->maxpacket * > > + video->ep->maxburst * > > If I'm not mistaken maxburst is 0 for FS and HS endpoints (from > config_ep_by_speed in drivers/usb/gadget/composite.c). The buffer size > will then be pretty small :-) > Actually my initial patch had this approach: video->req_buffer[i] = kmalloc(video->ep->maxpacket * (video->ep->maxburst + 1) * (video->ep->mult + 1)) but due to a patch from Felipe: usb: gadget: composite: fix ep->maxburst initialization bMaxBurst field on endpoint companion descriptor is supposed to contain the number of burst minus 1. When passing that to controller drivers, we should be passing the real number instead (by incrementing 1). While doing that, also fix the assumption on dwc3 that value comes decremented by one. Signed-off-by: Felipe Balbi <balbi@xxxxxx> as we have now: _ep->maxburst = comp_desc->bMaxBurst + 1; So, even if comp_desc->bMaxBurst is 0, _ep->maxburst will be 1. So, this patch seems correct to me. Regards, Bhupesh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
