Hi,
On 08/17/2012 12:30 PM, Gerd Hoffmann wrote:
Hi,
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
xhci does it too (although it is hard to test as xhci can happily submit
256k transfers where ehci and uhci have to use a bunch of smaller
packets instead).
Good point, I'm completely unfamiliar with the xhci code I'm afraid (at some
point
in time this will need to change, but not right now), any chance you could fill
in
the xhci part of this patch? Feel free to merge it with my patch so as to avoid
having a "broken" state in git.
Some minor nits below (the other two patches look good):
+ /* Submitting a new packet clears halt */
+ p->ep->halted = false;
check that the queue is empty when halted is set (i.e. enforce cancel on
error) ?
Good point will fix.
+
if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline) {
ret = usb_process_one(p);
if (ret == USB_RET_ASYNC) {
usb_packet_set_state(p, USB_PACKET_ASYNC);
QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
} else {
+ /*
+ * When pipelining is enabled usb-devices must always return async,
+ * otherwise packets can complete out of order!
+ */
+ assert(!p->ep->pipeline);
Strictly speaking returning something != async is fine for the first
package in the queue, but I guess in practice this doesn't matter as
enabling pipelining doesn't make sense unless you actually go async.
p->result = ret;
usb_packet_set_state(p, USB_PACKET_COMPLETE);
}
@@ -402,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
/* Notify the controller that an async packet is complete. This should only
be called for packets previously deferred by returning USB_RET_ASYNC from
handle_packet. */
That comments should be ...
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+ USBEndpoint *ep = p->ep;
+
+ assert(p->result != USB_RET_ASYNC && p->result != USB_RET_NAK);
+
+ if (p->result < 0) {
+ ep->halted = true;
+ }
+ usb_packet_set_state(p, USB_PACKET_COMPLETE);
+ QTAILQ_REMOVE(&ep->queue, p, queue);
+ dev->port->ops->complete(dev->port, p);
+}
+
... here, where the function for the external users is.
Fixed, will send a new version (without the xhci changes) right away, to
cut back on the spam I'm only going to resend 1/3.
Regards,
Hans
