Messages in this thread | | | Date | Thu, 14 Jul 2022 10:41:01 -0700 | Subject | Re: [PATCH v2 3/5] usb: dwc3: gadget: Adjust IRQ management during soft disconnect/connect | From | Wesley Cheng <> |
| |
Hi Thinh,
On 7/14/2022 10:38 AM, Thinh Nguyen wrote: > 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@quicinc.com> >> --- >> 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. >
Agreed.
> Hi Wesley, > > Did you try out my suggestion yet? >
In process of testing. Will update you in a few days, since it might take a day or so to reproduce.
Thanks Wesley Cheng
| |