On 7/12/2022, Wesley Cheng wrote: > Local interrupts are currently being disabled as part of aquiring the > spin lock before issuing the endxfer command. Leave interrupts enabled, so > that EP0 events can continue to be processed. Also, ensure that there are > no pending interrupts before attempting to handle any soft > connect/disconnect. > > Fixes: 861c010a2ee1 ("usb: dwc3: gadget: Refactor pullup()") > Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx> > --- > drivers/usb/dwc3/gadget.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index a455f8d4631d..ee85b773e3fe 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1674,6 +1674,7 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc) > static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool interrupt) > { > struct dwc3_gadget_ep_cmd_params params; > + struct dwc3 *dwc = dep->dwc; > u32 cmd; > int ret; > > @@ -1682,7 +1683,9 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int > cmd |= interrupt ? DWC3_DEPCMD_CMDIOC : 0; > cmd |= DWC3_DEPCMD_PARAM(dep->resource_index); > memset(¶ms, 0, sizeof(params)); > + spin_unlock(&dwc->lock); > ret = dwc3_send_gadget_ep_cmd(dep, cmd, ¶ms); > + spin_lock(&dwc->lock); > WARN_ON_ONCE(ret); > dep->resource_index = 0; > > @@ -2029,12 +2032,11 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > struct dwc3_ep *dep = to_dwc3_ep(ep); > struct dwc3 *dwc = dep->dwc; > > - unsigned long flags; > int ret = 0; > > trace_dwc3_ep_dequeue(req); > > - spin_lock_irqsave(&dwc->lock, flags); > + spin_lock(&dwc->lock); > > list_for_each_entry(r, &dep->cancelled_list, list) { > if (r == req) > @@ -2073,7 +2075,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, > request, ep->name); > ret = -EINVAL; > out: > - spin_unlock_irqrestore(&dwc->lock, flags); > + spin_unlock(&dwc->lock); > > return ret; > } > @@ -2489,9 +2491,7 @@ static int __dwc3_gadget_start(struct dwc3 *dwc); > > static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc) > { > - unsigned long flags; > - > - spin_lock_irqsave(&dwc->lock, flags); > + spin_lock(&dwc->lock); > dwc->connected = false; > > /* > @@ -2506,10 +2506,10 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc) > > reinit_completion(&dwc->ep0_in_setup); > > - spin_unlock_irqrestore(&dwc->lock, flags); > + spin_unlock(&dwc->lock); > ret = wait_for_completion_timeout(&dwc->ep0_in_setup, > msecs_to_jiffies(DWC3_PULL_UP_TIMEOUT)); > - spin_lock_irqsave(&dwc->lock, flags); > + spin_lock(&dwc->lock); > if (ret == 0) > dev_warn(dwc->dev, "timed out waiting for SETUP phase\n"); > } > @@ -2523,7 +2523,7 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc) > */ > dwc3_stop_active_transfers(dwc); > __dwc3_gadget_stop(dwc); > - spin_unlock_irqrestore(&dwc->lock, flags); > + spin_unlock(&dwc->lock); > > /* > * Note: if the GEVNTCOUNT indicates events in the event buffer, the > @@ -2569,6 +2569,8 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > return 0; > } > > + synchronize_irq(dwc->irq_gadget); > + > if (!is_on) { > ret = dwc3_gadget_soft_disconnect(dwc); > } else { > @@ -3729,6 +3731,7 @@ void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, > */ > > __dwc3_stop_active_transfer(dep, force, interrupt); > + > } > > static void dwc3_clear_stall_all_ep(struct dwc3 *dwc) Hi Greg, Please don't pick up this patch yet. We're still in discussion with this. I have some concern with unlocking/locking when sending End Transfer command. For example, this patch may cause issues with DWC3_EP_END_TRANSFER_PENDING checks. Hi Wesley, Did you try out my suggestion yet? Thanks, Thinh