RE: [PATCH 18/28] usb: dwc3: ep0: fix Data Phase for transfer sizes aligned to wMaxPacketSize

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux