| Message ID | 1536326029-16327-5-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 |
On 9/7/2018 6:14 AM, Anurag Kumar Vulisha wrote: > According to dwc3 databook when streams are used, it may be possible > for the host and device become out of sync, where device may wait for > host to issue prime transcation and host may wait for device to issue > erdy. To avoid such deadlock, timeout needs to be implemented. After > timeout occurs, device will first stop transfer and restart the transfer > again. This patch does the same. > > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> > Reviewed-by: Thinh Nguyen <thinhn@synopsys.com> > --- > Changes in v3: > 1. Added the changes suggested by "Thinh Nguyen" > > Changes in v2: > 1. Changed STREAM_TIMEOUT to STREAM_TIMEOUT_MS as suggested by > "Andy Shevchenko" > --- > drivers/usb/dwc3/core.h | 7 +++++++ > drivers/usb/dwc3/gadget.c | 36 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 43 insertions(+) > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 5bfb625..0b255e97 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -633,6 +633,11 @@ struct dwc3_event_buffer { > > #define DWC3_TRB_NUM 256 > > +/* > + * Timeout value in msecs used by stream_timeout_timer when streams are enabled > + */ > +#define STREAM_TIMEOUT_MS 50 > + > /** > * struct dwc3_ep - device side endpoint representation > * @endpoint: usb endpoint > @@ -656,6 +661,7 @@ struct dwc3_event_buffer { > * @name: a human readable name e.g. ep1out-bulk > * @direction: true for TX, false for RX > * @stream_capable: true when streams are enabled > + * @stream_timeout_timer: timer used to aviod deadlock when streams are used Change aviod -> avoid. Also, the timer is being used when there's a deadlock rather than to avoid it. You probably need to explain what deadlock it is if you mentioned it here, but I think it's ok to simply say timeout timer for streams (unless Felipe has any objection). Thinh
HI Thinh, >> --- >> drivers/usb/dwc3/core.h | 7 +++++++ >> drivers/usb/dwc3/gadget.c | 36 ++++++++++++++++++++++++++++++++++++ >> 2 files changed, 43 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index >> 5bfb625..0b255e97 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -633,6 +633,11 @@ struct dwc3_event_buffer { >> >> #define DWC3_TRB_NUM 256 >> >> +/* >> + * Timeout value in msecs used by stream_timeout_timer when streams >> +are enabled */ >> +#define STREAM_TIMEOUT_MS 50 >> + >> /** >> * struct dwc3_ep - device side endpoint representation >> * @endpoint: usb endpoint >> @@ -656,6 +661,7 @@ struct dwc3_event_buffer { >> * @name: a human readable name e.g. ep1out-bulk >> * @direction: true for TX, false for RX >> * @stream_capable: true when streams are enabled >> + * @stream_timeout_timer: timer used to aviod deadlock when streams >> + are used > >Change aviod -> avoid. Also, the timer is being used when there's a deadlock rather >than to avoid it. You probably need to explain what deadlock it is if you mentioned it >here, but I think it's ok to simply say timeout timer for streams (unless Felipe has any >objection). Thanks for correcting again , will fix it in the next version of patch, Thanks, Anurag Kumar Vulisha
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 5bfb625..0b255e97 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -633,6 +633,11 @@ struct dwc3_event_buffer { #define DWC3_TRB_NUM 256 +/* + * Timeout value in msecs used by stream_timeout_timer when streams are enabled + */ +#define STREAM_TIMEOUT_MS 50 + /** * struct dwc3_ep - device side endpoint representation * @endpoint: usb endpoint @@ -656,6 +661,7 @@ struct dwc3_event_buffer { * @name: a human readable name e.g. ep1out-bulk * @direction: true for TX, false for RX * @stream_capable: true when streams are enabled + * @stream_timeout_timer: timer used to aviod deadlock when streams are used */ struct dwc3_ep { struct usb_ep endpoint; @@ -705,6 +711,7 @@ struct dwc3_ep { unsigned direction:1; unsigned stream_capable:1; + struct timer_list stream_timeout_timer; }; enum dwc3_phy { diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 13ea282..98e5f39 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -254,6 +254,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned cmd, u32 param) } static int __dwc3_gadget_wakeup(struct dwc3 *dwc); +static void stream_timeout_function(struct timer_list *arg); /** * dwc3_send_gadget_ep_cmd - issue an endpoint command @@ -574,6 +575,8 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, unsigned int action) | DWC3_DEPCFG_STREAM_EVENT_EN | DWC3_DEPCFG_XFER_COMPLETE_EN; dep->stream_capable = true; + timer_setup(&dep->stream_timeout_timer, + stream_timeout_function, 0); } if (!usb_endpoint_xfer_control(desc)) @@ -730,6 +733,9 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) trace_dwc3_gadget_ep_disable(dep); + if (dep->stream_capable) + del_timer(&dep->stream_timeout_timer); + dwc3_remove_requests(dwc, dep); /* make sure HW endpoint isn't stalled */ @@ -1257,6 +1263,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep) return ret; } + if (starting && dep->stream_capable) { + dep->stream_timeout_timer.expires = jiffies + + msecs_to_jiffies(STREAM_TIMEOUT_MS); + add_timer(&dep->stream_timeout_timer); + } + return 0; } @@ -2403,6 +2415,13 @@ static void dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep, stop = true; } + /* + * Delete the timer that was started in __dwc3_gadget_kick_transfer() + * for stream capable endpoints. + */ + if (dep->stream_capable) + del_timer(&dep->stream_timeout_timer); + dwc3_gadget_ep_cleanup_completed_requests(dep, event, status); if (stop) { @@ -2487,6 +2506,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, } break; case DWC3_DEPEVT_STREAMEVT: + if (event->status == DEPEVT_STREAMEVT_FOUND) + del_timer(&dep->stream_timeout_timer); + else + dev_dbg(dwc->dev, "unable to find suitable stream"); + break; case DWC3_DEPEVT_RXTXFIFOEVT: break; } @@ -2588,6 +2612,18 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force) } } +static void stream_timeout_function(struct timer_list *arg) +{ + struct dwc3_ep *dep = from_timer(dep, arg, stream_timeout_timer); + struct dwc3 *dwc = dep->dwc; + unsigned long flags; + + spin_lock_irqsave(&dwc->lock, flags); + dwc3_stop_active_transfer(dep, true); + __dwc3_gadget_kick_transfer(dep); + spin_unlock_irqrestore(&dwc->lock, flags); +} + static void dwc3_clear_stall_all_ep(struct dwc3 *dwc) { u32 epnum;