Date: Sun, 31 Oct 2010 15:09:49 +0100 From: Hans Petter Selasky <hselasky@c2i.net> To: Weongyo Jeong <weongyo@freebsd.org> Cc: freebsd-usb@freebsd.org Subject: Re: [CFR] add usb_sleepout.[ch] Message-ID: <201010311509.49138.hselasky@c2i.net> In-Reply-To: <20101030231901.GA83161@weongyo> References: <20101030231901.GA83161@weongyo>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201010311509.49138.hselasky>