On Thu, 21 Aug 2014, Peter Chen wrote:
> On Wed, Aug 20, 2014 at 03:18:47PM -0400, Alan Stern wrote:
> > On Wed, 20 Aug 2014, Felipe Balbi wrote:
> >
> > > > > > --- a/drivers/usb/gadget/composite.c
> > > > > > +++ b/drivers/usb/gadget/composite.c
> > > > > > @@ -1956,6 +1956,7 @@ void composite_dev_cleanup(struct
> > > > > > usb_composite_dev *cdev)
> > > > > > }
> > > > > > if (cdev->req) {
> > > > > > kfree(cdev->req->buf);
> > > > > > + usb_ep_dequeue(cdev->gadget->ep0, cdev->req);
> > > > >
> > > > > it's best to dequeue the request before freeing its buffer.
> > > >
> > > > In fact, it's best to wait for the request's completion routine to be
> > > > called before freeing the buffer. The hardware can access the buffer's
> > > > memory at any time before the completion routine runs.
> > >
> > > dequeue should cause the transfer to be cancelled and given back.
> > > There's a slight possible race window because we release the controller
> > > lock in order to call gadget driver's ->complete().
> > >
>
> The dp has already been pulled down and the flush pending FIFO buffer
> has finished, so the controller should not try to access memory buffer
> after that.
In that case, why does the patch add a call to usb_ep_dequeue()?
Shouldn't the request be unlinked already?
> > > Other than that, it should be fine. No ?
> >
> > Dequeue causes the transfer to be cancelled and given back, yes. But
> > usb_ep_dequeue() is allowed to return before those things happen.
> >
>
> If usb_ep_dequeue is returned before doing any flush and cancel
> transfer, it means there is no request on the queue, we don't need
> to cancel any requests.
It can be different for different UDCs. The documentation doesn't say
that usb_ep_dequeue has to wait until the transfer has been cancelled
and flushed.
Alan Stern
--
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
