From 35f2ecb66299e63a76e51e6758c517cf1394c8f9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 1 Mar 2012 23:55:11 +0100 Subject: [PATCH] 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 --- 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);