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.
> +
> + 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
