Re: xhci not working with urtwn

[Takahiro Hayashi <t425hash%gmail.com@localhost>]

Hi,

On 2016/07/08 00:44, coypu%SDF.ORG@localhost wrote:
> hi,
> my urtwn (EW-7811Un) is failing with xhci.
> it still works if I use another machine which has USB2 ports on a recent
> kernel (i.e. it is not a urtwn(4) problem).
>
> in dmesg I see:
> urtwn0: could not load firmware page 0

Can you try attached patch?

It seems that Intel xHC may fail to handle control transfer
with Link TRB if it uses EVENT_DATA TRB.
This patch avoids using EVENT_DATA and adds some improvements.


--
t-hash

--- sys/dev/usb/xhci.c.orig	2016-07-10 11:45:17.000000000 +0900
+++ sys/dev/usb/xhci.c	2016-07-17 03:55:54.000000000 +0900
@@ -520,6 +520,17 @@ xhci_trb_get_idx(struct xhci_ring *xr, u
 	return 0;
 }
 
+static unsigned int
+xhci_get_epstate(struct xhci_softc * const sc, struct xhci_slot * const xs,
+    u_int dci)
+{
+	uint32_t *cp;
+
+	usb_syncmem(&xs->xs_dc_dma, 0, sc->sc_pgsz, BUS_DMASYNC_POSTREAD);
+	cp = xhci_slot_get_dcv(sc, xs, dci);
+	return XHCI_EPCTX_0_EPSTATE_GET(le32toh(cp[0]));
+}
+
 /* --- */
 
 void
@@ -1281,7 +1292,7 @@ xhci_unconfigure_endpoint(struct usbd_pi
 
 /* 4.6.8, 6.4.3.7 */
 static usbd_status
-xhci_reset_endpoint(struct usbd_pipe *pipe)
+xhci_reset_endpoint_locked(struct usbd_pipe *pipe)
 {
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
@@ -1292,17 +1303,31 @@ xhci_reset_endpoint(struct usbd_pipe *pi
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 	DPRINTFN(4, "slot %u dci %u", xs->xs_idx, dci, 0, 0);
 
+	KASSERT(mutex_owned(&sc->sc_lock));
+
 	trb.trb_0 = 0;
 	trb.trb_2 = 0;
 	trb.trb_3 = XHCI_TRB_3_SLOT_SET(xs->xs_idx) |
 	    XHCI_TRB_3_EP_SET(dci) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_RESET_EP);
 
-	err = xhci_do_command(sc, &trb, USBD_DEFAULT_TIMEOUT);
+	err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
 
 	return err;
 }
 
+static usbd_status
+xhci_reset_endpoint(struct usbd_pipe *pipe)
+{
+	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
+
+	mutex_enter(&sc->sc_lock);
+	usbd_status ret = xhci_reset_endpoint_locked(pipe);
+	mutex_exit(&sc->sc_lock);
+
+	return ret;
+}
+
 /*
  * 4.6.9, 6.4.3.8
  * Stop execution of TDs on xfer ring.
@@ -1342,7 +1367,7 @@ xhci_stop_endpoint(struct usbd_pipe *pip
  * error will be generated.
  */
 static usbd_status
-xhci_set_dequeue(struct usbd_pipe *pipe)
+xhci_set_dequeue_locked(struct usbd_pipe *pipe)
 {
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
@@ -1354,6 +1379,8 @@ xhci_set_dequeue(struct usbd_pipe *pipe)
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 	DPRINTFN(4, "slot %u dci %u", xs->xs_idx, dci, 0, 0);
 
+	KASSERT(mutex_owned(&sc->sc_lock));
+
 	xhci_host_dequeue(xr);
 
 	/* set DCS */
@@ -1363,11 +1390,23 @@ xhci_set_dequeue(struct usbd_pipe *pipe)
 	    XHCI_TRB_3_EP_SET(dci) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_SET_TR_DEQUEUE);
 
-	err = xhci_do_command(sc, &trb, USBD_DEFAULT_TIMEOUT);
+	err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
 
 	return err;
 }
 
+static usbd_status
+xhci_set_dequeue(struct usbd_pipe *pipe)
+{
+	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
+
+	mutex_enter(&sc->sc_lock);
+	usbd_status ret = xhci_set_dequeue_locked(pipe);
+	mutex_exit(&sc->sc_lock);
+
+	return ret;
+}
+
 /*
  * Open new pipe: called from usbd_setup_pipe_flags.
  * Fills methods of pipe.
@@ -1510,12 +1549,14 @@ xhci_close_pipe(struct usbd_pipe *pipe)
 
 /*
  * Abort transfer.
- * May be called from softintr context.
+ * Should be called with sc_lock held.
  */
 static void
 xhci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
 {
 	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
+	struct xhci_slot * const xs = xfer->ux_pipe->up_dev->ud_hcpriv;
+	const u_int dci = xhci_ep_get_dci(xfer->ux_pipe->up_endpoint->ue_edesc);
 
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 	DPRINTFN(4, "xfer %p pipe %p status %d",
@@ -1532,10 +1573,66 @@ xhci_abort_xfer(struct usbd_xfer *xfer, 
 		return;
 	}
 
-	/* XXX need more stuff */
+	/*
+	 * If an abort is already in progress then just wait for it to
+	 * complete and return.
+	 */
+	if (xfer->ux_hcflags & UXFER_ABORTING) {
+		DPRINTFN(4, "already aborting", 0, 0, 0, 0);
+#ifdef DIAGNOSTIC
+		if (status == USBD_TIMEOUT)
+			DPRINTFN(4, "TIMEOUT while aborting", 0, 0, 0, 0);
+#endif
+		/* Override the status which might be USBD_TIMEOUT. */
+		xfer->ux_status = status;
+		DPRINTFN(4, "xfer %p waiting for abort to finish", xfer, 0, 0,
+		    0);
+		xfer->ux_hcflags |= UXFER_ABORTWAIT;
+		while (xfer->ux_hcflags & UXFER_ABORTING)
+			cv_wait(&xfer->ux_hccv, &sc->sc_lock);
+		return;
+	}
+	xfer->ux_hcflags |= UXFER_ABORTING;
+
+	/*
+	 * Step 1: Stop xfer timeout timer.
+	 */
 	xfer->ux_status = status;
 	callout_stop(&xfer->ux_callout);
+
+	/*
+	 * Step 2: Stop execution of TD on the ring.
+	 */
+	switch (xhci_get_epstate(sc, xs, dci)) {
+	case XHCI_EPSTATE_HALTED:
+		(void)xhci_reset_endpoint_locked(xfer->ux_pipe);
+		break;
+	case XHCI_EPSTATE_STOPPED:
+		break;
+	default:
+		(void)xhci_stop_endpoint(xfer->ux_pipe);
+		break;
+	}
+#ifdef DIAGNOSTIC
+	uint32_t epst = xhci_get_epstate(sc, xs, dci);
+	if (epst != XHCI_EPSTATE_STOPPED)
+		DPRINTFN(4, "dci %u not stopped %u", dci, epst, 0, 0);
+#endif
+
+	/*
+	 * Step 3: Remove any vestiges of the xfer from the ring.
+	 */
+	xhci_set_dequeue_locked(xfer->ux_pipe);
+
+	/*
+	 * Step 4: Notify completion to waiting xfers.
+	 */
+	int wake = xfer->ux_hcflags & UXFER_ABORTWAIT;
+	xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT);
 	usb_transfer_complete(xfer);
+	if (wake) {
+		cv_broadcast(&xfer->ux_hccv);
+	}
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
@@ -1667,12 +1764,12 @@ xhci_event_transfer(struct xhci_softc * 
 		}
 		xx = xr->xr_cookies[idx];
 
+		/* clear cookie of consumed TRB */
+		xr->xr_cookies[idx] = NULL;
+
 		/*
-		 * If endpoint is stopped between TDs, TRB pointer points at
-		 * next TRB, however, it is not put yet or is a garbage TRB.
-		 * That's why xr_cookies may be NULL or look like broken.
-		 * Note: this ev happens only when hciversion >= 1.0 or
-		 * hciversion == 0.96 and FSE of hcc1 is set.
+		 * xx is NULL if pipe is opened but xfer is not started.
+		 * It happens when stopping idle pipe.
 		 */
 		if (xx == NULL || trbcode == XHCI_TRB_ERROR_LENGTH) {
 			DPRINTFN(1, "Ignore #%u: cookie %p cc %u dci %u",
@@ -1680,6 +1777,7 @@ xhci_event_transfer(struct xhci_softc * 
 			DPRINTFN(1, " orig TRB %"PRIx64" type %u", trb_0,
 			    XHCI_TRB_3_TYPE_GET(le32toh(xr->xr_trb[idx].trb_3)),
 			    0, 0);
+			return;
 		}
 	} else {
 		/* When ED != 0, trb_0 is virtual addr of struct xhci_xfer. */
@@ -1739,10 +1837,32 @@ xhci_event_transfer(struct xhci_softc * 
 		 * ctrl xfer uses EVENT_DATA, and others do not.
 		 * Thus driver can switch the flow by checking ED bit.
 		 */
-		xfer->ux_actlen =
-		    xfer->ux_length - XHCI_TRB_2_REM_GET(trb_2);
+		if ((trb_3 & XHCI_TRB_3_ED_BIT) == 0) {
+			if (xfer->ux_actlen == 0)
+				xfer->ux_actlen = xfer->ux_length -
+				    XHCI_TRB_2_REM_GET(trb_2);
+			if (XHCI_TRB_3_TYPE_GET(le32toh(xr->xr_trb[idx].trb_3))
+			    == XHCI_TRB_TYPE_DATA_STAGE) {
+				return;
+			}
+		} else if ((trb_0 & 0x3) == 0x3) {
+			return;
+		}
 		err = USBD_NORMAL_COMPLETION;
 		break;
+	case XHCI_TRB_ERROR_STOPPED:
+	case XHCI_TRB_ERROR_LENGTH:
+	case XHCI_TRB_ERROR_STOPPED_SHORT:
+		/*
+		 * don't complete the transfer being aborted
+		 * as abort_xfer does instead.
+		 */
+		if (xfer->ux_hcflags & UXFER_ABORTING) {
+			DPRINTFN(14, "ignore aborting xfer %p", xfer, 0, 0, 0);
+			return;
+		}
+		err = USBD_CANCELLED;
+		break;
 	case XHCI_TRB_ERROR_STALL:
 	case XHCI_TRB_ERROR_BABBLE:
 		DPRINTFN(1, "ERR %u slot %u dci %u", trbcode, slot, dci, 0);
@@ -2079,8 +2199,10 @@ xhci_new_device(device_t parent, struct 
 
 		/* 4.3.2 */
 		err = xhci_enable_slot(sc, &slot);
-		if (err)
+		if (err) {
+			DPRINTFN(1, "enable slot %u", err, 0, 0, 0);
 			goto bad;
+		}
 
 		xs = &sc->sc_slots[slot];
 		dev->ud_hcpriv = xs;
@@ -2088,6 +2210,7 @@ xhci_new_device(device_t parent, struct 
 		/* 4.3.3 initialize slot structure */
 		err = xhci_init_slot(dev, slot);
 		if (err) {
+			DPRINTFN(1, "init slot %u", err, 0, 0, 0);
 			dev->ud_hcpriv = NULL;
 			/*
 			 * We have to disable_slot here because
@@ -2235,7 +2358,7 @@ xhci_ring_put(struct xhci_softc * const 
 		DPRINTFN(12, " %016"PRIx64" %08"PRIx32" %08"PRIx32,
 		    trbs[i].trb_0, trbs[i].trb_2, trbs[i].trb_3, 0);
 		KASSERTMSG(XHCI_TRB_3_TYPE_GET(trbs[i].trb_3) !=
-		    XHCI_TRB_TYPE_LINK, "trb3 type %d", trbs[i].trb_3);
+		    XHCI_TRB_TYPE_LINK, "trbs[%zu].trb3 %#x", i, trbs[i].trb_3);
 	}
 
 	DPRINTFN(12, "%p xr_ep 0x%x xr_cs %u", xr, xr->xr_ep, xr->xr_cs, 0);
@@ -3466,7 +3589,7 @@ xhci_device_ctrl_start(struct usbd_xfer 
 	i = 0;
 
 	/* setup phase */
-	memcpy(&parameter, req, sizeof(*req));
+	memcpy(&parameter, req, sizeof(parameter));
 	status = XHCI_TRB_2_IRQ_SET(0) | XHCI_TRB_2_BYTES_SET(sizeof(*req));
 	control = ((len == 0) ? XHCI_TRB_3_TRT_NONE :
 	     (isread ? XHCI_TRB_3_TRT_IN : XHCI_TRB_3_TRT_OUT)) |
@@ -3483,12 +3606,7 @@ xhci_device_ctrl_start(struct usbd_xfer 
 		    XHCI_TRB_2_BYTES_SET(len);
 		control = (isread ? XHCI_TRB_3_DIR_IN : 0) |
 		    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_DATA_STAGE) |
-		    XHCI_TRB_3_CHAIN_BIT | XHCI_TRB_3_ENT_BIT;
-		xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
-
-		parameter = (uintptr_t)xfer | 0x3;
-		status = XHCI_TRB_2_IRQ_SET(0);
-		control = XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_EVENT_DATA) |
+		    (usbd_xfer_isread(xfer) ? XHCI_TRB_3_ISP_BIT : 0) |
 		    XHCI_TRB_3_IOC_BIT;
 		xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
 	}
@@ -3498,12 +3616,6 @@ xhci_device_ctrl_start(struct usbd_xfer 
 	/* the status stage has inverted direction */
 	control = ((isread && (len > 0)) ? 0 : XHCI_TRB_3_DIR_IN) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_STATUS_STAGE) |
-	    XHCI_TRB_3_CHAIN_BIT | XHCI_TRB_3_ENT_BIT;
-	xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
-
-	parameter = (uintptr_t)xfer | 0x0;
-	status = XHCI_TRB_2_IRQ_SET(0);
-	control = XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_EVENT_DATA) |
 	    XHCI_TRB_3_IOC_BIT;
 	xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
 
@@ -3619,7 +3731,8 @@ xhci_device_bulk_start(struct usbd_xfer 
 	    XHCI_TRB_2_TDSZ_SET(1) |
 	    XHCI_TRB_2_BYTES_SET(len);
 	control = XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_NORMAL) |
-	    XHCI_TRB_3_ISP_BIT | XHCI_TRB_3_IOC_BIT;
+	    (usbd_xfer_isread(xfer) ? XHCI_TRB_3_ISP_BIT : 0) |
+	    XHCI_TRB_3_IOC_BIT;
 	xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
 
 	mutex_enter(&tr->xr_lock);
@@ -3724,7 +3837,8 @@ xhci_device_intr_start(struct usbd_xfer 
 	    XHCI_TRB_2_TDSZ_SET(1) |
 	    XHCI_TRB_2_BYTES_SET(len);
 	control = XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_NORMAL) |
-	    XHCI_TRB_3_ISP_BIT | XHCI_TRB_3_IOC_BIT;
+	    (usbd_xfer_isread(xfer) ? XHCI_TRB_3_ISP_BIT : 0) |
+	    XHCI_TRB_3_IOC_BIT;
 	xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
 
 	mutex_enter(&tr->xr_lock);
@@ -3815,7 +3929,7 @@ xhci_timeout_task(void *addr)
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
 	mutex_enter(&sc->sc_lock);
-#if 0
+#if 1
 	xhci_abort_xfer(xfer, USBD_TIMEOUT);
 #else
 	xfer->ux_status = USBD_TIMEOUT;