Hi Laurent,
> Hi Bhupesh,
>
> On Monday 11 February 2013 21:27:50 Bhupesh SHARMA wrote:
> > On Friday, February 08, 2013 4:22 AM Laurent Pinchart wrote:
> > > 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
> > > > orIsochronous:
> > > >
> > > > - 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.sha...@st.com>
> > > > ---
> > > >
> > > > 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 <ba...@ti.com>
> >
> > 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.
>
> For SS, sure, but if you connect the gadget to a USB 2.0 host maxburst will be
> set to 0, so buffer allocation will break.
>
> Please make sure you test all your patches with both USB 2.0 and USB 3.0
> hosts.
>
Ok. So will something like this suffice:
video->req_buffer[i] = kmalloc(video->ep->maxpacket *
(video->ep->comp_desc ?
video->ep->maxburst : 1) *
(video->ep->mult + 1),
GFP_KERNEL);
As ep->comp_desc will be NULL for HS and FS devices..
Please let me know.
Regards,
Bhupesh
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
