From owner-freebsd-usb@FreeBSD.ORG Sun Oct 31 15:11:41 2010 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5A6B71065672; Sun, 31 Oct 2010 15:11:41 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe05.swip.net [212.247.154.129]) by mx1.freebsd.org (Postfix) with ESMTP id 8650B8FC0C; Sun, 31 Oct 2010 15:11:40 +0000 (UTC) X-Cloudmark-Score: 0.000000 [] X-Cloudmark-Analysis: v=1.1 cv=5OBHFxb9I47YZ7HELXzI6cL6pwPTRnd5uxbD1DPQ4WY= c=1 sm=1 a=5o9ETn4JDi4A:10 a=8nJEP1OIZ-IA:10 a=CL8lFSKtTFcA:10 a=i9M/sDlu2rpZ9XS819oYzg==:17 a=I9t67j24wRIK5ayr5iIA:9 a=yF99CFNFJJJYSbHfWScrEzui1W0A:4 a=wPNLvfGTeEIA:10 a=i9M/sDlu2rpZ9XS819oYzg==:117 Received: from [188.126.198.129] (account mc467741@c2i.net HELO laptop002.hselasky.homeunix.org) by mailfe05.swip.net (CommuniGate Pro SMTP 5.2.19) with ESMTPA id 42174207; Sun, 31 Oct 2010 15:08:37 +0100 From: Hans Petter Selasky To: Weongyo Jeong Date: Sun, 31 Oct 2010 15:09:49 +0100 User-Agent: KMail/1.13.5 (FreeBSD/8.1-STABLE; KDE/4.4.5; amd64; ; ) References: <20101030231901.GA83161@weongyo> In-Reply-To: <20101030231901.GA83161@weongyo> X-Face: +~\`s("[*|O,="7?X@L.elg*F"OA\I/3%^p8g?ab%RN'(; _IjlA: hGE..Ew, XAQ*o#\/M~SC=S1-f9{EzRfT'|Hhll5Q]ha5Bt-s|oTlKMusi:1e[wJl}kd}GR Z0adGx-x_0zGbZj'e(Y[(UNle~)8CQWXW@:DX+9)_YlB[tIccCPN$7/L' MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201010311509.49138.hselasky@c2i.net> Cc: freebsd-usb@freebsd.org Subject: Re: [CFR] add usb_sleepout.[ch] X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 31 Oct 2010 15:11:41 -0000 On Sunday 31 October 2010 01:19:01 Weongyo Jeong wrote: > Hello USB guys, > > The following patch is to add a implementation, called `sleepout'. > Please reviews. I'd like to commit it into HEAD if no objections. > > Adds `sleepout' prototype which is a comic combination of callout(9) and > taskqueue(8) only for USB drivers to implement one step timer. In > current USB drivers using callout(9) interface they all have two step > execution flow as follows: > > 1. callout callback is fired by the interrupt context. Then it needs > to pass it to USB process context because it could sleep(!) while > callout(9) don't allow it. > 2. In the USB process context it operates USB commands that most of > times it'd be blocked at least 125 us (it'd be always true for USB) > > In a view of driver developer it'd be more convenient if USB stack has a > feature like this (timer supporting blocking). > Hi, I think this is a good idea too. However, there are some atomic issues I think you need to fix first. 1) All the sleepout_xxx() functions need mutex asserts. 2) Is it allowed to call callout_stop() / callout_reset() unlocked at all? What are the concequences if the mutex associated with the sleepout is NULL ? 3) As per the current implementation it can happen that the task'ed timeout is called after that sleepout_stop() is used. The code should have a check in the task function to see if the sleepout() has been cancelled or not, when the mutex associated with the sleepout is locked. 4) Should the functions be prefixed by usb_ ? 5) In drain you should drain the callout first, then drain the tasqueue. Else the timer can trigger after the taskqueue is drained. --HPS