Re: [PATCH v3] usb: dwc3: gadget: Prevent EP resource conflicts during StartTransfer

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Tue, Sep 29, 2026, Selvarasu Ganesan wrote:
> 
> On 9/29/2026 8:38 AM, Thinh Nguyen wrote:
> > On Thu, Sep 24, 2026, Selvarasu Ganesan wrote:
> >>
> >> HI Thinh,
> >>
> >> Sorry for the delayed response. We're seeing this issue a lot in Exynos
> >> platform, so we want to get it fixed.
> >>
> >> Thanks for the explanation about UASP in last comment.
> >>
> >> We agree that for UASP, the new StartTransfer sent in ep_enable() is
> >> what makes the controller send ERDY for the host's first PRIME. The old
> >> patch skipped that StartTransfer that was wrong. If it is skipped and no
> >> request is queued right away, the controller never sends ERDY for the
> >> first PRIME and the UAS device possible hangs as you mentioned.
> >>
> >> The new patch is simpler, we won't skip it and just trigger it once the
> >> inflight end transfer is done.
> >>
> >> Please see the proposed sequence,
> >>
> >>     1. usb_ep_disable() --> EndTransfer deferred due to pending control
> >> transfer data/status stages (DWC3_EP_DELAY_STOP), old transfer still
> >> active in HW.
> >>     2. usb_ep_enable() on the same endpoint --> instead of issuing the
> >> new StartTransfer, set DWC3_EP_PENDING_START_TRANSFER (New flag) as below,
> >>
> >>     /* __dwc3_gadget_ep_enable() */
> >>     if (dep->flags & (DWC3_EP_DELAY_STOP |
> >>                       DWC3_EP_END_TRANSFER_PENDING |
> >>                       DWC3_EP_TRANSFER_STARTED))
> >>         dep->flags |= DWC3_EP_PENDING_START_TRANSFER;
> >>     else
> >>         ret = dwc3_gadget_ep_start_noop_transfer(dep);   /* unchanged path */
> >>
> >>     3. Control request finishes --> next SETUP --> existing delayed stop
> >> retry in dwc3_ep0_out_start() sends the EndTransfer for all delayed stop
> >> endpoints.
> >>     4. EndTransfer completion and trigger a previously skipped
> >> StartTransfer in ep_enable by checking DWC3_EP_PENDING_START_TRANSFER.
> >>
> >>     /* dwc3_gadget_endpoint_command_complete() */
> >>     if (dep->flags & DWC3_EP_PENDING_START_TRANSFER) {
> >>         dep->flags &= ~DWC3_EP_PENDING_START_TRANSFER;
> >>         dwc3_gadget_ep_start_noop_transfer(dep);
> >>     }
> >>
> >> So the endpoint gets the same start/stop sequence you described, only
> >> sent later, it goes out as soon as the EndTransfer finishes. So this is
> >> high possible of happens before the host's first PRIME arrives. ERDY is
> >> still sent, no UAS regression.
> >>
> >> The below testing log is for the reference. You can see the timestamp
> >> where pending start transfer triggered.
> >>
> >> [ 3273.597476]  Entry __dwc3_gadget_ep_disable
> >> [ 3273.597481]  dwc3_remove_requests ep1out skip stop transfer due to
> >> DWC3_EP_DELAY_STOP
> >> [ 3273.597510]  Entry __dwc3_gadget_ep_enable 1045 dep->name =ep1out
> >> dep->flags =e009
> >> [ 3273.597590]  dwc3_gadget_endpoint_command_complete 3857 dep->name
> >> =ep1out dep->flags =4009 --> Triggered skipped Start transfer when
> >> endpoint command completion is done.
> >>
> > Hi Selvarasu,
> >
> > I just wanted to point out that dwc3_gadget_ep_start_noop_transfer()
> > should only be needed for stream endpoints. I'll take a closer look at
> > the rest of the changes later this week.
> Hi Thinh,
> 
> Thanks for you feedback.
> 
> Agreed, The dwc3_gadget_ep_start_noop_transfer() should only be needed 
> for stream endpoints.
> 

Hi,

Can you try the below instead.

Thanks,
Thinh

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 608daeb7ef10..093627a6a409 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -770,6 +770,7 @@ struct dwc3_ep {
 #define DWC3_EP_TXFIFO_RESIZED		BIT(12)
 #define DWC3_EP_DELAY_STOP             BIT(13)
 #define DWC3_EP_RESOURCE_ALLOCATED	BIT(14)
+#define DWC3_EP_PENDING_NOOP_START	BIT(15)
 
 	/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN			BIT(31)
@@ -1676,7 +1677,8 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
 int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd,
 		u32 param);
 void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc);
-void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status);
+void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status,
+			  bool interrupt);
 #else
 static inline int dwc3_gadget_init(struct dwc3 *dwc)
 { return 0; }
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 310b5ffb236a..ff80c3a0c983 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -306,7 +306,7 @@ void dwc3_ep0_out_start(struct dwc3 *dwc)
 		if (dwc->connected)
 			dwc3_stop_active_transfer(dwc3_ep, false, true);
 		else
-			dwc3_remove_requests(dwc, dwc3_ep, -ESHUTDOWN);
+			dwc3_remove_requests(dwc, dwc3_ep, -ESHUTDOWN, false);
 	}
 }
 
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f245e66cd13d..4381a9f60f60 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -906,6 +906,54 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
 	return 0;
 }
 
+static int dwc3_gadget_ep_start_noop_transfer(struct dwc3_ep *dep)
+{
+	struct dwc3_gadget_ep_cmd_params params;
+	struct dwc3		*dwc = dep->dwc;
+	struct dwc3_trb		*trb;
+	dma_addr_t		trb_dma;
+	u32			cmd;
+	int			ret;
+
+	memset(&params, 0, sizeof(params));
+	trb = &dep->trb_pool[0];
+	trb_dma = dwc3_trb_dma_offset(dep, trb);
+
+	params.param0 = upper_32_bits(trb_dma);
+	params.param1 = lower_32_bits(trb_dma);
+
+	cmd = DWC3_DEPCMD_STARTTRANSFER;
+
+	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * For streams, at start, there maybe a race where the host primes the
+	 * endpoint before the function driver queues a request to initiate a
+	 * stream. In that case, the controller will not see the prime to
+	 * generate the ERDY and start stream. To workaround this, issue a
+	 * no-op TRB as normal, but end it immediately. As a result, when the
+	 * function driver queues the request, the next START_TRANSFER command
+	 * will cause the controller to generate an ERDY to initiate the
+	 * stream.
+	 */
+	dwc3_stop_active_transfer(dep, false, true);
+
+	/*
+	 * All stream eps will reinitiate stream on NoStream rejection.
+	 *
+	 * However, if the controller is capable of TXF_FLUSH_BYPASS, then IN
+	 * direction endpoints will automatically restart the stream without
+	 * the driver initiation.
+	 */
+	if (!dep->direction ||
+	    !(dwc->hwparams.hwparams9 & DWC3_GHWPARAMS9_DEV_TXF_FLUSH_BYPASS))
+		dep->flags |= DWC3_EP_FORCE_RESTART_STREAM;
+
+	return 0;
+}
+
 /**
  * __dwc3_gadget_ep_enable - initializes a hw endpoint
  * @dep: endpoint to be initialized
@@ -971,54 +1019,15 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action)
 	 * Issue StartTransfer here with no-op TRB so we can always rely on No
 	 * Response Update Transfer command.
 	 */
-	if (usb_endpoint_xfer_bulk(desc) ||
-			usb_endpoint_xfer_int(desc)) {
-		struct dwc3_gadget_ep_cmd_params params;
-		struct dwc3_trb	*trb;
-		dma_addr_t trb_dma;
-		u32 cmd;
-
-		memset(&params, 0, sizeof(params));
-		trb = &dep->trb_pool[0];
-		trb_dma = dwc3_trb_dma_offset(dep, trb);
-
-		params.param0 = upper_32_bits(trb_dma);
-		params.param1 = lower_32_bits(trb_dma);
-
-		cmd = DWC3_DEPCMD_STARTTRANSFER;
-
-		ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
-		if (ret < 0)
-			return ret;
-
-		if (dep->stream_capable) {
-			/*
-			 * For streams, at start, there maybe a race where the
-			 * host primes the endpoint before the function driver
-			 * queues a request to initiate a stream. In that case,
-			 * the controller will not see the prime to generate the
-			 * ERDY and start stream. To workaround this, issue a
-			 * no-op TRB as normal, but end it immediately. As a
-			 * result, when the function driver queues the request,
-			 * the next START_TRANSFER command will cause the
-			 * controller to generate an ERDY to initiate the
-			 * stream.
-			 */
-			dwc3_stop_active_transfer(dep, false, true);
-
-			/*
-			 * All stream eps will reinitiate stream on NoStream
-			 * rejection.
-			 *
-			 * However, if the controller is capable of
-			 * TXF_FLUSH_BYPASS, then IN direction endpoints will
-			 * automatically restart the stream without the driver
-			 * initiation.
-			 */
-			if (!dep->direction ||
-			    !(dwc->hwparams.hwparams9 &
-			      DWC3_GHWPARAMS9_DEV_TXF_FLUSH_BYPASS))
-				dep->flags |= DWC3_EP_FORCE_RESTART_STREAM;
+	if (dep->stream_capable) {
+		/* Defer if a previous transfer is still being retired */
+		if (dep->flags & (DWC3_EP_END_TRANSFER_PENDING |
+				  DWC3_EP_DELAY_STOP)) {
+			dep->flags |= DWC3_EP_PENDING_NOOP_START;
+		} else {
+			ret = dwc3_gadget_ep_start_noop_transfer(dep);
+			if (ret < 0)
+				return ret;
 		}
 	}
 
@@ -1028,16 +1037,21 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action)
 	return 0;
 }
 
-void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status)
+void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status,
+			  bool interrupt)
 {
 	struct dwc3_request		*req;
 
-	dwc3_stop_active_transfer(dep, false, false);
+	dwc3_stop_active_transfer(dep, false, interrupt);
 
 	/* If endxfer is delayed, avoid unmapping requests */
 	if (dep->flags & DWC3_EP_DELAY_STOP)
 		return;
 
+	/* Give the controller time to stop accessing the request buffers */
+	if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
+		mdelay(1);
+
 	/* - giveback all requests to gadget driver */
 	while (!list_empty(&dep->started_list)) {
 		req = next_request(&dep->started_list);
@@ -1084,7 +1098,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 	reg &= ~DWC3_DALEPENA_EP(dep->number);
 	dwc3_writel(dwc, DWC3_DALEPENA, reg);
 
-	dwc3_remove_requests(dwc, dep, -ESHUTDOWN);
+	dwc3_remove_requests(dwc, dep, -ESHUTDOWN, true);
 
 	dep->stream_capable = false;
 	dep->type = 0;
@@ -1096,6 +1110,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 	 */
 	if (dep->flags & DWC3_EP_DELAY_STOP)
 		mask |= (DWC3_EP_DELAY_STOP | DWC3_EP_TRANSFER_STARTED);
+
+	/*
+	 * The End Transfer command is still in progress.  Do not clear the
+	 * flags, so that the ep is only rearmed once the command completes.
+	 */
+	if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
+		mask |= (DWC3_EP_END_TRANSFER_PENDING |
+			 DWC3_EP_TRANSFER_STARTED);
+
 	dep->flags &= mask;
 
 	/* Clear out the ep descriptors for non-ep0 */
@@ -1792,9 +1815,9 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int
 
 	dep->resource_index = 0;
 
-	if (!interrupt)
+	if (!interrupt || ret)
 		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
-	else if (!ret)
+	else
 		dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
 
 	dep->flags &= ~DWC3_EP_DELAY_STOP;
@@ -2523,7 +2546,7 @@ static void dwc3_stop_active_transfers(struct dwc3 *dwc)
 		if (!dep)
 			continue;
 
-		dwc3_remove_requests(dwc, dep, -ESHUTDOWN);
+		dwc3_remove_requests(dwc, dep, -ESHUTDOWN, false);
 	}
 }
 
@@ -3867,7 +3890,16 @@ static void dwc3_gadget_endpoint_command_complete(struct dwc3_ep *dep,
 			dwc3_ep0_send_delayed_status(dwc);
 	}
 
-	if ((dep->flags & DWC3_EP_DELAY_START) &&
+	if (dep->flags & DWC3_EP_PENDING_NOOP_START) {
+		dep->flags &= ~DWC3_EP_PENDING_NOOP_START;
+
+		/* A delayed kick will arm the stream with a real TRB instead */
+		if (!(dep->flags & DWC3_EP_DELAY_START) &&
+		    (dep->flags & DWC3_EP_ENABLED) && dep->endpoint.desc)
+			dwc3_gadget_ep_start_noop_transfer(dep);
+	}
+
+	if ((dep->flags & DWC3_EP_DELAY_START) && dep->endpoint.desc &&
 	    !usb_endpoint_xfer_isoc(dep->endpoint.desc))
 		__dwc3_gadget_kick_transfer(dep);