From patchwork Fri Mar 2 18:32:31 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 144313 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 64BD4B6F9D for ; Sat, 3 Mar 2012 06:14:02 +1100 (EST) Received: from localhost ([::1]:48039 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S3XGw-0002ba-2V for incoming@patchwork.ozlabs.org; Fri, 02 Mar 2012 13:32:06 -0500 Received: from eggs.gnu.org ([208.118.235.92]:49186) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S3XGJ-0001FO-6S for qemu-devel@nongnu.org; Fri, 02 Mar 2012 13:31:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S3XFv-0006BJ-KP for qemu-devel@nongnu.org; Fri, 02 Mar 2012 13:31:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33124) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S3XFv-0006Aa-Bc for qemu-devel@nongnu.org; Fri, 02 Mar 2012 13:31:03 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q22IV1ns015075 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 2 Mar 2012 13:31:02 -0500 Received: from shalem.localdomain.com (vpn1-7-1.ams2.redhat.com [10.36.7.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q22IUltD003764; Fri, 2 Mar 2012 13:31:00 -0500 From: Hans de Goede To: Gerd Hoffmann Date: Fri, 2 Mar 2012 19:32:31 +0100 Message-Id: <1330713154-3229-11-git-send-email-hdegoede@redhat.com> In-Reply-To: <1330713154-3229-1-git-send-email-hdegoede@redhat.com> References: <1330713154-3229-1-git-send-email-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: Hans de Goede , qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH 10/13] usb-ehci: Fix and simplify nakcnt handling X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 files changed, 4 insertions(+), 28 deletions(-) diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index 07bcd1f..9197298 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1291,8 +1291,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; @@ -1311,16 +1309,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); @@ -1353,7 +1343,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); } } @@ -1877,7 +1867,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) { @@ -1897,21 +1886,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);