* [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).