diff mbox

[U-Boot] usb: ci_udc: fix emissions of ZLPs

Message ID 1442013002-1136-1-git-send-email-swarren@wwwdotorg.org
State Deferred
Delegated to: Łukasz Majewski
Headers show

Commit Message

Stephen Warren Sept. 11, 2015, 11:10 p.m. UTC 
From: Stephen Warren <swarren@nvidia.com>

Commit 6a132416359e "ci_udc: Update the ci_udc driver to support bulk
transfers" caused the value of "len" to change without updating subsquent
users of that variable in ci_ep_submit_next_request(). This caused the
code that detects when to emit ZLPs (Zero Length Packets) never to
trigger, which in turn caused host timeouts when a ZLP was required,
which in turn broke tests/dfu/, even despite the assertion in that
commit's description that "These changes are tested for both the DFU and
lthor."

Fix this by modifying the added dtd iteration code not to modify "len",
but rather to keep state in a separate variable. Rename the variables
while we're at it so they describe their purpose better.

Fixes: 6a132416359e ("ci_udc: Update the ci_udc driver to support bulk transfers")
Cc: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/usb/gadget/ci_udc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Comments

Marek Vasut Sept. 12, 2015, 6:26 p.m. UTC | #1 
On Saturday, September 12, 2015 at 01:10:02 AM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Commit 6a132416359e "ci_udc: Update the ci_udc driver to support bulk
> transfers" caused the value of "len" to change without updating subsquent
> users of that variable in ci_ep_submit_next_request(). This caused the
> code that detects when to emit ZLPs (Zero Length Packets) never to
> trigger, which in turn caused host timeouts when a ZLP was required,
> which in turn broke tests/dfu/, even despite the assertion in that
> commit's description that "These changes are tested for both the DFU and
> lthor."
> 
> Fix this by modifying the added dtd iteration code not to modify "len",
> but rather to keep state in a separate variable. Rename the variables
> while we're at it so they describe their purpose better.
> 
> Fixes: 6a132416359e ("ci_udc: Update the ci_udc driver to support bulk
> transfers") Cc: Siva Durga Prasad Paladugu
> <siva.durga.paladugu@xilinx.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Applied, thanks!

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
index 3e8eb8799f4e..b875c5c148b4 100644
--- a/drivers/usb/gadget/ci_udc.c
+++ b/drivers/usb/gadget/ci_udc.c
@@ -424,7 +424,7 @@  static void ci_ep_submit_next_request(struct ci_ep *ci_ep)
 	int bit, num, len, in;
 	struct ci_req *ci_req;
 	u8 *buf;
-	uint32_t length, actlen;
+	uint32_t len_left, len_this_dtd;
 	struct ept_queue_item *dtd, *qtd;
 
 	ci_ep->req_primed = true;
@@ -442,25 +442,23 @@  static void ci_ep_submit_next_request(struct ci_ep *ci_ep)
 
 	ci_req->dtd_count = 0;
 	buf = ci_req->hw_buf;
-	actlen = 0;
+	len_left = len;
 	dtd = item;
 
 	do {
-		length = min(ci_req->req.length - actlen,
-			     (unsigned)EP_MAX_LENGTH_TRANSFER);
+		len_this_dtd = min(len_left, (unsigned)EP_MAX_LENGTH_TRANSFER);
 
-		dtd->info = INFO_BYTES(length) | INFO_ACTIVE;
+		dtd->info = INFO_BYTES(len_this_dtd) | INFO_ACTIVE;
 		dtd->page0 = (unsigned long)buf;
 		dtd->page1 = ((unsigned long)buf & 0xfffff000) + 0x1000;
 		dtd->page2 = ((unsigned long)buf & 0xfffff000) + 0x2000;
 		dtd->page3 = ((unsigned long)buf & 0xfffff000) + 0x3000;
 		dtd->page4 = ((unsigned long)buf & 0xfffff000) + 0x4000;
 
-		len -= length;
-		actlen += length;
-		buf += length;
+		len_left -= len_this_dtd;
+		buf += len_this_dtd;
 
-		if (len) {
+		if (len_left) {
 			qtd = (struct ept_queue_item *)
 			       memalign(ILIST_ALIGN, ILIST_ENT_SZ);
 			dtd->next = (unsigned long)qtd;
@@ -469,7 +467,7 @@  static void ci_ep_submit_next_request(struct ci_ep *ci_ep)
 		}
 
 		ci_req->dtd_count++;
-	} while (len);
+	} while (len_left);
 
 	item = dtd;
 	/*