[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251205003723.rum7bexy2tazcdwb@synopsys.com>
Date: Fri, 5 Dec 2025 00:37:31 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Selvarasu Ganesan <selvarasu.g@...sung.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Alan Stern <stern@...land.harvard.edu>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jh0801.jung@...sung.com" <jh0801.jung@...sung.com>,
"dh10.jung@...sung.com" <dh10.jung@...sung.com>,
"naushad@...sung.com" <naushad@...sung.com>,
"akash.m5@...sung.com" <akash.m5@...sung.com>,
"h10.kim@...sung.com" <h10.kim@...sung.com>,
"eomji.oh@...sung.com" <eomji.oh@...sung.com>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
"thiagu.r@...sung.com" <thiagu.r@...sung.com>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict
during StartTransfer
On Thu, Dec 04, 2025, Selvarasu Ganesan wrote:
>
> On 12/4/2025 7:21 AM, Thinh Nguyen wrote:
> > At the moment, I can't think of a way to workaround for all cases. Let's
> > just leave bulk streams alone for now. Until we have proper fixes to the
> > gadget framework, let's just try the below.
> >
>
>
> Hi Thinh,
>
> Thanks for the changes. We understand the given fix and have verified
> that the original issue is resolved, but a similar below warning appears
> again in `dwc3_gadget_ep_queue` when we run a long duration our test.
> And we confirmed this is not due to this new given changes.
>
> This warning is caused by a race between `dwc3_gadget_ep_disable` and
> `dwc3_gadget_ep_queue` that manipulates `dep->flags`.
>
> Please refer the below sequence for the reference.
>
> The warning originates from a race condition between
> dwc3_gadget_ep_disable and dwc3_send_gadget_ep_cmd from
> dwc3_gadget_ep_queue that both manipulate dep->flags. Proper
> synchronization or a check is needed when masking (dep->flags &= mask)
> inside dwc3_gadget_ep_disable.
>
I was hoping that the dwc3_gadget_ep_queue() won't come early to run
into this scenario. What I've provided will only mitigate and will not
resolve for all cases. It seems adding more checks in dwc3 will be
more messy.
Probably we should try rework the usb gadget framework instead of
workaround the problem in dwc3. Here is a potential solution I'm
thinking: introduce usb_ep_disable_with_flush().
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 0474c92ac41a..f5bb064c2e9c 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -144,10 +144,9 @@ EXPORT_SYMBOL_GPL(usb_ep_enable);
* usb_ep_disable - endpoint is no longer usable
* @ep:the endpoint being unconfigured. may not be the endpoint named "ep0".
*
- * no other task may be using this endpoint when this is called.
- * any pending and uncompleted requests will complete with status
- * indicating disconnect (-ESHUTDOWN) before this call returns.
- * gadget drivers must call usb_ep_enable() again before queueing
+ * No other task may be using this endpoint when this is called.
+ * Pending and incompleted requests must be flushed before executing
+ * this. Gadget drivers must call usb_ep_enable() again before queueing
* requests to the endpoint.
*
* This routine may be called in an atomic (interrupt) context.
@@ -174,6 +173,40 @@ int usb_ep_disable(struct usb_ep *ep)
}
EXPORT_SYMBOL_GPL(usb_ep_disable);
+/**
+ * usb_ep_disable_with_flush - disable and flush endpoint
+ * @ep: the non-control endpoint to be disabled and flushed.
+ *
+ * No other task may be using this endpoint when this is called.
+ * Any pending and incompleted requests will be completed with status
+ * indicating disconnect (-ESHUTDOWN) before this call returns.
+ * gadget drivers must call usb_ep_enable() again before queueing
+ * requests to the endpoint.
+ *
+ * This routine must be called in process context.
+ *
+ * returns zero, or a negative error code.
+ */
+int usb_ep_disable_with_flush(struct usb_ep *ep)
+{
+ int ret = 0;
+
+ if (!ep->enabled)
+ goto out;
+
+ if (!ep->ops->disable_with_flush)
+ return -EOPNOTSUPP;
+
+ ep->enabled = false;
+
+ ret = ep->ops->disable_with_flush(ep);
+out:
+ trace_usb_ep_disable_with_flush(ep, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(usb_ep_disable_with_flush);
+
/**
* usb_ep_alloc_request - allocate a request object to use with this endpoint
* @ep:the endpoint to be used with with the request
---
Then we'll gradually start auditing and replacing usb_ep_disable() with
usb_ep_disable_with_flush() where needed. We'll also need to rework the
composite framework for set_alt() to be run in process context.
BR,
Thinh
Powered by blists - more mailing lists