On 14/01/16(Thu) 22:14, Martin Pieuchot wrote:
> This should fix the problem various people reported when using urtwn(4)
> over xhci(4). This problem did not disappear but after iwm(4) got
> imported bugs@ reports stopped ;)
>
> It's a screw up in my initial understanding of the documentation. When
> a multiple-TRB transfer descriptor span the end of the "ring", the link
> TRB must include the Chain bit.
Updated diff that to not add the Chain bit for control transfers, this
should fix the issue jcs@ reported. More tests are welcome.
Index: xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.66
diff -u -p -r1.66 xhci.c
--- xhci.c 2 Dec 2015 09:23:23 -0000 1.66
+++ xhci.c 15 Jan 2016 10:05:53 -0000
@@ -91,10 +91,11 @@ int xhci_ring_alloc(struct xhci_softc *,
void xhci_ring_free(struct xhci_softc *, struct xhci_ring *);
void xhci_ring_reset(struct xhci_softc *, struct xhci_ring *);
struct xhci_trb *xhci_ring_consume(struct xhci_softc *, struct xhci_ring *);
-struct xhci_trb *xhci_ring_produce(struct xhci_softc *, struct xhci_ring *);
+struct xhci_trb *xhci_ring_produce(struct xhci_softc *, struct xhci_ring *,
+ int, int);
struct xhci_trb *xhci_xfer_get_trb(struct xhci_softc *, struct usbd_xfer*,
- uint8_t *, int);
+ uint8_t *, int, int);
void xhci_xfer_done(struct usbd_xfer *xfer);
/* xHCI command helpers. */
int xhci_command_submit(struct xhci_softc *, struct xhci_trb *, int);
@@ -1499,7 +1500,8 @@ xhci_ring_consume(struct xhci_softc *sc,
}
struct xhci_trb*
-xhci_ring_produce(struct xhci_softc *sc, struct xhci_ring *ring)
+xhci_ring_produce(struct xhci_softc *sc, struct xhci_ring *ring, int last,
+ int chain)
{
struct xhci_trb *trb = &ring->trbs[ring->index];
@@ -1517,6 +1519,15 @@ xhci_ring_produce(struct xhci_softc *sc,
bus_dmamap_sync(ring->dma.tag, ring->dma.map, TRBOFF(ring, lnk),
sizeof(struct xhci_trb), BUS_DMASYNC_POSTREAD);
+ /*
+ * Section 4.11.7 implies that the Chain bit should
+ * be added to the Link TRB if a TD is including it.
+ */
+ if (last && chain)
+ lnk->trb_flags |= htole32(XHCI_TRB_CHAIN);
+ else
+ lnk->trb_flags &= htole32(~XHCI_TRB_CHAIN);
+
lnk->trb_flags ^= htole32(XHCI_TRB_CYCLE);
bus_dmamap_sync(ring->dma.tag, ring->dma.map, TRBOFF(ring, lnk),
@@ -1531,7 +1542,7 @@ xhci_ring_produce(struct xhci_softc *sc,
struct xhci_trb *
xhci_xfer_get_trb(struct xhci_softc *sc, struct usbd_xfer *xfer,
- uint8_t *togglep, int last)
+ uint8_t *togglep, int last, int chain)
{
struct xhci_pipe *xp = (struct xhci_pipe *)xfer->pipe;
struct xhci_xfer *xx = (struct xhci_xfer *)xfer;
@@ -1546,7 +1557,7 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
xx->ntrb += 1;
*togglep = xp->ring.toggle;
- return (xhci_ring_produce(sc, &xp->ring));
+ return (xhci_ring_produce(sc, &xp->ring, last, chain));
}
int
@@ -1559,7 +1570,7 @@ xhci_command_submit(struct xhci_softc *s
trb0->trb_flags |= htole32(sc->sc_cmd_ring.toggle);
- trb = xhci_ring_produce(sc, &sc->sc_cmd_ring);
+ trb = xhci_ring_produce(sc, &sc->sc_cmd_ring, 1, 0);
if (trb == NULL)
return (EAGAIN);
memcpy(trb, trb0, sizeof(struct xhci_trb));
@@ -2462,11 +2473,11 @@ xhci_device_ctrl_start(struct usbd_xfer
return (USBD_NOMEM);
/* We'll do the setup TRB once we're finished with the other stages. */
- trb0 = xhci_xfer_get_trb(sc, xfer, &toggle0, 0);
+ trb0 = xhci_xfer_get_trb(sc, xfer, &toggle0, 0, 0);
/* Data TRB */
if (len != 0) {
- trb = xhci_xfer_get_trb(sc, xfer, &toggle, 0);
+ trb = xhci_xfer_get_trb(sc, xfer, &toggle, 0, 0);
flags = XHCI_TRB_TYPE_DATA | toggle;
if (usbd_xfer_isread(xfer))
@@ -2482,7 +2493,7 @@ xhci_device_ctrl_start(struct usbd_xfer
}
/* Status TRB */
- trb = xhci_xfer_get_trb(sc, xfer, &toggle, 1);
+ trb = xhci_xfer_get_trb(sc, xfer, &toggle, 1, 0);
flags = XHCI_TRB_TYPE_STATUS | XHCI_TRB_IOC | toggle;
if (len == 0 || !usbd_xfer_isread(xfer))
@@ -2581,7 +2592,7 @@ xhci_device_generic_start(struct usbd_xf
return (USBD_NOMEM);
/* We'll do the first TRB once we're finished with the chain. */
- trb0 = xhci_xfer_get_trb(sc, xfer, &toggle0, (ntrb == 1));
+ trb0 = xhci_xfer_get_trb(sc, xfer, &toggle0, (ntrb == 1), 1);
remain = xfer->length - len0;
paddr += len0;
@@ -2590,7 +2601,7 @@ xhci_device_generic_start(struct usbd_xf
/* Chain more TRBs if needed. */
for (i = ntrb - 1; i > 0; i--) {
/* Next (or Last) TRB. */
- trb = xhci_xfer_get_trb(sc, xfer, &toggle, (i == 1));
+ trb = xhci_xfer_get_trb(sc, xfer, &toggle, (i == 1), 1);
flags = XHCI_TRB_TYPE_NORMAL | toggle;
if (usbd_xfer_isread(xfer))
flags |= XHCI_TRB_ISP;
