[PATCH 0/3] usb: dwc3: Fix TRB reclaim regression and clean up reclaim logic

[PATCH 0/3] usb: dwc3: Fix TRB reclaim regression and clean up reclaim logic

[Johannes Schneider]

Hoi,

This patch series fixes a subtle regression introduced in the recent
scatter-gather cleanup for the DWC3 USB gadget driver, and follows up
with two clean-up patches to simplify and clarify related logic.

Background:

Commit 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling") removed
some redundant state tracking in the DWC3 gadget driver, including how
scatter-gather TRBs are reclaimed after use. However, the reclaim logic
began relying on the TRB CHN (chain) bit to determine whether TRBs
belonged to a chain — which led to missed TRB reclamation in some
cases.

This broke userspace-facing protocols like MTP (Media Transfer Protocol)
when used via FunctionFS, causing incomplete transfers due to skipped
zero-length packets (ZLPs) or improperly reclaimed short TRBs.

The "offending" chunk from 61440628a4ff:
80                 ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
81 -                               trb, event, status, true);
82 +                               trb, event, status,
83 +                               !!(trb->ctrl & DWC3_TRB_CTRL_CHN));

Patch 1 fixes the issue by ensuring the HWO bit is always cleared
on reclaimed TRBs, regardless of the CHN bit.

Patches 2 and 3 follow up with simplifications:
- Patch 2 removes the now-redundant `chain` argument to the reclaim function
- Patch 3 simplifies the logic in `dwc3_needs_extra_trb()` to make the conditions easier to read and maintain

All three patches have been tested on a imx8mp based hardware, with
userspace MTP (viveris/uMTP-Responder) over FunctionFS and resolve the
regression while preserving the recent cleanup work.

Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
---
Johannes Schneider (3):
      usb: dwc3: gadget: Fix TRB reclaim logic for short transfers and ZLPs
      usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument
      usb: dwc3: gadget: Simplify logic in dwc3_needs_extra_trb()

 drivers/usb/dwc3/gadget.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
---
base-commit: d0c22de9995b624f563bc5004d44ac2655712a56
change-id: 20250621-dwc3-fix-gadget-mtp-3c09a6ab84c6

Best regards,
-- 
Johannes Schneider <johannes.schneider@leica-geosystems.com>

[PATCH 1/3] usb: dwc3: gadget: Fix TRB reclaim logic for short transfers and ZLPs

[Johannes Schneider]

Commit 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling") updated
the TRB reclaim path to use the TRB CHN (Chain) bit to determine whether
a TRB was part of a chain. However, this inadvertently changed the
behavior of reclaiming the final TRB in some scatter-gather or short
transfer cases.

In particular, if the final TRB did not have the CHN bit set, the
cleanup path could incorrectly skip clearing the HWO (Hardware Own)
bit, leaving stale TRBs in the ring. This resulted in broken data
transfer completions in userspace, notably for MTP over FunctionFS.

Fix this by unconditionally clearing the HWO bit during TRB reclaim,
regardless of the CHN bit state. This restores correct behavior
especially for transfers that require ZLPs or end on non-CHN TRBs.

Fixes: 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling")
Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
---
 drivers/usb/dwc3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 321361288935db4b773cd06235a16670a6adda1a..99fbd29d8f46d30df558ceb23d2afe7187b4244c 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3516,7 +3516,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	 * We're going to do that here to avoid problems of HW trying
 	 * to use bogus TRBs for transfers.
 	 */
-	if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
+	if (trb->ctrl & DWC3_TRB_CTRL_HWO)
 		trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
 
 	/*

-- 
2.34.1

Re: [PATCH 1/3] usb: dwc3: gadget: Fix TRB reclaim logic for short transfers and ZLPs

[Thinh Nguyen]

On Sat, Jun 21, 2025, Johannes Schneider wrote:
> Commit 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling") updated
> the TRB reclaim path to use the TRB CHN (Chain) bit to determine whether
> a TRB was part of a chain. However, this inadvertently changed the
> behavior of reclaiming the final TRB in some scatter-gather or short
> transfer cases.
> 
> In particular, if the final TRB did not have the CHN bit set, the
> cleanup path could incorrectly skip clearing the HWO (Hardware Own)
> bit, leaving stale TRBs in the ring. This resulted in broken data
> transfer completions in userspace, notably for MTP over FunctionFS.
> 
> Fix this by unconditionally clearing the HWO bit during TRB reclaim,
> regardless of the CHN bit state. This restores correct behavior
> especially for transfers that require ZLPs or end on non-CHN TRBs.
> 
> Fixes: 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling")
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> ---
>  drivers/usb/dwc3/gadget.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 321361288935db4b773cd06235a16670a6adda1a..99fbd29d8f46d30df558ceb23d2afe7187b4244c 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3516,7 +3516,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  	 * We're going to do that here to avoid problems of HW trying
>  	 * to use bogus TRBs for transfers.
>  	 */
> -	if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
> +	if (trb->ctrl & DWC3_TRB_CTRL_HWO)
>  		trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
>  
>  	/*
> 
> -- 
> 2.34.1
> 

Thanks for the catch. Please also Cc stable.

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

BR,,
Thinh

[PATCH v3] usb: dwc3: gadget: Fix TRB reclaim logic for short transfers and ZLPs

[SCHNEIDER Johannes]

Commit 96c7bf8f6b3e ("usb: dwc3: gadget: Cleanup SG handling") updated
the TRB reclaim path to use the TRB CHN (Chain) bit to determine whether
a TRB was part of a chain. However, this inadvertently changed the
behavior of reclaiming the final TRB in some scatter-gather or short
transfer cases.

In particular, if the final TRB did not have the CHN bit set, the
cleanup path could incorrectly skip clearing the HWO (Hardware Own)
bit, leaving stale TRBs in the ring. This resulted in broken data
transfer completions in userspace, notably for MTP over FunctionFS.

Fix this by unconditionally clearing the HWO bit during TRB reclaim,
regardless of the CHN bit state. This restores correct behavior
especially for transfers that require ZLPs or end on non-CHN TRBs.

Fixes 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling")
Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: <stable@vger.kernel.org> # v6.13
---
v3: no changes, re-submission as single patch, with Cc stable
v2: no changes to the patch, "faulty" re-submission
v1: initial submission as part of a series
Link: https://lore.kernel.org/lkml/AM8PR06MB7521CFF1CD8A93622A537EEDBC78A@AM8PR06MB7521.eurprd06.prod.outlook.com/

 drivers/usb/dwc3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 321361288935..99fbd29d8f46 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3516,7 +3516,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	 * We're going to do that here to avoid problems of HW trying
 	 * to use bogus TRBs for transfers.
 	 */
-	if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
+	if (trb->ctrl & DWC3_TRB_CTRL_HWO)
 		trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
 
 	/*
-- 
2.43.0

Re: [PATCH v3] usb: dwc3: gadget: Fix TRB reclaim logic for short transfers and ZLPs

[Greg Kroah-Hartman]

On Wed, Jun 25, 2025 at 07:49:16AM +0000, SCHNEIDER Johannes wrote:
> Commit 96c7bf8f6b3e ("usb: dwc3: gadget: Cleanup SG handling") updated
> the TRB reclaim path to use the TRB CHN (Chain) bit to determine whether
> a TRB was part of a chain. However, this inadvertently changed the
> behavior of reclaiming the final TRB in some scatter-gather or short
> transfer cases.
> 
> In particular, if the final TRB did not have the CHN bit set, the
> cleanup path could incorrectly skip clearing the HWO (Hardware Own)
> bit, leaving stale TRBs in the ring. This resulted in broken data
> transfer completions in userspace, notably for MTP over FunctionFS.
> 
> Fix this by unconditionally clearing the HWO bit during TRB reclaim,
> regardless of the CHN bit state. This restores correct behavior
> especially for transfers that require ZLPs or end on non-CHN TRBs.
> 
> Fixes 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling")
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Cc: <stable@vger.kernel.org> # v6.13
> ---
> v3: no changes, re-submission as single patch, with Cc stable
> v2: no changes to the patch, "faulty" re-submission
> v1: initial submission as part of a series
> Link: https://lore.kernel.org/lkml/AM8PR06MB7521CFF1CD8A93622A537EEDBC78A@AM8PR06MB7521.eurprd06.prod.outlook.com/
> 
>  drivers/usb/dwc3/gadget.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 321361288935..99fbd29d8f46 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3516,7 +3516,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  	 * We're going to do that here to avoid problems of HW trying
>  	 * to use bogus TRBs for transfers.
>  	 */
> -	if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
> +	if (trb->ctrl & DWC3_TRB_CTRL_HWO)
>  		trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
>  
>  	/*
> -- 
> 2.43.0
> 

How was this tested:

drivers/usb/dwc3/gadget.c: In function ‘dwc3_gadget_ep_reclaim_completed_trb’:
drivers/usb/dwc3/gadget.c:3519:13: error: ‘chain’ undeclared (first use in this function)
 3519 |         if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
      |             ^~~~~
drivers/usb/dwc3/gadget.c:3519:13: note: each undeclared identifier is reported only once for each function it appears in

Ugh, b4 is picking up the wrong thing here...

Can you resend these as NOT part of an existing email thread if you want
them to be applied on their own?

thanks,

greg k-h

Re: [PATCH v3] usb: dwc3: gadget: Fix TRB reclaim logic for short transfers and ZLPs

[Greg Kroah-Hartman]

On Wed, Jun 25, 2025 at 07:49:16AM +0000, SCHNEIDER Johannes wrote:
> Commit 96c7bf8f6b3e ("usb: dwc3: gadget: Cleanup SG handling") updated
> the TRB reclaim path to use the TRB CHN (Chain) bit to determine whether
> a TRB was part of a chain. However, this inadvertently changed the
> behavior of reclaiming the final TRB in some scatter-gather or short
> transfer cases.
> 
> In particular, if the final TRB did not have the CHN bit set, the
> cleanup path could incorrectly skip clearing the HWO (Hardware Own)
> bit, leaving stale TRBs in the ring. This resulted in broken data
> transfer completions in userspace, notably for MTP over FunctionFS.
> 
> Fix this by unconditionally clearing the HWO bit during TRB reclaim,
> regardless of the CHN bit state. This restores correct behavior
> especially for transfers that require ZLPs or end on non-CHN TRBs.
> 
> Fixes 61440628a4ff ("usb: dwc3: gadget: Cleanup SG handling")

Forgot the ":" here :(

I've fixed it up...

[PATCH 2/3] usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument

[Johannes Schneider]

Now that the TRB reclaim logic always inspects the TRB's CHN (Chain) bit
directly to determine whether a TRB is part of a chain, the explicit
'chain' parameter passed into dwc3_gadget_ep_reclaim_completed_trb()
is no longer necessary.

This cleanup simplifies the reclaim code by avoiding duplication of
chain state tracking, and makes the reclaim logic rely entirely on the
hardware descriptor flags — which are already present and accurate at
this stage.

No functional changes intended.

Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
---
 drivers/usb/dwc3/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 99fbd29d8f46d30df558ceb23d2afe7187b4244c..a4a2bf273f943fa112f49979297023a732e0af2e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3497,7 +3497,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 
 static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 		struct dwc3_request *req, struct dwc3_trb *trb,
-		const struct dwc3_event_depevt *event, int status, int chain)
+		const struct dwc3_event_depevt *event, int status)
 {
 	unsigned int		count;
 
@@ -3549,7 +3549,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN)
 		return 1;
 
-	if (event->status & DEPEVT_STATUS_SHORT && !chain)
+	if (event->status & DEPEVT_STATUS_SHORT &&
+	    !(trb->ctrl & DWC3_TRB_CTRL_CHN))
 		return 1;
 
 	if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
@@ -3576,8 +3577,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
 		trb = &dep->trb_pool[dep->trb_dequeue];
 
 		ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
-				trb, event, status,
-				!!(trb->ctrl & DWC3_TRB_CTRL_CHN));
+				trb, event, status);
 		if (ret)
 			break;
 	}

-- 
2.34.1

Re: [PATCH 2/3] usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument

[Thinh Nguyen]

On Sat, Jun 21, 2025, Johannes Schneider wrote:
> Now that the TRB reclaim logic always inspects the TRB's CHN (Chain) bit
> directly to determine whether a TRB is part of a chain, the explicit
> 'chain' parameter passed into dwc3_gadget_ep_reclaim_completed_trb()
> is no longer necessary.
> 
> This cleanup simplifies the reclaim code by avoiding duplication of
> chain state tracking, and makes the reclaim logic rely entirely on the
> hardware descriptor flags — which are already present and accurate at
> this stage.
> 
> No functional changes intended.
> 
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> ---
>  drivers/usb/dwc3/gadget.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 99fbd29d8f46d30df558ceb23d2afe7187b4244c..a4a2bf273f943fa112f49979297023a732e0af2e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3497,7 +3497,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
>  
>  static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  		struct dwc3_request *req, struct dwc3_trb *trb,
> -		const struct dwc3_event_depevt *event, int status, int chain)
> +		const struct dwc3_event_depevt *event, int status)
>  {
>  	unsigned int		count;
>  
> @@ -3549,7 +3549,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  	if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN)
>  		return 1;
>  
> -	if (event->status & DEPEVT_STATUS_SHORT && !chain)
> +	if (event->status & DEPEVT_STATUS_SHORT &&
> +	    !(trb->ctrl & DWC3_TRB_CTRL_CHN))
>  		return 1;
>  
>  	if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
> @@ -3576,8 +3577,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
>  		trb = &dep->trb_pool[dep->trb_dequeue];
>  
>  		ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
> -				trb, event, status,
> -				!!(trb->ctrl & DWC3_TRB_CTRL_CHN));
> +				trb, event, status);
>  		if (ret)
>  			break;
>  	}
> 
> -- 
> 2.34.1
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Note: if you want the fix [Patch 1/3] to be picked up early, then it needs to
be separated from this series. Otherwise this series won't be picked up
until the next rc1 release.

Thanks,
Thinh

[PATCH v3] usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument

[SCHNEIDER Johannes]

Now that the TRB reclaim logic always inspects the TRB's CHN (Chain) bit
directly to determine whether a TRB is part of a chain, the explicit
'chain' parameter passed into dwc3_gadget_ep_reclaim_completed_trb()
is no longer necessary.

This cleanup simplifies the reclaim code by avoiding duplication of
chain state tracking, and makes the reclaim logic rely entirely on the
hardware descriptor flags — which are already present and accurate at
this stage.

No functional changes intended.

Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
v3: no changes, re-submission as single patch
v2: no changes to the patch, "faulty" re-submission
v1: initial submission as part of a series
Link: https://lore.kernel.org/lkml/AM8PR06MB7521CFF1CD8A93622A537EEDBC78A@AM8PR06MB7521.eurprd06.prod.outlook.com/

 drivers/usb/dwc3/gadget.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 99fbd29d8f46..a4a2bf273f94 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3497,7 +3497,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
 
 static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 		struct dwc3_request *req, struct dwc3_trb *trb,
-		const struct dwc3_event_depevt *event, int status, int chain)
+		const struct dwc3_event_depevt *event, int status)
 {
 	unsigned int		count;
 
@@ -3549,7 +3549,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
 	if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN)
 		return 1;
 
-	if (event->status & DEPEVT_STATUS_SHORT && !chain)
+	if (event->status & DEPEVT_STATUS_SHORT &&
+	    !(trb->ctrl & DWC3_TRB_CTRL_CHN))
 		return 1;
 
 	if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
@@ -3576,8 +3577,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
 		trb = &dep->trb_pool[dep->trb_dequeue];
 
 		ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
-				trb, event, status,
-				!!(trb->ctrl & DWC3_TRB_CTRL_CHN));
+				trb, event, status);
 		if (ret)
 			break;
 	}
-- 
2.43.0

Re: [PATCH v3] usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument

[Greg Kroah-Hartman]

On Wed, Jun 25, 2025 at 07:53:17AM +0000, SCHNEIDER Johannes wrote:
> Now that the TRB reclaim logic always inspects the TRB's CHN (Chain) bit
> directly to determine whether a TRB is part of a chain, the explicit
> 'chain' parameter passed into dwc3_gadget_ep_reclaim_completed_trb()
> is no longer necessary.
> 
> This cleanup simplifies the reclaim code by avoiding duplication of
> chain state tracking, and makes the reclaim logic rely entirely on the
> hardware descriptor flags — which are already present and accurate at
> this stage.
> 
> No functional changes intended.
> 
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
> v3: no changes, re-submission as single patch
> v2: no changes to the patch, "faulty" re-submission
> v1: initial submission as part of a series
> Link: https://lore.kernel.org/lkml/AM8PR06MB7521CFF1CD8A93622A537EEDBC78A@AM8PR06MB7521.eurprd06.prod.outlook.com/
> 
>  drivers/usb/dwc3/gadget.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 99fbd29d8f46..a4a2bf273f94 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3497,7 +3497,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
>  
>  static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  		struct dwc3_request *req, struct dwc3_trb *trb,
> -		const struct dwc3_event_depevt *event, int status, int chain)
> +		const struct dwc3_event_depevt *event, int status)
>  {
>  	unsigned int		count;
>  
> @@ -3549,7 +3549,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
>  	if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN)
>  		return 1;
>  
> -	if (event->status & DEPEVT_STATUS_SHORT && !chain)
> +	if (event->status & DEPEVT_STATUS_SHORT &&
> +	    !(trb->ctrl & DWC3_TRB_CTRL_CHN))
>  		return 1;
>  
>  	if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
> @@ -3576,8 +3577,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
>  		trb = &dep->trb_pool[dep->trb_dequeue];
>  
>  		ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
> -				trb, event, status,
> -				!!(trb->ctrl & DWC3_TRB_CTRL_CHN));
> +				trb, event, status);
>  		if (ret)
>  			break;
>  	}
> -- 
> 2.43.0
> 

Applying just this patch I get:

  CC [M]  drivers/usb/dwc3/gadget.o
drivers/usb/dwc3/gadget.c: In function ‘dwc3_gadget_ep_reclaim_completed_trb’:
drivers/usb/dwc3/gadget.c:3517:13: error: ‘chain’ undeclared (first use in this function)
 3517 |         if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
      |             ^~~~~
drivers/usb/dwc3/gadget.c:3517:13: note: each undeclared identifier is reported only once for each function it appears in

So that's not going to work :(

Re: [PATCH v3] usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument

[SCHNEIDER Johannes]

Hoi Greg,


> > Now that the TRB reclaim logic always inspects the TRB's CHN (Chain) bit
> > directly to determine whether a TRB is part of a chain, the explicit
> > 'chain' parameter passed into dwc3_gadget_ep_reclaim_completed_trb()
> > is no longer necessary.
> >
> > This cleanup simplifies the reclaim code by avoiding duplication of
> > chain state tracking, and makes the reclaim logic rely entirely on the
> > hardware descriptor flags — which are already present and accurate at
> > this stage.
> >
> > No functional changes intended.
> >
> > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> > ---
> > v3: no changes, re-submission as single patch
> > v2: no changes to the patch, "faulty" re-submission
> > v1: initial submission as part of a series
> > Link:
> >
> >  drivers/usb/dwc3/gadget.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 99fbd29d8f46..a4a2bf273f94 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -3497,7 +3497,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
> >
> >  static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
> >               struct dwc3_request *req, struct dwc3_trb *trb,
> > -             const struct dwc3_event_depevt *event, int status, int chain)
> > +             const struct dwc3_event_depevt *event, int status)
> >  {
> >       unsigned int            count;
> >
> > @@ -3549,7 +3549,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
> >       if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN)
> >               return 1;
> >
> > -     if (event->status & DEPEVT_STATUS_SHORT && !chain)
> > +     if (event->status & DEPEVT_STATUS_SHORT &&
> > +         !(trb->ctrl & DWC3_TRB_CTRL_CHN))
> >               return 1;
> >
> >       if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
> > @@ -3576,8 +3577,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
> >               trb = &dep->trb_pool[dep->trb_dequeue];
> >
> >               ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
> > -                             trb, event, status,
> > -                             !!(trb->ctrl & DWC3_TRB_CTRL_CHN));
> > +                             trb, event, status);
> >               if (ret)
> >                       break;
> >       }
> > --
> > 2.43.0
> >
> 
> Applying just this patch I get:
> 
>   CC [M]  drivers/usb/dwc3/gadget.o
> drivers/usb/dwc3/gadget.c: In function ‘dwc3_gadget_ep_reclaim_completed_trb’:
> drivers/usb/dwc3/gadget.c:3517:13: error: ‘chain’ undeclared (first use in this function)
>  3517 |         if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
>       |             ^~~~~
> drivers/usb/dwc3/gadget.c:3517:13: note: each undeclared identifier is reported only once for each function it appears in
> 
> So that's not going to work :(
> 

Oh - sorry! i shouldn't have pulled the initial series apart :-(

I'll re-submit both patches together as v4 (this time both with cc-stable)
... without b4 to have a new mail tread


Thanks for the patience
Gruß
Johannes

Re: [PATCH v3] usb: dwc3: gadget: Simplify TRB reclaim logic by removing redundant 'chain' argument

[Greg Kroah-Hartman]

On Sun, Jun 29, 2025 at 08:45:45AM +0000, SCHNEIDER Johannes wrote:
> Hoi Greg,
> 
> 
> > > Now that the TRB reclaim logic always inspects the TRB's CHN (Chain) bit
> > > directly to determine whether a TRB is part of a chain, the explicit
> > > 'chain' parameter passed into dwc3_gadget_ep_reclaim_completed_trb()
> > > is no longer necessary.
> > >
> > > This cleanup simplifies the reclaim code by avoiding duplication of
> > > chain state tracking, and makes the reclaim logic rely entirely on the
> > > hardware descriptor flags — which are already present and accurate at
> > > this stage.
> > >
> > > No functional changes intended.
> > >
> > > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> > > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> > > ---
> > > v3: no changes, re-submission as single patch
> > > v2: no changes to the patch, "faulty" re-submission
> > > v1: initial submission as part of a series
> > > Link:
> > >
> > >  drivers/usb/dwc3/gadget.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 99fbd29d8f46..a4a2bf273f94 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -3497,7 +3497,7 @@ static void dwc3_gadget_free_endpoints(struct dwc3 *dwc)
> > >
> > >  static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
> > >               struct dwc3_request *req, struct dwc3_trb *trb,
> > > -             const struct dwc3_event_depevt *event, int status, int chain)
> > > +             const struct dwc3_event_depevt *event, int status)
> > >  {
> > >       unsigned int            count;
> > >
> > > @@ -3549,7 +3549,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep,
> > >       if ((trb->ctrl & DWC3_TRB_CTRL_HWO) && status != -ESHUTDOWN)
> > >               return 1;
> > >
> > > -     if (event->status & DEPEVT_STATUS_SHORT && !chain)
> > > +     if (event->status & DEPEVT_STATUS_SHORT &&
> > > +         !(trb->ctrl & DWC3_TRB_CTRL_CHN))
> > >               return 1;
> > >
> > >       if ((trb->ctrl & DWC3_TRB_CTRL_ISP_IMI) &&
> > > @@ -3576,8 +3577,7 @@ static int dwc3_gadget_ep_reclaim_trb_sg(struct dwc3_ep *dep,
> > >               trb = &dep->trb_pool[dep->trb_dequeue];
> > >
> > >               ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
> > > -                             trb, event, status,
> > > -                             !!(trb->ctrl & DWC3_TRB_CTRL_CHN));
> > > +                             trb, event, status);
> > >               if (ret)
> > >                       break;
> > >       }
> > > --
> > > 2.43.0
> > >
> > 
> > Applying just this patch I get:
> > 
> >   CC [M]  drivers/usb/dwc3/gadget.o
> > drivers/usb/dwc3/gadget.c: In function ‘dwc3_gadget_ep_reclaim_completed_trb’:
> > drivers/usb/dwc3/gadget.c:3517:13: error: ‘chain’ undeclared (first use in this function)
> >  3517 |         if (chain && (trb->ctrl & DWC3_TRB_CTRL_HWO))
> >       |             ^~~~~
> > drivers/usb/dwc3/gadget.c:3517:13: note: each undeclared identifier is reported only once for each function it appears in
> > 
> > So that's not going to work :(
> > 
> 
> Oh - sorry! i shouldn't have pulled the initial series apart :-(
> 
> I'll re-submit both patches together as v4 (this time both with cc-stable)
> ... without b4 to have a new mail tread

The first patch is already in my tree, I can't drop that, sorry.  Just
resend this one on it's own please.

thanks,

greg k-h

[PATCH 3/3] usb: dwc3: gadget: Simplify logic in dwc3_needs_extra_trb()

[Johannes Schneider]

The existing logic in dwc3_needs_extra_trb() checks multiple conditions
in a compound expression to determine whether an extra TRB is needed,
either for a ZLP or to handle short OUT transfers.

This commit simplifies the logic without changing behavior:
- Returns false early for isochronous endpoints
- Separates the conditions for IN vs OUT transfers
- Makes intent and flow easier to read and reason about

No functional changes intended.

Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
---
 drivers/usb/dwc3/gadget.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index a4a2bf273f943fa112f49979297023a732e0af2e..32d0fb090f4c2ffab61ae6eee29a02efd32ed032 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1420,12 +1420,13 @@ static bool dwc3_needs_extra_trb(struct dwc3_ep *dep, struct dwc3_request *req)
 	unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
 	unsigned int rem = req->request.length % maxp;
 
-	if ((req->request.length && req->request.zero && !rem &&
-			!usb_endpoint_xfer_isoc(dep->endpoint.desc)) ||
-			(!req->direction && rem))
-		return true;
+	if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
+		return false;
+
+	if (!req->direction) /* OUT transfers */
+		return rem != 0;
 
-	return false;
+	return rem == 0;
 }
 
 /**

-- 
2.34.1

Re: [PATCH 3/3] usb: dwc3: gadget: Simplify logic in dwc3_needs_extra_trb()

[Thinh Nguyen]

On Sat, Jun 21, 2025, Johannes Schneider wrote:
> The existing logic in dwc3_needs_extra_trb() checks multiple conditions
> in a compound expression to determine whether an extra TRB is needed,
> either for a ZLP or to handle short OUT transfers.
> 
> This commit simplifies the logic without changing behavior:
> - Returns false early for isochronous endpoints
> - Separates the conditions for IN vs OUT transfers
> - Makes intent and flow easier to read and reason about
> 
> No functional changes intended.
> 
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> ---
>  drivers/usb/dwc3/gadget.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a4a2bf273f943fa112f49979297023a732e0af2e..32d0fb090f4c2ffab61ae6eee29a02efd32ed032 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1420,12 +1420,13 @@ static bool dwc3_needs_extra_trb(struct dwc3_ep *dep, struct dwc3_request *req)
>  	unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
>  	unsigned int rem = req->request.length % maxp;
>  
> -	if ((req->request.length && req->request.zero && !rem &&
> -			!usb_endpoint_xfer_isoc(dep->endpoint.desc)) ||
> -			(!req->direction && rem))
> -		return true;
> +	if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
> +		return false;

I know the current flow is unsightly, but this is not the same logic.
Please help fix it.

Thanks,
Thinh

> +
> +	if (!req->direction) /* OUT transfers */
> +		return rem != 0;
>  
> -	return false;
> +	return rem == 0;
>  }
>  
>  /**
> 
> -- 
> 2.34.1
> 

Re: [PATCH 3/3] usb: dwc3: gadget: Simplify logic in dwc3_needs_extra_trb()

[SCHNEIDER Johannes]

> On Sat, Jun 21, 2025, Johannes Schneider wrote:
> > The existing logic in dwc3_needs_extra_trb() checks multiple conditions
> > in a compound expression to determine whether an extra TRB is needed,
> > either for a ZLP or to handle short OUT transfers.
> >
> > This commit simplifies the logic without changing behavior:
> > - Returns false early for isochronous endpoints
> > - Separates the conditions for IN vs OUT transfers
> > - Makes intent and flow easier to read and reason about
> >
> > No functional changes intended.
> >
> > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> > ---
> >  drivers/usb/dwc3/gadget.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index a4a2bf273f943fa112f49979297023a732e0af2e..32d0fb090f4c2ffab61ae6eee29a02efd32ed032 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -1420,12 +1420,13 @@ static bool dwc3_needs_extra_trb(struct dwc3_ep *dep, struct dwc3_request *req)
> >       unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
> >       unsigned int rem = req->request.length % maxp;
> >
> > -     if ((req->request.length && req->request.zero && !rem &&
> > -                     !usb_endpoint_xfer_isoc(dep->endpoint.desc)) ||
> > -                     (!req->direction && rem))
> > -             return true;
> > +     if (usb_endpoint_xfer_isoc(dep->endpoint.desc))
> > +             return false;
> 
> I know the current flow is unsightly, but this is not the same logic.
> Please help fix it.
>

Ups, you're right - re-verified it with a short test program
i'll just drop the third patch...

Gruß
Johannes

>
> Thanks,
> Thinh
>
> > +
> > +     if (!req->direction) /* OUT transfers */
> > +             return rem != 0;
> >
> > -     return false;
> > +     return rem == 0;
> >  }
> >
> >  /**
> >
> > --
> > 2.34.1
> >
>