PERFORCE change 131806 for review
Hans Petter Selasky
hselasky at FreeBSD.org
Thu Dec 27 13:57:37 PST 2007
http://perforce.freebsd.org/chv.cgi?CH=131806
Change 131806 by hselasky at hselasky_laptop001 on 2007/12/27 21:57:31
This commit fixes some problems with the recently
introduced DMA delay. Alfred Perlstein suggested that
I return an error code from the function
"usbd_transfer_stop()". The error code would be
something like EWOULDBLOCK, in case stopping the
USB transfer does not happen immediately.
I have not done that, hence it is very rarely you need
to know that. Instead I have introduced a new function
called "usbd_transfer_drain()" that will only return
when everything is safe and sound. This function also
requires a context unlike "usbd_transfer_stop()". Also
I've replaced all DELAY() in the critical path with
callouts. This introduce some extra complexity, but
I think that that is ok, given that it will save
some complexity in the USB device drivers and save
CPU cost.
After this commit it should always be safe to free
DMA'ed memory when the callback is called, regardless
of state.
With regard to function naming I've tried to mimic
the callout system:
callout_stop -> usbd_transfer_stop
callout_drain -> usbd_transfer_drain
Affected files ...
.. //depot/projects/usb/src/sys/dev/usb/ehci.c#65 edit
.. //depot/projects/usb/src/sys/dev/usb/ohci.c#54 edit
.. //depot/projects/usb/src/sys/dev/usb/uhci.c#55 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_subr.c#80 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_subr.h#88 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#93 edit
Differences ...
==== //depot/projects/usb/src/sys/dev/usb/ehci.c#65 (text+ko) ====
@@ -3851,13 +3851,13 @@
}
static void
-ehci_do_dma_delay(struct usbd_bus *bus)
+ehci_get_dma_delay(struct usbd_bus *bus, uint32_t *pus)
{
/*
* Wait until the hardware has finished any possible use of
* the transfer descriptor(s) and QH
*/
- DELAY(188);
+ *pus = (188); /* microseconds */
return;
}
@@ -3867,5 +3867,5 @@
.xfer_setup = ehci_xfer_setup,
.xfer_unsetup = ehci_xfer_unsetup,
.do_poll = ehci_do_poll,
- .do_dma_delay = ehci_do_dma_delay,
+ .get_dma_delay = ehci_get_dma_delay,
};
==== //depot/projects/usb/src/sys/dev/usb/ohci.c#54 (text+ko) ====
@@ -2808,13 +2808,13 @@
}
static void
-ohci_do_dma_delay(struct usbd_bus *bus)
+ohci_get_dma_delay(struct usbd_bus *bus, uint32_t *pus)
{
/*
* Wait until hardware has finished any possible use of the
* transfer descriptor(s) and QH
*/
- DELAY(1125);
+ *pus = (1125); /* microseconds */
return;
}
@@ -2824,5 +2824,5 @@
.xfer_setup = ohci_xfer_setup,
.xfer_unsetup = ohci_xfer_unsetup,
.do_poll = ohci_do_poll,
- .do_dma_delay = ohci_do_dma_delay,
+ .get_dma_delay = ohci_get_dma_delay,
};
==== //depot/projects/usb/src/sys/dev/usb/uhci.c#55 (text+ko) ====
@@ -3255,13 +3255,13 @@
}
static void
-uhci_do_dma_delay(struct usbd_bus *bus)
+uhci_get_dma_delay(struct usbd_bus *bus, uint32_t *pus)
{
/*
* Wait until hardware has finished any possible use of the
* transfer descriptor(s) and QH
*/
- DELAY(1125);
+ *pus = (1125); /* microseconds */
return;
}
@@ -3271,5 +3271,5 @@
.xfer_setup = uhci_xfer_setup,
.xfer_unsetup = uhci_xfer_unsetup,
.do_poll = uhci_do_poll,
- .do_dma_delay = uhci_do_dma_delay,
+ .get_dma_delay = uhci_get_dma_delay,
};
==== //depot/projects/usb/src/sys/dev/usb/usb_subr.c#80 (text+ko) ====
@@ -256,8 +256,6 @@
void
usbd_pause_mtx(struct mtx *mtx, uint32_t ms)
{
- int error;
-
if (cold) {
ms = (ms + 1) * 1024;
DELAY(ms);
@@ -265,11 +263,12 @@
} else {
ms = USBD_MS_TO_TICKS(ms);
+ ms++; /* be sure that we don't return too
+ * early */
- if (ms == 0) {
- ms = 1;
+ if (mtx_sleep(&ms, mtx, 0, "pause_mtx", ms)) {
+ /* should not happen */
}
- error = mtx_sleep(&error, mtx, 0, "pause_mtx", ms);
}
return;
}
==== //depot/projects/usb/src/sys/dev/usb/usb_subr.h#88 (text+ko) ====
@@ -167,8 +167,7 @@
void (*do_poll) (struct usbd_bus *);
void (*xfer_setup) (struct usbd_setup_params *parm);
void (*xfer_unsetup) (struct usbd_xfer *xfer);
- void (*do_dma_delay) (struct usbd_bus *); /* only used if
- * "bdma_enable" is set */
+ void (*get_dma_delay) (struct usbd_bus *, uint32_t *pdelay);
/* USB Device and Host mode - Optional */
@@ -403,7 +402,6 @@
uint8_t toggle_next:1; /* next data toggle value */
uint8_t is_stalled:1; /* set if pipe is stalled */
- uint8_t did_dma_delay:1; /* set if we waited for DMA */
uint8_t unused:5;
uint8_t iface_index; /* not used by "default pipe" */
};
@@ -517,6 +515,10 @@
uint8_t recursed_1:1;
uint8_t recursed_2:1;
uint8_t transferring:1;
+ uint8_t did_dma_delay:1; /* set if we waited for HW DMA */
+ uint8_t draining:1; /* set if we are draining an USB
+ * transfer */
+ uint8_t started:1; /* keeps track of started or stopped */
uint8_t bandwidth_reclaimed:1;
uint8_t control_xfr:1; /* set if control transfer */
uint8_t control_hdr:1; /* set if control header should be
@@ -533,7 +535,6 @@
uint8_t isochronous_xfr:1; /* set if isochronous transfer */
uint8_t usb_mode:1; /* shadow copy of "udev->usb_mode" */
uint8_t context:2; /* see USBD_CONTEXT_XXX */
- uint8_t needs_dma_delay:1; /* set if we need to wait for DMA */
uint8_t curr_dma_set:1; /* used by USB HC/DC driver */
};
@@ -654,11 +655,6 @@
uint8_t dma_error; /* set if virtual memory could not be
* loaded */
- uint8_t dma_draining; /* set if someone is waiting for a
- * BUS-DMA load operation to complete */
- uint8_t dma_no_callback; /* set if callback should not be
- * called */
-
uint8_t dma_tag_max;
uint8_t done_sleep; /* set if done thread is sleeping */
};
==== //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#93 (text+ko) ====
@@ -61,10 +61,9 @@
static void usbd_premature_callback(struct usbd_xfer *xfer, usbd_status_t error);
static void usbd_delayed_transfer_start(void *arg);
static void usbd_bdma_work_loop(struct usbd_memory_info *info);
-static void usbd_bdma_cancel_event(struct usbd_xfer *xfer);
static usbd_status_t usbd_handle_request(struct usbd_xfer *xfer);
static void usbd_callback_intr_td(void *arg);
-static void usbd_transfer_unsetup_sub(struct usbd_memory_info *info);
+static void usbd_transfer_unsetup_sub(struct usbd_memory_info *info, uint8_t need_delay);
static void usbd_callback_intr_sched(struct usbd_memory_info *info);
@@ -1022,7 +1021,7 @@
if (buf) {
if (info->setup_refcount == 0) {
/* something went wrong */
- usbd_transfer_unsetup_sub(info);
+ usbd_transfer_unsetup_sub(info, 0);
}
}
if (parm.err) {
@@ -1032,47 +1031,42 @@
}
/*------------------------------------------------------------------------*
- * usbd_dma_delay
+ * usbd_get_dma_delay
*
* The following function is called when we need to
* synchronize with DMA hardware.
+ *
+ * Returns:
+ * 0: no DMA delay required
+ * Else: milliseconds of DMA delay
*------------------------------------------------------------------------*/
-static void
-usbd_dma_delay(struct usbd_xfer *xfer)
+static uint32_t
+usbd_get_dma_delay(struct usbd_bus *bus)
{
- struct usbd_pipe *pipe;
- struct usbd_pipe *pipe_end;
+ uint32_t temp = 0;
- if (!xfer->flags_int.needs_dma_delay) {
- return;
+ if (bus->methods->get_dma_delay) {
+ (bus->methods->get_dma_delay)(bus, &temp);
+ /* round up and convert to milliseconds */
+ temp += 0x3FF;
+ temp /= 0x400;
}
- xfer->flags_int.needs_dma_delay = 0;
-
- if (xfer->pipe->did_dma_delay) {
- return;
- }
- (xfer->udev->bus->methods->do_dma_delay)
- (xfer->udev->bus);
-
- /*
- * Tell the other USB transfers that we did a delay to avoid
- * extra delays !
- */
- pipe = xfer->udev->pipes;
- pipe_end = xfer->udev->pipes_end;
-
- while (pipe != pipe_end) {
- pipe->did_dma_delay = 1;
- pipe++;
- }
- return;
+ return (temp);
}
static void
-usbd_transfer_unsetup_sub(struct usbd_memory_info *info)
+usbd_transfer_unsetup_sub(struct usbd_memory_info *info, uint8_t needs_delay)
{
struct usbd_page_cache *pc;
+ uint32_t temp;
+ /* wait for any outstanding DMA operations */
+
+ if (needs_delay) {
+ temp = usbd_get_dma_delay(info->bus);
+ usbd_pause_mtx(info->usb_mtx, temp);
+ }
+
/* wait for interrupt thread to exit */
while (info->done_thread) {
@@ -1200,17 +1194,8 @@
info->setup_refcount--;
if (info->setup_refcount == 0) {
- /*
- * We need to wait for any DMA
- * transactions to complete to be
- * absolutely sure that nobody will
- * write to freed memory !
- */
- if (needs_delay) {
- (info->bus->methods->do_dma_delay)
- (info->bus);
- }
- usbd_transfer_unsetup_sub(info);
+ usbd_transfer_unsetup_sub(info,
+ needs_delay);
} else {
mtx_unlock(info->usb_mtx);
}
@@ -1559,13 +1544,27 @@
/* sanity checks */
- if (!xfer->flags_int.open || xfer->flags_int.transferring) {
+ if (!xfer->flags_int.started || xfer->flags_int.transferring) {
+ PRINTFN(-1, ("sanity checks failed\n"));
return;
}
+
+ /* Only open the USB transfer once! */
+ if (!xfer->flags_int.open) {
+ xfer->flags_int.open = 1;
+
+ mtx_lock(xfer->usb_mtx);
+ (xfer->pipe->methods->open) (xfer);
+ mtx_unlock(xfer->usb_mtx);
+ }
+
/* set "transferring" and "recursed_2" flags */
xfer->flags_int.transferring = 1;
xfer->flags_int.recursed_2 = 1;
+ /* clear "did_dma_delay" flag */
+ xfer->flags_int.did_dma_delay = 0;
+
/* clear lengths and frame counts by default */
xfer->sumlen = 0;
xfer->actlen = 0;
@@ -1635,10 +1634,6 @@
if (xfer->flags_int.bdma_enable) {
- /* synchronize with DMA hardware */
-
- usbd_dma_delay(xfer);
-
/*
* If the transfer is not inserted, insert
* the transfer into the DMA queue
@@ -1650,13 +1645,6 @@
info = xfer->usb_root;
/*
- * If there is no current transfer, set one
- */
- if (info->dma_curr_xfer == NULL) {
- info->dma_curr_xfer = xfer;
- info->dma_no_callback = 1;
- }
- /*
* Only call the BUS-DMA work loop when it is not busy
*/
if (info->dma_refcount == 0) {
@@ -1697,33 +1685,6 @@
}
/*------------------------------------------------------------------------*
- * usbd_bdma_cancel_event
- *
- * This function will cancel any BUS-DMA operations.
- *------------------------------------------------------------------------*/
-static void
-usbd_bdma_cancel_event(struct usbd_xfer *xfer)
-{
- struct usbd_memory_info *info;
-
- info = xfer->usb_root;
-
- mtx_assert(info->priv_mtx, MA_OWNED);
-
- if (info->dma_curr_xfer == xfer) {
- /* prepare next USB transfer to load, if any */
- info->dma_curr_xfer =
- LIST_PREV(&(info->dma_head), xfer, dma_list);
- info->dma_no_callback = 1;
- }
- if (xfer->dma_list.le_prev) {
- LIST_REMOVE(xfer, dma_list);
- xfer->dma_list.le_prev = NULL;
- }
- return;
-}
-
-/*------------------------------------------------------------------------*
* usbd_bdma_work_loop
*
* This function handles loading of virtual buffers into DMA.
@@ -1739,29 +1700,28 @@
mtx_assert(info->priv_mtx, MA_OWNED);
- if (info->dma_draining) {
- /* someone is waiting for us to drain */
- info->dma_draining = 0;
- wakeup(&(info->dma_draining));
- }
-repeat:
+load_complete:
xfer = info->dma_curr_xfer;
-
if (xfer) {
-
- if (!info->dma_no_callback) {
-
/* prepare next USB transfer to load, if any */
info->dma_curr_xfer =
LIST_PREV(&(info->dma_head), xfer, dma_list);
- info->dma_no_callback = 1;
LIST_REMOVE(xfer, dma_list);
xfer->dma_list.le_prev = NULL;
/* check for DMA error */
- if (info->dma_error) {
+ if (!xfer->flags_int.open) {
+
+ /* we got cancelled */
+
+ info->dma_refcount++;
+ usbd_premature_callback(xfer,
+ USBD_CANCELLED);
+ info->dma_refcount--;
+
+ } else if (info->dma_error) {
/* prevent recursion by increasing refcount */
@@ -1774,27 +1734,35 @@
info->dma_refcount--;
- goto repeat;
- }
+ } else {
/* go ahead */
usbd_bdma_pre_sync(xfer);
/* finally start the hardware */
usbd_pipe_enter_wrapper(xfer);
- /* load next USB transfer, if any */
- goto repeat;
- }
+ }
} else {
+ /* get first USB transfer */
+ info->dma_curr_xfer =
+ LIST_FIRST(&(info->dma_head));
+ }
+
+ xfer = info->dma_curr_xfer;
+ if (xfer == NULL) {
/* nothing more to do */
return;
}
+ if (!xfer->flags_int.open) {
+ /* we got cancelled */
+ goto load_complete;
+ }
+
/* reset BUS-DMA load state */
info->dma_refcount = 1;
info->dma_error = 0;
- info->dma_no_callback = 0;
if (xfer->flags_int.isochronous_xfr) {
/* only one frame buffer */
@@ -1848,7 +1816,7 @@
if (--(info->dma_refcount) == 0) {
/* we are complete */
- goto repeat;
+ goto load_complete;
}
return;
}
@@ -1954,20 +1922,20 @@
}
mtx_assert(xfer->priv_mtx, MA_OWNED);
- if (!xfer->flags_int.open) {
- xfer->flags_int.open = 1;
+ /* mark the USB transfer started */
- /*
- * open transfer
- */
- mtx_lock(xfer->usb_mtx);
- (xfer->pipe->methods->open) (xfer);
- mtx_unlock(xfer->usb_mtx);
+ if (!xfer->flags_int.started) {
+ xfer->flags_int.started = 1;
}
+
+ /* check if the USB transfer callback is already transferring */
+
if (xfer->flags_int.transferring) {
return;
}
+
/* call callback */
+
usbd_callback_wrapper(xfer, NULL, USBD_CONTEXT_START);
return;
@@ -1978,6 +1946,8 @@
*
* NOTE: Calling this function more than one time will only
* result in a single transfer stop.
+ * NOTE: When this function returns it is not safe to free nor
+ * reuse any DMA buffers. See "usbd_transfer_drain()".
*------------------------------------------------------------------------*/
void
usbd_transfer_stop(struct usbd_xfer *xfer)
@@ -1988,26 +1958,27 @@
}
mtx_assert(xfer->priv_mtx, MA_OWNED);
+ /* check if the USB transfer was ever opened */
+
if (!xfer->flags_int.open) {
+ /* nothing to do except clearing the "started" flag */
+ xfer->flags_int.started = 0;
return;
}
/*
* close transfer
*/
mtx_lock(xfer->usb_mtx);
+ (xfer->pipe->methods->close) (xfer);
+ mtx_unlock(xfer->usb_mtx);
/*
- * NOTE: we need to exclude the
- * host controller driver from
- * reading "flags_int" when we
- * clear "flags_int.open" !
+ * Clear "open" and "started" after closing the USB transfer
+ * so that we don't get a race updating "flags_int" !
*/
xfer->flags_int.open = 0;
+ xfer->flags_int.started = 0;
- (xfer->pipe->methods->close) (xfer);
-
- mtx_unlock(xfer->usb_mtx);
-
#ifdef USB_DEBUG
/* check error value */
if (xfer->error != USBD_CANCELLED) {
@@ -2015,8 +1986,6 @@
usbd_errstr(xfer->error)));
}
#endif
- /* cancel loading of virtual buffers, if any */
- usbd_bdma_cancel_event(xfer);
/*
* Check if we are doing a transfer and if so
@@ -2036,14 +2005,13 @@
* usbd_transfer_drain
*
* This function will stop the USB transfer and wait for any
- * additional BUS-DMA operations to complete. Buffers that are loaded
- * into DMA can safely be freed after that this function has returned.
+ * additional BUS-DMA and HW-DMA operations to complete. Buffers that
+ * are loaded into DMA can safely be freed or reused after that this
+ * function has returned.
*------------------------------------------------------------------------*/
void
usbd_transfer_drain(struct usbd_xfer *xfer)
{
- struct usbd_memory_info *info;
-
WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL,
"usbd_transfer_drain can sleep!");
@@ -2058,19 +2026,13 @@
usbd_transfer_stop(xfer);
- info = xfer->usb_root;
-
- if (info->dma_refcount > 0) {
-
+ while (xfer->flags_int.transferring) {
+ xfer->flags_int.draining = 1;
/*
- * Wait until the current outstanding DMA
- * operation is complete before we return.
- * That way we are sure that no DMA operation
- * belonging to our USB transfer is pending!
+ * Wait until the current outstanding USB
+ * transfer is complete !
*/
- info->dma_draining = 1;
-
- if (mtx_sleep(&(info->dma_draining), xfer->priv_mtx,
+ if (mtx_sleep(&(xfer->flags_int), xfer->priv_mtx,
0, "usbdrain", 0)) {
/* should not happen */
}
@@ -2220,7 +2182,6 @@
goto repeat;
}
}
-
wakeup(&(info->done_thread));
info->done_thread = NULL;
mtx_unlock(info->usb_mtx);
@@ -2228,6 +2189,20 @@
return;
}
+static void
+usbd_dma_delay_done_cb(struct usbd_xfer *xfer)
+{
+ mtx_assert(xfer->usb_mtx, MA_OWNED);
+
+ PRINTFN(2, ("Completed %p\n", xfer));
+
+ /* queue callback for execution */
+ usbd_callback_wrapper(xfer, NULL, USBD_CONTEXT_CALLBACK);
+
+ mtx_unlock(xfer->usb_mtx);
+ return;
+}
+
/*------------------------------------------------------------------------*
* usbd_callback_wrapper
*
@@ -2245,7 +2220,7 @@
{
struct usbd_bus *bus;
uint32_t timeout;
- uint8_t temp;
+ uint32_t temp;
uint8_t dropped_usb_mtx = 0;
if (mtx_owned(xfer->priv_mtx)) {
@@ -2362,6 +2337,36 @@
/* check first recurse flag */
if (!xfer->flags_int.recursed_1) {
do {
+ /*
+ * If we have a non-hardware induced error we
+ * need to do the DMA delay!
+ */
+ if (xfer->flags_int.transferring &&
+ xfer->flags_int.bdma_enable &&
+ ((xfer->error == USBD_CANCELLED) ||
+ (xfer->error == USBD_TIMEOUT)) &&
+ (!xfer->flags_int.did_dma_delay)) {
+
+ /* only do this one time */
+ xfer->flags_int.did_dma_delay = 1;
+
+ temp = usbd_get_dma_delay(xfer->udev->bus);
+
+ PRINTFN(2, ("DMA delay, %u ms, "
+ "on %p\n", temp, xfer));
+
+ if (xfer->flags.use_polling) {
+ DELAY(temp * 1024);
+ } else {
+ mtx_lock(xfer->usb_mtx);
+ usb_callout_reset(&(xfer->timeout_handle),
+ USBD_MS_TO_TICKS(temp) + 1,
+ (void *)&usbd_dma_delay_done_cb, xfer);
+ mtx_unlock(xfer->usb_mtx);
+ break; /* wait for callback */
+ }
+ }
+
/* set both recurse flags */
xfer->flags_int.recursed_1 = 1;
xfer->flags_int.recursed_2 = 1;
@@ -2377,6 +2382,17 @@
xfer->flags_int.transferring = 0;
if (xfer->error) {
+ /*
+ * Check if we got started after that
+ * we got cancelled, but before we
+ * managed to deliver the
+ * USBD_CANCELLED message!
+ */
+ if ((xfer->error == USBD_CANCELLED) &&
+ (xfer->flags_int.started)) {
+ /* restart by doing a second loop */
+ xfer->flags_int.recursed_2 = 0;
+ }
xfer->usb_state = USBD_ST_ERROR;
goto callback;
}
@@ -2450,6 +2466,16 @@
/* check second recurse flag */
} while (!xfer->flags_int.recursed_2);
+ if (xfer->flags_int.draining &&
+ (!xfer->flags_int.transferring)) {
+ /*
+ * "usbd_transfer_drain()" is waiting for us
+ * to stop transferring.
+ */
+ xfer->flags_int.draining = 0;
+ wakeup(&(xfer->flags_int));
+ }
+
/* clear first recurse flag */
xfer->flags_int.recursed_1 = 0;
} else {
@@ -2695,19 +2721,6 @@
if (error) {
/* end of control transfer, if any */
xfer->flags_int.control_act = 0;
-
- if ((error == USBD_CANCELLED) ||
- (error == USBD_TIMEOUT)) {
- if (xfer->flags_int.bdma_enable &&
- xfer->flags_int.transferring) {
- /*
- * We need to wait for DMA
- * transactions to complete !
- */
- xfer->flags_int.needs_dma_delay = 1;
- xfer->pipe->did_dma_delay = 0;
- }
- }
} else {
/* check for short transfers */
if (xfer->actlen < xfer->sumlen) {
More information about the p4-projects
mailing list