On Thu, Dec 19, 2013 at 11:32:08AM +0900, Anton Tikhomirov wrote:
> Hi,
>
> > 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.
>
> USB2.0 spec., 8.5.3.2 Variable-length Data Stage
>
> >
> > > 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.
>
> Our customer reported this issue. In their case, Windows USB2.0 host
> requests Configuration descriptor with wLength = 255. Device replies
> with two 64 bytes IN transactions, and stalls EP0 on 3rd IN transaction,
> where it has to reply with ZLP. Unfortunately, we don’t have full
> picture of what's happening on their side and why host requests
> more bytes than actual length of Configuration descriptor.
that's a bug on the host side.
> > > 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)
>
> By the way, why do we need this check? If r is NULL, we will have
> panic above in this function, where ur is dereferenced.
probably comes from earlier code.
> > 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.
>
> Thank you for review. I will rework the patch and test carefully.
Thanks for that, I appreciate it :-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
