PERFORCE change 116832 for review

[Hans Petter Selasky]

http://perforce.freebsd.org/chv.cgi?CH=116832

Change 116832 by hselasky at hselasky_mini_itx on 2007/03/29 13:50:38

	Add more documentation to the config thread system.
	Make "usbd_config_td_stop()" lock the mutex it requires.
	Check if the config thread is gone in "usbd_config_td_queue_command()"
	before queueing a command.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/usb_subr.c#30 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/usb_subr.c#30 (text+ko) ====

@@ -2412,6 +2412,52 @@
 	        break;
 	    }
 
+	    /* NOTE to reimplementors: dequeueing a command from the
+	     * "used" queue and executing it must be atomic, with
+	     * regard to the "p_mtx" mutex. That means any attempt to
+	     * queue a command by another thread must be blocked until
+	     * either:
+	     *
+	     * 1) the command sleeps
+	     *
+	     * 2) the command returns
+	     *
+	     * Here is a practical example that shows how this
+	     * helps solving a problem:
+	     *
+	     * Assume that you want to set the baud rate on a USB
+	     * serial device. During the programming of the device you
+	     * don't want to receive nor transmit any data, because it
+	     * will be garbage most likely anyway. The programming of
+	     * our USB device takes 20 milliseconds and it needs to
+	     * call functions that sleep.
+	     *
+	     * Non-working solution: Before we queue the programming command,
+	     * we stop transmission and reception of data. Then we
+	     * queue a programming command. At the end of the programming
+	     * command we enable transmission and reception of data.
+	     *
+	     * Problem: If a second programming command is queued
+	     * while the first one is sleeping, we end up enabling
+	     * transmission and reception of data too early.
+	     *
+	     * Working solution: Before we queue the programming
+	     * command, we stop transmission and reception of
+	     * data. Then we queue a programming command. Then we
+	     * queue a second command that only enables transmission
+	     * and reception of data.
+	     *
+	     * Why it works: If a second programming command is queued
+	     * while the first one is sleeping, then the queueing of a
+	     * second command to enable the data transfers, will cause
+	     * the previous one, which is still on the queue, to be
+	     * removed from the queue, and re-inserted after the last
+	     * baud rate programming command, which then gives the
+	     * desired result.
+	     *
+	     * This example assumes that you use a "qcount" of zero.
+	     */
+
 	    USBD_IF_DEQUEUE(&(ctd->cmd_used), m);
 
 	    if (m) {
@@ -2519,24 +2565,37 @@
 void
 usbd_config_td_stop(struct usbd_config_td *ctd)
 {
-	register int error;
+	uint32_t level;
+	int error;
 
-	while (ctd->config_thread) {
+	if (ctd->p_mtx) {
 
-	    mtx_assert(ctd->p_mtx, MA_OWNED);
+	  mtx_lock(ctd->p_mtx);
 
-	    ctd->flag_config_td_gone = 1;
+	  while (ctd->config_thread) {
 
 	    usbd_config_td_queue_command(ctd, NULL, &usbd_config_td_dummy_cmd, 
 					 0, 0);
 
+	    /* set the gone flag after queueing the
+	     * last command:
+	     */
+	    ctd->flag_config_td_gone = 1;
+
 	    if (cold) {
 	        panic("%s:%d: cannot stop config thread!\n",
 		      __FUNCTION__, __LINE__);
 	    }
 
+	    level = mtx_drop_recurse(ctd->p_mtx);
+
 	    error = msleep(&(ctd->wakeup_config_td_gone), 
 			   ctd->p_mtx, 0, "wait config TD", 0);
+
+	    mtx_pickup_recurse(ctd->p_mtx, level);
+	  }
+
+	  mtx_unlock(ctd->p_mtx);
 	}
 	return;
 }
@@ -2550,11 +2609,8 @@
 void
 usbd_config_td_unsetup(struct usbd_config_td *ctd)
 {
-	if (ctd->p_mtx) {
-	    mtx_lock(ctd->p_mtx);
-	    usbd_config_td_stop(ctd);
-	    mtx_unlock(ctd->p_mtx);
-	}
+	usbd_config_td_stop(ctd);
+
 	if (ctd->p_cmd_queue) {
 	    free(ctd->p_cmd_queue, M_DEVBUF);
 	    ctd->p_cmd_queue = NULL;
@@ -2576,7 +2632,10 @@
 	struct usbd_mbuf *m;
 	int32_t qlen;
 
-	mtx_assert(ctd->p_mtx, MA_OWNED);
+	if (usbd_config_td_is_gone(ctd)) {
+	    /* nothing more to do */
+	    return;
+	}
 
 	/*
 	 * first check if the command was