Hi,
On Mon, Aug 29, 2011 at 01:28:50PM +0200, Sebastian Andrzej Siewior wrote:
> * Felipe Balbi | 2011-08-29 11:51:34 [+0300]:
>
> >diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> >index a6fc5c3..9d8ec02 100644
> >--- a/drivers/usb/dwc3/ep0.c
> >+++ b/drivers/usb/dwc3/ep0.c
> >@@ -185,10 +185,27 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
> > req->epnum = dep->number;
> >
> > list_add_tail(&req->list, &dep->request_list);
> >- dwc3_map_buffer_to_dma(req);
> >+ if (req->request.length == 0) {
> >+ ret = dwc3_ep0_start_trans(dwc, dep->number,
> >+ dwc->ctrl_req_addr, 0);
> >+ } else if ((req->request.length % dep->endpoint.maxpacket)
> >+ && (dep->number == 0)) {
>
> This "problem" is not limited to ep0. And I would beat the gadget
> drivers to queue [bw]MaxPacketSize0? length requests even if they know
> that they expect only 5 bytes of data. Otherwise this workaround as to
> be implemented in other driver as well. It gets a little more
> complicated if the requests is maxpacket+3 or so.
>
> So I'm for fixing the gadget.
We could go that path, sure. But then this means that gadget driver will
be doing something different than requested on specification, just
because this one controller is quirky.
A better approach might be to introduce a "quirks" field on struct
usb_gadget, as have gadget drivers check for that. Something like:
if (gadget->quirks & GADGET_NEEDS_ALIGNED_OUT)
req->length = gadget->ep0->maxpacket;
else
req->length = len;
but then, gadget driver needs to cope with whatever size we might need
to send. In case of most gadget drivers, it's very simple because they
will only receive data which is less than or equal to wMaxPacketSize,
but on gadget drivers such as DFU, we might need to be careful here.
> >+ dwc->ep0_bounced = true;
> >+
> >+ /*
> >+ * REVISIT in case request length is bigger than EP0
> >+ * wMaxPacketSize, we will need two chained TRBs to handle
> >+ * the transfer.
> >+ */
>
> A warn_on() in case we hit the problem so we know we need to fix this
> _now_.
sure, why not.
> >+ ret = dwc3_ep0_start_trans(dwc, dep->number,
> >+ dwc->ep0_bounce_addr, dep->endpoint.maxpacket);
> >+ } else {
> >+ dwc3_map_buffer_to_dma(req);
> >+
> >+ ret = dwc3_ep0_start_trans(dwc, dep->number,
> >+ req->request.dma, req->request.length);
> >+ }
> >
> >- ret = dwc3_ep0_start_trans(dwc, dep->number, req->request.dma,
> >- req->request.length);
> > if (ret < 0) {
> > list_del(&req->list);
> > dwc3_unmap_buffer_from_dma(req);
> >@@ -658,6 +675,11 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
> > transferred = ur->length - trb.length;
> > ur->actual += transferred;
> >
> >+ if (dwc->ep0_bounced) {
>
> transferred = min(req->length, dep->endpoint.maxpacket - trb.length)
yeah, good point.
--
balbi
Attachment:
signature.asc
Description: Digital signature
