On Wed, 23 Dec 2020 09:47:44 +0100
Marcus Glocker <mar...@nazgul.ch> wrote:
> On Tue, 22 Dec 2020 20:55:41 +0100
> Marcus Glocker <mar...@nazgul.ch> wrote:
>
> > > > Did you consider incrementing xx->ntrb instead?
> >
> > >That doesn't work either, because the status completion code needs
> > >xx->ntrb to be correct for the data TD to be handled correctly.
> > >Incrementing xx->ntrb means the number of TRBs for the data TD is
> > >incorrect, since it includes the (optional) zero TD's TRB.
> > >
> > >In this case the zero TD allocates a TRB but doesn't do proper
> > >accounting, and currently there's no place where this could be
> > >accounted properly.
> > >
> > >In the end it's all software, so I guess the diff will simply have
> > >to be bigger than just a one-liner.
> >
> > > > With the diff below the produced TRB isn't accounted which might
> > > > lead
> > > > to and off-by-one.
> >
> > Sorry, I missed this thread and had to re-grab the last mail from
> > MARC.
> >
> > Can't we just take account of the zero trb separately?
>
> We also want to reset the zerotrb.
Re-thinking this again I think we should only increase the zerotrb to
avoid again a possible miss match for free_trbs, and leave the
responsibility to the caller of xhci_xfer_get_trb() to request the
right amount of zerotrb.
Index: dev/usb/xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.119
diff -u -p -u -p -r1.119 xhci.c
--- dev/usb/xhci.c 31 Jul 2020 19:27:57 -0000 1.119
+++ dev/usb/xhci.c 23 Dec 2020 09:38:58 -0000
@@ -1135,8 +1135,10 @@ xhci_xfer_done(struct usbd_xfer *xfer)
i = (xp->ring.ntrb - 1);
}
xp->free_trbs += xx->ntrb;
+ xp->free_trbs += xx->zerotrb;
xx->index = -1;
xx->ntrb = 0;
+ xx->zerotrb = 0;
timeout_del(&xfer->timeout_handle);
usb_rem_task(xfer->device, &xfer->abort_task);
@@ -1842,6 +1844,7 @@ xhci_xfer_get_trb(struct xhci_softc *sc,
switch (last) {
case -1: /* This will be a zero-length TD. */
xp->pending_xfers[xp->ring.index] = NULL;
+ xx->zerotrb += 1;
break;
case 0: /* This will be in a chain. */
xp->pending_xfers[xp->ring.index] = xfer;
Index: dev/usb/xhcivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhcivar.h,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 xhcivar.h
--- dev/usb/xhcivar.h 6 Oct 2019 17:30:00 -0000 1.11
+++ dev/usb/xhcivar.h 23 Dec 2020 09:38:58 -0000
@@ -40,6 +40,7 @@ struct xhci_xfer {
struct usbd_xfer xfer;
int index; /* Index of the last TRB */
size_t ntrb; /* Number of associated TRBs */
+ size_t zerotrb; /* Is zero len TRB required? */
};
struct xhci_ring {
