When we change the USB function with configfs frequently, sometimes it will hang the system to crash. The reason is the gadget driver can not hanle the end transfer complete event after free the gadget irq (since the xHCI will share the same interrupt number with gadget, thus when free the gadget irq, it will not shutdown this gadget irq line), which will trigger the interrupt all the time. Thus we should check if we need wait for the end transfer command completion before free gadget irq. Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx> --- Changes since v1: - Simply the operation of cleaning the dep flags. --- drivers/usb/dwc3/core.h | 3 ++ drivers/usb/dwc3/gadget.c | 73 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 9128725..f01d8fd 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -537,6 +537,7 @@ struct dwc3_ep { #define DWC3_EP_BUSY (1 << 4) #define DWC3_EP_PENDING_REQUEST (1 << 5) #define DWC3_EP_MISSED_ISOC (1 << 6) +#define DWC3_EP_CMDCMPLT_BUSY (1 << 7) /* This last one is specific to EP0 */ #define DWC3_EP0_DIR_IN (1 << 31) @@ -746,6 +747,7 @@ struct dwc3_scratchpad_array { * @ep0_bounce_addr: dma address of ep0_bounce * @scratch_addr: dma address of scratchbuf * @ep0_in_setup: One control transfer is completed and enter setup phase + * @cmd_complete: endpoint command completion * @lock: for synchronizing * @dev: pointer to our struct device * @xhci: pointer to our xHCI child @@ -845,6 +847,7 @@ struct dwc3 { dma_addr_t scratch_addr; struct dwc3_request ep0_usb_req; struct completion ep0_in_setup; + struct completion cmd_complete; /* device lock */ spinlock_t lock; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index fef023a..32e3d4d 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -573,6 +573,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, dep->comp_desc = comp_desc; dep->type = usb_endpoint_type(desc); dep->flags |= DWC3_EP_ENABLED; + dep->flags &= ~DWC3_EP_CMDCMPLT_BUSY; reg = dwc3_readl(dwc->regs, DWC3_DALEPENA); reg |= DWC3_DALEPENA_EP(dep->number); @@ -650,7 +651,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) dep->endpoint.desc = NULL; dep->comp_desc = NULL; dep->type = 0; - dep->flags = 0; + dep->flags &= DWC3_EP_CMDCMPLT_BUSY; return 0; } @@ -1732,6 +1733,54 @@ static void __dwc3_gadget_stop(struct dwc3 *dwc) __dwc3_gadget_ep_disable(dwc->eps[1]); } +static void dwc3_wait_command_complete(struct dwc3 *dwc) +{ + u32 epnum, epstart = 2; + int ret, wait_cmd_complete = 0; + unsigned long flags; + +check_next: + spin_lock_irqsave(&dwc->lock, flags); + /* + * If the gadget has been in suspend state, then don't + * need to wait for the end transfer complete event. + */ + if (pm_runtime_suspended(dwc->dev)) { + spin_unlock_irqrestore(&dwc->lock, flags); + return; + } + + for (epnum = epstart; epnum < DWC3_ENDPOINTS_NUM; epnum++) { + struct dwc3_ep *dep; + + dep = dwc->eps[epnum]; + if (!dep) + continue; + + if (dep->flags & DWC3_EP_CMDCMPLT_BUSY) { + reinit_completion(&dwc->cmd_complete); + epstart = epnum + 1; + wait_cmd_complete = 1; + break; + } + } + spin_unlock_irqrestore(&dwc->lock, flags); + + /* + * Wait for 500ms to complete the end transfer command before free irq. + */ + if (wait_cmd_complete) { + wait_cmd_complete = 0; + ret = wait_for_completion_timeout(&dwc->cmd_complete, + msecs_to_jiffies(500)); + if (ret == 0) + dev_warn(dwc->dev, + "timeout to wait for command complete.\n"); + + goto check_next; + } +} + static int dwc3_gadget_stop(struct usb_gadget *g) { struct dwc3 *dwc = gadget_to_dwc(g); @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g) dwc->gadget_driver = NULL; spin_unlock_irqrestore(&dwc->lock, flags); + /* + * Since the xHCI will share the same interrupt with gadget, thus when + * free the gadget irq, it will not shutdown this gadget irq line. Then + * the gadget driver can not handle the end transfer command complete + * event after free the gadget irq, which will hang the system to crash. + * + * So we should wait for the end transfer command complete event before + * free it to avoid this situation. + */ + dwc3_wait_command_complete(dwc); + free_irq(dwc->irq_gadget, dwc->ev_buf); return 0; @@ -2108,7 +2168,8 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, dep = dwc->eps[epnum]; - if (!(dep->flags & DWC3_EP_ENABLED)) + if (!(dep->flags & DWC3_EP_ENABLED) && + !(dep->flags & DWC3_EP_CMDCMPLT_BUSY)) return; if (epnum == 0 || epnum == 1) { @@ -2180,6 +2241,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc, dwc3_trace(trace_dwc3_gadget, "%s FIFO Overrun", dep->name); break; case DWC3_DEPEVT_EPCMDCMPLT: + if (dep->flags & DWC3_EP_CMDCMPLT_BUSY) { + dep->flags &= ~DWC3_EP_CMDCMPLT_BUSY; + complete(&dwc->cmd_complete); + } + dwc3_trace(trace_dwc3_gadget, "Endpoint Command Complete"); break; } @@ -2278,7 +2344,7 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum, bool force) dep->flags &= ~DWC3_EP_BUSY; if (dwc3_is_usb31(dwc) || dwc->revision < DWC3_REVISION_310A) - udelay(100); + dep->flags |= DWC3_EP_CMDCMPLT_BUSY; } static void dwc3_stop_active_transfers(struct dwc3 *dwc) @@ -2936,6 +3002,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) } init_completion(&dwc->ep0_in_setup); + init_completion(&dwc->cmd_complete); dwc->gadget.ops = &dwc3_gadget_ops; dwc->gadget.speed = USB_SPEED_UNKNOWN; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html