Blame 0135-usb-ehci-Fix-and-simplify-nakcnt-handling.patch

Hans de Goede 1b1995
From 2d9b6cb9bd00ede47635dc4db413f647143d5a1d Mon Sep 17 00:00:00 2001
Hans de Goede 1b1995
From: Hans de Goede <hdegoede@redhat.com>
Hans de Goede 1b1995
Date: Thu, 1 Mar 2012 23:55:11 +0100
Hans de Goede 1b1995
Subject: [PATCH 135/140] usb-ehci: Fix and simplify nakcnt handling
Hans de Goede 1b1995
Hans de Goede 1b1995
The nakcnt code in ehci_execute_complete() marked transactions as finished
Hans de Goede 1b1995
when a packet completed with a result of USB_RET_NAK, but USB_RET_NAK
Hans de Goede 1b1995
means that the device cannot receive / send data at that time and that
Hans de Goede 1b1995
the transaction should be retried later, which is also what the usb-uhci
Hans de Goede 1b1995
and usb-ohci code does.
Hans de Goede 1b1995
Hans de Goede 1b1995
Note that there already was some special code in place to handle this
Hans de Goede 1b1995
for interrupt endpoints in the form of doing a return from
Hans de Goede 1b1995
ehci_execute_complete() when reload == 0, but that for bulk transactions
Hans de Goede 1b1995
this was not handled correctly (where as for example the usb-ccid device does
Hans de Goede 1b1995
return USB_RET_NAK for bulk packets).
Hans de Goede 1b1995
Hans de Goede 1b1995
Besides that the code in ehci_execute_complete() decrement nakcnt by 1
Hans de Goede 1b1995
on a packet result of USB_RET_NAK, but
Hans de Goede 1b1995
-since the transaction got marked as finished,
Hans de Goede 1b1995
 nakcnt would never be decremented again
Hans de Goede 1b1995
-there is no code checking for nakcnt becoming 0
Hans de Goede 1b1995
-there is no use in re-trying the transaction within the same usb frame /
Hans de Goede 1b1995
 usb-ehci frame-timer call, since the status of emulated devices won't change
Hans de Goede 1b1995
 as long as the usb-ehci frame-timer is running
Hans de Goede 1b1995
So we should simply set the nakcnt to 0 when we get a USB_RET_NAK, thus
Hans de Goede 1b1995
claiming that we've tried reload times (or as many times as possible if
Hans de Goede 1b1995
reload is 0).
Hans de Goede 1b1995
Hans de Goede 1b1995
Besides the code in ehci_execute_complete() handling USB_RET_NAK there
Hans de Goede 1b1995
was also code handling it in ehci_state_executing(), which calls
Hans de Goede 1b1995
ehci_execute_complete(), and then does its own handling on top of the handling
Hans de Goede 1b1995
in ehci_execute_complete(), this code would decrement nakcnt *again* (if not
Hans de Goede 1b1995
already 0), or restore the reload value (which was never changed) on success.
Hans de Goede 1b1995
Hans de Goede 1b1995
Since the double decrement was wrong to begin with, and is no longer needed
Hans de Goede 1b1995
now that we set nakcnt directly to 0 on USB_RET_NAK, and the restore of reload
Hans de Goede 1b1995
is not needed either, this patch simply removes all nakcnt handling from
Hans de Goede 1b1995
ehci_state_executing().
Hans de Goede 1b1995
Hans de Goede 1b1995
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Hans de Goede 1b1995
---
Hans de Goede 1b1995
 hw/usb-ehci.c |   32 ++++----------------------------
Hans de Goede 1b1995
 1 file changed, 4 insertions(+), 28 deletions(-)
Hans de Goede 1b1995
Hans de Goede 1b1995
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
Hans de Goede 1b1995
index 92cdf2a..aa6fae5 100644
Hans de Goede 1b1995
--- a/hw/usb-ehci.c
Hans de Goede 1b1995
+++ b/hw/usb-ehci.c
Hans de Goede 1b1995
@@ -1269,8 +1269,6 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet)
Hans de Goede 1b1995
 
Hans de Goede 1b1995
 static void ehci_execute_complete(EHCIQueue *q)
Hans de Goede 1b1995
 {
Hans de Goede 1b1995
-    int reload;
Hans de Goede 1b1995
-
Hans de Goede 1b1995
     assert(q->async != EHCI_ASYNC_INFLIGHT);
Hans de Goede 1b1995
     q->async = EHCI_ASYNC_NONE;
Hans de Goede 1b1995
 
Hans de Goede 1b1995
@@ -1289,16 +1287,8 @@ static void ehci_execute_complete(EHCIQueue *q)
Hans de Goede 1b1995
             ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
Hans de Goede 1b1995
             break;
Hans de Goede 1b1995
         case USB_RET_NAK:
Hans de Goede 1b1995
-            /* 4.10.3 */
Hans de Goede 1b1995
-            reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
Hans de Goede 1b1995
-            if ((q->pid == USB_TOKEN_IN) && reload) {
Hans de Goede 1b1995
-                int nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
Hans de Goede 1b1995
-                nakcnt--;
Hans de Goede 1b1995
-                set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
Hans de Goede 1b1995
-            } else if (!reload) {
Hans de Goede 1b1995
-                return;
Hans de Goede 1b1995
-            }
Hans de Goede 1b1995
-            break;
Hans de Goede 1b1995
+            set_field(&q->qh.altnext_qtd, 0, QH_ALTNEXT_NAKCNT);
Hans de Goede 1b1995
+            return; /* We're not done yet with this transaction */
Hans de Goede 1b1995
         case USB_RET_BABBLE:
Hans de Goede 1b1995
             q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
Hans de Goede 1b1995
             ehci_record_interrupt(q->ehci, USBSTS_ERRINT);
Hans de Goede 1b1995
@@ -1331,7 +1321,7 @@ static void ehci_execute_complete(EHCIQueue *q)
Hans de Goede 1b1995
     q->qh.token ^= QTD_TOKEN_DTOGGLE;
Hans de Goede 1b1995
     q->qh.token &= ~QTD_TOKEN_ACTIVE;
Hans de Goede 1b1995
 
Hans de Goede 1b1995
-    if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) {
Hans de Goede 1b1995
+    if (q->qh.token & QTD_TOKEN_IOC) {
Hans de Goede 1b1995
         ehci_record_interrupt(q->ehci, USBSTS_INT);
Hans de Goede 1b1995
     }
Hans de Goede 1b1995
 }
Hans de Goede 1b1995
@@ -1905,7 +1895,6 @@ out:
Hans de Goede 1b1995
 static int ehci_state_executing(EHCIQueue *q, int async)
Hans de Goede 1b1995
 {
Hans de Goede 1b1995
     int again = 0;
Hans de Goede 1b1995
-    int reload, nakcnt;
Hans de Goede 1b1995
 
Hans de Goede 1b1995
     ehci_execute_complete(q);
Hans de Goede 1b1995
     if (q->usb_status == USB_RET_ASYNC) {
Hans de Goede 1b1995
@@ -1925,21 +1914,8 @@ static int ehci_state_executing(EHCIQueue *q, int async)
Hans de Goede 1b1995
         // counter decrements to 0
Hans de Goede 1b1995
     }
Hans de Goede 1b1995
 
Hans de Goede 1b1995
-    reload = get_field(q->qh.epchar, QH_EPCHAR_RL);
Hans de Goede 1b1995
-    if (reload) {
Hans de Goede 1b1995
-        nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);
Hans de Goede 1b1995
-        if (q->usb_status == USB_RET_NAK) {
Hans de Goede 1b1995
-            if (nakcnt) {
Hans de Goede 1b1995
-                nakcnt--;
Hans de Goede 1b1995
-            }
Hans de Goede 1b1995
-        } else {
Hans de Goede 1b1995
-            nakcnt = reload;
Hans de Goede 1b1995
-        }
Hans de Goede 1b1995
-        set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);
Hans de Goede 1b1995
-    }
Hans de Goede 1b1995
-
Hans de Goede 1b1995
     /* 4.10.5 */
Hans de Goede 1b1995
-    if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) {
Hans de Goede 1b1995
+    if (q->usb_status == USB_RET_NAK) {
Hans de Goede 1b1995
         ehci_set_state(q->ehci, async, EST_HORIZONTALQH);
Hans de Goede 1b1995
     } else {
Hans de Goede 1b1995
         ehci_set_state(q->ehci, async, EST_WRITEBACK);
Hans de Goede 1b1995
-- 
Hans de Goede 1b1995
1.7.9.3
Hans de Goede 1b1995