Hi Bhupesh,
On Tuesday 12 February 2013 16:48:08 Bhupesh SHARMA wrote:
> > 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:
[snip]
> > > > > 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.
> >
> > 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..
What about the following patch ?
diff --git a/drivers/usb/gadget/uvc_video.c b/drivers/usb/gadget/uvc_video.c
index cd067a6..f5193cc 100644
--- a/drivers/usb/gadget/uvc_video.c
+++ b/drivers/usb/gadget/uvc_video.c
@@ -229,13 +229,18 @@ uvc_video_free_requests(struct uvc_video *video)
static int
uvc_video_alloc_requests(struct uvc_video *video)
{
+ unsigned int req_size;
unsigned int i;
int ret = -ENOMEM;
BUG_ON(video->req_size);
+ req_size = video->ep->maxpacket
+ * max_t(unsigned int, video->ep->maxburst, 1)
+ * (video->ep->mult + 1);
+
for (i = 0; i < UVC_NUM_REQUESTS; ++i) {
- video->req_buffer[i] = kmalloc(video->ep->maxpacket, GFP_KERNEL);
+ video->req_buffer[i] = kmalloc(req_size, GFP_KERNEL);
if (video->req_buffer[i] == NULL)
goto error;
@@ -252,7 +257,8 @@ uvc_video_alloc_requests(struct uvc_video *video)
list_add_tail(&video->req[i]->list, &video->req_free);
}
- video->req_size = video->ep->maxpacket;
+ video->req_size = req_size;
+
return 0;
error:
--
Regards,
Laurent Pinchart
--
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
