Skip to content

Commit

Permalink
usb: dwc3: gadget: Stall and restart EP0 if host is unresponsive
Browse files Browse the repository at this point in the history
commit 02435a7 upstream.

It was observed that there are hosts that may complete pending SETUP
transactions before the stop active transfers and controller halt occurs,
leading to lingering endxfer commands on DEPs on subsequent pullup/gadget
start iterations.

  dwc3_gadget_ep_disable   name=ep8in flags=0x3009  direction=1
  dwc3_gadget_ep_disable   name=ep4in flags=1  direction=1
  dwc3_gadget_ep_disable   name=ep3out flags=1  direction=0
  usb_gadget_disconnect   deactivated=0  connected=0  ret=0

The sequence shows that the USB gadget disconnect (dwc3_gadget_pullup(0))
routine completed successfully, allowing for the USB gadget to proceed with
a USB gadget connect.  However, if this occurs the system runs into an
issue where:

  BUG: spinlock already unlocked on CPU
  spin_bug+0x0
  dwc3_remove_requests+0x278
  dwc3_ep0_out_start+0xb0
  __dwc3_gadget_start+0x25c

This is due to the pending endxfers, leading to gadget start (w/o lock
held) to execute the remove requests, which will unlock the dwc3
spinlock as part of giveback.

To mitigate this, resolve the pending endxfers on the pullup disable
path by re-locating the SETUP phase check after stop active transfers, since
that is where the DWC3_EP_DELAY_STOP is potentially set.  This also allows
for handling of a host that may be unresponsive by using the completion
timeout to trigger the stall and restart for EP0.

Fixes: c966837 ("usb: dwc3: ep0: Don't prepare beyond Setup stage")
Cc: stable@vger.kernel.org
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
Link: https://lore.kernel.org/r/20230413195742.11821-2-quic_wcheng@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Wesley Cheng authored and gregkh committed May 11, 2023
1 parent f58a3da commit 3713276
Showing 1 changed file with 32 additions and 17 deletions.
49 changes: 32 additions & 17 deletions drivers/usb/dwc3/gadget.c
Original file line number Diff line number Diff line change
Expand Up @@ -2532,29 +2532,17 @@ static int __dwc3_gadget_start(struct dwc3 *dwc);
static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
{
unsigned long flags;
int ret;

spin_lock_irqsave(&dwc->lock, flags);
dwc->connected = false;

/*
* Per databook, when we want to stop the gadget, if a control transfer
* is still in process, complete it and get the core into setup phase.
* Attempt to end pending SETUP status phase, and not wait for the
* function to do so.
*/
if (dwc->ep0state != EP0_SETUP_PHASE) {
int ret;

if (dwc->delayed_status)
dwc3_ep0_send_delayed_status(dwc);

reinit_completion(&dwc->ep0_in_setup);

spin_unlock_irqrestore(&dwc->lock, flags);
ret = wait_for_completion_timeout(&dwc->ep0_in_setup,
msecs_to_jiffies(DWC3_PULL_UP_TIMEOUT));
spin_lock_irqsave(&dwc->lock, flags);
if (ret == 0)
dev_warn(dwc->dev, "timed out waiting for SETUP phase\n");
}
if (dwc->delayed_status)
dwc3_ep0_send_delayed_status(dwc);

/*
* In the Synopsys DesignWare Cores USB3 Databook Rev. 3.30a
Expand All @@ -2567,6 +2555,33 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc)
__dwc3_gadget_stop(dwc);
spin_unlock_irqrestore(&dwc->lock, flags);

/*
* Per databook, when we want to stop the gadget, if a control transfer
* is still in process, complete it and get the core into setup phase.
* In case the host is unresponsive to a SETUP transaction, forcefully
* stall the transfer, and move back to the SETUP phase, so that any
* pending endxfers can be executed.
*/
if (dwc->ep0state != EP0_SETUP_PHASE) {
reinit_completion(&dwc->ep0_in_setup);

ret = wait_for_completion_timeout(&dwc->ep0_in_setup,
msecs_to_jiffies(DWC3_PULL_UP_TIMEOUT));
if (ret == 0) {
unsigned int dir;

dev_warn(dwc->dev, "wait for SETUP phase timed out\n");
spin_lock_irqsave(&dwc->lock, flags);
dir = !!dwc->ep0_expect_in;
if (dwc->ep0state == EP0_DATA_PHASE)
dwc3_ep0_end_control_data(dwc, dwc->eps[dir]);
else
dwc3_ep0_end_control_data(dwc, dwc->eps[!dir]);
dwc3_ep0_stall_and_restart(dwc);
spin_unlock_irqrestore(&dwc->lock, flags);
}
}

/*
* Note: if the GEVNTCOUNT indicates events in the event buffer, the
* driver needs to acknowledge them before the controller can halt.
Expand Down

0 comments on commit 3713276

Please sign in to comment.