From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47908) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TOUo2-00048C-4g for qemu-devel@nongnu.org; Wed, 17 Oct 2012 10:41:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TOUnz-0004N4-2d for qemu-devel@nongnu.org; Wed, 17 Oct 2012 10:41:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46558) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TOUny-0004Mt-Qa for qemu-devel@nongnu.org; Wed, 17 Oct 2012 10:41:07 -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 q9HEf597002934 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 17 Oct 2012 10:41:06 -0400 Message-ID: <507EC3A3.6080201@redhat.com> Date: Wed, 17 Oct 2012 16:41:39 +0200 From: Hans de Goede MIME-Version: 1.0 References: <1350297511-25437-1-git-send-email-hdegoede@redhat.com> <1350297511-25437-15-git-send-email-hdegoede@redhat.com> <507E96A3.6020101@redhat.com> In-Reply-To: <507E96A3.6020101@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 14/22] usb: Add packet combining functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org Hi, On 10/17/2012 01:29 PM, Gerd Hoffmann wrote: > Hi, > >> + /* >> + * Process / cancel combined packets, called from >> + * usb_ep_combine_input_packets() / usb_combined_packet_cancel(). >> + * Only called for devices which call these functions themselves. >> + */ >> + int (*handle_combined_data)(USBDevice *dev, USBPacket *p); >> + void (*cancel_combined_packet)(USBDevice *dev, USBPacket *p); > > Do we really need these? For handle_combined_data, yes, as usb_ep_combine_input_packets can cause multiple packets to get submitted, since if a combined packet ends with a packet, which does not have its short_not_ok flag set, another packet can be safely pipelined after it. This is not useful for usb mass storage, but very usefull for usb serial port converters. I actually first did not have cancel_combined_packet :) Instead I had usb_combined_packet_cancel() return a boolean indicating if it had handled the cancel, or if the caller (which would be a device's cancel method) needs to handle the cancel itself. I personally find the cancel_combined_packet callback somewhat cleaner, esp. since with the return boolean method, after calling usb_combined_packet_cancel(p) p->combined will be NULL, so if the device's cancel method needs access to p->combined to deal with combined packets, things get more difficult. But if you prefer getting rid of the callback we can do that. > I think it isn't much work for the callers to > do that themself. Saves them providing a callback. And makes the code > flow easier to follow by removing a pointless indirection. > > For handle_combined_data we probably must make > usb_ep_combine_input_packets return a status code. Regards, Hans