Hi,
On Thu, 23 Oct 2014, Alan Stern wrote:
> On Thu, 23 Oct 2014, Felipe Balbi wrote:
>
> > here's v2:
> >
> > 8<--------------------------------------------------------------
> >
> > From 1080b54d66e3e77410b41732e76746ed8e2c01c7 Mon Sep 17 00:00:00
> 2001
> > From: Felipe Balbi <balbi@xxxxxx>
> > Date: Tue, 30 Sep 2014 10:39:14 -0500
> > Subject: [PATCH] usb: dwc3: ep0: fix Data Phase for transfer sizes
> aligned to
> > wMaxPacketSize
> >
> > According to Section 8.5.3.2 of the USB 2.0 specification,
> > a USB device must terminate a Data Phase with either a
> > short packet or a ZLP (if the previous transfer was
> > a multiple of wMaxPacketSize).
> >
> > For reference, here's what the USB 2.0 specification, section
> > 8.5.3.2 says:
> >
> > "
> > 8.5.3.2 Variable-length Data Stage
> >
> > A control pipe may have a variable-length data phase
> > in which the host requests more data than is contained
> > in the specified data structure. When all of the data
> > structure is returned to the host, the function should
> > indicate that the Data stage is ended by returning a
> > packet that is shorter than the MaxPacketSize for the
> > pipe. If the data structure is an exact multiple of
> > wMaxPacketSize for the pipe, the function will return
> > a zero-length packet to indicate the end of the Data
> > stage.
> > "
> >
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > drivers/usb/dwc3/ep0.c | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> > index a47cc1e..ce6b0c7 100644
> > --- a/drivers/usb/dwc3/ep0.c
> > +++ b/drivers/usb/dwc3/ep0.c
> > @@ -828,12 +828,19 @@ static void dwc3_ep0_complete_data(struct dwc3
> *dwc,
> >
> > dwc3_ep0_stall_and_restart(dwc);
> > } else {
> > - /*
> > - * handle the case where we have to send a zero packet.
> This
> > - * seems to be case when req.length > maxpacket. Could it
> be?
> > - */
> > - if (r)
> > - dwc3_gadget_giveback(ep0, r, 0);
> > + dwc3_gadget_giveback(ep0, r, 0);
>
> Don't you want to wait until the ZLP has completed before doing the
> giveback?
>
> In fact, shouldn't all this ZLP code run when the transfer is
> submitted, rather than when it completes? The idea is that you get a
> request, you queue up all the necessary TRBs or whatever, and if an
> extra ZLP is needed then you queue up an extra TRB.
You may want to check my patch one more time. I sent it for review 10
monthes ago:
[PATCH] usb: dwc3: ep0: Handle variable-length Data Stage
It works just fine for us in our custom kernel.
>
> > +
> > + if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket) &&
> > + ur->zero) {
>
> Is this correct in the case where ur->length is 0? When that happens,
> there should be only one ZLP, not two.
>
> > + int ret;
> > +
> > + dwc->ep0_next_event = DWC3_EP0_COMPLETE;
> > +
> > + ret = dwc3_ep0_start_trans(dwc, epnum,
> > + dwc->ctrl_req_addr, 0,
> > + DWC3_TRBCTL_CONTROL_DATA);
> > + WARN_ON(ret < 0);
> > + }
> > }
> > }
>
> Alan Stern
--
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
