Hi,

I tested this patch. From what I've seen, it works perfectly.
After suspend / resume, I did not notice any problem.

Thanks for the resolution of this bug.



On Tue, Oct 3, 2017 at 11:27 AM, Martin Pieuchot <m...@openbsd.org> wrote:
> On 01/10/17(Sun) 20:35, Olivier Antoine wrote:
>> Hi,
>>
>> Looks like this bug: <https://marc.info/?l=openbsd-bugs&m=147306540522232>
>>
>> I can also reproduce this with:
>>
>> $ while true ; do adb shell ls / ; adb kill-server ; done
>>
>> The code which is triggered in /sys/dev/usb/ehci.c:
>>
>> ehci_device_clear_toggle(struct usbd_pipe *pipe)
>> {
>>         struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
>>
>> #ifdef DIAGNOSTIC
>>         if ((epipe->sqh->qh.qh_qtd.qtd_status & htole32(EHCI_QTD_ACTIVE)) != 
>> 0)
>>                 panic("ehci_device_clear_toggle: queue active");
>> #endif
>>         epipe->sqh->qh.qh_qtd.qtd_status &= htole32(~EHCI_QTD_TOGGLE_MASK);
>> }
>>
>> Don't know if it's hardware specific. But I can confirm that it hit me too.
>
> Bug reports without dmesg are useless, see sendbug(1).
>
> Anyway, here's a diff that should fix the problem.  However last I
> couldn't narrow down possible regressions.  So make sure everything
> works with it, including suspend/resume.
>
> Index: ehci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ehci.c,v
> retrieving revision 1.200
> diff -u -p -r1.200 ehci.c
> --- ehci.c      15 May 2017 10:52:08 -0000      1.200
> +++ ehci.c      3 Oct 2017 09:24:08 -0000
> @@ -116,6 +116,7 @@ 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_start(struct ehci_softc *);
>  int            ehci_intr1(struct ehci_softc *);
>  void           ehci_check_intr(struct ehci_softc *, struct usbd_xfer *);
>  void           ehci_check_qh_intr(struct ehci_softc *, struct usbd_xfer *);
> @@ -188,12 +189,11 @@ int               ehci_alloc_sitd_chain(struct ehci_s
>  void           ehci_abort_isoc_xfer(struct usbd_xfer *xfer,
>                     usbd_status status);
>
> -usbd_status    ehci_device_setintr(struct ehci_softc *, struct ehci_soft_qh 
> *,
> -                           int ival);
> +struct ehci_soft_qh * ehci_intr_get_sqh(struct usbd_pipe *);
>
> -void           ehci_add_qh(struct ehci_soft_qh *, struct ehci_soft_qh *);
> -void           ehci_rem_qh(struct ehci_softc *, struct ehci_soft_qh *);
> -void           ehci_set_qh_qtd(struct ehci_soft_qh *, struct ehci_soft_qtd 
> *);
> +void           ehci_add_qh(struct usbd_pipe *, struct ehci_soft_qh *,
> +                   struct ehci_soft_qtd *);
> +void           ehci_rem_qh(struct ehci_softc *, struct usbd_pipe *);
>  void           ehci_sync_hc(struct ehci_softc *);
>
>  void           ehci_close_pipe(struct usbd_pipe *);
> @@ -295,7 +295,7 @@ ehci_reverse_bits(u_int8_t c, int nbits)
>  usbd_status
>  ehci_init(struct ehci_softc *sc)
>  {
> -       u_int32_t sparams, cparams, hcr;
> +       uint32_t sparams;
>         u_int i, j;
>         usbd_status err;
>         struct ehci_soft_qh *sqh;
> @@ -316,20 +316,8 @@ ehci_init(struct ehci_softc *sc)
>         sparams = EREAD4(sc, EHCI_HCSPARAMS);
>         DPRINTF(("ehci_init: sparams=0x%x\n", sparams));
>         sc->sc_noport = EHCI_HCS_N_PORTS(sparams);
> -       cparams = EREAD4(sc, EHCI_HCCPARAMS);
> -       DPRINTF(("ehci_init: cparams=0x%x\n", cparams));
> -
> -       /* MUST clear segment register if 64 bit capable. */
> -       if (EHCI_HCC_64BIT(cparams))
> -               EWRITE4(sc, EHCI_CTRLDSSEGMENT, 0);
> -
>         sc->sc_bus.usbrev = USBREV_2_0;
>
> -       DPRINTF(("%s: resetting\n", sc->sc_bus.bdev.dv_xname));
> -       err = ehci_reset(sc);
> -       if (err)
> -               return (err);
> -
>         if (ehcixfer == NULL) {
>                 ehcixfer = malloc(sizeof(struct pool), M_DEVBUF, M_NOWAIT);
>                 if (ehcixfer == NULL) {
> @@ -365,8 +353,6 @@ ehci_init(struct ehci_softc *sc)
>         for (i = 0; i < sc->sc_flsize; i++)
>                 sc->sc_flist[i] = htole32(EHCI_LINK_TERMINATE);
>
> -       EOWRITE4(sc, EHCI_PERIODICLISTBASE, DMAADDR(&sc->sc_fldma, 0));
> -
>         sc->sc_softitds = mallocarray(sc->sc_flsize,
>             sizeof(struct ehci_soft_itd *), M_USB, M_NOWAIT | M_ZERO);
>         if (sc->sc_softitds == NULL) {
> @@ -412,7 +398,6 @@ ehci_init(struct ehci_softc *sc)
>                 sqh->qh.qh_qtd.qtd_next = htole32(EHCI_LINK_TERMINATE);
>                 sqh->qh.qh_qtd.qtd_altnext = htole32(EHCI_LINK_TERMINATE);
>                 sqh->qh.qh_qtd.qtd_status = htole32(EHCI_QTD_HALTED);
> -               sqh->sqtd = NULL;
>                 usb_syncmem(&sqh->dma, sqh->offs, sizeof(sqh->qh),
>                     BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
>         }
> @@ -443,18 +428,47 @@ ehci_init(struct ehci_softc *sc)
>         sqh->qh.qh_qtd.qtd_next = htole32(EHCI_LINK_TERMINATE);
>         sqh->qh.qh_qtd.qtd_altnext = htole32(EHCI_LINK_TERMINATE);
>         sqh->qh.qh_qtd.qtd_status = htole32(EHCI_QTD_HALTED);
> -       sqh->sqtd = NULL;
>         usb_syncmem(&sqh->dma, sqh->offs, sizeof(sqh->qh),
>             BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
>
>         /* Point to async list */
>         sc->sc_async_head = sqh;
> -       EOWRITE4(sc, EHCI_ASYNCLISTADDR, sqh->physaddr | EHCI_LINK_QH);
>
>         timeout_set(&sc->sc_tmo_intrlist, ehci_intrlist_timeout, sc);
>
>         rw_init(&sc->sc_doorbell_lock, "ehcidb");
>
> +       return (ehci_start(sc));
> +
> +#if 0
> + bad2:
> +       ehci_free_sqh(sc, sc->sc_async_head);
> +#endif
> + bad1:
> +       free(sc->sc_softitds, M_USB, sc->sc_flsize);
> +       usb_freemem(&sc->sc_bus, &sc->sc_fldma);
> +       return (err);
> +}
> +
> +int
> +ehci_start(struct ehci_softc *sc)
> +{
> +       uint32_t hcr, cparams;
> +       int error, i;
> +
> +       error = ehci_reset(sc);
> +       if (error)
> +               return (error);
> +
> +       cparams = EREAD4(sc, EHCI_HCCPARAMS);
> +       /* MUST clear segment register if 64 bit capable. */
> +       if (EHCI_HCC_64BIT(cparams))
> +               EWRITE4(sc, EHCI_CTRLDSSEGMENT, 0);
> +
> +       EOWRITE4(sc, EHCI_PERIODICLISTBASE, DMAADDR(&sc->sc_fldma, 0));
> +       EOWRITE4(sc, EHCI_ASYNCLISTADDR,
> +           sc->sc_async_head->physaddr | EHCI_LINK_QH);
> +
>         /* Turn on controller */
>         EOWRITE4(sc, EHCI_USBCMD,
>             EHCI_CMD_ITC_2 | /* 2 microframes interrupt delay */
> @@ -474,23 +488,13 @@ ehci_init(struct ehci_softc *sc)
>         }
>         if (hcr) {
>                 printf("%s: run timeout\n", sc->sc_bus.bdev.dv_xname);
> -               return (USBD_IOERROR);
> +               return (EIO);
>         }
>
>         /* Enable interrupts */
>         EOWRITE4(sc, EHCI_USBINTR, sc->sc_eintrs);
>
> -       return (USBD_NORMAL_COMPLETION);
> -
> -#if 0
> - bad2:
> -       ehci_free_sqh(sc, sc->sc_async_head);
> -#endif
> - bad1:
> -       free(sc->sc_softitds, M_USB,
> -           sc->sc_flsize * sizeof(struct ehci_soft_itd *));
> -       usb_freemem(&sc->sc_bus, &sc->sc_fldma);
> -       return (err);
> +       return (0);
>  }
>
>  int
> @@ -732,6 +736,7 @@ ehci_check_qh_intr(struct ehci_softc *sc
>                 return;
>         }
>   done:
> +       ehci_rem_qh(sc, xfer->pipe);
>         TAILQ_REMOVE(&sc->sc_intrhead, ex, inext);
>         timeout_del(&xfer->timeout_handle);
>         usb_rem_task(xfer->pipe->device, &xfer->abort_task);
> @@ -864,7 +869,7 @@ ehci_idone(struct usbd_xfer *xfer)
>  {
>         struct ehci_xfer *ex = (struct ehci_xfer *)xfer;
>         struct ehci_soft_qtd *sqtd;
> -       u_int32_t status = 0, nstatus = 0;
> +       uint32_t status = 0, nstatus = 0;
>         int actlen, cerr;
>
>  #ifdef DIAGNOSTIC
> @@ -956,7 +961,7 @@ int
>  ehci_activate(struct device *self, int act)
>  {
>         struct ehci_softc *sc = (struct ehci_softc *)self;
> -       u_int32_t cmd, hcr, cparams;
> +       uint32_t cmd, hcr;
>         int i, rv = 0;
>
>         switch (act) {
> @@ -969,6 +974,11 @@ ehci_activate(struct device *self, int a
>                             sc->sc_bus.bdev.dv_xname);
>                         return (-1);
>                 }
> +               if (sc->sc_async_head->next != sc->sc_async_head) {
> +                       printf("%s: asynchronous list not empty\n",
> +                           sc->sc_bus.bdev.dv_xname);
> +                       return (-1);
> +               }
>  #endif
>
>                 sc->sc_bus.use_polling++;
> @@ -1010,16 +1020,9 @@ ehci_activate(struct device *self, int a
>         case DVACT_RESUME:
>                 sc->sc_bus.use_polling++;
>
> -               ehci_reset(sc);
> +               ehci_start(sc);
>
> -               cparams = EREAD4(sc, EHCI_HCCPARAMS);
> -               /* MUST clear segment register if 64 bit capable. */
> -               if (EHCI_HCC_64BIT(cparams))
> -                       EWRITE4(sc, EHCI_CTRLDSSEGMENT, 0);
> -
> -               EOWRITE4(sc, EHCI_PERIODICLISTBASE, DMAADDR(&sc->sc_fldma, 
> 0));
> -               EOWRITE4(sc, EHCI_ASYNCLISTADDR,
> -                   sc->sc_async_head->physaddr | EHCI_LINK_QH);
> +               usb_delay_ms(&sc->sc_bus, USB_RESUME_WAIT);
>
>                 hcr = 0;
>                 for (i = 1; i <= sc->sc_noport; i++) {
> @@ -1041,32 +1044,6 @@ ehci_activate(struct device *self, int a
>                         }
>                 }
>
> -               /* Turn on controller */
> -               EOWRITE4(sc, EHCI_USBCMD,
> -                   EHCI_CMD_ITC_2 | /* 2 microframes interrupt delay */
> -                   (EOREAD4(sc, EHCI_USBCMD) & EHCI_CMD_FLS_M) |
> -                   EHCI_CMD_ASE |
> -                   EHCI_CMD_PSE |
> -                   EHCI_CMD_RS);
> -
> -               /* Take over port ownership */
> -               EOWRITE4(sc, EHCI_CONFIGFLAG, EHCI_CONF_CF);
> -               for (i = 0; i < 100; i++) {
> -                       usb_delay_ms(&sc->sc_bus, 1);
> -                       hcr = EOREAD4(sc, EHCI_USBSTS) & EHCI_STS_HCH;
> -                       if (!hcr)
> -                               break;
> -               }
> -
> -               if (hcr) {
> -                       printf("%s: run timeout\n", sc->sc_bus.bdev.dv_xname);
> -                       /* XXX should we bail here? */
> -               }
> -
> -               EOWRITE4(sc, EHCI_USBINTR, sc->sc_eintrs);
> -
> -               usb_delay_ms(&sc->sc_bus, USB_RESUME_WAIT);
> -
>                 sc->sc_bus.use_polling--;
>                 rv = config_activate_children(self, act);
>                 break;
> @@ -1150,13 +1127,7 @@ ehci_freex(struct usbd_bus *bus, struct
>  void
>  ehci_device_clear_toggle(struct usbd_pipe *pipe)
>  {
> -       struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
> -
> -#ifdef DIAGNOSTIC
> -       if ((epipe->sqh->qh.qh_qtd.qtd_status & htole32(EHCI_QTD_ACTIVE)) != 
> 0)
> -               panic("ehci_device_clear_toggle: queue active");
> -#endif
> -       epipe->sqh->qh.qh_qtd.qtd_status &= htole32(~EHCI_QTD_TOGGLE_MASK);
> +       pipe->endpoint->savedtoggle = 0;
>  }
>
>  #ifdef EHCI_DEBUG
> @@ -1353,29 +1324,17 @@ ehci_open(struct usbd_pipe *pipe)
>         struct usbd_device *dev = pipe->device;
>         struct ehci_softc *sc = (struct ehci_softc *)dev->bus;
>         usb_endpoint_descriptor_t *ed = pipe->endpoint->edesc;
> -       u_int8_t addr = dev->address;
>         u_int8_t xfertype = ed->bmAttributes & UE_XFERTYPE;
>         struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
>         struct ehci_soft_qh *sqh;
>         usbd_status err;
> -       int s;
> -       int ival, speed, naks;
> -       int hshubaddr, hshubport;
>
> -       DPRINTFN(1, ("ehci_open: pipe=%p, addr=%d, endpt=%d\n",
> +       DPRINTFN(1, ("%s: pipe=%p, addr=%d, endpt=%d\n", __func__,
>             pipe, addr, ed->bEndpointAddress));
>
>         if (sc->sc_bus.dying)
>                 return (USBD_IOERROR);
>
> -       if (dev->myhsport) {
> -               hshubaddr = dev->myhsport->parent->address;
> -               hshubport = dev->myhsport->portno;
> -       } else {
> -               hshubaddr = 0;
> -               hshubport = 0;
> -       }
> -
>         /* Root Hub */
>         if (pipe->device->depth == 0) {
>                 switch (ed->bEndpointAddress) {
> @@ -1391,59 +1350,11 @@ ehci_open(struct usbd_pipe *pipe)
>                 return (USBD_NORMAL_COMPLETION);
>         }
>
> -       /* XXX All this stuff is only valid for async. */
> -       switch (dev->speed) {
> -       case USB_SPEED_LOW:
> -               speed = EHCI_QH_SPEED_LOW;
> -               break;
> -       case USB_SPEED_FULL:
> -               speed = EHCI_QH_SPEED_FULL;
> -               break;
> -       case USB_SPEED_HIGH:
> -               speed = EHCI_QH_SPEED_HIGH;
> -               break;
> -       default:
> -               panic("ehci_open: bad device speed %d", dev->speed);
> -       }
> -
> -       naks = 8;               /* XXX */
> -
>         /* Allocate sqh for everything, save isoc xfers */
>         if (xfertype != UE_ISOCHRONOUS) {
>                 sqh = ehci_alloc_sqh(sc);
>                 if (sqh == NULL)
>                         return (USBD_NOMEM);
> -               /* qh_link filled when the QH is added */
> -               sqh->qh.qh_endp = htole32(
> -                   EHCI_QH_SET_ADDR(addr) |
> -                   EHCI_QH_SET_ENDPT(UE_GET_ADDR(ed->bEndpointAddress)) |
> -                   EHCI_QH_SET_EPS(speed) |
> -                   (xfertype == UE_CONTROL ? EHCI_QH_DTC : 0) |
> -                   EHCI_QH_SET_MPL(UGETW(ed->wMaxPacketSize)) |
> -                   (speed != EHCI_QH_SPEED_HIGH && xfertype == UE_CONTROL ?
> -                   EHCI_QH_CTL : 0) |
> -                   EHCI_QH_SET_NRL(naks)
> -               );
> -               sqh->qh.qh_endphub = htole32(
> -                   EHCI_QH_SET_MULT(1) |
> -                   EHCI_QH_SET_SMASK(xfertype == UE_INTERRUPT ? 0x01 : 0)
> -               );
> -               if (speed != EHCI_QH_SPEED_HIGH) {
> -                       sqh->qh.qh_endphub |= htole32(
> -                           EHCI_QH_SET_HUBA(hshubaddr) |
> -                           EHCI_QH_SET_PORT(hshubport) |
> -                           EHCI_QH_SET_CMASK(0x1c) /* XXX */
> -                       );
> -               }
> -               sqh->qh.qh_curqtd = htole32(EHCI_LINK_TERMINATE);
> -               /* Fill the overlay qTD */
> -               sqh->qh.qh_qtd.qtd_next = htole32(EHCI_LINK_TERMINATE);
> -               sqh->qh.qh_qtd.qtd_altnext = htole32(EHCI_LINK_TERMINATE);
> -               sqh->qh.qh_qtd.qtd_status =
> -                   htole32(EHCI_QTD_SET_TOGGLE(pipe->endpoint->savedtoggle));
> -
> -               usb_syncmem(&sqh->dma, sqh->offs, sizeof(sqh->qh),
> -                   BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
>                 epipe->sqh = sqh;
>         } /*xfertype == UE_ISOC*/
>
> @@ -1456,32 +1367,20 @@ ehci_open(struct usbd_pipe *pipe)
>                         return (err);
>                 }
>                 pipe->methods = &ehci_device_ctrl_methods;
> -               s = splusb();
> -               ehci_add_qh(sqh, sc->sc_async_head);
> -               splx(s);
>                 break;
>         case UE_BULK:
>                 pipe->methods = &ehci_device_bulk_methods;
> -               s = splusb();
> -               ehci_add_qh(sqh, sc->sc_async_head);
> -               splx(s);
>                 break;
>         case UE_INTERRUPT:
>                 pipe->methods = &ehci_device_intr_methods;
> -               ival = pipe->interval;
> -               if (ival == USBD_DEFAULT_INTERVAL)
> -                       ival = ed->bInterval;
> -               s = splusb();
> -               err = ehci_device_setintr(sc, sqh, ival);
> -               splx(s);
> -               return (err);
> +               break;
>         case UE_ISOCHRONOUS:
> -               switch (speed) {
> -               case EHCI_QH_SPEED_HIGH:
> -               case EHCI_QH_SPEED_FULL:
> +               switch (pipe->device->speed) {
> +               case USB_SPEED_HIGH:
> +               case USB_SPEED_FULL:
>                         pipe->methods = &ehci_device_isoc_methods;
>                         break;
> -               case EHCI_QH_SPEED_LOW:
> +               case USB_SPEED_LOW:
>                 default:
>                         return (USBD_INVAL);
>                 }
> @@ -1504,16 +1403,92 @@ ehci_open(struct usbd_pipe *pipe)
>         return (USBD_NORMAL_COMPLETION);
>  }
>
> -/*
> - * Add an ED to the schedule.  Called at splusb().
> - * If in the async schedule, it will always have a next.
> - * If in the intr schedule it may not.
> - */
>  void
> -ehci_add_qh(struct ehci_soft_qh *sqh, struct ehci_soft_qh *head)
> +ehci_add_qh(struct usbd_pipe *pipe, struct ehci_soft_qh *head,
> +    struct ehci_soft_qtd *start)
>  {
> +       struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
> +       struct ehci_soft_qh *sqh = epipe->sqh;
> +       usb_endpoint_descriptor_t *ed = pipe->endpoint->edesc;
> +       uint8_t xfertype = ed->bmAttributes & UE_XFERTYPE;
> +       uint8_t addr = pipe->device->address;
> +       int i, hshubaddr, hshubport, speed;
> +       int naks = 8;           /* XXX */
> +
> +       KASSERT(xfertype != UE_ISOCHRONOUS);
> +
>         splsoftassert(IPL_SOFTUSB);
>
> +       if (pipe->device->myhsport) {
> +               hshubaddr = pipe->device->myhsport->parent->address;
> +               hshubport = pipe->device->myhsport->portno;
> +       } else {
> +               hshubaddr = 0;
> +               hshubport = 0;
> +       }
> +
> +       /* XXX All this stuff is only valid for async. */
> +       switch (pipe->device->speed) {
> +       case USB_SPEED_LOW:
> +               speed = EHCI_QH_SPEED_LOW;
> +               break;
> +       case USB_SPEED_FULL:
> +               speed = EHCI_QH_SPEED_FULL;
> +               break;
> +       case USB_SPEED_HIGH:
> +               speed = EHCI_QH_SPEED_HIGH;
> +               break;
> +       default:
> +               panic("%s: bad device speed %d", __func__, 
> pipe->device->speed);
> +       }
> +
> +       /* qh_link is filled below */
> +       sqh->qh.qh_endp = htole32(
> +           EHCI_QH_SET_ADDR(addr) |
> +           EHCI_QH_SET_ENDPT(UE_GET_ADDR(ed->bEndpointAddress)) |
> +           EHCI_QH_SET_EPS(speed) |
> +           (xfertype == UE_CONTROL ? EHCI_QH_DTC : 0) |
> +           EHCI_QH_SET_MPL(UGETW(ed->wMaxPacketSize)) |
> +           (speed != EHCI_QH_SPEED_HIGH && xfertype == UE_CONTROL ?
> +           EHCI_QH_CTL : 0) |
> +           EHCI_QH_SET_NRL(naks)
> +           );
> +       sqh->qh.qh_endphub = htole32(
> +           EHCI_QH_SET_MULT(1) |
> +           EHCI_QH_SET_SMASK(xfertype == UE_INTERRUPT ? 0x01 : 0)
> +       );
> +       if (speed != EHCI_QH_SPEED_HIGH) {
> +               sqh->qh.qh_endphub |= htole32(
> +                   EHCI_QH_SET_HUBA(hshubaddr) |
> +                   EHCI_QH_SET_PORT(hshubport) |
> +                   EHCI_QH_SET_CMASK(0x1c) /* XXX */
> +               );
> +       }
> +       sqh->qh.qh_curqtd = 0;
> +
> +       sqh->qh.qh_qtd.qtd_next = htole32(start->physaddr);
> +       sqh->qh.qh_qtd.qtd_altnext = htole32(EHCI_LINK_TERMINATE);
> +       if (xfertype == UE_CONTROL)
> +               sqh->qh.qh_qtd.qtd_status = 0;
> +       else
> +               sqh->qh.qh_qtd.qtd_status =
> +                   htole32(EHCI_QTD_SET_TOGGLE(pipe->endpoint->savedtoggle));
> +
> +       /* Reset reserved fields */
> +       for (i = 0; i < EHCI_QTD_NBUFFERS; i++) {
> +               sqh->qh.qh_qtd.qtd_buffer[i] = 0;
> +               sqh->qh.qh_qtd.qtd_buffer_hi[i] = 0;
> +       }
> +       usb_syncmem(&sqh->dma, sqh->offs, sizeof(sqh->qh),
> +           BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
> +
> +       /*
> +        * Add an ED to the schedule.
> +        *
> +        * If in the async schedule, it will always have a next.
> +        * If in the intr schedule it may not.
> +        */
> +
>         usb_syncmem(&head->dma, head->offs + offsetof(struct ehci_qh, 
> qh_link),
>             sizeof(head->qh.qh_link), BUS_DMASYNC_POSTWRITE);
>         sqh->next = head->next;
> @@ -1534,10 +1509,13 @@ ehci_add_qh(struct ehci_soft_qh *sqh, st
>   * Will always have a 'next' if it's in the async list as it's circular.
>   */
>  void
> -ehci_rem_qh(struct ehci_softc *sc, struct ehci_soft_qh *sqh)
> +ehci_rem_qh(struct ehci_softc *sc, struct usbd_pipe *pipe)
>  {
> +       struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
> +       struct ehci_soft_qh *sqh = epipe->sqh;
> +
>         splsoftassert(IPL_SOFTUSB);
> -       /* XXX */
> +
>         usb_syncmem(&sqh->dma, sqh->offs + offsetof(struct ehci_qh, qh_link),
>             sizeof(sqh->qh.qh_link), BUS_DMASYNC_POSTWRITE);
>         sqh->prev->qh.qh_link = sqh->qh.qh_link;
> @@ -1548,42 +1526,8 @@ ehci_rem_qh(struct ehci_softc *sc, struc
>             sqh->prev->offs + offsetof(struct ehci_qh, qh_link),
>             sizeof(sqh->prev->qh.qh_link), BUS_DMASYNC_PREWRITE);
>
> -       ehci_sync_hc(sc);
> -}
> -
> -void
> -ehci_set_qh_qtd(struct ehci_soft_qh *sqh, struct ehci_soft_qtd *sqtd)
> -{
> -       int i;
> -       u_int32_t status;
> -
> -       /* Save toggle bit and ping status. */
> -       usb_syncmem(&sqh->dma, sqh->offs, sizeof(sqh->qh),
> -           BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
> -       status = sqh->qh.qh_qtd.qtd_status &
> -           htole32(EHCI_QTD_TOGGLE_MASK |
> -               EHCI_QTD_SET_STATUS(EHCI_QTD_PINGSTATE));
> -       /* Set HALTED to make hw leave it alone. */
> -       sqh->qh.qh_qtd.qtd_status =
> -           htole32(EHCI_QTD_SET_STATUS(EHCI_QTD_HALTED));
> -       usb_syncmem(&sqh->dma,
> -           sqh->offs + offsetof(struct ehci_qh, qh_qtd.qtd_status),
> -           sizeof(sqh->qh.qh_qtd.qtd_status),
> -           BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
> -       sqh->qh.qh_curqtd = 0;
> -       sqh->qh.qh_qtd.qtd_next = htole32(sqtd->physaddr);
> -       sqh->qh.qh_qtd.qtd_altnext = htole32(EHCI_LINK_TERMINATE);
> -       for (i = 0; i < EHCI_QTD_NBUFFERS; i++)
> -               sqh->qh.qh_qtd.qtd_buffer[i] = 0;
> -       sqh->sqtd = sqtd;
> -       usb_syncmem(&sqh->dma, sqh->offs, sizeof(sqh->qh),
> -           BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
> -       /* Set !HALTED && !ACTIVE to start execution, preserve some fields */
> -       sqh->qh.qh_qtd.qtd_status = status;
> -       usb_syncmem(&sqh->dma,
> -           sqh->offs + offsetof(struct ehci_qh, qh_qtd.qtd_status),
> -           sizeof(sqh->qh.qh_qtd.qtd_status),
> -           BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
> +       pipe->endpoint->savedtoggle =
> +           EHCI_QTD_GET_TOGGLE(letoh32(sqh->qh.qh_qtd.qtd_status));
>  }
>
>  /*
> @@ -2471,7 +2415,6 @@ ehci_alloc_sqtd_chain(struct ehci_softc
>  void
>  ehci_free_sqtd_chain(struct ehci_softc *sc, struct ehci_xfer *ex)
>  {
> -       struct ehci_pipe *epipe = (struct ehci_pipe *)ex->xfer.pipe;
>         struct ehci_soft_qtd *sqtd, *next;
>
>         DPRINTFN(10,("ehci_free_sqtd_chain: sqtd=%p\n", ex->sqtdstart));
> @@ -2481,7 +2424,6 @@ ehci_free_sqtd_chain(struct ehci_softc *
>                 ehci_free_sqtd(sc, sqtd);
>         }
>         ex->sqtdstart = ex->sqtdend = NULL;
> -       epipe->sqh->sqtd = NULL;
>  }
>
>  struct ehci_soft_itd *
> @@ -2565,14 +2507,7 @@ ehci_close_pipe(struct usbd_pipe *pipe)
>  {
>         struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
>         struct ehci_softc *sc = (struct ehci_softc *)pipe->device->bus;
> -       struct ehci_soft_qh *sqh = epipe->sqh;
> -       int s;
>
> -       s = splusb();
> -       ehci_rem_qh(sc, sqh);
> -       splx(s);
> -       pipe->endpoint->savedtoggle =
> -           EHCI_QTD_GET_TOGGLE(letoh32(sqh->qh.qh_qtd.qtd_status));
>         ehci_free_sqh(sc, epipe->sqh);
>  }
>
> @@ -2590,16 +2525,15 @@ void
>  ehci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
>  {
>         struct ehci_softc *sc = (struct ehci_softc *)xfer->device->bus;
> -       struct ehci_pipe *epipe = (struct ehci_pipe *)xfer->pipe;
>         struct ehci_xfer *ex = (struct ehci_xfer*)xfer;
> -       struct ehci_soft_qh *sqh = epipe->sqh;
> -       struct ehci_soft_qtd *sqtd;
>         int s;
>
>         if (sc->sc_bus.dying || xfer->status == USBD_NOT_STARTED) {
>                 s = splusb();
> -               if (xfer->status != USBD_NOT_STARTED)
> +               if (xfer->status != USBD_NOT_STARTED) {
> +                       ehci_rem_qh(sc, xfer->pipe);
>                         TAILQ_REMOVE(&sc->sc_intrhead, ex, inext);
> +               }
>                 xfer->status = status;  /* make software ignore it */
>                 timeout_del(&xfer->timeout_handle);
>                 usb_rem_task(xfer->device, &xfer->abort_task);
> @@ -2632,51 +2566,21 @@ ehci_abort_xfer(struct usbd_xfer *xfer,
>                 return;
>         }
>
> -       /*
> -        * Step 1: Make interrupt routine and timeouts ignore xfer.
> -        */
>         s = splusb();
>         ex->ehci_xfer_flags |= EHCI_XFER_ABORTING;
> -       xfer->status = status;  /* make software ignore it */
> +
> +       /* Remove the Queue Head. */
> +       ehci_rem_qh(sc, xfer->pipe);
>         TAILQ_REMOVE(&sc->sc_intrhead, ex, inext);
> +
> +       xfer->status = status;  /* make software ignore it */
>         timeout_del(&xfer->timeout_handle);
>         usb_rem_task(xfer->device, &xfer->abort_task);
> -       splx(s);
>
>         /*
> -        * Step 2: Deactivate all of the qTDs that we will be removing,
> -        * otherwise the queue head may go active again.
> -        */
> -       usb_syncmem(&sqh->dma,
> -           sqh->offs + offsetof(struct ehci_qh, qh_qtd.qtd_status),
> -           sizeof(sqh->qh.qh_qtd.qtd_status),
> -           BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
> -       sqh->qh.qh_qtd.qtd_status = htole32(EHCI_QTD_HALTED);
> -       usb_syncmem(&sqh->dma,
> -           sqh->offs + offsetof(struct ehci_qh, qh_qtd.qtd_status),
> -           sizeof(sqh->qh.qh_qtd.qtd_status),
> -           BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
> -
> -       for (sqtd = ex->sqtdstart; sqtd != NULL; sqtd = sqtd->nextqtd) {
> -               usb_syncmem(&sqtd->dma,
> -                   sqtd->offs + offsetof(struct ehci_qtd, qtd_status),
> -                   sizeof(sqtd->qtd.qtd_status),
> -                   BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
> -               sqtd->qtd.qtd_status = htole32(EHCI_QTD_HALTED);
> -               usb_syncmem(&sqtd->dma,
> -                   sqtd->offs + offsetof(struct ehci_qtd, qtd_status),
> -                   sizeof(sqtd->qtd.qtd_status),
> -                   BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
> -       }
> -       ehci_sync_hc(sc);
> -
> -       /*
> -        * Step 3: Make sure the soft interrupt routine has run. This
> +        * Make sure the soft interrupt routine has run. This
>          * should remove any completed items off the queue.
> -        * The hardware has no reference to completed items (TDs).
> -        * It's safe to remove them at any time.
>          */
> -       s = splusb();
>         sc->sc_softwake = 1;
>         usb_schedsoftintr(&sc->sc_bus);
>         tsleep(&sc->sc_softwake, PZERO, "ehciab", 0);
> @@ -2849,7 +2753,6 @@ ehci_device_ctrl_start(struct usbd_xfer
>         struct ehci_xfer *ex = (struct ehci_xfer *)xfer;
>         usb_device_request_t *req = &xfer->request;
>         struct ehci_soft_qtd *setup, *stat, *next;
> -       struct ehci_soft_qh *sqh;
>         u_int len = UGETW(req->wLength);
>         usbd_status err;
>         int s;
> @@ -2870,8 +2773,6 @@ ehci_device_ctrl_start(struct usbd_xfer
>                 goto bad2;
>         }
>
> -       sqh = epipe->sqh;
> -
>         /* Set up data transaction */
>         if (len != 0) {
>                 struct ehci_soft_qtd *end;
> @@ -2931,14 +2832,13 @@ ehci_device_ctrl_start(struct usbd_xfer
>         ex->isdone = 0;
>  #endif
>
> -       /* Insert qTD in QH list. */
>         s = splusb();
> -       ehci_set_qh_qtd(sqh, setup);
>         if (xfer->timeout && !sc->sc_bus.use_polling) {
>                 timeout_del(&xfer->timeout_handle);
>                 timeout_set(&xfer->timeout_handle, ehci_timeout, xfer);
>                 timeout_add_msec(&xfer->timeout_handle, xfer->timeout);
>         }
> +       ehci_add_qh(xfer->pipe, sc->sc_async_head, setup);
>         TAILQ_INSERT_TAIL(&sc->sc_intrhead, ex, inext);
>         xfer->status = USBD_IN_PROGRESS;
>         splx(s);
> @@ -2998,10 +2898,8 @@ usbd_status
>  ehci_device_bulk_start(struct usbd_xfer *xfer)
>  {
>         struct ehci_softc *sc = (struct ehci_softc *)xfer->device->bus;
> -       struct ehci_pipe *epipe = (struct ehci_pipe *)xfer->pipe;
>         struct ehci_xfer *ex = (struct ehci_xfer *)xfer;
>         struct ehci_soft_qtd *data, *dataend;
> -       struct ehci_soft_qh *sqh;
>         usbd_status err;
>         int s;
>
> @@ -3010,8 +2908,6 @@ ehci_device_bulk_start(struct usbd_xfer
>         if (sc->sc_bus.dying)
>                 return (USBD_IOERROR);
>
> -       sqh = epipe->sqh;
> -
>         err = ehci_alloc_sqtd_chain(sc, xfer->length, xfer, &data, &dataend);
>         if (err) {
>                 xfer->status = err;
> @@ -3030,12 +2926,12 @@ ehci_device_bulk_start(struct usbd_xfer
>  #endif
>
>         s = splusb();
> -       ehci_set_qh_qtd(sqh, data);
>         if (xfer->timeout && !sc->sc_bus.use_polling) {
>                 timeout_del(&xfer->timeout_handle);
>                 timeout_set(&xfer->timeout_handle, ehci_timeout, xfer);
>                 timeout_add_msec(&xfer->timeout_handle, xfer->timeout);
>         }
> +       ehci_add_qh(xfer->pipe, sc->sc_async_head, data);
>         TAILQ_INSERT_TAIL(&sc->sc_intrhead, ex, inext);
>         xfer->status = USBD_IN_PROGRESS;
>         splx(s);
> @@ -3069,12 +2965,18 @@ ehci_device_bulk_done(struct usbd_xfer *
>         }
>  }
>
> -usbd_status
> -ehci_device_setintr(struct ehci_softc *sc, struct ehci_soft_qh *sqh, int 
> ival)
> +struct ehci_soft_qh *
> +ehci_intr_get_sqh(struct usbd_pipe *pipe)
>  {
> +       struct ehci_softc *sc = (struct ehci_softc *)pipe->device->bus;
> +       struct ehci_pipe *epipe = (struct ehci_pipe *)pipe;
>         struct ehci_soft_islot *isp;
> +       int ival = pipe->interval;
>         int islot, lev;
>
> +       if (ival == USBD_DEFAULT_INTERVAL)
> +               ival = pipe->endpoint->edesc->bInterval;
> +
>         /* Find a poll rate that is large enough. */
>         for (lev = EHCI_IPOLLRATES - 1; lev > 0; lev--)
>                 if (EHCI_ILEV_IVAL(lev) <= ival)
> @@ -3084,11 +2986,10 @@ ehci_device_setintr(struct ehci_softc *s
>         /* XXX could do better than picking at random */
>         islot = EHCI_IQHIDX(lev, arc4random());
>
> -       sqh->islot = islot;
> +       epipe->sqh->islot = islot;
>         isp = &sc->sc_islots[islot];
> -       ehci_add_qh(sqh, isp->sqh);
>
> -       return (USBD_NORMAL_COMPLETION);
> +       return (isp->sqh);
>  }
>
>  usbd_status
> @@ -3113,9 +3014,7 @@ ehci_device_intr_start(struct usbd_xfer
>  {
>         struct ehci_softc *sc = (struct ehci_softc *)xfer->device->bus;
>         struct ehci_xfer *ex = (struct ehci_xfer *)xfer;
> -       struct ehci_pipe *epipe = (struct ehci_pipe *)xfer->pipe;
>         struct ehci_soft_qtd *data, *dataend;
> -       struct ehci_soft_qh *sqh;
>         usbd_status err;
>         int s;
>
> @@ -3124,8 +3023,6 @@ ehci_device_intr_start(struct usbd_xfer
>         if (sc->sc_bus.dying)
>                 return (USBD_IOERROR);
>
> -       sqh = epipe->sqh;
> -
>         err = ehci_alloc_sqtd_chain(sc, xfer->length, xfer, &data, &dataend);
>         if (err) {
>                 xfer->status = err;
> @@ -3143,12 +3040,12 @@ ehci_device_intr_start(struct usbd_xfer
>  #endif
>
>         s = splusb();
> -       ehci_set_qh_qtd(sqh, data);
>         if (xfer->timeout && !sc->sc_bus.use_polling) {
>                 timeout_del(&xfer->timeout_handle);
>                 timeout_set(&xfer->timeout_handle, ehci_timeout, xfer);
>                 timeout_add_msec(&xfer->timeout_handle, xfer->timeout);
>         }
> +       ehci_add_qh(xfer->pipe, sc->sc_async_head, data);
>         TAILQ_INSERT_TAIL(&sc->sc_intrhead, ex, inext);
>         xfer->status = USBD_IN_PROGRESS;
>         splx(s);
> @@ -3179,48 +3076,15 @@ void
>  ehci_device_intr_done(struct usbd_xfer *xfer)
>  {
>         struct ehci_softc *sc = (struct ehci_softc *)xfer->device->bus;
> -       struct ehci_pipe *epipe = (struct ehci_pipe *)xfer->pipe;
>         struct ehci_xfer *ex = (struct ehci_xfer *)xfer;
> -       struct ehci_soft_qtd *data, *dataend;
> -       struct ehci_soft_qh *sqh;
> -       usbd_status err;
> -       int s;
>
>         if (xfer->pipe->repeat) {
>                 ehci_free_sqtd_chain(sc, ex);
> -
>                 usb_syncmem(&xfer->dmabuf, 0, xfer->length,
>                     usbd_xfer_isread(xfer) ?
>                     BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
> -               sqh = epipe->sqh;
> -
> -               err = ehci_alloc_sqtd_chain(sc, xfer->length, xfer, &data, 
> &dataend);
> -               if (err) {
> -                       xfer->status = err;
> -                       return;
> -               }
>
> -               /* Set up interrupt info. */
> -               ex->sqtdstart = data;
> -               ex->sqtdend = dataend;
> -#ifdef DIAGNOSTIC
> -               if (!ex->isdone) {
> -                       printf("ehci_device_intr_done: not done, ex=%p\n",
> -                                       ex);
> -               }
> -               ex->isdone = 0;
> -#endif
> -
> -               s = splusb();
> -               ehci_set_qh_qtd(sqh, data);
> -               if (xfer->timeout && !sc->sc_bus.use_polling) {
> -                       timeout_del(&xfer->timeout_handle);
> -                       timeout_set(&xfer->timeout_handle, ehci_timeout, 
> xfer);
> -                       timeout_add_msec(&xfer->timeout_handle, 
> xfer->timeout);
> -               }
> -               TAILQ_INSERT_TAIL(&sc->sc_intrhead, ex, inext);
> -               xfer->status = USBD_IN_PROGRESS;
> -               splx(s);
> +               ehci_device_intr_start(xfer);
>         } else if (xfer->status != USBD_NOMEM) {
>                 ehci_free_sqtd_chain(sc, ex);
>         }
> Index: ehcivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ehcivar.h,v
> retrieving revision 1.37
> diff -u -p -r1.37 ehcivar.h
> --- ehcivar.h   2 Oct 2016 06:36:39 -0000       1.37
> +++ ehcivar.h   3 Oct 2017 09:24:08 -0000
> @@ -46,7 +46,6 @@ struct ehci_soft_qh {
>         struct ehci_qh qh;
>         struct ehci_soft_qh *next;
>         struct ehci_soft_qh *prev;
> -       struct ehci_soft_qtd *sqtd;
>         ehci_physaddr_t physaddr;
>         struct usb_dma dma;             /* QH's DMA infos */
>         int offs;                       /* QH's offset in struct usb_dma */

Reply via email to