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>
---
Hi Felipe,
My proposed change wasn't quite right. I have verified that the
following patch works for me.
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index d208fc8..420b913 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -890,7 +890,7 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
{
struct dwc3_request *req, *n;
- u32 trbs_left;
+ u32 trbs_left, max;
unsigned int last_one = 0;
BUILD_BUG_ON_NOT_POWER_OF_2(DWC3_TRB_NUM);
@@ -898,6 +898,13 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
/* 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)) {
+ max = DWC3_TRB_NUM - (dep->free_slot & DWC3_TRB_MASK);
+ if (trbs_left > max)
+ trbs_left = max;
+ }
+
/*
* If busy & slot are equal than it is either full or empty. If we are
* starting to process requests then we are empty. Otherwise we are
--
Paul
> From: Felipe Balbi [mailto:balbi@xxxxxx]
> Sent: Wednesday, February 15, 2012 11:12 PM
> 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
--
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
