linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation
@ 2020-08-06  0:44 Thinh Nguyen
  2020-08-06  0:44 ` [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested Thinh Nguyen
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:44 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb
  Cc: John Youn, stable

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
-- 
2.28.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
@ 2020-08-06  0:44 ` Thinh Nguyen
  2020-08-06  6:58   ` Thinh Nguyen
  2020-08-06  0:44 ` [PATCH 2/7] usb: dwc3: gadget: Fix handling ZLP Thinh Nguyen
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:44 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb
  Cc: John Youn, stable

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.

Cc: stable@vger.kernel.org
Fixes: a31e63b608ff ("usb: dwc3: gadget: Correct handling of scattergather lists")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 41 ++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index e44bfc3b5096..657616077502 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -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);
 	}
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/7] usb: dwc3: gadget: Fix handling ZLP
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
  2020-08-06  0:44 ` [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested Thinh Nguyen
@ 2020-08-06  0:44 ` Thinh Nguyen
  2020-08-06  0:45 ` [PATCH 3/7] usb: dwc3: gadget: Handle ZLP for sg requests Thinh Nguyen
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:44 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb
  Cc: John Youn, stable

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.

Cc: stable@vger.kernel.org
Fixes: d6e5a549cc4d ("usb: dwc3: simplify ZLP handling")
Fixes: 04c03d10e507 ("usb: dwc3: gadget: handle request->zero")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 657616077502..c0175dff194e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -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;
 	}
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/7] usb: dwc3: gadget: Handle ZLP for sg requests
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
  2020-08-06  0:44 ` [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested Thinh Nguyen
  2020-08-06  0:44 ` [PATCH 2/7] usb: dwc3: gadget: Fix handling ZLP Thinh Nguyen
@ 2020-08-06  0:45 ` Thinh Nguyen
  2020-08-06  0:45 ` [PATCH 4/7] usb: dwc3: gadget: Refactor preparing TRBs Thinh Nguyen
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:45 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb
  Cc: John Youn, stable

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

Cc: stable@vger.kernel.org
Fixes: 04c03d10e507 ("usb: dwc3: gadget: handle request->zero")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c0175dff194e..ced229aeccec 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -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);
 		}
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/7] usb: dwc3: gadget: Refactor preparing TRBs
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
                   ` (2 preceding siblings ...)
  2020-08-06  0:45 ` [PATCH 3/7] usb: dwc3: gadget: Handle ZLP for sg requests Thinh Nguyen
@ 2020-08-06  0:45 ` Thinh Nguyen
  2020-08-06  0:45 ` [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB Thinh Nguyen
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:45 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb; +Cc: John Youn

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

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 176 +++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 109 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index ced229aeccec..dcadef105c2a 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -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);
 }
 
 /*
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
                   ` (3 preceding siblings ...)
  2020-08-06  0:45 ` [PATCH 4/7] usb: dwc3: gadget: Refactor preparing TRBs Thinh Nguyen
@ 2020-08-06  0:45 ` Thinh Nguyen
  2020-08-06  4:03   ` kernel test robot
  2020-08-07  2:30   ` kernel test robot
  2020-08-06  0:45 ` [PATCH 6/7] usb: dwc3: gadget: Rename misleading function names Thinh Nguyen
  2020-08-06  0:45 ` [PATCH 7/7] usb: dwc3: ep0: Skip ZLP setup for OUT Thinh Nguyen
  6 siblings, 2 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:45 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb; +Cc: John Youn

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")
Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 79 ++++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index dcadef105c2a..64c2ebacc73c 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -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;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/7] usb: dwc3: gadget: Rename misleading function names
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
                   ` (4 preceding siblings ...)
  2020-08-06  0:45 ` [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB Thinh Nguyen
@ 2020-08-06  0:45 ` Thinh Nguyen
  2020-08-06  0:45 ` [PATCH 7/7] usb: dwc3: ep0: Skip ZLP setup for OUT Thinh Nguyen
  6 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:45 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb; +Cc: John Youn

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

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 64c2ebacc73c..401a72b30256 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -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;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/7] usb: dwc3: ep0: Skip ZLP setup for OUT
  2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
                   ` (5 preceding siblings ...)
  2020-08-06  0:45 ` [PATCH 6/7] usb: dwc3: gadget: Rename misleading function names Thinh Nguyen
@ 2020-08-06  0:45 ` Thinh Nguyen
  6 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  0:45 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Thinh Nguyen, linux-usb; +Cc: John Youn

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.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
 drivers/usb/dwc3/ep0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 59f2e8c31bd1..ade9503cf876 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -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);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB
  2020-08-06  0:45 ` [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB Thinh Nguyen
@ 2020-08-06  4:03   ` kernel test robot
  2020-08-07  2:30   ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-08-06  4:03 UTC (permalink / raw)
  To: Thinh Nguyen, Felipe Balbi, Greg Kroah-Hartman, linux-usb
  Cc: kbuild-all, clang-built-linux, John Youn

[-- Attachment #1: Type: text/plain, Size: 7540 bytes --]

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
Reported-by: kernel test robot <lkp@intel.com>

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@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43779 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested
  2020-08-06  0:44 ` [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested Thinh Nguyen
@ 2020-08-06  6:58   ` Thinh Nguyen
  0 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2020-08-06  6:58 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, linux-usb; +Cc: John Youn, stable

Thinh Nguyen wrote:
> 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.
>
> Cc: stable@vger.kernel.org
> Fixes: a31e63b608ff ("usb: dwc3: gadget: Correct handling of scattergather lists")
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB
  2020-08-06  0:45 ` [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB Thinh Nguyen
  2020-08-06  4:03   ` kernel test robot
@ 2020-08-07  2:30   ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-08-07  2:30 UTC (permalink / raw)
  To: Thinh Nguyen, Felipe Balbi, Greg Kroah-Hartman, linux-usb
  Cc: kbuild-all, John Youn

[-- Attachment #1: Type: text/plain, Size: 7518 bytes --]

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
Reported-by: kernel test robot <lkp@intel.com>

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@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32721 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-08-07  2:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06  0:44 [PATCH 0/7] usb: dwc3: gadget: Fix TRB preparation Thinh Nguyen
2020-08-06  0:44 ` [PATCH 1/7] usb: dwc3: gadget: Don't setup more than requested Thinh Nguyen
2020-08-06  6:58   ` Thinh Nguyen
2020-08-06  0:44 ` [PATCH 2/7] usb: dwc3: gadget: Fix handling ZLP Thinh Nguyen
2020-08-06  0:45 ` [PATCH 3/7] usb: dwc3: gadget: Handle ZLP for sg requests Thinh Nguyen
2020-08-06  0:45 ` [PATCH 4/7] usb: dwc3: gadget: Refactor preparing TRBs Thinh Nguyen
2020-08-06  0:45 ` [PATCH 5/7] usb: dwc3: gadget: Account for extra TRB Thinh Nguyen
2020-08-06  4:03   ` kernel test robot
2020-08-07  2:30   ` kernel test robot
2020-08-06  0:45 ` [PATCH 6/7] usb: dwc3: gadget: Rename misleading function names Thinh Nguyen
2020-08-06  0:45 ` [PATCH 7/7] usb: dwc3: ep0: Skip ZLP setup for OUT Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).