Hi,

On Tue, Dec 17, 2013 at 03:59:31PM +0900, Anton Tikhomirov wrote:
> In accordance with specification, when sent data length is

please mention section of specification.

> an exact multiple of wMaxPacketSize for the pipe and less
> than requested by host, the function shall return a zero-length
> packet (ZLP) to indicate the end of the Data stage to a USB host.

hmm... so in USB3 mode that would be host requesting 513 bytes and us
sending only 512.

> @@ -619,6 +619,7 @@ struct dwc3_scratchpad_array {
>   * @three_stage_setup: set if we perform a three phase setup
>   * @ep0_bounced: true when we used bounce buffer
>   * @ep0_expect_in: true when we expect a DATA IN transfer
> + * @ep0_zlp_sent: true when ZLP was sent

I would rather have a ep0_needs_zlp flag.

> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 21a3520..cf72561 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -782,6 +782,9 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>               return;
>       }
>  
> +     if (dwc->ep0_zlp_sent)
> +             goto finish_zlp;
> +
>       length = trb->size & DWC3_TRB_SIZE_MASK;
>  
>       if (dwc->ep0_bounced) {
> @@ -802,14 +805,24 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>               /* for some reason we did not get everything out */
>  
>               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);
> +             return;
>       }
> +
> +     /* handle the case where we have to send a zero packet */
> +     if ((epnum & 1) && ur->zero &&
> +         (ur->length % ep0->endpoint.maxpacket == 0)) {
> +             int ret;
> +
> +             ret = dwc3_ep0_start_trans(dwc, epnum, dwc->ctrl_req_addr, 0,
> +                             DWC3_TRBCTL_CONTROL_DATA);
> +             WARN_ON(ret < 0);
> +             dwc->ep0_zlp_sent = 1;
> +             return;
> +     }

note that this causes a slight bug. Code expects to receive a
NRDY_STATUS, but we're sending another CONTROL_DATA which means we will
receive XFER_COMPLETE_DATA.

It's only working because Control(Data) lost its XferNotReady handling
due to a silicon bug we found. If someone ever patches that handler,
this will be a hard-to-track problem.

how have you tested this ? Any changes to ep0 handling must come with a
libusb-based testcase so I can make sure nothing else gets broken (yes,
new requirement :-)

Also make sure to run testusb for control endpoints and leave it running
for weeks. You should be able to survive 4 weeks of stress test without
any issues.

Don't forget to run through USB30CV ch9 on USB3 and USB2 modes.

Sorry dude, but I can't accept regressions and this code has been
exercised pretty well.

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to