Message-ID: <1539436498-24892-2-git-send-email-anurag.kumar.vulisha@xilinx.com>
Date:   Sat, 13 Oct 2018 18:44:48 +0530
From:   Anurag Kumar Vulisha <anurag.kumar.vulisha@...inx.com>
To:     Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Johan Hovold <johan@...nel.org>,
        Jaejoong Kim <climbbb.kim@...il.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Roger Quadros <rogerq@...com>
CC:     <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <v.anuragkumar@...il.com>, Thinh Nguyen <thinhn@...opsys.com>,
        Tejas Joglekar <tejas.joglekar@...opsys.com>,
        Ajay Yugalkishore Pandey <APANDEY@...inx.com>,
        Anurag Kumar Vulisha <anurag.kumar.vulisha@...inx.com>
Subject: [PATCH V6 01/10] usb: gadget: udc: Add timer for stream capable endpoints

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@...inx.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)
+		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;
 };
-- 
2.1.1

Powered by blists - more mailing lists