Commit 5f7bf077 authored Feb 24, 2021 by Sriharsha Allenki Committed by Gerrit - the friendly Code Review server Mar 14, 2021

usb: dwc3: gadget: Prevent queue on endpoints after bus reset



There is a race between ep_queue from function driver and
ep_disable during function disable as part of bus reset.
This race is leading to a scenario where the endpoint is
disabled with an active transfer. And when the endpoint is enabled
back, this leads to a stale buffer access by the controller.

The race is as below:
						qdss_disable
usb_qdss_write					    |  -----> Waits on qdss_lock
     |  ----> Acquires qdss_lock		    |
     |  ----> Checks for qdss_connected		    |
     |  ----> Releases qdss_lock		    |  ------> Acquires qdss_lock
     |						    |  ------> Clears usb_connected
     |						    |  ------> Releases qdss_lock
     |						usb_ep_disable
     |						    |  ------> Acquires dwc->lock
     |						dwc3_remove_reuests
     |						    |
     |						dwc3_stop_active_transfer
     |						    |
     |						    |  ------> Releases dwc->lock
usb_ep_queue					    |
     | ----> Acquires dwc->lock			usb_gadget_giveback
     |						    |
dwc3_gadget_kick_transfer			    |
     | ----> Releases dwc->lock			    |
						    | ----> Acquires dwc->lock
						    |
						disable_endpoint

After this state when the endpoint is enabled back, there is already
and active transfer on this endpoint. Any START TRANSFER command
on this endpoint would lead to "No Transfer Resource" error
from the controller.

Fix this by preventing the ep_queue when a reset of the gadget
is in progress. This is done by clearing the connected flag
as part of dwc3_gadget_reset and checking for this as part of
ep_queue. Set this flag once we have received the CONNECTION
DONE interrupt from the controller.

Change-Id: Idc52a72ed0a965c2043618046a5b0dc32bfd1985
Signed-off-by: Sriharsha Allenki <sallenki@codeaurora.org>

parent 8a7d4fed

drivers/usb/dwc3/gadget.c

+6 −3

Original line number	Original line	Diff line number	Diff line
	@@ -1596,7 +1596,8 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep dep, struct dwc3_request req)
	{		{
	struct dwc3 *dwc = dep->dwc;		struct dwc3 *dwc = dep->dwc;

	if (!dep->endpoint.desc \|\| !dwc->pullups_connected) {		if (!dep->endpoint.desc \|\| !dwc->pullups_connected \|\|
			!dwc->connected) {
	dev_err_ratelimited(dwc->dev, "%s: can't queue to disabled endpoint\n",		dev_err_ratelimited(dwc->dev, "%s: can't queue to disabled endpoint\n",
	dep->name);		dep->name);
	return -ESHUTDOWN;		return -ESHUTDOWN;
	@@ -3355,6 +3356,8 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
	{		{
	struct usb_gadget_driver *gadget_driver;		struct usb_gadget_driver *gadget_driver;

			dwc->connected = false;

	if (!dwc->gadget_driver)		if (!dwc->gadget_driver)
	return;		return;

	@@ -3488,8 +3491,6 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
	{		{
	u32 reg;		u32 reg;

	dwc->connected = true;

	/*		/*
	* WORKAROUND: DWC3 revisions <1.88a have an issue which		* WORKAROUND: DWC3 revisions <1.88a have an issue which
	* would cause a missing Disconnect Event if there's a		* would cause a missing Disconnect Event if there's a
	@@ -3675,6 +3676,8 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
	dwc3_writel(dwc->regs, DWC3_DCTL, reg);		dwc3_writel(dwc->regs, DWC3_DCTL, reg);
	}		}

			dwc->connected = true;

	dep = dwc->eps[0];		dep = dwc->eps[0];
	ret = __dwc3_gadget_ep_enable(dep, DWC3_DEPCFG_ACTION_MODIFY);		ret = __dwc3_gadget_ep_enable(dep, DWC3_DEPCFG_ACTION_MODIFY);
	if (ret) {		if (ret) {