| Message ID | 1543662811-5194-6-git-send-email-anurag.kumar.vulisha@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc3: Fix broken BULK stream support to dwc3 gadget driver | expand |
Hi, Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> writes: > @@ -2487,6 +2497,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, > } > > switch (event->endpoint_event) { > + case DWC3_DEPEVT_XFERCOMPLETE: > + if (!dep->stream_capable) > + break; > + dep->flags &= ~DWC3_EP_TRANSFER_STARTED; > + /* Fall Through */ instead, let's add a proper handler for this: dwc3_gadget_endpoint_transfer_complete(dep, event); That handler should be "self-sufficient". IOW, we shouldn't have this fall through here. This means that the other patch where you modify dwc3_gadget_transfer_in_progress() shouldn't be necessary, since that event shouldn't run for stream capable endpoints. While rewriting the patches, please rebase on my testing/next as I have applied a few of the patches in this series.
HI Felipe, >-----Original Message----- >From: Felipe Balbi [mailto:balbi@kernel.org] >Sent: Wednesday, December 05, 2018 2:32 PM >To: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman ><gregkh@linuxfoundation.org>; Shuah Khan <shuah@kernel.org>; Alan Stern ><stern@rowland.harvard.edu>; Johan Hovold <johan@kernel.org>; Jaejoong Kim ><climbbb.kim@gmail.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; >Roger Quadros <rogerq@ti.com>; Manu Gautam <mgautam@codeaurora.org>; >martin.petersen@oracle.com; Bart Van Assche <bvanassche@acm.org>; Mike >Christie <mchristi@redhat.com>; Matthew Wilcox <willy@infradead.org>; Colin Ian >King <colin.king@canonical.com> >Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; >v.anuragkumar@gmail.com; Thinh Nguyen <thinhn@synopsys.com>; Tejas Joglekar ><tejas.joglekar@synopsys.com>; Ajay Yugalkishore Pandey <APANDEY@xilinx.com>; >Anurag Kumar Vulisha <anuragku@xilinx.com> >Subject: Re: [PATCH v7 05/10] usb: dwc3: make controller clear transfer resources >after complete > > >Hi, > >Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> writes: >> @@ -2487,6 +2497,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, >> } >> >> switch (event->endpoint_event) { >> + case DWC3_DEPEVT_XFERCOMPLETE: >> + if (!dep->stream_capable) >> + break; >> + dep->flags &= ~DWC3_EP_TRANSFER_STARTED; >> + /* Fall Through */ > >instead, let's add a proper handler for this: > >dwc3_gadget_endpoint_transfer_complete(dep, event); > >That handler should be "self-sufficient". IOW, we shouldn't have this >fall through here. This means that the other patch where you modify >dwc3_gadget_transfer_in_progress() shouldn't be necessary, since that >event shouldn't run for stream capable endpoints. > Thanks for your suggestion, will implement the changes as said and send the patches >While rewriting the patches, please rebase on my testing/next as I have >applied a few of the patches in this series. Okay , will rebase on top of testing/next and send the patches Best Regards, Anurag Kumar Vulisha
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 3171c24..3edfc0b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -579,7 +579,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, unsigned int action) if (usb_ss_max_streams(comp_desc) && usb_endpoint_xfer_bulk(desc)) { params.param1 |= DWC3_DEPCFG_STREAM_CAPABLE - | DWC3_DEPCFG_STREAM_EVENT_EN; + | DWC3_DEPCFG_STREAM_EVENT_EN + | DWC3_DEPCFG_XFER_COMPLETE_EN; dep->stream_capable = true; } @@ -1005,6 +1006,15 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, if (chain) trb->ctrl |= DWC3_TRB_CTRL_CHN; + /* + * To issue start transfer on another stream, controller need to free + * previously acquired transfer resource. Setting the LST bit in + * last TRB makes the controller clear transfer resource for that + * endpoint, allowing to start another stream on that endpoint. + */ + else if (dep->stream_capable) + trb->ctrl |= DWC3_TRB_CTRL_LST; + if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable) trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id); @@ -2276,7 +2286,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, if (event->status & DEPEVT_STATUS_SHORT && !chain) return 1; - if (event->status & DEPEVT_STATUS_IOC) + if (event->status & (DEPEVT_STATUS_IOC | DEPEVT_STATUS_LST)) return 1; return 0; @@ -2487,6 +2497,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, } switch (event->endpoint_event) { + case DWC3_DEPEVT_XFERCOMPLETE: + if (!dep->stream_capable) + break; + dep->flags &= ~DWC3_EP_TRANSFER_STARTED; + /* Fall Through */ case DWC3_DEPEVT_XFERINPROGRESS: dwc3_gadget_endpoint_transfer_in_progress(dep, event); break; @@ -2505,7 +2520,6 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, if (event->status == DEPEVT_STREAMEVT_FOUND) dwc3_endpoint_stream_event(dwc, event); - case DWC3_DEPEVT_XFERCOMPLETE: case DWC3_DEPEVT_RXTXFIFOEVT: break; }
To start transfer with another stream id, controller needs to free previously allocated transfer resource. This will be automatically done by the controller at the time of XferComplete Event. This patch updates the code to issue XferComplete event once all transfers are done by setting LST bit in the ctrl field of the last TRB. Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> --- Changes in v7: 1. Reverted to dep->stream_capable from dep->endpoint.stream_capable Changes in v6: 1. Used dep->endpoint.stream_capable instead of dep->stream_capable flag Changes in v5: 1. None Changes in v4: 1. None Changes in v3: 1. Added the changes suggested by "Thinh Nguyen" Changes in v2: 1. None --- drivers/usb/dwc3/gadget.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)