Re: Patch: xhci controller driver improvements

[Takahiro HAYASHI <t.hash425%gmail.com@localhost>]

On 08/11/14 19:40, Nick Hudson wrote:
> > > > @@ -2889,6 +3028,7 @@ xhci_timeout(void *addr)
> > > >       struct xhci_softc * const sc = xfer->pipe->device->bus->hci_private;
> > > >       if (sc->sc_dying) {
> > > > +        xhci_abort_xfer(xfer, USBD_TIMEOUT);
> > > >           return;
> > > >       }
> > >
> > > This looks very strange.
> >
> > Does it need sc_lock and unlock?
> > or it's strange that xhci_abort_xfer is called
> > from callout(softclock) interrupt context?
>
> Calling xhci_abort_xfer while dying.

well, should it be like this?

--- xhci.c.orig 2014-08-11 20:14:13.000000000 +0900
+++ xhci.c      2014-08-12 06:41:55.000000000 +0900
@@ -2896,6 +3492,11 @@ xhci_timeout(void *addr)
        struct xhci_softc * const sc = xfer->pipe->device->bus->hci_private;
 
 	if (sc->sc_dying) {
+               mutex_enter(&sc->sc_lock);
+               xfer->status = USBD_TIMEOUT;
+               callout_stop(&xfer->timeout_handle);
+               usb_transfer_complete(xfer);
+               mutex_exit(&sc->sc_lock);
                return;
        }
 

This resembles {u,o,e}hci.c do in dying path.

> > > The driver makes far too much use of device_printf and all USB should move to 
> > > KERNHIST.
> >
> > I didn't know about KERNHIST, thanks for notifying.
> > I've replaced device_printf with DPRINTF or DPRINTFN in
> > my local tree.
> Please send diff :)

Wait for a while plz.


--
t-hash