From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58385) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QTaIy-0004DX-8P for qemu-devel@nongnu.org; Mon, 06 Jun 2011 09:57:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QTaIw-00041i-Am for qemu-devel@nongnu.org; Mon, 06 Jun 2011 09:57:20 -0400 Received: from sj-iport-1.cisco.com ([171.71.176.70]:29570) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QTaIv-00041Q-Kz for qemu-devel@nongnu.org; Mon, 06 Jun 2011 09:57:18 -0400 Message-ID: <4DECDCBB.1010609@cisco.com> Date: Mon, 06 Jun 2011 07:57:15 -0600 From: David Ahern MIME-Version: 1.0 References: <1307363962-27223-1-git-send-email-kraxel@redhat.com> <1307363962-27223-14-git-send-email-kraxel@redhat.com> <4DECD780.5020807@cisco.com> In-Reply-To: <4DECD780.5020807@cisco.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 13/31] usb-ehci: drop EXECUTING checks. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org On 06/06/2011 07:34 AM, David Ahern wrote: > On 06/06/2011 06:39 AM, Gerd Hoffmann wrote: >> The state machine doesn't stop in EXECUTING state any more when async >> packets are in flight, so the checks are not needed any more and can >> be dropped. >> >> Also kick out the check for the frame timer. As we don't stop & sleep >> any more on async packets this is obsolete. >> >> Signed-off-by: Gerd Hoffmann >> --- >> hw/usb-ehci.c | 32 ++------------------------------ >> 1 files changed, 2 insertions(+), 30 deletions(-) >> >> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c >> index e345a1c..8b1ae3a 100644 >> --- a/hw/usb-ehci.c >> +++ b/hw/usb-ehci.c >> @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async) >> int again = 0; >> uint32_t entry = ehci_get_fetch_addr(ehci, async); >> >> -#if EHCI_DEBUG == 0 >> - if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) { >> - if (async) { >> - DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n"); >> - goto out; >> - } else { >> - DPRINTF("FETCHENTRY: WARNING " >> - "- frame timer elapsed during periodic\n"); >> - } >> - } >> -#endif This check was added to per Section 4 of the EHCI spec -- the HC should not start transactions that will not be completed before the end of the micro-frame. If you remove this what causes the EHCI model to take a breather? David >> if (entry < 0x1000) { >> DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry); >> ehci_set_state(ehci, async, EST_ACTIVE); >> @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci) >> } >> >> ehci_set_state(ehci, async, EST_WAITLISTHEAD); >> - /* fall through */ >> - >> - case EST_FETCHENTRY: >> - /* fall through */ > > Why drop this case too? As I recall that is needed for proper execution. > > David > >> - >> - case EST_EXECUTING: >> ehci_advance_state(ehci, async); >> break; >> >> @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci) >> ehci_advance_state(ehci, async); >> break; >> >> - case EST_EXECUTING: >> - DPRINTF("PERIODIC state adv for executing\n"); >> - ehci_advance_state(ehci, async); >> - break; >> - >> default: >> /* this should only be due to a developer mistake */ >> fprintf(stderr, "ehci: Bad periodic state %d. " >> @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque) >> if (frames - i > 10) { >> skipped_frames++; >> } else { >> - // TODO could this cause periodic frames to get skipped if async >> - // active? >> - if (ehci_get_state(ehci, 1) != EST_EXECUTING) { >> - ehci_advance_periodic_state(ehci); >> - } >> + ehci_advance_periodic_state(ehci); >> } >> >> ehci->last_run_usec += FRAME_TIMER_USEC; >> @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque) >> /* Async is not inside loop since it executes everything it can once >> * called >> */ >> - if (ehci_get_state(ehci, 0) != EST_EXECUTING) { >> - ehci_advance_async_state(ehci); >> - } >> + ehci_advance_async_state(ehci); >> >> qemu_mod_timer(ehci->frame_timer, expire_time); >> }