Re: Call for testing of ehci(4), ohci(4), slhci(4), uhci(4) data toggle patch

[Matthias Drochner <M.Drochner%fz-juelich.de@localhost>]

There is a remaining problem with data toggle handling, at least
with bulk endpoints: The toggle state must persist if a usb
pipe is closed and opened again, so it needs to be stored
outside the host controller's soft pipe state.
The appended patch fixes this for me, at least on uhci/ehci.
(tested only on uhci - this fixes annoying timeouts with
some device accessed through ugen/libusb)

Can someone have a look at it?

best regards
Matthias



------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------
Forschungszentrum Juelich GmbH
52425 Juelich
Sitz der Gesellschaft: Juelich
Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498
Vorsitzender des Aufsichtsrats: MinDirig Dr. Karl Eugen Huthmacher
Geschaeftsfuehrung: Prof. Dr. Achim Bachem (Vorsitzender),
Dr. Ulrich Krafft (stellv. Vorsitzender), Prof. Dr.-Ing. Harald Bolt,
Prof. Dr. Sebastian M. Schmidt
------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------

#
# old_revision [1d86a40e92abd84616fbb24c167ab7721d5e3715]
#
# patch "sys/dev/usb/ehci.c"
#  from [e80a4d6ee7911d95a79104579abf15c38efd41d9]
#    to [f72802102c17253ec4a89ef019aa49fbc82f5cf8]
# 
# patch "sys/dev/usb/uhci.c"
#  from [7dd7c7747d374e882d3b33e24f7b59657857acf2]
#    to [a63cfa191b06e1166582f06e19a114b660d3c7d1]
# 
# patch "sys/dev/usb/usb_subr.c"
#  from [26d889890f4245a9a806aa74382505a54eb339ae]
#    to [f9a3827440aa3897e79d81769ac5e7f3269c05bc]
# 
# patch "sys/dev/usb/usbdivar.h"
#  from [502afde4a010f8ad432133b3962d295c9bdb8fc1]
#    to [4dbee31ca0d7222630d71eafea84d98416219a65]
#
============================================================
--- sys/dev/usb/ehci.c  e80a4d6ee7911d95a79104579abf15c38efd41d9
+++ sys/dev/usb/ehci.c  f72802102c17253ec4a89ef019aa49fbc82f5cf8
@@ -1543,7 +1546,8 @@ ehci_open(usbd_pipe_handle pipe)
        if (sc->sc_dying)
                return (USBD_IOERROR);
 
-       epipe->nexttoggle = 0;
+       /* toggle state needed for bulk endpoints */
+       epipe->nexttoggle = pipe->endpoint->datatoggle;
 
        if (addr == sc->sc_addr) {
                switch (ed->bEndpointAddress) {
@@ -3516,8 +3520,10 @@ ehci_device_bulk_close(usbd_pipe_handle 
 ehci_device_bulk_close(usbd_pipe_handle pipe)
 {
        ehci_softc_t *sc = pipe->device->bus->hci_private;
+       struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
 
        DPRINTF(("ehci_device_bulk_close: pipe=%p\n", pipe));
+       pipe->endpoint->datatoggle = epipe->nexttoggle;
        ehci_close_pipe(pipe, sc->sc_async_head);
 }
 
============================================================
--- sys/dev/usb/uhci.c  7dd7c7747d374e882d3b33e24f7b59657857acf2
+++ sys/dev/usb/uhci.c  a63cfa191b06e1166582f06e19a114b660d3c7d1
@@ -2177,6 +2177,8 @@ uhci_device_bulk_close(usbd_pipe_handle 
        uhci_softc_t *sc = dev->bus->hci_private;
 
        uhci_free_sqh(sc, upipe->u.bulk.sqh);
+
+       pipe->endpoint->datatoggle = upipe->nexttoggle;
 }
 
 usbd_status
@@ -3185,7 +3187,8 @@ uhci_open(usbd_pipe_handle pipe)
                     ed->bEndpointAddress, sc->sc_addr));
 
        upipe->aborting = 0;
-       upipe->nexttoggle = 0;
+       /* toggle state needed for bulk endpoints */
+       upipe->nexttoggle = pipe->endpoint->datatoggle;
 
        if (pipe->device->address == sc->sc_addr) {
                switch (ed->bEndpointAddress) {
============================================================
--- sys/dev/usb/usb_subr.c      26d889890f4245a9a806aa74382505a54eb339ae
+++ sys/dev/usb/usb_subr.c      f9a3827440aa3897e79d81769ac5e7f3269c05bc
@@ -494,6 +494,7 @@ usbd_fill_iface_data(usbd_device_handle 
                        }
                }
                ifc->endpoints[endpt].refcnt = 0;
+               ifc->endpoints[endpt].datatoggle = 0;
                p += ed->bLength;
        }
 #undef ed
@@ -1104,6 +1107,9 @@ usbd_new_device(device_t parent, usbd_bu
        USETW(dev->def_ep_desc.wMaxPacketSize, 64);
        dev->def_ep_desc.bInterval = 0;
 
+       /* doesn't matter, just don't let it uninitialized */
+       dev->def_ep.datatoggle = 0;
+
        dev->quirks = &usbd_no_quirk;
        dev->address = USB_START_ADDR;
        dev->ddesc.bMaxPacketSize = 0;
============================================================
--- sys/dev/usb/usbdivar.h      502afde4a010f8ad432133b3962d295c9bdb8fc1
+++ sys/dev/usb/usbdivar.h      4dbee31ca0d7222630d71eafea84d98416219a65
@@ -46,6 +46,7 @@ struct usbd_endpoint {
 struct usbd_endpoint {
        usb_endpoint_descriptor_t *edesc;
        int                     refcnt;
+       int datatoggle;
 };
 
 struct usbd_bus_methods {