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, Oct 23, 2014 at 10:18:41PM -0500, Felipe Balbi wrote:
> > HI,
> >
> > On Fri, Oct 24, 2014 at 10:14:19AM +0900, Anton Tikhomirov wrote:
> > >
> > > 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.
> >
> > you also said you'd send another version (see [1]) and never did. 10
> > months passed and I had even forgotten about this issue until I
> decided
> > to run all gadget drivers through USB[23]0CV and found that g_ncm
> falls
> > into this same bug, so I wrote the patch above.

I'm sorry about this. I was busy with current project at that time and
finally forgot about this issue too.

> >
> > considering you never sent another version even after 10 months, I'll
> > just go ahead with this one and work on improving TRB handling on
> this
> > driver so that when req->zero is true we can actually allocate a
> > separate TRB (as has been discussed on this and previous threads).
> >
> > I'll add a note to commit log stating that you provided the original
> > patch but failed to provide a follow up.
> 
> actually, I can't do that anymore as I have already moved this commit
> to
> my 'fixes' branch.

It's ok

> 
> --
> balbi

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