|
Oliver Neukum |
10e64a |
From c91815b596245fd7da349ecc43c8def670d2269e Mon Sep 17 00:00:00 2001
|
|
Oliver Neukum |
10e64a |
From: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Oliver Neukum |
10e64a |
Date: Mon, 26 Mar 2018 13:14:47 +0300
|
|
Oliver Neukum |
10e64a |
Subject: [PATCH] usb: dwc3: gadget: never call ->complete() from ->ep_queue()
|
|
Oliver Neukum |
10e64a |
Git-commit: c91815b596245fd7da349ecc43c8def670d2269e
|
|
Oliver Neukum |
10e64a |
References: git-fixes
|
|
Oliver Neukum |
10e64a |
Patch-mainline: v4.17-rc1
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
This is a requirement which has always existed but, somehow, wasn't
|
|
Oliver Neukum |
10e64a |
reflected in the documentation and problems weren't found until now
|
|
Oliver Neukum |
10e64a |
when Tuba Yavuz found a possible deadlock happening between dwc3 and
|
|
Oliver Neukum |
10e64a |
f_hid. She described the situation as follows:
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
spin_lock_irqsave(&hidg->write_spinlock, flags); // first acquire
|
|
Oliver Neukum |
10e64a |
/* we our function has been disabled by host */
|
|
Oliver Neukum |
10e64a |
if (!hidg->req) {
|
|
Oliver Neukum |
10e64a |
free_ep_req(hidg->in_ep, hidg->req);
|
|
Oliver Neukum |
10e64a |
goto try_again;
|
|
Oliver Neukum |
10e64a |
}
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
[...]
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);
|
|
Oliver Neukum |
10e64a |
=>
|
|
Oliver Neukum |
10e64a |
[...]
|
|
Oliver Neukum |
10e64a |
=> usb_gadget_giveback_request
|
|
Oliver Neukum |
10e64a |
=>
|
|
Oliver Neukum |
10e64a |
f_hidg_req_complete
|
|
Oliver Neukum |
10e64a |
=>
|
|
Oliver Neukum |
10e64a |
spin_lock_irqsave(&hidg->write_spinlock, flags); // second acquire
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
Note that this happens because dwc3 would call ->complete() on a
|
|
Oliver Neukum |
10e64a |
failed usb_ep_queue() due to failed Start Transfer command. This is,
|
|
Oliver Neukum |
10e64a |
anyway, a theoretical situation because dwc3 currently uses "No
|
|
Oliver Neukum |
10e64a |
Response Update Transfer" command for Bulk and Interrupt endpoints.
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
It's still good to make this case impossible to happen even if the "No
|
|
Oliver Neukum |
10e64a |
Reponse Update Transfer" command is changed.
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
Reported-by: Tuba Yavuz <tuba@ece.ufl.edu>
|
|
Oliver Neukum |
10e64a |
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
|
|
Oliver Neukum |
10e64a |
Cc: stable <stable@vger.kernel.org>
|
|
Oliver Neukum |
10e64a |
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Oliver Neukum |
10e64a |
Signed-off-by: Oliver Neukum <oneukum@suse.com>
|
|
Oliver Neukum |
10e64a |
---
|
|
Oliver Neukum |
10e64a |
drivers/usb/dwc3/gadget.c | 31 ++++++++++++++++++++++++-------
|
|
Oliver Neukum |
10e64a |
1 file changed, 24 insertions(+), 7 deletions(-)
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
--- a/drivers/usb/dwc3/gadget.c
|
|
Oliver Neukum |
10e64a |
+++ b/drivers/usb/dwc3/gadget.c
|
|
Oliver Neukum |
10e64a |
@@ -167,8 +167,8 @@ static void dwc3_ep_inc_deq(struct dwc3_
|
|
Oliver Neukum |
10e64a |
dwc3_ep_inc_trb(&dep->trb_dequeue);
|
|
Oliver Neukum |
10e64a |
}
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
-void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
|
|
Oliver Neukum |
10e64a |
- int status)
|
|
Oliver Neukum |
10e64a |
+void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep,
|
|
Oliver Neukum |
10e64a |
+ struct dwc3_request *req, int status)
|
|
Oliver Neukum |
10e64a |
{
|
|
Oliver Neukum |
10e64a |
struct dwc3 *dwc = dep->dwc;
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
@@ -181,16 +181,33 @@ void dwc3_gadget_giveback(struct dwc3_ep
|
|
Oliver Neukum |
10e64a |
req->request.status = status;
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
usb_gadget_unmap_request_by_dev(dwc->sysdev,
|
|
Oliver Neukum |
10e64a |
- &req->request, req->direction);
|
|
Oliver Neukum |
10e64a |
+ &req->request, req->direction);
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
trace_dwc3_gadget_giveback(req);
|
|
Oliver Neukum |
10e64a |
+ if (dep->number > 1)
|
|
Oliver Neukum |
10e64a |
+ pm_runtime_put(dwc->dev);
|
|
Oliver Neukum |
10e64a |
+}
|
|
Oliver Neukum |
10e64a |
+
|
|
Oliver Neukum |
10e64a |
+/**
|
|
Oliver Neukum |
10e64a |
+ * dwc3_gadget_giveback - call struct usb_request's ->complete callback
|
|
Oliver Neukum |
10e64a |
+ * @dep: The endpoint to whom the request belongs to
|
|
Oliver Neukum |
10e64a |
+ * @req: The request we're giving back
|
|
Oliver Neukum |
10e64a |
+ * @status: completion code for the request
|
|
Oliver Neukum |
10e64a |
+ *
|
|
Oliver Neukum |
10e64a |
+ * Must be called with controller's lock held and interrupts disabled. This
|
|
Oliver Neukum |
10e64a |
+ * function will unmap @req and call its ->complete() callback to notify upper
|
|
Oliver Neukum |
10e64a |
+ * layers that it has completed.
|
|
Oliver Neukum |
10e64a |
+ */
|
|
Oliver Neukum |
10e64a |
+void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
|
|
Oliver Neukum |
10e64a |
+ int status)
|
|
Oliver Neukum |
10e64a |
+{
|
|
Oliver Neukum |
10e64a |
+ struct dwc3 *dwc = dep->dwc;
|
|
Oliver Neukum |
10e64a |
+
|
|
Oliver Neukum |
10e64a |
+ dwc3_gadget_del_and_unmap_request(dep, req, status);
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
spin_unlock(&dwc->lock);
|
|
Oliver Neukum |
10e64a |
usb_gadget_giveback_request(&dep->endpoint, &req->request);
|
|
Oliver Neukum |
10e64a |
spin_lock(&dwc->lock);
|
|
Oliver Neukum |
10e64a |
-
|
|
Oliver Neukum |
10e64a |
- if (dep->number > 1)
|
|
Oliver Neukum |
10e64a |
- pm_runtime_put(dwc->dev);
|
|
Oliver Neukum |
10e64a |
}
|
|
Oliver Neukum |
10e64a |
|
|
Oliver Neukum |
10e64a |
int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned cmd, u32 param)
|
|
Oliver Neukum |
10e64a |
@@ -1150,7 +1167,7 @@ static int __dwc3_gadget_kick_transfer(s
|
|
Oliver Neukum |
10e64a |
if (req->trb)
|
|
Oliver Neukum |
10e64a |
memset(req->trb, 0, sizeof(struct dwc3_trb));
|
|
Oliver Neukum |
10e64a |
dep->queued_requests--;
|
|
Oliver Neukum |
10e64a |
- dwc3_gadget_giveback(dep, req, ret);
|
|
Oliver Neukum |
10e64a |
+ dwc3_gadget_del_and_unmap_request(dep, req, ret);
|
|
Oliver Neukum |
10e64a |
return ret;
|
|
Oliver Neukum |
10e64a |
}
|
|
Oliver Neukum |
10e64a |
|