On Wed, Feb 15, 2012 at 02:26:00PM -0800, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > Sent: Wednesday, February 15, 2012 3:38 AM > > > > From: Paul Zimmerman <paulz@xxxxxxxxxxxx> > > > > If we have a non-ISOC endpoint, we will not have a Link TRB > > pointing to the beginning of the TRB pool. On such endpoints, > > we don't want to let the driver wrap around the TRB pool > > otherwise controller will hang waiting for a valid TRB. > > > > Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx> > > Signed-off-by: Felipe Balbi <balbi@xxxxxx> > > --- > > > > Paul, are you happy with this fix instead ? I figured that > > the same condition will happen on the scatterlist case too. > > > > drivers/usb/dwc3/gadget.c | 13 +++++++++++-- > > 1 files changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index cbe9bef..62e4793 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -672,8 +672,17 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting) > > > > BUILD_BUG_ON_NOT_POWER_OF_2(DWC3_TRB_NUM); > > > > - /* the first request must not be queued */ > > - trbs_left = (dep->busy_slot - dep->free_slot) & DWC3_TRB_MASK; > > + /* > > + * Can't wrap around on a non-isoc EP since there's no > > + * link TRB > > + */ > > + if (!usb_endpoint_xfer_isoc(dep->desc) && > > + (dep->free_slot & DWC3_TRB_MASK) == DWC3_TRB_MASK) { > > + trbs_left = 1; > > + } else { > > + /* the first request must not be queued */ > > + trbs_left = (dep->busy_slot - dep->free_slot) & DWC3_TRB_MASK; > > + } > > > > /* > > * if busy & slot are equal than it is either full or empty. If we are > > -- > > 1.7.9 > > Hi Felipe, > > I don't think that will work. > > If there are 3 requests in the queue, and the initial free slot number > is DWC3_TRB_MASK - 1, then your check will pass. But after it enters the > loop, when it gets to the 3rd request it will still get wrapped. > > So either the check needs to be inside the loop like mine was, or you > need to account for the number of requests in the queue somehow. I think > if you made it: > > if (!usb_endpoint_xfer_isoc(dep->desc)) > trbs_left = DWC3_TRB_MASK - (dep->free_slot & DWC3_TRB_MASK); > > that might work? that's true, good catch. Will update the patch. Thanks -- balbi
Attachment:
signature.asc
Description: Digital signature
