Re: [PATCH v2 3/5] usb: dwc3: gadget: Adjust IRQ management during soft disconnect/connect

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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(&params, 0, sizeof(params));
+	spin_unlock(&dwc->lock);
   	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
+	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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux