From 0d6c0ce348f18af4028f7f8168ea816919cd801d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 4 Mar 2012 11:57:53 +0100 Subject: [PATCH 31/35] usb-ehci: Fix and simplify nakcnt handling RH-Author: Hans de Goede Message-id: <1330862278-22314-17-git-send-email-hdegoede@redhat.com> Patchwork-id: 37933 O-Subject: [PATCH 16/21] usb-ehci: Fix and simplify nakcnt handling Bugzilla: 758104 RH-Acked-by: Hans de Goede RH-Acked-by: Gerd Hoffmann RH-Acked-by: Alon Levy RH-Acked-by: Markus Armbruster 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 Upstream commit: 553a6a59f6931bf3a034945e0c1585f4b05d6000 Upstream: http://patchwork.ozlabs.org/patch/144362/ (waiting for next usb pull request) --- hw/usb-ehci.c | 32 ++++---------------------------- 1 files changed, 4 insertions(+), 28 deletions(-) Signed-off-by: Michal Novotny --- hw/usb-ehci.c | 32 ++++---------------------------- 1 files changed, 4 insertions(+), 28 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index cbf6e45..05e76d0 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1258,8 +1258,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; @@ -1278,16 +1276,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); @@ -1322,7 +1312,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); } } @@ -1864,7 +1854,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) { @@ -1884,21 +1873,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.7.6