tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Patch: xhci controller driver improvements



Hello,

(05/11/14 02:57), Rajasekhar Pulluru wrote:
Hi,

The attached patch addresses following points.

- With some usb devices, often some endpoints gets stalled. Resetting the
endpoint helps recover usb transactions.

- When a usb device is removed while data is being copied to/from, all the
queued transfer requests needs to be aborted gracefully so that transfer
status is reported up to the application.

- Some boards require an additional interrupt acknowledgement specific to
the architecture. Added a function pointer that needs to be set during that
board arch-specific initialization. If this is not required, function
pointer needs to be set to NULL.

- Added debug functions to dump xhci registers that could help us in case
of any issue.

Note: I have manually patched the code changes from my older
version(verified) of code to the latest as of today in MAIN branch. And the
code is not compiled either, sorry about that. If anyone is interested to
patch this and verify, it would be really helpful.

I reformatted whitespace and made several changes:

- avoid lock error when calling xhci_do_command from
  xhci_stop_endpoint
- assume usb_uncallout(a,b,c) is callout_stop(&a)
- comment out checking xfer->device->bus->intr_context
- declare gsc used in debug code (would be set in ddb?)

It should build at least, but still experimental.


--
t-hash








--- src/sys/dev/usb/xhci.c.orig 2014-08-05 22:24:27.000000000 +0900
+++ src/sys/dev/usb/xhci.c      2014-08-07 17:35:16.000000000 +0900
@@ -55,6 +55,8 @@ __KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.2
 #include <dev/usb/xhcivar.h>
 #include <dev/usb/usbroothub_subr.h>
 
+#include <uvm/uvm.h> /* for vtophys on arm */
+
 #ifdef XHCI_DEBUG
 int xhcidebug = 0;
 #define DPRINTF(x)     do { if (xhcidebug) printf x; } while(0)
@@ -79,6 +81,7 @@ struct xhci_pipe {
 #define XHCI_EVENT_RING_SEGMENTS 1
 #define XHCI_TRB_3_ED_BIT XHCI_TRB_3_ISP_BIT
 
+static void xhci_abort_xfer(usbd_xfer_handle xfer, usbd_status status);
 static usbd_status xhci_open(usbd_pipe_handle);
 static int xhci_intr1(struct xhci_softc * const);
 static void xhci_softintr(void *);
@@ -94,12 +97,16 @@ static usbd_status xhci_new_device(devic
 static usbd_status xhci_configure_endpoint(usbd_pipe_handle);
 static usbd_status xhci_unconfigure_endpoint(usbd_pipe_handle);
 static usbd_status xhci_reset_endpoint(usbd_pipe_handle);
-//static usbd_status xhci_stop_endpoint(usbd_pipe_handle);
+static usbd_status xhci_stop_endpoint(usbd_pipe_handle);
 
 static usbd_status xhci_set_dequeue(usbd_pipe_handle);
 
 static usbd_status xhci_do_command(struct xhci_softc * const,
     struct xhci_trb * const, int);
+static usbd_status xhci_do_command1(struct xhci_softc * const,
+    struct xhci_trb * const, int, int);
+static usbd_status xhci_do_command_locked(struct xhci_softc * const,
+    struct xhci_trb * const, int);
 static usbd_status xhci_init_slot(struct xhci_softc * const, uint32_t,
     int, int, int, int);
 static usbd_status xhci_enable_slot(struct xhci_softc * const,
@@ -336,13 +343,13 @@ xhci_rt_write_8(const struct xhci_softc 
        }
 }
 
-#if 0 /* unused */
+#ifdef XHCI_DEBUG
 static inline uint32_t
 xhci_db_read_4(const struct xhci_softc * const sc, bus_size_t offset)
 {
        return bus_space_read_4(sc->sc_iot, sc->sc_dbh, offset);
 }
-#endif /* unused */
+#endif
 
 static inline void
 xhci_db_write_4(const struct xhci_softc * const sc, bus_size_t offset,
@@ -888,6 +895,14 @@ xhci_intr1(struct xhci_softc * const sc)
        usbsts = xhci_op_read_4(sc, XHCI_USBSTS);
        //device_printf(sc->sc_dev, "%s USBSTS %08x\n", __func__, usbsts);
 
+       /*
+        * some arch requires additional arch specific xhci intr ack
+        * this function pointer needs to be implemented and assigned 
+        * in arch specifc code
+        */ 
+       if (sc->sc_quirk_intr_ack != NULL)
+               sc->sc_quirk_intr_ack((void*)sc);
+
        iman = xhci_rt_read_4(sc, XHCI_IMAN(0));
        //device_printf(sc->sc_dev, "%s IMAN0 %08x\n", __func__, iman);
        if ((iman & XHCI_IMAN_INTR_PEND) == 0) {
@@ -1011,7 +1026,6 @@ xhci_reset_endpoint(usbd_pipe_handle pip
        return err;
 }
 
-#if 0
 static usbd_status
 xhci_stop_endpoint(usbd_pipe_handle pipe)
 {
@@ -1029,11 +1043,10 @@ xhci_stop_endpoint(usbd_pipe_handle pipe
            XHCI_TRB_3_EP_SET(dci) |
            XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_STOP_EP);
 
-       err = xhci_do_command(sc, &trb, USBD_DEFAULT_TIMEOUT);
+       err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
 
        return err;
 }
-#endif
 
 static usbd_status
 xhci_set_dequeue(usbd_pipe_handle pipe)
@@ -1065,6 +1078,26 @@ xhci_set_dequeue(usbd_pipe_handle pipe)
        return err;
 }
 
+#ifdef XHCI_DEBUG
+static void
+dump_endpoint_info(usb_endpoint_descriptor_t * const ed)
+{
+       const uint8_t xfertype = UE_GET_XFERTYPE(ed->bmAttributes);
+       printf("direction (%08x) : %s\n, xfertype (%08x) : ", 
ed->bEndpointAddress,
+               UE_GET_DIR(ed->bEndpointAddress) ? "in" : "out", xfertype);
+       if (xfertype == UE_CONTROL)
+               printf("CONTROL\n");
+       else if (xfertype == UE_ISOCHRONOUS)
+               printf("ISOCHRONOUS\n");
+       else if (xfertype == UE_BULK)
+               printf("BULK\n");
+       else if (xfertype == UE_INTERRUPT)
+               printf("INTERRUPT\n");
+       else
+               printf("unknown\n");
+}
+#endif
+
 static usbd_status
 xhci_open(usbd_pipe_handle pipe)
 {
@@ -1079,6 +1112,11 @@ xhci_open(usbd_pipe_handle pipe)
        device_printf(sc->sc_dev, "%s addr %d depth %d port %d speed %d\n",
            __func__, addr, dev->depth, dev->powersrc->portno, dev->speed);
 
+#ifdef XHCI_DEBUG
+       if (xhcidebug)
+               dump_endpoint_info(ed);
+#endif
+
        if (sc->sc_dying)
                return USBD_IOERROR;
 
@@ -1127,6 +1165,81 @@ xhci_open(usbd_pipe_handle pipe)
 }
 
 static void
+xhci_abort_xfer(usbd_xfer_handle xfer, usbd_status status)
+{
+       int s;
+       struct xhci_softc * const sc = xfer->pipe->device->bus->hci_private;
+       int wake;
+
+       DPRINTF(("%s: xfer=%p pipe=%p\n", __func__, xfer, xfer->pipe));
+
+       if (sc->sc_dying) {
+               aprint_normal("%s: dying\n", __func__);
+               /* If we're dying, just do the software part. */
+               s = splusb();
+               xfer->status = status;  /* make software ignore it */
+               callout_stop(&xfer->timeout_handle);
+               usb_transfer_complete(xfer);
+               splx(s);
+               return;
+       }
+
+#if 0
+       if (xfer->device->bus->intr_context)
+               panic("xhci_abort_xfer: not in process context");
+#endif
+
+       /*
+        * If an abort is already in progress then just wait for it to
+        * complete and return.
+        */
+       if (xfer->hcflags & UXFER_ABORTING) {
+               DPRINTFN(2, ("%s: already aborting\n", __func__));
+#ifdef DIAGNOSTIC
+               if (status == USBD_TIMEOUT)
+                       printf("%s: TIMEOUT while aborting\n", __func__);
+#endif
+               /* Override the status which might be USBD_TIMEOUT. */
+               xfer->status = status;
+               DPRINTFN(2, ("%s: waiting for abort to finish\n", __func__));
+               xfer->hcflags |= UXFER_ABORTWAIT;
+               while (xfer->hcflags & UXFER_ABORTING)
+                       tsleep(&xfer->hcflags, PZERO, "xhciaw", 0);
+               return;
+       }
+       xfer->hcflags |= UXFER_ABORTING;
+
+       /*
+        * Step 1: Make interrupt routine and hardware ignore xfer.
+        */
+       s = splusb();
+       xfer->status = status;  /* make software ignore it */
+       callout_stop(&xfer->timeout_handle);
+       splx(s);
+       xhci_stop_endpoint(xfer->pipe);
+
+       /*
+        * Step 2: Wait until we know hardware has finished any possible
+        * use of the xfer.  Also make sure the soft interrupt routine
+        * has run.
+        */
+       s = splusb();
+       usb_schedsoftintr(&sc->sc_bus);
+       splx(s);
+
+       /*
+        * Step 3: Execute callback.
+        */
+       wake = xfer->hcflags & UXFER_ABORTWAIT;
+       xfer->hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT);
+       usb_transfer_complete(xfer);
+       if (wake)
+               wakeup(&xfer->hcflags);
+
+       splx(s);
+}
+
+static void
 xhci_rhpsc(struct xhci_softc * const sc, u_int port)
 {
        usbd_xfer_handle const xfer = sc->sc_intrxfer;
@@ -1160,6 +1273,7 @@ xhci_handle_event(struct xhci_softc * co
        uint32_t trb_2, trb_3;
 
        DPRINTF(("%s: %s\n", __func__, device_xname(sc->sc_dev)));
+       KASSERT(mutex_owned(&sc->sc_lock));
 
        trb_0 = le64toh(trb->trb_0);
        trb_2 = le32toh(trb->trb_2);
@@ -1226,7 +1340,6 @@ xhci_handle_event(struct xhci_softc * co
                }
                xfer->status = err;
 
-               //mutex_enter(&sc->sc_lock); /* XXX ??? */
                if ((trb_3 & XHCI_TRB_3_ED_BIT) != 0) {
                        if ((trb_0 & 0x3) == 0x0) {
                                usb_transfer_complete(xfer);
@@ -1234,7 +1347,6 @@ xhci_handle_event(struct xhci_softc * co
                } else {
                        usb_transfer_complete(xfer);
                }
-               //mutex_exit(&sc->sc_lock); /* XXX ??? */
 
                }
                break;
@@ -1704,8 +1816,8 @@ xhci_ring_put(struct xhci_softc * const 
 }
 
 static usbd_status
-xhci_do_command(struct xhci_softc * const sc, struct xhci_trb * const trb,
-    int timeout)
+xhci_do_command1(struct xhci_softc * const sc,
+    struct xhci_trb * const trb, int timeout, int locked)
 {
        struct xhci_ring * const cr = &sc->sc_cr;
        usbd_status err;
@@ -1714,7 +1826,8 @@ xhci_do_command(struct xhci_softc * cons
            "0x%016"PRIx64" 0x%08"PRIx32" 0x%08"PRIx32"\n", __func__,
            trb->trb_0, trb->trb_2, trb->trb_3);
 
-       mutex_enter(&sc->sc_lock);
+       if (!locked)
+               mutex_enter(&sc->sc_lock);
 
        KASSERT(sc->sc_command_addr == 0);
        sc->sc_command_addr = xhci_ring_trbp(cr, cr->xr_ep);
@@ -1754,11 +1867,26 @@ xhci_do_command(struct xhci_softc * cons
 
 timedout:
        sc->sc_command_addr = 0;
-       mutex_exit(&sc->sc_lock);
+       if (!locked)
+               mutex_exit(&sc->sc_lock);
        return err;
 }
 
 static usbd_status
+xhci_do_command(struct xhci_softc * const sc, struct xhci_trb * const trb,
+    int timeout)
+{
+       return xhci_do_command1(sc, trb, timeout, 0);
+}
+
+static usbd_status
+xhci_do_command_locked(struct xhci_softc * const sc,
+    struct xhci_trb * const trb, int timeout)
+{
+       return xhci_do_command1(sc, trb, timeout, 1);
+}
+
+static usbd_status
 xhci_enable_slot(struct xhci_softc * const sc, uint8_t * const slotp)
 {
        struct xhci_trb trb;
@@ -2616,6 +2744,7 @@ static void
 xhci_device_ctrl_abort(usbd_xfer_handle xfer)
 {
        DPRINTF(("%s\n", __func__));
+       xhci_abort_xfer(xfer, USBD_CANCELLED);
 }
 
 static void
@@ -2673,6 +2802,16 @@ xhci_device_bulk_start(usbd_xfer_handle 
        if (sc->sc_dying)
                return USBD_IOERROR;
 
+       if (tr->is_halted) {
+               /* If an endpoint is stalled for some reason, reset it */
+               device_printf(sc->sc_dev, "%s %p slot %u dci %u\n", __func__, 
xfer,
+                   xs->xs_idx, dci);
+               device_printf(sc->sc_dev, "%s: transfer ring halted.. 
resetting...\n", __func__);
+               xhci_reset_endpoint(xfer->pipe);
+               tr->is_halted = false;
+               xhci_set_dequeue(xfer->pipe);
+       }
+
        KASSERT((xfer->rqflags & URQ_REQUEST) == 0);
 
        parameter = DMAADDR(dma, 0);
@@ -2737,6 +2876,7 @@ static void
 xhci_device_bulk_abort(usbd_xfer_handle xfer)
 {
        DPRINTF(("%s\n", __func__));
+       xhci_abort_xfer(xfer, USBD_CANCELLED);
 }
 
 static void
@@ -2866,8 +3006,7 @@ xhci_device_intr_abort(usbd_xfer_handle 
        KASSERT(mutex_owned(&sc->sc_lock));
        device_printf(sc->sc_dev, "%s %p\n", __func__, xfer);
        KASSERT(xfer->pipe->intrxfer == xfer);
-       xfer->status = USBD_CANCELLED;
-       usb_transfer_complete(xfer);
+       xhci_abort_xfer(xfer, USBD_CANCELLED);
 }
 
 static void
@@ -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;
        }
 
@@ -2905,7 +3045,7 @@ xhci_timeout_task(void *addr)
        struct xhci_softc * const sc = xfer->pipe->device->bus->hci_private;
 
        mutex_enter(&sc->sc_lock);
-#if 0
+#if 1
        xhci_abort_xfer(xfer, USBD_TIMEOUT);
 #else
        xfer->status = USBD_TIMEOUT;
@@ -2913,3 +3053,115 @@ xhci_timeout_task(void *addr)
 #endif
        mutex_exit(&sc->sc_lock);
 }
+
+#ifdef XHCI_DEBUG
+
+void xhci_dump_capability_reg(void);
+void xhci_dump_operational_reg(void);
+void xhci_dump_runtime_reg(void);
+void xhci_dump_doorbell_reg(void);
+void xhci_dump_ring_reg(void);
+void xhci_dump_reg(void);
+void xhci_ring_doorbell(int, int);
+
+const struct xhci_softc * volatile gsc = NULL;
+
+void
+xhci_dump_capability_reg(void)
+{
+       printf("Capability Registers \n");
+       printf("\tCapLength     %08x\n", xhci_cap_read_4(gsc, XHCI_CAPLENGTH));
+       printf("\tHCSPARAMS 1   %08x\n", xhci_cap_read_4(gsc, XHCI_HCSPARAMS1));
+       printf("\tHCSPARAMS 2   %08x\n", xhci_cap_read_4(gsc, XHCI_HCSPARAMS2));
+       printf("\tHCSPARAMS 3   %08x\n", xhci_cap_read_4(gsc, XHCI_HCSPARAMS3));
+       printf("\tHCSPARAMS     %08x\n", xhci_cap_read_4(gsc, XHCI_HCCPARAMS));
+       printf("\tDBOFF         %08x\n", xhci_cap_read_4(gsc, XHCI_DBOFF));
+}
+
+void
+xhci_dump_operational_reg(void)
+{
+       int nports = 2;
+       int i = 0;
+
+       printf("Operational Registers \n");
+       printf("\tUSB Command   %08x\n", xhci_op_read_4(gsc, XHCI_USBCMD));
+       printf("\tUSB Status    %08x\n", xhci_op_read_4(gsc, XHCI_USBSTS));
+       printf("\tPage Size     %08x\n", xhci_op_read_4(gsc, XHCI_PAGESIZE));
+       printf("\tDNCTRL        %08x\n", xhci_op_read_4(gsc, XHCI_DNCTRL));
+       printf("\tCRCR          %08x\n", xhci_op_read_4(gsc, XHCI_CRCR));
+       printf("\tDCBAAP        %08x\n", xhci_op_read_4(gsc, XHCI_DCBAAP));
+       printf("\tCONFIG        %08x\n", xhci_op_read_4(gsc, XHCI_CONFIG));
+       printf("\tPort Status Registers \n");
+       for (i=0;i<nports;i++)
+       {
+               printf("\tPORTSC[%d]    %08x\n", i, xhci_op_read_4(gsc, 
XHCI_PORTSC(i)));
+               printf("\tPORTPMSC[%d]  %08x\n", i, xhci_op_read_4(gsc, 
XHCI_PORTPMSC(i)));
+               printf("\tPORTLI[%d]    %08x\n", i, xhci_op_read_4(gsc, 
XHCI_PORTLI(i)));
+       }
+}
+
+void
+xhci_dump_runtime_reg(void)
+{
+       printf("Runtime Registers \n");
+       printf("\tIMAN0         %08x\n", xhci_rt_read_4(gsc, XHCI_IMAN(0)));
+       printf("\tIMOD0         %08x\n", xhci_rt_read_4(gsc, XHCI_IMOD(0)));
+       printf("\tERSTSZ0       %08x\n", xhci_rt_read_4(gsc, XHCI_ERSTSZ(0)));
+       printf("\tERSTBA0       %08x\n", xhci_rt_read_4(gsc, XHCI_ERSTBA(0)));
+       printf("\tERSTBA-HI0    %08x\n", xhci_rt_read_4(gsc, 
XHCI_ERSTBA_HI(0)));
+       printf("\tERDB0         %08x\n", xhci_rt_read_4(gsc, XHCI_ERDP(0)));
+       printf("\tERDB-HI0      %08x\n", xhci_rt_read_4(gsc, XHCI_ERDP_HI(0)));
+}
+
+void
+xhci_dump_doorbell_reg(void)
+{
+       int i=0;
+       int nports = 2;
+
+       printf("DoorBell Registers \n");
+       for (i=0;i<nports;i++)
+       {
+               printf("\tDoorbell[%d] : %08x\n",
+                   i, xhci_db_read_4(gsc, XHCI_DOORBELL(i)));
+       }
+}
+
+void
+xhci_dump_ring_reg(void)
+{
+       int i=0, j=0, k=0;
+       struct xhci_ring * const er = __UNCONST(&gsc->sc_er);
+       struct xhci_trb *trb;
+
+       i = er->xr_ep;
+       j = er->xr_cs;
+       printf("Event Ring enque index %d, cycle state %d\n", i, j);
+       usb_syncmem(&er->xr_dma, XHCI_TRB_SIZE * i, XHCI_TRB_SIZE,
+                   BUS_DMASYNC_POSTREAD);
+       trb = &er->xr_trb[i];
+       k = (le32toh(trb->trb_3) & XHCI_TRB_3_CYCLE_BIT) ? 1 : 0;
+       if (j == k)
+               printf("Event occurred\n");
+       else
+               printf("No Event\n");
+
+}
+
+void
+xhci_dump_reg(void)
+{
+       xhci_dump_capability_reg();
+       xhci_dump_operational_reg();
+       xhci_dump_runtime_reg();
+       xhci_dump_doorbell_reg();
+       xhci_dump_ring_reg();
+}
+
+void
+xhci_ring_doorbell(int slot, int epindex)
+{
+       xhci_db_write_4(gsc, XHCI_DOORBELL(slot), epindex);
+}
+#endif /* XHCI_DEBUG */
Index: src/sys/dev/usb/xhcivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/xhcivar.h,v
retrieving revision 1.4
diff -u -p -r1.4 xhcivar.h
--- src/sys/dev/usb/xhcivar.h   10 Mar 2014 13:12:02 -0000      1.4
+++ src/sys/dev/usb/xhcivar.h   1 Aug 2014 11:00:37 -0000
@@ -114,6 +114,7 @@ struct xhci_softc {
 
        uint8_t sc_addr;
        uint8_t sc_conf;
+       int (*sc_quirk_intr_ack) (void *);
 };
 
 int    xhci_init(struct xhci_softc *);


Home | Main Index | Thread Index | Old Index