From owner-p4-projects@FreeBSD.ORG Thu Mar 29 13:51:20 2007 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 66B6616A406; Thu, 29 Mar 2007 13:51:20 +0000 (UTC) X-Original-To: perforce@FreeBSD.org Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 2C33E16A404 for ; Thu, 29 Mar 2007 13:51:20 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [69.147.83.41]) by mx1.freebsd.org (Postfix) with ESMTP id 1CCAF13C45B for ; Thu, 29 Mar 2007 13:51:20 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.8/8.13.8) with ESMTP id l2TDpJQ5025574 for ; Thu, 29 Mar 2007 13:51:19 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.8/8.13.8/Submit) id l2TDpJpB025569 for perforce@freebsd.org; Thu, 29 Mar 2007 13:51:19 GMT (envelope-from hselasky@FreeBSD.org) Date: Thu, 29 Mar 2007 13:51:19 GMT Message-Id: <200703291351.l2TDpJpB025569@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 116832 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Mar 2007 13:51:20 -0000 http://perforce.freebsd.org/chv.cgi?CH=116832 Change 116832 by hselasky@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