From 2d9b6cb9bd00ede47635dc4db413f647143d5a1d Mon Sep 17 00:00:00 2001

From: Hans de Goede <hdegoede@redhat.com>

Date: Thu, 1 Mar 2012 23:55:11 +0100

Subject: [PATCH 135/140] usb-ehci: Fix and simplify nakcnt handling

The nakcnt code in ehci_execute_complete() marked transactions as finished

when a packet completed with a result of USB_RET_NAK, but USB_RET_NAK

means that the device cannot receive / send data at that time and that

the transaction should be retried later, which is also what the usb-uhci

and usb-ohci code does.

Note that there already was some special code in place to handle this

for interrupt endpoints in the form of doing a return from

ehci_execute_complete() when reload == 0, but that for bulk transactions

this was not handled correctly (where as for example the usb-ccid device does

return USB_RET_NAK for bulk packets).

Besides that the code in ehci_execute_complete() decrement nakcnt by 1

on a packet result of USB_RET_NAK, but

-since the transaction got marked as finished,

 nakcnt would never be decremented again

-there is no code checking for nakcnt becoming 0

-there is no use in re-trying the transaction within the same usb frame /

 usb-ehci frame-timer call, since the status of emulated devices won't change

 as long as the usb-ehci frame-timer is running

So we should simply set the nakcnt to 0 when we get a USB_RET_NAK, thus

claiming that we've tried reload times (or as many times as possible if

reload is 0).

Besides the code in ehci_execute_complete() handling USB_RET_NAK there

was also code handling it in ehci_state_executing(), which calls

ehci_execute_complete(), and then does its own handling on top of the handling

in ehci_execute_complete(), this code would decrement nakcnt *again* (if not

already 0), or restore the reload value (which was never changed) on success.

Since the double decrement was wrong to begin with, and is no longer needed

now that we set nakcnt directly to 0 on USB_RET_NAK, and the restore of reload

is not needed either, this patch simply removes all nakcnt handling from

ehci_state_executing().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>

---

 hw/usb-ehci.c |   32 ++++----------------------------

 1 file changed, 4 insertions(+), 28 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c

index 92cdf2a..aa6fae5 100644

--- a/hw/usb-ehci.c

+++ b/hw/usb-ehci.c

@@ -1269,8 +1269,6 @@ static void ehci_async_complete_packet(USBPort *port, USBPacket *packet)

 static void ehci_execute_complete(EHCIQueue *q)

 {

-    int reload;

-

     assert(q->async != EHCI_ASYNC_INFLIGHT);

     q->async = EHCI_ASYNC_NONE;

@@ -1289,16 +1287,8 @@ static void ehci_execute_complete(EHCIQueue *q)

             ehci_record_interrupt(q->ehci, USBSTS_ERRINT);

             break;

         case USB_RET_NAK:

-            /* 4.10.3 */

-            reload = get_field(q->qh.epchar, QH_EPCHAR_RL);

-            if ((q->pid == USB_TOKEN_IN) && reload) {

-                int nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);

-                nakcnt--;

-                set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);

-            } else if (!reload) {

-                return;

-            }

-            break;

+            set_field(&q->qh.altnext_qtd, 0, QH_ALTNEXT_NAKCNT);

+            return; /* We're not done yet with this transaction */

         case USB_RET_BABBLE:

             q->qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);

             ehci_record_interrupt(q->ehci, USBSTS_ERRINT);

@@ -1331,7 +1321,7 @@ static void ehci_execute_complete(EHCIQueue *q)

     q->qh.token ^= QTD_TOKEN_DTOGGLE;

     q->qh.token &= ~QTD_TOKEN_ACTIVE;

-    if ((q->usb_status != USB_RET_NAK) && (q->qh.token & QTD_TOKEN_IOC)) {

+    if (q->qh.token & QTD_TOKEN_IOC) {

         ehci_record_interrupt(q->ehci, USBSTS_INT);

     }

 }

@@ -1905,7 +1895,6 @@ out:

 static int ehci_state_executing(EHCIQueue *q, int async)

 {

     int again = 0;

-    int reload, nakcnt;

     ehci_execute_complete(q);

     if (q->usb_status == USB_RET_ASYNC) {

@@ -1925,21 +1914,8 @@ static int ehci_state_executing(EHCIQueue *q, int async)

         // counter decrements to 0

     }

-    reload = get_field(q->qh.epchar, QH_EPCHAR_RL);

-    if (reload) {

-        nakcnt = get_field(q->qh.altnext_qtd, QH_ALTNEXT_NAKCNT);

-        if (q->usb_status == USB_RET_NAK) {

-            if (nakcnt) {

-                nakcnt--;

-            }

-        } else {

-            nakcnt = reload;

-        }

-        set_field(&q->qh.altnext_qtd, nakcnt, QH_ALTNEXT_NAKCNT);

-    }

-

     /* 4.10.5 */

-    if ((q->usb_status == USB_RET_NAK) || (q->qh.token & QTD_TOKEN_ACTIVE)) {

+    if (q->usb_status == USB_RET_NAK) {

         ehci_set_state(q->ehci, async, EST_HORIZONTALQH);

     } else {

         ehci_set_state(q->ehci, async, EST_WRITEBACK);

-- 

1.7.9.3