| 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 |
| |
| |
| @@ -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 |
| |