From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37575) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLZuE-0004NS-LW for qemu-devel@nongnu.org; Tue, 09 Oct 2012 09:31:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLZu7-0008VF-Fi for qemu-devel@nongnu.org; Tue, 09 Oct 2012 09:31:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3444) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLZu7-0008V2-5e for qemu-devel@nongnu.org; Tue, 09 Oct 2012 09:31:23 -0400 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 q99DVM7F011699 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Oct 2012 09:31:22 -0400 Message-ID: <50742728.2020601@redhat.com> Date: Tue, 09 Oct 2012 15:31:20 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <1349682696-3046-1-git-send-email-hdegoede@redhat.com> <1349682696-3046-5-git-send-email-hdegoede@redhat.com> <5072CC04.4010300@redhat.com> <5072E838.8080104@redhat.com> <5073ECB6.4090408@redhat.com> <5073FF0A.6080304@redhat.com> In-Reply-To: <5073FF0A.6080304@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/12] usb: Add support for input pipelining List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hans de Goede Cc: qemu-devel@nongnu.org Hi, >>> 2a) The combining entity needs to know when the hcd is done with >>> queuing up packets for an ep. >> >> Yes. We can add a USBDeviceClass method for that. > > Ok, this assumes that the combining / uncombining gets moved down > into the usb-device level code. Which clearly has your preference, > but I don't completely agree with, so lets have that discussion first, > once we've decided what to do there, details like this will likely > sort out themselves. Yes. >> The core can and should do that for packets it owns (USBPacketState == >> USB_PACKET_QUEUED) because they are not (yet) passed to the USBDevice. >> >> Packets owned by USBDevice (USBPacketState == USB_PACKET_ASYNC) must be >> handled by the USBDevice itself. > > Getting offtopic a bit here, but this not how we currently handle > things, currently the hcd code cancels packets after a queue halt, Ah, right. Well, that should continue to work. USB_RET_NOT_USED would make the hcd code just free the packet, and anything not-yet freed will be zapped by the queue halt handling. > Notice that arguments 2 and 3 could be made for combining output packets > too, but what we do their now is nice and KISS, and the possible speed > improvement is not worth the complication IMHO. Handling in/out eps the same way has its merits too (code sharing). > So again lets try to split the discussion in answering multiple questions: > > 1) I believe it is better, and would greatly prefer to, do the combining > on the qemu side rather then on the usb-redir-host side, do you agree? Your call, you know the bits much better than I do. Your arguments make sense. > 2) If you agree with 1, then I assume you agree we will want to share > the combining code between host-linux.c (or host-* for that matter) and > redirect.c ? I'm not sure there is that much to share. A helper function which takes a USBEndpoint and returns an iovec for all USBPackets lined up there would probably be useful. Likewise for one for completing the packets (takes xfer length + status, then fill USBPacket->result & call usb_packet_complete for each packet). But beyond that? > 3) If you agree with 2, then all we need is a place for the shared logic > to live, we could put it in a new file called input-pipeline.c ? Just stick the helpers into hw/usb/core.c? cheers, Gerd