| Message ID | 1539436498-24892-2-git-send-email-anurag.kumar.vulisha@xilinx.com (mailing list archive) |
|---|---|
| State | Superseded |
| 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: > When bulk streams are enabled for an endpoint, there can > be a condition where the gadget controller waits for the > host to issue prime transaction and the host controller > waits for the gadget to issue ERDY. This condition could > create a deadlock. To avoid such potential deadlocks, a > timer is started after queuing any request for the stream > capable endpoint in usb_ep_queue(). The gadget driver is > expected to stop the timer if a valid stream event is found. > If no stream event is found, the timer expires after the > STREAM_TIMEOUT_MS value and a callback function registered > by gadget driver to endpoint ops->stream_timeout API would be > called, so that the gadget driver can handle this situation. > This kind of behaviour is observed in dwc3 controller and > expected to be generic issue with other controllers supporting > bulk streams also. > > Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> > --- > Changes in v6: > 1. This patch is newly added in this series to add timer into udc/core.c > --- > drivers/usb/gadget/udc/core.c | 71 ++++++++++++++++++++++++++++++++++++++++++- > include/linux/usb/gadget.h | 12 ++++++++ > 2 files changed, 82 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index af88b48..41cc23b 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc, > /* ------------------------------------------------------------------------- */ > > /** > + * usb_ep_stream_timeout - callback function for endpoint stream timeout timer > + * @arg: pointer to struct timer_list > + * > + * This function gets called only when bulk streams are enabled in the endpoint > + * and only after ep->stream_timeout_timer has expired. The timer gets expired > + * only when the gadget controller failed to find a valid stream event for this > + * endpoint. On timer expiry, this function calls the endpoint-specific timeout > + * handler registered to endpoint ops->stream_timeout API. > + */ > +static void usb_ep_stream_timeout(struct timer_list *arg) > +{ > + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer); > + > + if (ep->stream_capable && ep->ops->stream_timeout) why is the timer only for stream endpoints? What if we want to run this on non-stream capable eps? > + ep->ops->stream_timeout(ep); you don't ned an extra operation here, ep_dequeue should be enough. > @@ -102,6 +132,10 @@ int usb_ep_enable(struct usb_ep *ep) > if (ret) > goto out; > > + if (ep->stream_capable) > + timer_setup(&ep->stream_timeout_timer, > + usb_ep_stream_timeout, 0); the timer should be per-request, not per-endpoint. Otherwise in case of multiple requests being queued, you can't give them different timeouts > @@ -269,6 +321,13 @@ int usb_ep_queue(struct usb_ep *ep, > > ret = ep->ops->queue(ep, req, gfp_flags); > > + if (ep->stream_capable) { > + ep->stream_timeout_timer.expires = jiffies + > + msecs_to_jiffies(STREAM_TIMEOUT_MS); timeout value should be passed by the gadget driver. Add a new usb_ep_queue_timeout() that takes the new argument. Rename the current usb_ep_queue() to static int __usb_ep_queue() with an extra argument for timeout and introduce usb_ep_queue() without the argument, calling __usb_ep_queue() passing timeout as 0. > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index e5cd84a..2ebaef0 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -144,6 +144,7 @@ struct usb_ep_ops { > > int (*fifo_status) (struct usb_ep *ep); > void (*fifo_flush) (struct usb_ep *ep); > + void (*stream_timeout) (struct usb_ep *ep); why?
Hi Felipe, Thanks a lot for spending your time in reviewing this patch. Please find my comments inline >-----Original Message----- >From: Felipe Balbi [mailto:balbi@kernel.org] >Sent: Wednesday, November 14, 2018 7:28 PM >To: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman ><gregkh@linuxfoundation.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> >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 V6 01/10] usb: gadget: udc: Add timer for stream capable >endpoints > > >Hi, > >Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> writes: >> When bulk streams are enabled for an endpoint, there can >> be a condition where the gadget controller waits for the >> host to issue prime transaction and the host controller >> waits for the gadget to issue ERDY. This condition could >> create a deadlock. To avoid such potential deadlocks, a >> timer is started after queuing any request for the stream >> capable endpoint in usb_ep_queue(). The gadget driver is >> expected to stop the timer if a valid stream event is found. >> If no stream event is found, the timer expires after the >> STREAM_TIMEOUT_MS value and a callback function registered >> by gadget driver to endpoint ops->stream_timeout API would be >> called, so that the gadget driver can handle this situation. >> This kind of behaviour is observed in dwc3 controller and >> expected to be generic issue with other controllers supporting >> bulk streams also. >> >> Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> >> --- >> Changes in v6: >> 1. This patch is newly added in this series to add timer into udc/core.c >> --- >> drivers/usb/gadget/udc/core.c | 71 >++++++++++++++++++++++++++++++++++++++++++- >> include/linux/usb/gadget.h | 12 ++++++++ >> 2 files changed, 82 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c >> index af88b48..41cc23b 100644 >> --- a/drivers/usb/gadget/udc/core.c >> +++ b/drivers/usb/gadget/udc/core.c >> @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc, >> /* ------------------------------------------------------------------------- */ >> >> /** >> + * usb_ep_stream_timeout - callback function for endpoint stream timeout timer >> + * @arg: pointer to struct timer_list >> + * >> + * This function gets called only when bulk streams are enabled in the endpoint >> + * and only after ep->stream_timeout_timer has expired. The timer gets expired >> + * only when the gadget controller failed to find a valid stream event for this >> + * endpoint. On timer expiry, this function calls the endpoint-specific timeout >> + * handler registered to endpoint ops->stream_timeout API. >> + */ >> +static void usb_ep_stream_timeout(struct timer_list *arg) >> +{ >> + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer); >> + >> + if (ep->stream_capable && ep->ops->stream_timeout) > >why is the timer only for stream endpoints? What if we want to run this >on non-stream capable eps? > I have seen this issue only with stream capable eps between PRIME & EPRDY. But this issue can potentially occur with non-stream endpoints also. Will remove this stream capable checks in next series of patch. >> + ep->ops->stream_timeout(ep); > >you don't ned an extra operation here, ep_dequeue should be enough. > I think issuing ep_dequeue() would be more trickier than calling ep->ops->stream_timeout(). This is because, every gadget driver has their own way of handling ep_dequeue. Some drivers (like dwc3) may sleep for an event (wait_event_lock_irq) in the ep_dequeue routine. Since we are calling ep_dequeue from timer timeout callback which is in softirq context, sleeping or waiting for an event would hang the system. Also adding ep->ops->stream_timeout() would make the gadget drivers handle the issue in better way based on their implementation. Another advantage is the drivers which doesn't have this timeout issue doesn't have to register the timeout handler and can avoid the logic of deleting the timer for every request. So, I think ep->ops->stream_timeout() would be better than having a generic handler which does ep_dequeue() & ep_queue(). Please provide your suggestion on this implementation. >> @@ -102,6 +132,10 @@ int usb_ep_enable(struct usb_ep *ep) >> if (ret) >> goto out; >> >> + if (ep->stream_capable) >> + timer_setup(&ep->stream_timeout_timer, >> + usb_ep_stream_timeout, 0); > >the timer should be per-request, not per-endpoint. Otherwise in case of >multiple requests being queued, you can't give them different timeouts I agree with you. Will correct this in next series of patch. > >> @@ -269,6 +321,13 @@ int usb_ep_queue(struct usb_ep *ep, >> >> ret = ep->ops->queue(ep, req, gfp_flags); >> >> + if (ep->stream_capable) { >> + ep->stream_timeout_timer.expires = jiffies + >> + msecs_to_jiffies(STREAM_TIMEOUT_MS); > >timeout value should be passed by the gadget driver. Add a new >usb_ep_queue_timeout() that takes the new argument. Rename the current >usb_ep_queue() to static int __usb_ep_queue() with an extra argument for >timeout and introduce usb_ep_queue() without the argument, calling >__usb_ep_queue() passing timeout as 0. > Thanks for correcting and providing the steps for implementing. Will modify as suggested in next series of patch. >> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h >> index e5cd84a..2ebaef0 100644 >> --- a/include/linux/usb/gadget.h >> +++ b/include/linux/usb/gadget.h >> @@ -144,6 +144,7 @@ struct usb_ep_ops { >> >> int (*fifo_status) (struct usb_ep *ep); >> void (*fifo_flush) (struct usb_ep *ep); >> + void (*stream_timeout) (struct usb_ep *ep); > >why? > As discussed above, the common timeout handler with ep_dequeue() and ep_queue() may not work for all the gadget drivers, adding the stream_timeout() in ep->ops would enable the drivers to implement their own handler for handling the stream timeout issue. Thanks, Anurag Kumar Vulisha >-- >balbi
Hi, Anurag Kumar Vulisha <anuragku@xilinx.com> writes: >>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c >>> index af88b48..41cc23b 100644 >>> --- a/drivers/usb/gadget/udc/core.c >>> +++ b/drivers/usb/gadget/udc/core.c >>> @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc, >>> /* ------------------------------------------------------------------------- */ >>> >>> /** >>> + * usb_ep_stream_timeout - callback function for endpoint stream timeout timer >>> + * @arg: pointer to struct timer_list >>> + * >>> + * This function gets called only when bulk streams are enabled in the endpoint >>> + * and only after ep->stream_timeout_timer has expired. The timer gets expired >>> + * only when the gadget controller failed to find a valid stream event for this >>> + * endpoint. On timer expiry, this function calls the endpoint-specific timeout >>> + * handler registered to endpoint ops->stream_timeout API. >>> + */ >>> +static void usb_ep_stream_timeout(struct timer_list *arg) >>> +{ >>> + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer); >>> + >>> + if (ep->stream_capable && ep->ops->stream_timeout) >> >>why is the timer only for stream endpoints? What if we want to run this >>on non-stream capable eps? >> > > I have seen this issue only with stream capable eps between PRIME & > EPRDY. But this issue can potentially occur with non-stream endpoints > also. Will remove this stream capable checks in next series of patch. you're adding support for transfer timeouts, which some gadgets may want regardless of endpoint type. One use is to correct cases of streams going out of sync. >>> + ep->ops->stream_timeout(ep); >> >>you don't ned an extra operation here, ep_dequeue should be enough. >> > > I think issuing ep_dequeue() would be more trickier than calling ep->ops->stream_timeout(). > This is because, every gadget driver has their own way of handling ep_dequeue. Some > drivers (like dwc3) may sleep for an event (wait_event_lock_irq) in the ep_dequeue routine. not anymore. There's, now, a requirement that ->dequeue can be called from any context. > Since we are calling ep_dequeue from timer timeout callback which is in softirq context, > sleeping or waiting for an event would hang the system. Also adding ep->ops->stream_timeout() > would make the gadget drivers handle the issue in better way based on their implementation. no problems > Another advantage is the drivers which doesn't have this timeout issue doesn't have to register the > timeout handler and can avoid the logic of deleting the timer for every request. So, I think > ep->ops->stream_timeout() would be better than having a generic handler which does > ep_dequeue() & ep_queue(). Please provide your suggestion on this implementation. call ep_dequeue()
Hi Felipe, >-----Original Message----- >From: Felipe Balbi [mailto:balbi@kernel.org] >Sent: Thursday, November 29, 2018 6:22 PM >To: Anurag Kumar Vulisha <anuragku@xilinx.com>; Greg Kroah-Hartman ><gregkh@linuxfoundation.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> >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> >Subject: RE: [PATCH V6 01/10] usb: gadget: udc: Add timer for stream capable >endpoints > > >Hi, > >Anurag Kumar Vulisha <anuragku@xilinx.com> writes: >>>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c >>>> index af88b48..41cc23b 100644 >>>> --- a/drivers/usb/gadget/udc/core.c >>>> +++ b/drivers/usb/gadget/udc/core.c >>>> @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc, >>>> /* ------------------------------------------------------------------------- */ >>>> >>>> /** >>>> + * usb_ep_stream_timeout - callback function for endpoint stream timeout >timer >>>> + * @arg: pointer to struct timer_list >>>> + * >>>> + * This function gets called only when bulk streams are enabled in the endpoint >>>> + * and only after ep->stream_timeout_timer has expired. The timer gets expired >>>> + * only when the gadget controller failed to find a valid stream event for this >>>> + * endpoint. On timer expiry, this function calls the endpoint-specific timeout >>>> + * handler registered to endpoint ops->stream_timeout API. >>>> + */ >>>> +static void usb_ep_stream_timeout(struct timer_list *arg) >>>> +{ >>>> + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer); >>>> + >>>> + if (ep->stream_capable && ep->ops->stream_timeout) >>> >>>why is the timer only for stream endpoints? What if we want to run this >>>on non-stream capable eps? >>> >> >> I have seen this issue only with stream capable eps between PRIME & >> EPRDY. But this issue can potentially occur with non-stream endpoints >> also. Will remove this stream capable checks in next series of patch. > >you're adding support for transfer timeouts, which some gadgets may want >regardless of endpoint type. One use is to correct cases of streams >going out of sync. > Thanks for making me understand, will implement your suggestions and resend the patches soon. Best Regards, Anurag Kumar Vulisha >>>> + ep->ops->stream_timeout(ep); >>> >>>you don't ned an extra operation here, ep_dequeue should be enough. >>> >> >> I think issuing ep_dequeue() would be more trickier than calling ep->ops- >>stream_timeout(). >> This is because, every gadget driver has their own way of handling ep_dequeue. >Some >> drivers (like dwc3) may sleep for an event (wait_event_lock_irq) in the ep_dequeue >routine. > >not anymore. There's, now, a requirement that ->dequeue can be called >from any context. > >> Since we are calling ep_dequeue from timer timeout callback which is in softirq >context, >> sleeping or waiting for an event would hang the system. Also adding ep->ops- >>stream_timeout() >> would make the gadget drivers handle the issue in better way based on their >implementation. > >no problems > >> Another advantage is the drivers which doesn't have this timeout issue doesn't have >to register the >> timeout handler and can avoid the logic of deleting the timer for every request. So, I >think >> ep->ops->stream_timeout() would be better than having a generic handler which >does >> ep_dequeue() & ep_queue(). Please provide your suggestion on this >implementation. > >call ep_dequeue() > >-- >balbi
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index af88b48..41cc23b 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -52,6 +52,24 @@ static int udc_bind_to_driver(struct usb_udc *udc, /* ------------------------------------------------------------------------- */ /** + * usb_ep_stream_timeout - callback function for endpoint stream timeout timer + * @arg: pointer to struct timer_list + * + * This function gets called only when bulk streams are enabled in the endpoint + * and only after ep->stream_timeout_timer has expired. The timer gets expired + * only when the gadget controller failed to find a valid stream event for this + * endpoint. On timer expiry, this function calls the endpoint-specific timeout + * handler registered to endpoint ops->stream_timeout API. + */ +static void usb_ep_stream_timeout(struct timer_list *arg) +{ + struct usb_ep *ep = from_timer(ep, arg, stream_timeout_timer); + + if (ep->stream_capable && ep->ops->stream_timeout) + ep->ops->stream_timeout(ep); +} + +/** * usb_ep_set_maxpacket_limit - set maximum packet size limit for endpoint * @ep:the endpoint being configured * @maxpacket_limit:value of maximum packet size limit @@ -87,6 +105,18 @@ EXPORT_SYMBOL_GPL(usb_ep_set_maxpacket_limit); * configurable, with more generic names like "ep-a". (remember that for * USB, "in" means "towards the USB master".) * + * When bulk streams are enabled (stream_capable == true), a timer is setup + * by this function, which would be started at the time of queuing the request + * in usb_ep_queue(). This is because, when streams are enabled the host and + * gadget can go out sync, the gadget may wait until the host issues a prime + * transaction and the host may wait until gadget issues a ERDY. This behaviour + * may create a deadlock. To avoid such a deadlock, the timer is started after + * submitting the request in usb_ep_queue(). If a valid stream event is + * generated, the gadget driver stops the timer. If no valid stream event is + * found, the timer gets expired and usb_ep_stream_timeout() function which is + * registered as a callback to stream_timeout_timer is called. This callback + * function handles the deadlock. + * * This routine must be called in process context. * * returns zero, or a negative error code. @@ -102,6 +132,10 @@ int usb_ep_enable(struct usb_ep *ep) if (ret) goto out; + if (ep->stream_capable) + timer_setup(&ep->stream_timeout_timer, + usb_ep_stream_timeout, 0); + ep->enabled = true; out: @@ -121,6 +155,9 @@ EXPORT_SYMBOL_GPL(usb_ep_enable); * gadget drivers must call usb_ep_enable() again before queueing * requests to the endpoint. * + * For stream capable endpoints, the timer which was started in usb_ep_enable() + * would be removed. + * * This routine must be called in process context. * * returns zero, or a negative error code. @@ -132,6 +169,9 @@ int usb_ep_disable(struct usb_ep *ep) if (!ep->enabled) goto out; + if (ep->stream_capable && timer_pending(&ep->stream_timeout_timer)) + del_timer(&ep->stream_timeout_timer); + ret = ep->ops->disable(ep); if (ret) goto out; @@ -245,6 +285,18 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request); * Note that @req's ->complete() callback must never be called from * within usb_ep_queue() as that can create deadlock situations. * + * For stream capable endpoints (stream_capable == true), a timer which was + * setup by usb_ep_enable() is started in this function to avoid deadlock. + * When streams are enabled the host and gadget can go out sync, the gadget + * may wait until the host issues prime transaction and the host may wait + * until gadget issues a ERDY. This behaviour may create a deadlock. To avoid + * such a deadlock, when endpoint is bulk stream capable, the timer is started + * after submitting the request. If a valid stream event is generated by the + * gadget controller, the gadget driver stops the timer. If no valid stream + * event is found, the timer keeps running until expired after STREAM_TIMEOUT_MS + * value and the stream timeout function - usb_ep_stream_timeout() gets + * called, which takes necessary action to avoid the deadlock condition. + * * This routine may be called in interrupt context. * * Returns zero, or a negative error code. Endpoints that are not enabled @@ -269,6 +321,13 @@ int usb_ep_queue(struct usb_ep *ep, ret = ep->ops->queue(ep, req, gfp_flags); + if (ep->stream_capable) { + ep->stream_timeout_timer.expires = jiffies + + msecs_to_jiffies(STREAM_TIMEOUT_MS); + mod_timer(&ep->stream_timeout_timer, + ep->stream_timeout_timer.expires); + } + out: trace_usb_ep_queue(ep, req, ret); @@ -291,6 +350,9 @@ EXPORT_SYMBOL_GPL(usb_ep_queue); * restrictions prevent drivers from supporting configuration changes, * even to configuration zero (a "chapter 9" requirement). * + * For stream capable endpoints, the timer which was started in usb_ep_queue() + * would be removed. + * * This routine may be called in interrupt context. */ int usb_ep_dequeue(struct usb_ep *ep, struct usb_request *req) @@ -300,6 +362,9 @@ int usb_ep_dequeue(struct usb_ep *ep, struct usb_request *req) ret = ep->ops->dequeue(ep, req); trace_usb_ep_dequeue(ep, req, ret); + if (ep->stream_capable && timer_pending(&ep->stream_timeout_timer)) + del_timer(&ep->stream_timeout_timer); + return ret; } EXPORT_SYMBOL_GPL(usb_ep_dequeue); @@ -878,7 +943,8 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request); * Context: in_interrupt() * * This is called by device controller drivers in order to return the - * completed request back to the gadget layer. + * completed request back to the gadget layer. For stream capable endpoints, + * the timer which was started in usb_ep_queue() would be removed. */ void usb_gadget_giveback_request(struct usb_ep *ep, struct usb_request *req) @@ -886,6 +952,9 @@ void usb_gadget_giveback_request(struct usb_ep *ep, if (likely(req->status == 0)) usb_led_activity(USB_LED_EVENT_GADGET); + if (ep->stream_capable && timer_pending(&ep->stream_timeout_timer)) + del_timer(&ep->stream_timeout_timer); + trace_usb_gadget_giveback_request(ep, req, 0); req->complete(ep, req); diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index e5cd84a..2ebaef0 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -144,6 +144,7 @@ struct usb_ep_ops { int (*fifo_status) (struct usb_ep *ep); void (*fifo_flush) (struct usb_ep *ep); + void (*stream_timeout) (struct usb_ep *ep); }; /** @@ -184,6 +185,11 @@ struct usb_ep_caps { .dir_out = !!(_dir & USB_EP_CAPS_DIR_OUT), \ } +/* + * Timeout value in msecs used by stream_timeout_timer when streams are enabled + */ +#define STREAM_TIMEOUT_MS 50 + /** * struct usb_ep - device side representation of USB endpoint * @name:identifier for the endpoint, such as "ep-a" or "ep9in-bulk" @@ -191,6 +197,7 @@ struct usb_ep_caps { * @ep_list:the gadget's ep_list holds all of its endpoints * @caps:The structure describing types and directions supported by endoint. * @enabled: The current endpoint enabled/disabled state. + * @stream_capable: Set to true when ep supports bulk streams. * @claimed: True if this endpoint is claimed by a function. * @maxpacket:The maximum packet size used on this endpoint. The initial * value can sometimes be reduced (hardware allowing), according to @@ -209,6 +216,9 @@ struct usb_ep_caps { * enabled and remains valid until the endpoint is disabled. * @comp_desc: In case of SuperSpeed support, this is the endpoint companion * descriptor that is used to configure the endpoint + * @stream_timeout_timer: timeout timer used by bulk streams to avoid deadlock + * where host waits for the gadget to issue ERDY and gadget waits for host + * to issue prime transaction. * * the bus controller driver lists all the general purpose endpoints in * gadget->ep_list. the control endpoint (gadget->ep0) is not in that list, @@ -224,12 +234,14 @@ struct usb_ep { struct usb_ep_caps caps; bool claimed; bool enabled; + bool stream_capable; unsigned maxpacket:16; unsigned maxpacket_limit:16; unsigned max_streams:16; unsigned mult:2; unsigned maxburst:5; u8 address; + struct timer_list stream_timeout_timer; const struct usb_endpoint_descriptor *desc; const struct usb_ss_ep_comp_descriptor *comp_desc; };
When bulk streams are enabled for an endpoint, there can be a condition where the gadget controller waits for the host to issue prime transaction and the host controller waits for the gadget to issue ERDY. This condition could create a deadlock. To avoid such potential deadlocks, a timer is started after queuing any request for the stream capable endpoint in usb_ep_queue(). The gadget driver is expected to stop the timer if a valid stream event is found. If no stream event is found, the timer expires after the STREAM_TIMEOUT_MS value and a callback function registered by gadget driver to endpoint ops->stream_timeout API would be called, so that the gadget driver can handle this situation. This kind of behaviour is observed in dwc3 controller and expected to be generic issue with other controllers supporting bulk streams also. Signed-off-by: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com> --- Changes in v6: 1. This patch is newly added in this series to add timer into udc/core.c --- drivers/usb/gadget/udc/core.c | 71 ++++++++++++++++++++++++++++++++++++++++++- include/linux/usb/gadget.h | 12 ++++++++ 2 files changed, 82 insertions(+), 1 deletion(-)