Skip to contentSkip to conversation

usb: dwc3: gadget: Fix TRB preparation

PATCH Superseded Thinh Nguyen 2 participants 11 messages Aug 6, 2020 – Aug 7, 2020

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Thinh Nguyen

author maintainer
#0 lore raw

cover usb: dwc3: gadget: Fix TRB preparation

There are a few issues in DWC3 driver when preparing for TRB. The driver needs to account the following:

* MPS alignment for ZLP OUT direction
* Extra TRBs when checking for available TRBs
* SG entries size > request length

Along with these fixes, there are some cleanup/refactoring patches in this series .

Thinh Nguyen (7):
usb: dwc3: gadget: Don't setup more than requested
usb: dwc3: gadget: Fix handling ZLP
usb: dwc3: gadget: Handle ZLP for sg requests
usb: dwc3: gadget: Refactor preparing TRBs
usb: dwc3: gadget: Account for extra TRB
usb: dwc3: gadget: Rename misleading function names
usb: dwc3: ep0: Skip ZLP setup for OUT

 drivers/usb/dwc3/ep0.c    |   2 +-
 drivers/usb/dwc3/gadget.c | 232 ++++++++++++++++++++++----------------
 2 files changed, 137 insertions(+), 97 deletions(-)

base-commit: e3ee0e740c3887d2293e8d54a8707218d70d86ca

Signature · 1 line
2.28.0
author maintainer
#1 lore raw

PATCH 1/7 usb: dwc3: gadget: Don't setup more than requested

The SG list may be set up with entry size more than the requested length. Check the usb_request->length and make sure that we don't setup the TRBs to send/receive more than requested. This case may occur when the SG entry is allocated up to a certain minimum size, but the request length is less than that. It can also occur when the request is reused for a different request length.

Fixes a31e63b608ff ("usb: dwc3: gadget: Correct handling of scattergather lists")
---
 drivers/usb/dwc3/gadget.c | 41 ++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 14 deletions(-)
drivers/usb/dwc3/gadget.c +27 -14
index e44bfc3b5096..657616077502 100644
@@ -1054,27 +1054,25 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb,
  * dwc3_prepare_one_trb - setup one TRB from one request
  * @dep: endpoint for which this request is prepared
  * @req: dwc3_request pointer
+ * @trb_length: buffer size of the TRB
  * @chain: should this TRB be chained to the next?
  * @node: only for isochronous endpoints. First TRB needs different type.
  */
 static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
-		struct dwc3_request *req, unsigned chain, unsigned node)
+		struct dwc3_request *req, unsigned int trb_length,
+		unsigned chain, unsigned node)
 {
 	struct dwc3_trb		*trb;
-	unsigned int		length;
 	dma_addr_t		dma;
 	unsigned		stream_id = req->request.stream_id;
 	unsigned		short_not_ok = req->request.short_not_ok;
 	unsigned		no_interrupt = req->request.no_interrupt;
 	unsigned		is_last = req->request.is_last;
 
-	if (req->request.num_sgs > 0) {
-		length = sg_dma_len(req->start_sg);
+	if (req->request.num_sgs > 0)
 		dma = sg_dma_address(req->start_sg);
-	} else {
-		length = req->request.length;
+	else
 		dma = req->request.dma;
-	}
 
 	trb = &dep->trb_pool[dep->trb_enqueue];
 
@@ -1086,7 +1084,7 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
 
 	req->num_trbs++;
 
-	__dwc3_prepare_one_trb(dep, trb, dma, length, chain, node,
+	__dwc3_prepare_one_trb(dep, trb, dma, trb_length, chain, node,
 			stream_id, short_not_ok, no_interrupt, is_last);
 }
 
@@ -1104,8 +1102,13 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		unsigned int length = req->request.length;
 		unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
 		unsigned int rem = length % maxp;
+		unsigned int trb_length;
 		unsigned chain = true;
 
+		trb_length = min_t(unsigned int, length, sg_dma_len(req->start_sg));
+
+		length -= trb_length;
+
 		/*
 		 * IOMMU driver is coalescing the list of sgs which shares a
 		 * page boundary into one and giving it to USB driver. With
@@ -1113,7 +1116,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		 * sgs passed. So mark the chain bit to false if it isthe last
 		 * mapped sg.
 		 */
-		if (i == remaining - 1)
+		if ((i == remaining - 1) || !length)
 			chain = false;
 
 		if (rem && usb_endpoint_dir_out(dep->endpoint.desc) && !chain) {
@@ -1123,7 +1126,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 			req->needs_extra_trb = true;
 
 			/* prepare normal TRB */
-			dwc3_prepare_one_trb(dep, req, true, i);
+			dwc3_prepare_one_trb(dep, req, trb_length, true, i);
 
 			/* Now prepare one extra TRB to align transfer size */
 			trb = &dep->trb_pool[dep->trb_enqueue];
@@ -1135,7 +1138,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 					req->request.no_interrupt,
 					req->request.is_last);
 		} else {
-			dwc3_prepare_one_trb(dep, req, chain, i);
+			dwc3_prepare_one_trb(dep, req, trb_length, chain, i);
 		}
 
 		/*
@@ -1150,6 +1153,16 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 
 		req->num_queued_sgs++;
 
+		/*
+		 * The number of pending SG entries may not correspond to the
+		 * number of mapped SG entries. If all the data are queued, then
+		 * don't include unused SG entries.
+		 */
+		if (length == 0) {
+			req->num_pending_sgs -= req->request.num_mapped_sgs - req->num_queued_sgs;
+			break;
+		}
+
 		if (!dwc3_calc_trbs_left(dep))
 			break;
 	}
@@ -1169,7 +1182,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 		req->needs_extra_trb = true;
 
 		/* prepare normal TRB */
-		dwc3_prepare_one_trb(dep, req, true, 0);
+		dwc3_prepare_one_trb(dep, req, length, true, 0);
 
 		/* Now prepare one extra TRB to align transfer size */
 		trb = &dep->trb_pool[dep->trb_enqueue];
@@ -1187,7 +1200,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 		req->needs_extra_trb = true;
 
 		/* prepare normal TRB */
-		dwc3_prepare_one_trb(dep, req, true, 0);
+		dwc3_prepare_one_trb(dep, req, length, true, 0);
 
 		/* Now prepare one extra TRB to handle ZLP */
 		trb = &dep->trb_pool[dep->trb_enqueue];
@@ -1198,7 +1211,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 				req->request.no_interrupt,
 				req->request.is_last);
 	} else {
-		dwc3_prepare_one_trb(dep, req, false, 0);
+		dwc3_prepare_one_trb(dep, req, length, false, 0);
 	}
 }
 
Signature · 1 line
2.28.0
author maintainer
#2 lore raw

Re: [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested

Quoted 7 lines · Thinh Nguyen

The SG list may be set up with entry size more than the requested length. Check the usb_request->length and make sure that we don't setup the TRBs to send/receive more than requested. This case may occur when the SG entry is allocated up to a certain minimum size, but the request length is less than that. It can also occur when the request is reused for a different request length.

Quoted 8 lines
Fixes a31e63b608ff ("usb: dwc3: gadget: Correct handling of scattergather lists")
---
 drivers/usb/dwc3/gadget.c | 41 ++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 14 deletions(-)

Got a little trigger happy with this patch. Too many obvious issues. Please ignore this patch for now. Will revise and resubmit.

Thanks,
Thinh

author maintainer
#3 lore raw

PATCH 2/7 usb: dwc3: gadget: Fix handling ZLP

In reply to Thinh Nguyen

The usb_request->zero doesn't apply for isoc. Also, if we prepare a 0-length (ZLP) TRB for the OUT direction, we need to prepare an extra TRB to pad up to the MPS alignment. Use the same bounce buffer for the ZLP TRB and the extra pad TRB.

Fixes d6e5a549cc4d ("usb: dwc3: simplify ZLP handling")
Fixes 04c03d10e507 ("usb: dwc3: gadget: handle request->zero")
---
 drivers/usb/dwc3/gadget.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)
drivers/usb/dwc3/gadget.c +23 -2
index 657616077502..c0175dff194e 100644
@@ -1193,6 +1193,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 				req->request.no_interrupt,
 				req->request.is_last);
 	} else if (req->request.zero && req->request.length &&
+		   !usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
 		   (IS_ALIGNED(req->request.length, maxp))) {
 		struct dwc3	*dwc = dep->dwc;
 		struct dwc3_trb	*trb;
@@ -1202,14 +1203,25 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 		/* prepare normal TRB */
 		dwc3_prepare_one_trb(dep, req, length, true, 0);
 
-		/* Now prepare one extra TRB to handle ZLP */
+		/* Prepare one extra TRB to handle ZLP */
 		trb = &dep->trb_pool[dep->trb_enqueue];
 		req->num_trbs++;
 		__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, 0,
-				false, 1, req->request.stream_id,
+				!req->direction, 1, req->request.stream_id,
 				req->request.short_not_ok,
 				req->request.no_interrupt,
 				req->request.is_last);
+
+		/* Prepare one more TRB to handle MPS alignment for OUT */
+		if (!req->direction) {
+			trb = &dep->trb_pool[dep->trb_enqueue];
+			req->num_trbs++;
+			__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp,
+					       false, 1, req->request.stream_id,
+					       req->request.short_not_ok,
+					       req->request.no_interrupt,
+					       req->request.is_last);
+		}
 	} else {
 		dwc3_prepare_one_trb(dep, req, length, false, 0);
 	}
@@ -2684,8 +2696,17 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep,
 				status);
 
 	if (req->needs_extra_trb) {
+		unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
+
 		ret = dwc3_gadget_ep_reclaim_trb_linear(dep, req, event,
 				status);
+
+		/* Reclaim MPS padding TRB for ZLP */
+		if (!req->direction && req->request.zero && req->request.length &&
+		    !usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
+		    (IS_ALIGNED(req->request.length, maxp)))
+			ret = dwc3_gadget_ep_reclaim_trb_linear(dep, req, event, status);
+
 		req->needs_extra_trb = false;
 	}
 
Signature · 1 line
2.28.0
author maintainer
#4 lore raw

PATCH 3/7 usb: dwc3: gadget: Handle ZLP for sg requests

In reply to Thinh Nguyen

Currently dwc3 doesn't handle usb_request->zero for SG requests. This change checks and prepares extra TRBs for the ZLP for SG requests.

Fixes 04c03d10e507 ("usb: dwc3: gadget: handle request->zero")
---
 drivers/usb/dwc3/gadget.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
drivers/usb/dwc3/gadget.c +31 -0
index c0175dff194e..ced229aeccec 100644
@@ -1137,6 +1137,37 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 					req->request.short_not_ok,
 					req->request.no_interrupt,
 					req->request.is_last);
+		} else if (req->request.zero && req->request.length &&
+			   !usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
+			   !rem && !chain) {
+			struct dwc3	*dwc = dep->dwc;
+			struct dwc3_trb	*trb;
+
+			req->needs_extra_trb = true;
+
+			/* Prepare normal TRB */
+			dwc3_prepare_one_trb(dep, req, trb_length, true, i);
+
+			/* Prepare one extra TRB to handle ZLP */
+			trb = &dep->trb_pool[dep->trb_enqueue];
+			req->num_trbs++;
+			__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, 0,
+					       !req->direction, 1,
+					       req->request.stream_id,
+					       req->request.short_not_ok,
+					       req->request.no_interrupt,
+					       req->request.is_last);
+
+			/* Prepare one more TRB to handle MPS alignment */
+			if (!req->direction) {
+				trb = &dep->trb_pool[dep->trb_enqueue];
+				req->num_trbs++;
+				__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp,
+						       false, 1, req->request.stream_id,
+						       req->request.short_not_ok,
+						       req->request.no_interrupt,
+						       req->request.is_last);
+			}
 		} else {
 			dwc3_prepare_one_trb(dep, req, trb_length, chain, i);
 		}
Signature · 1 line
2.28.0
author maintainer
#5 lore raw

PATCH 4/7 usb: dwc3: gadget: Refactor preparing TRBs

In reply to Thinh Nguyen

There are a lot of common codes for preparing SG and linear TRBs. Let's refactor them for easier read. No functional change here.

---
 drivers/usb/dwc3/gadget.c | 176 +++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 109 deletions(-)
drivers/usb/dwc3/gadget.c +67 -109
index ced229aeccec..dcadef105c2a 100644
@@ -1088,6 +1088,65 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
 			stream_id, short_not_ok, no_interrupt, is_last);
 }
 
+/**
+ * dwc3_prepare_last_sg - prepare TRBs for the last SG entry
+ * @dep: The endpoint that the request belongs to
+ * @req: The request to prepare
+ * @entry_length: The last SG entry size
+ * @node: Indicates whether this is not the first entry (for isoc only)
+ */
+static void dwc3_prepare_last_sg(struct dwc3_ep *dep,
+				 struct dwc3_request *req,
+				 unsigned int entry_length,
+				 unsigned int node)
+{
+	unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
+	unsigned int rem = req->request.length % maxp;
+	unsigned int num_extra_trbs = 0;
+	unsigned int i;
+	bool do_zlp = false;
+
+	if (!usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
+	    req->request.zero && req->request.length && !rem) {
+		num_extra_trbs++;
+		do_zlp = true;
+	}
+
+	if (!req->direction && (!req->request.length || rem || do_zlp))
+		num_extra_trbs++;
+
+	if (num_extra_trbs > 0)
+		req->needs_extra_trb = true;
+
+	/* Prepare a normal TRB */
+	dwc3_prepare_one_trb(dep, req, entry_length, req->needs_extra_trb, node);
+
+	/* Prepare extra TRBs for ZLP and MPS OUT transfer alignment */
+	for (i = 0; i < num_extra_trbs; i++) {
+		struct dwc3 *dwc = dep->dwc;
+		struct dwc3_trb	*trb;
+		unsigned int extra_trb_length;
+		bool chain = true;
+
+		if (do_zlp && !i)
+			extra_trb_length = 0;
+		else
+			extra_trb_length = maxp - rem;
+
+		if (i == num_extra_trbs - 1)
+			chain = false;
+
+		trb = &dep->trb_pool[dep->trb_enqueue];
+		req->num_trbs++;
+		__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr,
+				       extra_trb_length, chain, 1,
+				       req->request.stream_id,
+				       req->request.short_not_ok,
+				       req->request.no_interrupt,
+				       req->request.is_last);
+	}
+}
+
 static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		struct dwc3_request *req)
 {
@@ -1103,7 +1162,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
 		unsigned int rem = length % maxp;
 		unsigned int trb_length;
-		unsigned chain = true;
+		bool last_sg = false;
 
 		trb_length = min_t(unsigned int, length, sg_dma_len(req->start_sg));
 
@@ -1117,60 +1176,12 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		 * mapped sg.
 		 */
 		if ((i == remaining - 1) || !length)
-			chain = false;
+			last_sg = true;
 
-		if (rem && usb_endpoint_dir_out(dep->endpoint.desc) && !chain) {
-			struct dwc3	*dwc = dep->dwc;
-			struct dwc3_trb	*trb;
-
-			req->needs_extra_trb = true;
-
-			/* prepare normal TRB */
-			dwc3_prepare_one_trb(dep, req, trb_length, true, i);
-
-			/* Now prepare one extra TRB to align transfer size */
-			trb = &dep->trb_pool[dep->trb_enqueue];
-			req->num_trbs++;
-			__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr,
-					maxp - rem, false, 1,
-					req->request.stream_id,
-					req->request.short_not_ok,
-					req->request.no_interrupt,
-					req->request.is_last);
-		} else if (req->request.zero && req->request.length &&
-			   !usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
-			   !rem && !chain) {
-			struct dwc3	*dwc = dep->dwc;
-			struct dwc3_trb	*trb;
-
-			req->needs_extra_trb = true;
-
-			/* Prepare normal TRB */
-			dwc3_prepare_one_trb(dep, req, trb_length, true, i);
-
-			/* Prepare one extra TRB to handle ZLP */
-			trb = &dep->trb_pool[dep->trb_enqueue];
-			req->num_trbs++;
-			__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, 0,
-					       !req->direction, 1,
-					       req->request.stream_id,
-					       req->request.short_not_ok,
-					       req->request.no_interrupt,
-					       req->request.is_last);
-
-			/* Prepare one more TRB to handle MPS alignment */
-			if (!req->direction) {
-				trb = &dep->trb_pool[dep->trb_enqueue];
-				req->num_trbs++;
-				__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp,
-						       false, 1, req->request.stream_id,
-						       req->request.short_not_ok,
-						       req->request.no_interrupt,
-						       req->request.is_last);
-			}
-		} else {
-			dwc3_prepare_one_trb(dep, req, trb_length, chain, i);
-		}
+		if (last_sg)
+			dwc3_prepare_last_sg(dep, req, trb_length, i);
+		else
+			dwc3_prepare_one_trb(dep, req, trb_length, 1, i);
 
 		/*
 		 * There can be a situation where all sgs in sglist are not
@@ -1179,7 +1190,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		 * we have free trbs we can continue queuing from where we
 		 * previously stopped
 		 */
-		if (chain)
+		if (!last_sg)
 			req->start_sg = sg_next(s);
 
 		req->num_queued_sgs++;
@@ -1202,60 +1213,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 		struct dwc3_request *req)
 {
-	unsigned int length = req->request.length;
-	unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
-	unsigned int rem = length % maxp;
-
-	if ((!length || rem) && usb_endpoint_dir_out(dep->endpoint.desc)) {
-		struct dwc3	*dwc = dep->dwc;
-		struct dwc3_trb	*trb;
-
-		req->needs_extra_trb = true;
-
-		/* prepare normal TRB */
-		dwc3_prepare_one_trb(dep, req, length, true, 0);
-
-		/* Now prepare one extra TRB to align transfer size */
-		trb = &dep->trb_pool[dep->trb_enqueue];
-		req->num_trbs++;
-		__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp - rem,
-				false, 1, req->request.stream_id,
-				req->request.short_not_ok,
-				req->request.no_interrupt,
-				req->request.is_last);
-	} else if (req->request.zero && req->request.length &&
-		   !usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
-		   (IS_ALIGNED(req->request.length, maxp))) {
-		struct dwc3	*dwc = dep->dwc;
-		struct dwc3_trb	*trb;
-
-		req->needs_extra_trb = true;
-
-		/* prepare normal TRB */
-		dwc3_prepare_one_trb(dep, req, length, true, 0);
-
-		/* Prepare one extra TRB to handle ZLP */
-		trb = &dep->trb_pool[dep->trb_enqueue];
-		req->num_trbs++;
-		__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, 0,
-				!req->direction, 1, req->request.stream_id,
-				req->request.short_not_ok,
-				req->request.no_interrupt,
-				req->request.is_last);
-
-		/* Prepare one more TRB to handle MPS alignment for OUT */
-		if (!req->direction) {
-			trb = &dep->trb_pool[dep->trb_enqueue];
-			req->num_trbs++;
-			__dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp,
-					       false, 1, req->request.stream_id,
-					       req->request.short_not_ok,
-					       req->request.no_interrupt,
-					       req->request.is_last);
-		}
-	} else {
-		dwc3_prepare_one_trb(dep, req, length, false, 0);
-	}
+	dwc3_prepare_last_sg(dep, req, req->request.length, 0);
 }
 
 /*
Signature · 1 line
2.28.0
author maintainer
#6 lore raw

PATCH 5/7 usb: dwc3: gadget: Account for extra TRB

In reply to Thinh Nguyen

When checking for how many TRB remaining, make sure to account for extra TRBs for ZLP or MPS alignment transfers. Since the dwc3_prepare_trb* functions should know if we need the extra TRBs, make those functions return a status code -EAGAIN if there isn't enough TRB. Check against those status when preparing TRB instead.

Fixes c6267a51639b ("usb: dwc3: gadget: align transfers to wMaxPacketSize")
---
 drivers/usb/dwc3/gadget.c | 79 ++++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 31 deletions(-)
drivers/usb/dwc3/gadget.c +48 -31
index dcadef105c2a..64c2ebacc73c 100644
@@ -1057,8 +1057,10 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb,
  * @trb_length: buffer size of the TRB
  * @chain: should this TRB be chained to the next?
  * @node: only for isochronous endpoints. First TRB needs different type.
+ *
+ * Return 0 on success or -EAGAIN if there is not enough TRBs.
  */
-static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
+static int dwc3_prepare_one_trb(struct dwc3_ep *dep,
 		struct dwc3_request *req, unsigned int trb_length,
 		unsigned chain, unsigned node)
 {
@@ -1069,6 +1071,9 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
 	unsigned		no_interrupt = req->request.no_interrupt;
 	unsigned		is_last = req->request.is_last;
 
+	if (!dwc3_calc_trbs_left(dep))
+		return -EAGAIN;
+
 	if (req->request.num_sgs > 0)
 		dma = sg_dma_address(req->start_sg);
 	else
@@ -1086,6 +1091,8 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
 
 	__dwc3_prepare_one_trb(dep, trb, dma, trb_length, chain, node,
 			stream_id, short_not_ok, no_interrupt, is_last);
+
+	return 0;
 }
 
 /**
@@ -1094,11 +1101,13 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
  * @req: The request to prepare
  * @entry_length: The last SG entry size
  * @node: Indicates whether this is not the first entry (for isoc only)
+ *
+ * Returns 0 on success or -EAGAIN if there is not enough TRBs.
  */
-static void dwc3_prepare_last_sg(struct dwc3_ep *dep,
-				 struct dwc3_request *req,
-				 unsigned int entry_length,
-				 unsigned int node)
+static int dwc3_prepare_last_sg(struct dwc3_ep *dep,
+				struct dwc3_request *req,
+				unsigned int entry_length,
+				unsigned int node)
 {
 	unsigned int maxp = usb_endpoint_maxp(dep->endpoint.desc);
 	unsigned int rem = req->request.length % maxp;
@@ -1118,6 +1127,9 @@ static void dwc3_prepare_last_sg(struct dwc3_ep *dep,
 	if (num_extra_trbs > 0)
 		req->needs_extra_trb = true;
 
+	if (dwc3_calc_trbs_left(dep) < num_extra_trbs + 1)
+		return -EAGAIN;
+
 	/* Prepare a normal TRB */
 	dwc3_prepare_one_trb(dep, req, entry_length, req->needs_extra_trb, node);
 
@@ -1145,9 +1157,11 @@ static void dwc3_prepare_last_sg(struct dwc3_ep *dep,
 				       req->request.no_interrupt,
 				       req->request.is_last);
 	}
+
+	return 0;
 }
 
-static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
+static int dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		struct dwc3_request *req)
 {
 	struct scatterlist *sg = req->start_sg;
@@ -1163,6 +1177,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 		unsigned int rem = length % maxp;
 		unsigned int trb_length;
 		bool last_sg = false;
+		int ret = 0;
 
 		trb_length = min_t(unsigned int, length, sg_dma_len(req->start_sg));
 
@@ -1179,9 +1194,13 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 			last_sg = true;
 
 		if (last_sg)
-			dwc3_prepare_last_sg(dep, req, trb_length, i);
+			ret = dwc3_prepare_last_sg(dep, req, trb_length, i);
 		else
-			dwc3_prepare_one_trb(dep, req, trb_length, 1, i);
+			ret = dwc3_prepare_one_trb(dep, req, trb_length, 1, i);
+
+		/* Ran out of TRBs */
+		if (ret)
+			return ret;
 
 		/*
 		 * There can be a situation where all sgs in sglist are not
@@ -1204,16 +1223,14 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 			req->num_pending_sgs -= req->request.num_mapped_sgs - req->num_queued_sgs;
 			break;
 		}
-
-		if (!dwc3_calc_trbs_left(dep))
-			break;
 	}
+	return 0;
 }
 
-static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
+static int dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
 		struct dwc3_request *req)
 {
-	dwc3_prepare_last_sg(dep, req, req->request.length, 0);
+	return dwc3_prepare_last_sg(dep, req, req->request.length, 0);
 }
 
 /*
@@ -1224,9 +1241,10 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
  * transfers. The function returns once there are no more TRBs available or
  * it runs out of requests.
  */
-static void dwc3_prepare_trbs(struct dwc3_ep *dep)
+static int dwc3_prepare_trbs(struct dwc3_ep *dep)
 {
 	struct dwc3_request	*req, *n;
+	int ret = 0;
 
 	BUILD_BUG_ON_NOT_POWER_OF_2(DWC3_TRB_NUM);
 
@@ -1241,11 +1259,11 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 	 * break things.
 	 */
 	list_for_each_entry(req, &dep->started_list, list) {
-		if (req->num_pending_sgs > 0)
-			dwc3_prepare_one_trb_sg(dep, req);
-
-		if (!dwc3_calc_trbs_left(dep))
-			return;
+		if (req->num_pending_sgs > 0) {
+			ret = dwc3_prepare_one_trb_sg(dep, req);
+			if (ret)
+				return ret;
+		}
 
 		/*
 		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
@@ -1253,17 +1271,16 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 		 * active transfer instead of stopping.
 		 */
 		if (dep->stream_capable && req->request.is_last)
-			return;
+			return 0;
 	}
 
 	list_for_each_entry_safe(req, n, &dep->pending_list, list) {
 		struct dwc3	*dwc = dep->dwc;
-		int		ret;
 
 		ret = usb_gadget_map_request_by_dev(dwc->sysdev, &req->request,
 						    dep->direction);
 		if (ret)
-			return;
+			return ret;
 
 		req->sg			= req->request.sg;
 		req->start_sg		= req->sg;
@@ -1271,12 +1288,12 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 		req->num_pending_sgs	= req->request.num_mapped_sgs;
 
 		if (req->num_pending_sgs > 0)
-			dwc3_prepare_one_trb_sg(dep, req);
+			ret = dwc3_prepare_one_trb_sg(dep, req);
 		else
-			dwc3_prepare_one_trb_linear(dep, req);
+			ret = dwc3_prepare_one_trb_linear(dep, req);
 
-		if (!dwc3_calc_trbs_left(dep))
-			return;
+		if (ret)
+			return ret;
 
 		/*
 		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
@@ -1284,7 +1301,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
 		 * active transfer instead of stopping.
 		 */
 		if (dep->stream_capable && req->request.is_last)
-			return;
+			return 0;
 	}
 }
 
@@ -1298,12 +1315,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)
 	int				ret;
 	u32				cmd;
 
-	if (!dwc3_calc_trbs_left(dep))
-		return 0;
-
 	starting = !(dep->flags & DWC3_EP_TRANSFER_STARTED);
 
-	dwc3_prepare_trbs(dep);
+	ret = dwc3_prepare_trbs(dep);
+	if (ret)
+		return 0;
+
 	req = next_request(&dep->started_list);
 	if (!req) {
 		dep->flags |= DWC3_EP_PENDING_REQUEST;
Signature · 1 line
2.28.0
#7 lore raw

Re: [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB

Thank you for the patch! Perhaps something to improve:

Hi Thinh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on e3ee0e740c3887d2293e8d54a8707218d70d86ca]

url:    https://github.com/0day-ci/linux/commits/Thinh-Nguyen/usb-dwc3-gadget-Fix-TRB-preparation/20200806-084719
base:    e3ee0e740c3887d2293e8d54a8707218d70d86ca
config: x86_64-randconfig-a004-20200805 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 076b120bebfd727b502208601012a44ab2e1028e)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate

All warnings (new ones prefixed by >>):

drivers/usb/dwc3/gadget.c:1177:16: warning: unused variable 'rem' [-Wunused-variable]

                   unsigned int rem = length % maxp;
                                ^
>> drivers/usb/dwc3/gadget.c:1306:1: warning: non-void function does not return a value in all control paths [-Wreturn-type]

}
^
2 warnings generated.

vim +1306 drivers/usb/dwc3/gadget.c

5ee85d890f8de5 Felipe Balbi         2016-05-13  1235  
5ee85d890f8de5 Felipe Balbi         2016-05-13  1236  /*
5ee85d890f8de5 Felipe Balbi         2016-05-13  1237   * dwc3_prepare_trbs - setup TRBs from requests
5ee85d890f8de5 Felipe Balbi         2016-05-13  1238   * @dep: endpoint for which requests are being prepared
5ee85d890f8de5 Felipe Balbi         2016-05-13  1239   *
5ee85d890f8de5 Felipe Balbi         2016-05-13  1240   * The function goes through the requests list and sets up TRBs for the
5ee85d890f8de5 Felipe Balbi         2016-05-13  1241   * transfers. The function returns once there are no more TRBs available or
5ee85d890f8de5 Felipe Balbi         2016-05-13  1242   * it runs out of requests.
5ee85d890f8de5 Felipe Balbi         2016-05-13  1243   */
e1a8607778079c Thinh Nguyen         2020-08-05  1244  static int dwc3_prepare_trbs(struct dwc3_ep *dep)
5ee85d890f8de5 Felipe Balbi         2016-05-13  1245  {
5ee85d890f8de5 Felipe Balbi         2016-05-13  1246  	struct dwc3_request	*req, *n;
e1a8607778079c Thinh Nguyen         2020-08-05  1247  	int ret = 0;
5ee85d890f8de5 Felipe Balbi         2016-05-13  1248  
5ee85d890f8de5 Felipe Balbi         2016-05-13  1249  	BUILD_BUG_ON_NOT_POWER_OF_2(DWC3_TRB_NUM);
5ee85d890f8de5 Felipe Balbi         2016-05-13  1250  
d86c5a676e5b1e Felipe Balbi         2016-10-25  1251  	/*
d86c5a676e5b1e Felipe Balbi         2016-10-25  1252  	 * We can get in a situation where there's a request in the started list
d86c5a676e5b1e Felipe Balbi         2016-10-25  1253  	 * but there weren't enough TRBs to fully kick it in the first time
d86c5a676e5b1e Felipe Balbi         2016-10-25  1254  	 * around, so it has been waiting for more TRBs to be freed up.
d86c5a676e5b1e Felipe Balbi         2016-10-25  1255  	 *
d86c5a676e5b1e Felipe Balbi         2016-10-25  1256  	 * In that case, we should check if we have a request with pending_sgs
d86c5a676e5b1e Felipe Balbi         2016-10-25  1257  	 * in the started list and prepare TRBs for that request first,
d86c5a676e5b1e Felipe Balbi         2016-10-25  1258  	 * otherwise we will prepare TRBs completely out of order and that will
d86c5a676e5b1e Felipe Balbi         2016-10-25  1259  	 * break things.
d86c5a676e5b1e Felipe Balbi         2016-10-25  1260  	 */
d86c5a676e5b1e Felipe Balbi         2016-10-25  1261  	list_for_each_entry(req, &dep->started_list, list) {
e1a8607778079c Thinh Nguyen         2020-08-05  1262  		if (req->num_pending_sgs > 0) {
e1a8607778079c Thinh Nguyen         2020-08-05  1263  			ret = dwc3_prepare_one_trb_sg(dep, req);
e1a8607778079c Thinh Nguyen         2020-08-05  1264  			if (ret)
e1a8607778079c Thinh Nguyen         2020-08-05  1265  				return ret;
e1a8607778079c Thinh Nguyen         2020-08-05  1266  		}
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1267  
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1268  		/*
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1269  		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1270  		 * burst capability may try to read and use TRBs beyond the
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1271  		 * active transfer instead of stopping.
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1272  		 */
63c7bb299fc9c4 Thinh Nguyen         2020-05-15  1273  		if (dep->stream_capable && req->request.is_last)
e1a8607778079c Thinh Nguyen         2020-08-05  1274  			return 0;
d86c5a676e5b1e Felipe Balbi         2016-10-25  1275  	}
d86c5a676e5b1e Felipe Balbi         2016-10-25  1276  
5ee85d890f8de5 Felipe Balbi         2016-05-13  1277  	list_for_each_entry_safe(req, n, &dep->pending_list, list) {
cdb55b39fab82b Felipe Balbi         2017-05-17  1278  		struct dwc3	*dwc = dep->dwc;
cdb55b39fab82b Felipe Balbi         2017-05-17  1279  
cdb55b39fab82b Felipe Balbi         2017-05-17  1280  		ret = usb_gadget_map_request_by_dev(dwc->sysdev, &req->request,
cdb55b39fab82b Felipe Balbi         2017-05-17  1281  						    dep->direction);
cdb55b39fab82b Felipe Balbi         2017-05-17  1282  		if (ret)
e1a8607778079c Thinh Nguyen         2020-08-05  1283  			return ret;
cdb55b39fab82b Felipe Balbi         2017-05-17  1284  
cdb55b39fab82b Felipe Balbi         2017-05-17  1285  		req->sg			= req->request.sg;
a31e63b608ff78 Anurag Kumar Vulisha 2018-03-27  1286  		req->start_sg		= req->sg;
c96e6725db9d6a Anurag Kumar Vulisha 2018-03-27  1287  		req->num_queued_sgs	= 0;
cdb55b39fab82b Felipe Balbi         2017-05-17  1288  		req->num_pending_sgs	= req->request.num_mapped_sgs;
cdb55b39fab82b Felipe Balbi         2017-05-17  1289  
1f512119a08c0d Felipe Balbi         2016-08-12  1290  		if (req->num_pending_sgs > 0)
e1a8607778079c Thinh Nguyen         2020-08-05  1291  			ret = dwc3_prepare_one_trb_sg(dep, req);
5ee85d890f8de5 Felipe Balbi         2016-05-13  1292  		else
e1a8607778079c Thinh Nguyen         2020-08-05  1293  			ret = dwc3_prepare_one_trb_linear(dep, req);
5ee85d890f8de5 Felipe Balbi         2016-05-13  1294  
e1a8607778079c Thinh Nguyen         2020-08-05  1295  		if (ret)
e1a8607778079c Thinh Nguyen         2020-08-05  1296  			return ret;
aefe3d232b6629 Thinh Nguyen         2020-05-05  1297  
aefe3d232b6629 Thinh Nguyen         2020-05-05  1298  		/*
aefe3d232b6629 Thinh Nguyen         2020-05-05  1299  		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
aefe3d232b6629 Thinh Nguyen         2020-05-05  1300  		 * burst capability may try to read and use TRBs beyond the
aefe3d232b6629 Thinh Nguyen         2020-05-05  1301  		 * active transfer instead of stopping.
aefe3d232b6629 Thinh Nguyen         2020-05-05  1302  		 */
aefe3d232b6629 Thinh Nguyen         2020-05-05  1303  		if (dep->stream_capable && req->request.is_last)
e1a8607778079c Thinh Nguyen         2020-08-05  1304  			return 0;
72246da40f3719 Felipe Balbi         2011-08-19  1305  	}

72246da40f3719 Felipe Balbi 2011-08-19 @1306 }
72246da40f3719 Felipe Balbi 2011-08-19 1307

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

#8 lore raw

Re: [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB

In reply to Thinh Nguyen
Thank you for the patch! Perhaps something to improve:

Hi Thinh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on e3ee0e740c3887d2293e8d54a8707218d70d86ca]

url:    https://github.com/0day-ci/linux/commits/Thinh-Nguyen/usb-dwc3-gadget-Fix-TRB-preparation/20200806-084719
base:    e3ee0e740c3887d2293e8d54a8707218d70d86ca
config: mips-randconfig-r005-20200807 (attached as .config)
compiler: mipsel-linux-gcc (GCC) 9.3.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips

If you fix the issue, kindly add following tag as appropriate

All warnings (new ones prefixed by >>):

drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_one_trb_sg':
drivers/usb/dwc3/gadget.c:1177:16: warning: unused variable 'rem' [-Wunused-variable]

    1177 |   unsigned int rem = length % maxp;
         |                ^~~
   drivers/usb/dwc3/gadget.c: In function 'dwc3_prepare_trbs':
>> drivers/usb/dwc3/gadget.c:1306:1: warning: control reaches end of non-void function [-Wreturn-type]
    1306 | }
         | ^

vim +1306 drivers/usb/dwc3/gadget.c

5ee85d890f8de5c Felipe Balbi         2016-05-13  1235  
5ee85d890f8de5c Felipe Balbi         2016-05-13  1236  /*
5ee85d890f8de5c Felipe Balbi         2016-05-13  1237   * dwc3_prepare_trbs - setup TRBs from requests
5ee85d890f8de5c Felipe Balbi         2016-05-13  1238   * @dep: endpoint for which requests are being prepared
5ee85d890f8de5c Felipe Balbi         2016-05-13  1239   *
5ee85d890f8de5c Felipe Balbi         2016-05-13  1240   * The function goes through the requests list and sets up TRBs for the
5ee85d890f8de5c Felipe Balbi         2016-05-13  1241   * transfers. The function returns once there are no more TRBs available or
5ee85d890f8de5c Felipe Balbi         2016-05-13  1242   * it runs out of requests.
5ee85d890f8de5c Felipe Balbi         2016-05-13  1243   */
e1a8607778079c1 Thinh Nguyen         2020-08-05  1244  static int dwc3_prepare_trbs(struct dwc3_ep *dep)
5ee85d890f8de5c Felipe Balbi         2016-05-13  1245  {
5ee85d890f8de5c Felipe Balbi         2016-05-13  1246  	struct dwc3_request	*req, *n;
e1a8607778079c1 Thinh Nguyen         2020-08-05  1247  	int ret = 0;
5ee85d890f8de5c Felipe Balbi         2016-05-13  1248  
5ee85d890f8de5c Felipe Balbi         2016-05-13  1249  	BUILD_BUG_ON_NOT_POWER_OF_2(DWC3_TRB_NUM);
5ee85d890f8de5c Felipe Balbi         2016-05-13  1250  
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1251  	/*
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1252  	 * We can get in a situation where there's a request in the started list
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1253  	 * but there weren't enough TRBs to fully kick it in the first time
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1254  	 * around, so it has been waiting for more TRBs to be freed up.
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1255  	 *
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1256  	 * In that case, we should check if we have a request with pending_sgs
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1257  	 * in the started list and prepare TRBs for that request first,
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1258  	 * otherwise we will prepare TRBs completely out of order and that will
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1259  	 * break things.
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1260  	 */
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1261  	list_for_each_entry(req, &dep->started_list, list) {
e1a8607778079c1 Thinh Nguyen         2020-08-05  1262  		if (req->num_pending_sgs > 0) {
e1a8607778079c1 Thinh Nguyen         2020-08-05  1263  			ret = dwc3_prepare_one_trb_sg(dep, req);
e1a8607778079c1 Thinh Nguyen         2020-08-05  1264  			if (ret)
e1a8607778079c1 Thinh Nguyen         2020-08-05  1265  				return ret;
e1a8607778079c1 Thinh Nguyen         2020-08-05  1266  		}
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1267  
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1268  		/*
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1269  		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1270  		 * burst capability may try to read and use TRBs beyond the
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1271  		 * active transfer instead of stopping.
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1272  		 */
63c7bb299fc9c43 Thinh Nguyen         2020-05-15  1273  		if (dep->stream_capable && req->request.is_last)
e1a8607778079c1 Thinh Nguyen         2020-08-05  1274  			return 0;
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1275  	}
d86c5a676e5b1ee Felipe Balbi         2016-10-25  1276  
5ee85d890f8de5c Felipe Balbi         2016-05-13  1277  	list_for_each_entry_safe(req, n, &dep->pending_list, list) {
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1278  		struct dwc3	*dwc = dep->dwc;
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1279  
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1280  		ret = usb_gadget_map_request_by_dev(dwc->sysdev, &req->request,
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1281  						    dep->direction);
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1282  		if (ret)
e1a8607778079c1 Thinh Nguyen         2020-08-05  1283  			return ret;
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1284  
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1285  		req->sg			= req->request.sg;
a31e63b608ff78c Anurag Kumar Vulisha 2018-03-27  1286  		req->start_sg		= req->sg;
c96e6725db9d6a0 Anurag Kumar Vulisha 2018-03-27  1287  		req->num_queued_sgs	= 0;
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1288  		req->num_pending_sgs	= req->request.num_mapped_sgs;
cdb55b39fab82b5 Felipe Balbi         2017-05-17  1289  
1f512119a08c0d4 Felipe Balbi         2016-08-12  1290  		if (req->num_pending_sgs > 0)
e1a8607778079c1 Thinh Nguyen         2020-08-05  1291  			ret = dwc3_prepare_one_trb_sg(dep, req);
5ee85d890f8de5c Felipe Balbi         2016-05-13  1292  		else
e1a8607778079c1 Thinh Nguyen         2020-08-05  1293  			ret = dwc3_prepare_one_trb_linear(dep, req);
5ee85d890f8de5c Felipe Balbi         2016-05-13  1294  
e1a8607778079c1 Thinh Nguyen         2020-08-05  1295  		if (ret)
e1a8607778079c1 Thinh Nguyen         2020-08-05  1296  			return ret;
aefe3d232b6629c Thinh Nguyen         2020-05-05  1297  
aefe3d232b6629c Thinh Nguyen         2020-05-05  1298  		/*
aefe3d232b6629c Thinh Nguyen         2020-05-05  1299  		 * Don't prepare beyond a transfer. In DWC_usb32, its transfer
aefe3d232b6629c Thinh Nguyen         2020-05-05  1300  		 * burst capability may try to read and use TRBs beyond the
aefe3d232b6629c Thinh Nguyen         2020-05-05  1301  		 * active transfer instead of stopping.
aefe3d232b6629c Thinh Nguyen         2020-05-05  1302  		 */
aefe3d232b6629c Thinh Nguyen         2020-05-05  1303  		if (dep->stream_capable && req->request.is_last)
e1a8607778079c1 Thinh Nguyen         2020-08-05  1304  			return 0;
72246da40f3719a Felipe Balbi         2011-08-19  1305  	}

72246da40f3719a Felipe Balbi 2011-08-19 @1306 }
72246da40f3719a Felipe Balbi 2011-08-19 1307

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

author maintainer
#9 lore raw

PATCH 6/7 usb: dwc3: gadget: Rename misleading function names

In reply to Thinh Nguyen

The functions dwc3_prepare_one_trb_sg and dwc3_prepare_one_trb_linear are not necessarily preparing "one" TRB, it can prepare multiple TRBs. Rename these functions as follow:

dwc3_prepare_one_trb_sg -> dwc3_prepare_trbs_sg dwc3_prepare_one_trb_linear -> dwc3_prepare_trbs_linear

---
 drivers/usb/dwc3/gadget.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/usb/dwc3/gadget.c +5 -5
index 64c2ebacc73c..401a72b30256 100644
@@ -1161,7 +1161,7 @@ static int dwc3_prepare_last_sg(struct dwc3_ep *dep,
 	return 0;
 }
 
-static int dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
+static int dwc3_prepare_trbs_sg(struct dwc3_ep *dep,
 		struct dwc3_request *req)
 {
 	struct scatterlist *sg = req->start_sg;
@@ -1227,7 +1227,7 @@ static int dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
 	return 0;
 }
 
-static int dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
+static int dwc3_prepare_trbs_linear(struct dwc3_ep *dep,
 		struct dwc3_request *req)
 {
 	return dwc3_prepare_last_sg(dep, req, req->request.length, 0);
@@ -1260,7 +1260,7 @@ static int dwc3_prepare_trbs(struct dwc3_ep *dep)
 	 */
 	list_for_each_entry(req, &dep->started_list, list) {
 		if (req->num_pending_sgs > 0) {
-			ret = dwc3_prepare_one_trb_sg(dep, req);
+			ret = dwc3_prepare_trbs_sg(dep, req);
 			if (ret)
 				return ret;
 		}
@@ -1288,9 +1288,9 @@ static int dwc3_prepare_trbs(struct dwc3_ep *dep)
 		req->num_pending_sgs	= req->request.num_mapped_sgs;
 
 		if (req->num_pending_sgs > 0)
-			ret = dwc3_prepare_one_trb_sg(dep, req);
+			ret = dwc3_prepare_trbs_sg(dep, req);
 		else
-			ret = dwc3_prepare_one_trb_linear(dep, req);
+			ret = dwc3_prepare_trbs_linear(dep, req);
 
 		if (ret)
 			return ret;
Signature · 1 line
2.28.0
author maintainer
#10 lore raw

PATCH 7/7 usb: dwc3: ep0: Skip ZLP setup for OUT

In reply to Thinh Nguyen

The current implementation for ZLP handling of usb_request->zero for ep0 is only for control IN requests. For OUT direction, DWC3 needs to check and set up for MPS boundary alignment, and it doesn't do that at the moment.

Usually, control OUT requests can indicate its transfer size via the wLength field of the control message. So usb_request->zero is usually not needed for OUT direction. To handle ZLP OUT for control endpoint, we'd need to allocate at least 3 TRBs for control requests (we have 2 at the moment). For now, let's just make sure the current ZLP setup is only for IN direction.

---
 drivers/usb/dwc3/ep0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/dwc3/ep0.c +1 -1
index 59f2e8c31bd1..ade9503cf876 100644
@@ -979,7 +979,7 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 					 false);
 		ret = dwc3_ep0_start_trans(dep);
 	} else if (IS_ALIGNED(req->request.length, dep->endpoint.maxpacket) &&
-		   req->request.length && req->request.zero) {
+		   req->request.length && req->request.zero && req->direction) {
 
 		ret = usb_gadget_map_request_by_dev(dwc->sysdev,
 				&req->request, dep->number);
Signature · 1 line
2.28.0