[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 15/22] usb: Add packet combining functions
From: |
Hans de Goede |
Subject: |
Re: [Qemu-devel] [PATCH 15/22] usb: Add packet combining functions |
Date: |
Tue, 30 Oct 2012 14:23:39 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121016 Thunderbird/16.0.1 |
Hi,
On 10/25/2012 08:55 AM, Gerd Hoffmann wrote:
On 10/24/12 18:14, Hans de Goede wrote:
+ /*
+ * 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);
I still think these should get a USBCombinedPacket not a USBPacket.
I just rebased my tree's USB bits to your usb.68 pull req, and then
tried to make this change, and then I realized again why at least
handle_combined_data is not getting a USBCombinedPacket as argument.
The call sequence goes like this:
1) hcd calls usb_handle_packet
2) usb_handle_packet calls devices handle_data (through process_one)
3) device's handle_data sees this is for a input ep on which it is
doing input pipelining, returns USB_RET_ADD_TO_QUEUE
4) hcd calls usb_device_flush_ep_queue
5) usb_device_flush_ep_queue calls usb_ep_combine_input_packets
6) usb_ep_combine_input_packets either ends up with a combined
packet, or with a single regular packet to send to
the device
Currently usb_ep_combine_input_packets calls the device's
handle_combined_data method in both cases, and that can distinguish
between the 2 scenarios by checking the passed in USBPacket's
combined field.
I did things this way, even though it may seem more logical for
usb_ep_combine_input_packets to call the device's "regular"
handle_data method in case no combining is done for a packet,
so it is submitting a single regular packet, but in that case
we would end up at step 3) again, and the device's handle_data
will again return USB_RET_ADD_TO_QUEUE which is not what we want.
This is why handle_combined_data takes a USBPacket, and then checks
USBPacket->combined to see what to do, rather then taking a
USBCombinedPacket, as usb_ep_combine_input_packets simply does
not always have a combined packet to pass.
Alternatives to allow handle_combined_data to take a
USBCombinedPacket, would be:
1) Some flag to the device's handle_data method to indicate
it should *really* process the packet and not return
USB_RET_ADD_TO_QUEUE
2) Always make Uc allocate a USBCombinedPacket,
even when the entire transfer consists of only a single
packet, note that this in essence means an unnecessary
malloc + free call per such packet, and for example with
(redirected) usb-mass-storage one can expect each scsi
sense phase transfer to be only a single packet large,
and for smaller reads the data phase packets as well!
IMHO either alternative is worse then simply passing
a USBPacket to handle_combined_data, and let the
device's handle_combined_data figure out what to do
based on USBPacket->combined. Note that if it were
to take a USBCombinedPacket, it would end up getting
back to the USBPacket itself through
USBCombinedPacket->first anyways to get info from there
such as the packet id.
Regards,
Hans
- [Qemu-devel] [PATCH 16/22] combined-packet: Add a workaround for Linux usbfs + live migration, (continued)
- [Qemu-devel] [PATCH 16/22] combined-packet: Add a workaround for Linux usbfs + live migration, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 12/22] usb: Move clearing of queue on halt to the core, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 13/22] usb: Move short-not-ok handling to the core, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 22/22] usb-redir: Allow redirecting super speed devices to high speed controllers, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 17/22] usb-redir: Add support for 32 bits bulk packet length, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 18/22] usb-redir: Add support for input pipelining, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 19/22] usb-redir: Add an usbredir_setup_usb_eps() helper function, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 14/22] usb: Add an int_req flag to USBPacket, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 15/22] usb: Add packet combining functions, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 20/22] usb-redir: Use reject rather the disconnect on bad ep info, Hans de Goede, 2012/10/24
- [Qemu-devel] [PATCH 21/22] usb-redir: Allow to attach USB 2.0 devices to 1.1 host controller, Hans de Goede, 2012/10/24
- Re: [Qemu-devel] usb: input-pipelining + speedups v3, Gerd Hoffmann, 2012/10/25