| From 02435a739b81ae24aff5d6e930efef9458e2af3c Mon Sep 17 00:00:00 2001 |
| From: Wesley Cheng <quic_wcheng@quicinc.com> |
| Date: Thu, 13 Apr 2023 12:57:40 -0700 |
| Subject: usb: dwc3: gadget: Stall and restart EP0 if host is unresponsive |
| |
| From: Wesley Cheng <quic_wcheng@quicinc.com> |
| |
| commit 02435a739b81ae24aff5d6e930efef9458e2af3c 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: c96683798e27 ("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> |
| --- |
| drivers/usb/dwc3/gadget.c | 49 ++++++++++++++++++++++++++++++---------------- |
| 1 file changed, 32 insertions(+), 17 deletions(-) |
| |
| --- a/drivers/usb/dwc3/gadget.c |
| +++ b/drivers/usb/dwc3/gadget.c |
| @@ -2522,29 +2522,17 @@ static int __dwc3_gadget_start(struct dw |
| 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 |
| @@ -2558,6 +2546,33 @@ static int dwc3_gadget_soft_disconnect(s |
| 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. |
| * Simply let the interrupt handler acknowledges and handle the |