On Sun, Aug 30, 2015 at 12:32:24PM +0200, Martin Pieuchot stated:
> That's good but the original design of allocating an xfer for every
> usbd_transfer(9) call does not make much sense.
>
> You could allocate two xfers (one for read and one for write) at the
> same time the pipes are opened and free them when they're closed.
>
> You could even allocate the DMA buffer with usb_alloc_buffer() and pass
> the USBD_NO_COPY flag to usbd_transfer(9) to avoid a supplementary copy.
Thanks for the feedback, Martin. I've been running with your first
suggestion for a bit with no issues. I wonder though if the xfer can't
just be shared for rx and tx? I also NULLed out the pipes on failure
since it looks like otherwise they'd be re-closed on detach. I'll look
into the usbd_alloc_buffer() idea.
Index: uow.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uow.c,v
retrieving revision 1.33
diff -u -p -s -r1.33 uow.c
--- uow.c 15 Apr 2013 09:23:02 -0000 1.33
+++ uow.c 30 Aug 2015 12:49:12 -0000
@@ -49,7 +49,8 @@ struct uow_softc {
struct usbd_pipe *sc_ph_obulk;
struct usbd_pipe *sc_ph_intr;
u_int8_t sc_regs[DS2490_NREGS];
- struct usbd_xfer *sc_xfer;
+ struct usbd_xfer *sc_xfer_in;
+ struct usbd_xfer *sc_xfer_out;
u_int8_t sc_fifo[DS2490_DATAFIFOSIZE];
};
@@ -186,14 +187,17 @@ uow_attach(struct device *parent, struct
goto fail;
}
-#if 0
- /* Allocate xfer for bulk transfers */
- if ((sc->sc_xfer = usbd_alloc_xfer(sc->sc_udev)) == NULL) {
- printf("%s: failed to alloc bulk xfer\n",
+ /* Allocate xfers for bulk transfers */
+ if ((sc->sc_xfer_in = usbd_alloc_xfer(sc->sc_udev)) == NULL) {
+ printf("%s: failed to alloc bulk-in xfer\n",
+ sc->sc_dev.dv_xname);
+ goto fail;
+ }
+ if ((sc->sc_xfer_out = usbd_alloc_xfer(sc->sc_udev)) == NULL) {
+ printf("%s: failed to alloc bulk-out xfer\n",
sc->sc_dev.dv_xname);
goto fail;
}
-#endif
memset(sc->sc_fifo, 0xff, sizeof(sc->sc_fifo));
@@ -220,14 +224,26 @@ uow_attach(struct device *parent, struct
return;
fail:
- if (sc->sc_ph_ibulk != NULL)
+ if (sc->sc_ph_ibulk != NULL) {
usbd_close_pipe(sc->sc_ph_ibulk);
- if (sc->sc_ph_obulk != NULL)
+ sc->sc_ph_ibulk = NULL;
+ }
+ if (sc->sc_ph_obulk != NULL) {
usbd_close_pipe(sc->sc_ph_obulk);
- if (sc->sc_ph_intr != NULL)
+ sc->sc_ph_obulk = NULL;
+ }
+ if (sc->sc_ph_intr != NULL) {
usbd_close_pipe(sc->sc_ph_intr);
- if (sc->sc_xfer != NULL)
- usbd_free_xfer(sc->sc_xfer);
+ sc->sc_ph_intr = NULL;
+ }
+ if (sc->sc_xfer_in != NULL) {
+ usbd_free_xfer(sc->sc_xfer_in);
+ sc->sc_xfer_in = NULL;
+ }
+ if (sc->sc_xfer_out != NULL) {
+ usbd_free_xfer(sc->sc_xfer_out);
+ sc->sc_xfer_out = NULL;
+ }
}
int
@@ -251,8 +267,10 @@ uow_detach(struct device *self, int flag
usbd_close_pipe(sc->sc_ph_intr);
}
- if (sc->sc_xfer != NULL)
- usbd_free_xfer(sc->sc_xfer);
+ if (sc->sc_xfer_in != NULL)
+ usbd_free_xfer(sc->sc_xfer_in);
+ if (sc->sc_xfer_out != NULL)
+ usbd_free_xfer(sc->sc_xfer_out);
if (sc->sc_ow_dev != NULL)
rv = config_detach(sc->sc_ow_dev, flags);
@@ -457,14 +475,9 @@ uow_read(struct uow_softc *sc, void *buf
return (-1);
}
- if ((sc->sc_xfer = usbd_alloc_xfer(sc->sc_udev)) == NULL) {
- printf("%s: failed to alloc xfer\n", sc->sc_dev.dv_xname);
- return (-1);
- }
- usbd_setup_xfer(sc->sc_xfer, sc->sc_ph_ibulk, sc, buf, len,
+ usbd_setup_xfer(sc->sc_xfer_in, sc->sc_ph_ibulk, sc, buf, len,
USBD_SHORT_XFER_OK | USBD_SYNCHRONOUS, UOW_TIMEOUT, NULL);
- error = usbd_transfer(sc->sc_xfer);
- usbd_free_xfer(sc->sc_xfer);
+ error = usbd_transfer(sc->sc_xfer_in);
if (error != 0) {
printf("%s: read failed, len %d: %s\n",
sc->sc_dev.dv_xname, len, usbd_errstr(error));
@@ -472,7 +485,7 @@ uow_read(struct uow_softc *sc, void *buf
return (-1);
}
- usbd_get_xfer_status(sc->sc_xfer, NULL, NULL, &count, &error);
+ usbd_get_xfer_status(sc->sc_xfer_in, NULL, NULL, &count, &error);
return (count);
}
@@ -488,14 +501,9 @@ uow_write(struct uow_softc *sc, const vo
return (1);
}
- if ((sc->sc_xfer = usbd_alloc_xfer(sc->sc_udev)) == NULL) {
- printf("%s: failed to alloc xfer\n", sc->sc_dev.dv_xname);
- return (-1);
- }
- usbd_setup_xfer(sc->sc_xfer, sc->sc_ph_obulk, sc, (void *)buf, len,
- USBD_SYNCHRONOUS, UOW_TIMEOUT, NULL);
- error = usbd_transfer(sc->sc_xfer);
- usbd_free_xfer(sc->sc_xfer);
+ usbd_setup_xfer(sc->sc_xfer_out, sc->sc_ph_obulk, sc, (void *)buf,
+ len, USBD_SYNCHRONOUS, UOW_TIMEOUT, NULL);
+ error = usbd_transfer(sc->sc_xfer_out);
if (error != 0) {
printf("%s: write failed, len %d: %s\n",
sc->sc_dev.dv_xname, len, usbd_errstr(error));
