On Thu, Oct 02, 2014 at 12:20:14PM +0200, Martin Pieuchot wrote:
> Our USB stack contains a hack needed for ehci(4) and ohci(4) that
> breaks xhci(4). The diff below moves this hack in these drivers,
> and makes it possible to have a working xhci(4) in GENERIC.
>
> I'd like this diff to be tested on as much machines as possible, because
> the code path it touches is very sensible. This also matters if you are
> using uhci(4)!
>
> Please test and report back.
Tested umass on x61s (amd64 MP) and everything seems to be working just
fine.
I also read the diff and I'm okay with it going in.
>
> Thanks,
> Martin
>
> Index: ehci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ehci.c,v
> retrieving revision 1.168
> diff -u -p -r1.168 ehci.c
> --- ehci.c 1 Sep 2014 08:13:02 -0000 1.168
> +++ ehci.c 2 Oct 2014 09:30:28 -0000
> @@ -99,6 +99,7 @@ struct ehci_pipe {
> u_int8_t ehci_reverse_bits(u_int8_t, int);
>
> usbd_status ehci_open(struct usbd_pipe *);
> +int ehci_setaddr(struct usbd_device *, int);
> void ehci_poll(struct usbd_bus *);
> void ehci_softintr(void *);
> int ehci_intr1(struct ehci_softc *);
> @@ -215,7 +216,7 @@ void ehci_dump_exfer(struct ehci_xfer *
>
> struct usbd_bus_methods ehci_bus_methods = {
> .open_pipe = ehci_open,
> - .dev_setaddr = usbd_set_address,
> + .dev_setaddr = ehci_setaddr,
> .soft_intr = ehci_softintr,
> .do_poll = ehci_poll,
> .allocx = ehci_allocx,
> @@ -603,6 +604,40 @@ ehci_pcd(struct ehci_softc *sc, struct u
> xfer->status = USBD_NORMAL_COMPLETION;
>
> usb_transfer_complete(xfer);
> +}
> +
> +/*
> + * Work around the half configured control (default) pipe when setting
> + * the address of a device.
> + *
> + * Because a single QH is setup per endpoint in ehci_open(), and the
> + * control pipe is configured before we could have set the address
> + * of the device or read the wMaxPacketSize of the endpoint, we have
> + * to re-open the pipe twice here.
> + */
> +int
> +ehci_setaddr(struct usbd_device *dev, int addr)
> +{
> + /* Root Hub */
> + if (dev->depth == 0)
> + return (0);
> +
> + /* Re-establish the default pipe with the new max packet size. */
> + ehci_close_pipe(dev->default_pipe);
> + if (ehci_open(dev->default_pipe))
> + return (EINVAL);
> +
> + if (usbd_set_address(dev, addr))
> + return (1);
> +
> + dev->address = addr;
> +
> + /* Re-establish the default pipe with the new address. */
> + ehci_close_pipe(dev->default_pipe);
> + if (ehci_open(dev->default_pipe))
> + return (EINVAL);
> +
> + return (0);
> }
>
> void
> Index: ohci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ohci.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 ohci.c
> --- ohci.c 10 Aug 2014 11:18:57 -0000 1.139
> +++ ohci.c 2 Oct 2014 09:33:03 -0000
> @@ -88,6 +88,7 @@ usbd_status ohci_alloc_std_chain(struct
> struct ohci_soft_td **);
>
> usbd_status ohci_open(struct usbd_pipe *);
> +int ohci_setaddr(struct usbd_device *, int);
> void ohci_poll(struct usbd_bus *);
> void ohci_softintr(void *);
> void ohci_waitintr(struct ohci_softc *, struct usbd_xfer *);
> @@ -232,7 +233,7 @@ struct ohci_pipe {
>
> struct usbd_bus_methods ohci_bus_methods = {
> .open_pipe = ohci_open,
> - .dev_setaddr = usbd_set_address,
> + .dev_setaddr = ohci_setaddr,
> .soft_intr = ohci_softintr,
> .do_poll = ohci_poll,
> .allocx = ohci_allocx,
> @@ -2003,6 +2004,40 @@ ohci_open(struct usbd_pipe *pipe)
> bad0:
> return (USBD_NOMEM);
>
> +}
> +
> +/*
> + * Work around the half configured control (default) pipe when setting
> + * the address of a device.
> + *
> + * Because a single ED is setup per endpoint in ohci_open(), and the
> + * control pipe is configured before we could have set the address
> + * of the device or read the wMaxPacketSize of the endpoint, we have
> + * to re-open the pipe twice here.
> + */
> +int
> +ohci_setaddr(struct usbd_device *dev, int addr)
> +{
> + /* Root Hub */
> + if (dev->depth == 0)
> + return (0);
> +
> + /* Re-establish the default pipe with the new max packet size. */
> + ohci_device_ctrl_close(dev->default_pipe);
> + if (ohci_open(dev->default_pipe))
> + return (EINVAL);
> +
> + if (usbd_set_address(dev, addr))
> + return (1);
> +
> + dev->address = addr;
> +
> + /* Re-establish the default pipe with the new address. */
> + ohci_device_ctrl_close(dev->default_pipe);
> + if (ohci_open(dev->default_pipe))
> + return (EINVAL);
> +
> + return (0);
> }
>
> /*
> Index: usb_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 usb_subr.c
> --- usb_subr.c 1 Oct 2014 08:29:01 -0000 1.109
> +++ usb_subr.c 2 Oct 2014 09:24:18 -0000
> @@ -901,8 +901,8 @@ usbd_probe_and_attach(struct device *par
> "error=%s\n", parent->dv_xname, port,
> addr, usbd_errstr(err)));
> #else
> - printf("%s: port %d, set config at addr %d failed\n",
> - parent->dv_xname, port, addr);
> + printf("%s: port %d, set config %d at addr %d failed\n",
> + parent->dv_xname, port, confi, addr);
> #endif
>
> goto fail;
> @@ -1160,23 +1160,6 @@ usbd_new_device(struct device *parent, s
>
> USETW(dev->def_ep_desc.wMaxPacketSize, dd->bMaxPacketSize);
>
> - /* Re-establish the default pipe with the new max packet size. */
> - usbd_close_pipe(dev->default_pipe);
> - err = usbd_setup_pipe(dev, 0, &dev->def_ep, USBD_DEFAULT_INTERVAL,
> - &dev->default_pipe);
> - if (err) {
> - usb_free_device(dev);
> - up->device = NULL;
> - return (err);
> - }
> -
> - err = usbd_reload_device_desc(dev);
> - if (err) {
> - usb_free_device(dev);
> - up->device = NULL;
> - return (err);
> - }
> -
> /* Set the address if the HC didn't do it already. */
> if (bus->methods->dev_setaddr != NULL &&
> bus->methods->dev_setaddr(dev, addr)) {
> @@ -1192,10 +1175,7 @@ usbd_new_device(struct device *parent, s
> dev->address = addr;
> bus->devices[addr] = dev;
>
> - /* Re-establish the default pipe with the new address. */
> - usbd_close_pipe(dev->default_pipe);
> - err = usbd_setup_pipe(dev, 0, &dev->def_ep, USBD_DEFAULT_INTERVAL,
> - &dev->default_pipe);
> + err = usbd_reload_device_desc(dev);
> if (err) {
> usb_free_device(dev);
> up->device = NULL;
>
