Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933070AbXJPGIW (ORCPT ); Tue, 16 Oct 2007 02:08:22 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1760993AbXJPGIJ (ORCPT ); Tue, 16 Oct 2007 02:08:09 -0400 Received: from fk-out-0910.google.com ([209.85.128.190]:34044 "EHLO fk-out-0910.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932644AbXJPGIG (ORCPT ); Tue, 16 Oct 2007 02:08:06 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=QnXfuLP9cSmcpEKSPXnScN4Qi0lFrEJ9ozjkBoNRfNeux/fEuWd5Ex6/uOMYVnF8YLR5zyMyeokmIE9cVqd3Wj2h5ukxO1+CsnYXtB8JtujahlKBONwgSFk78+I6ePlCH9LdseRzXbnhTXWwuY5HxLAhD0PnQHNpyacUYWhmkOI= Message-ID: <386072610710152308o3d4a04bfgfcecafc9c345286b@mail.gmail.com> Date: Tue, 16 Oct 2007 14:08:04 +0800 From: "Bryan Wu" To: "Ahmed S. Darwish" Subject: Re: [PATCH try #3] Input/Joystick Driver: add support AD7142 joystick driver Cc: "Dmitry Torokhov" , bryan.wu@analog.com, "Andrey Panin" , "Roel Kluin" <12o3l@tiscali.nl>, linux-input@atrey.karlin.mff.cuni.cz, linux-joystick@atrey.karlin.mff.cuni.cz, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, "Jean Delvare" In-Reply-To: <20071015182743.GA2529@Ahmed> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1192459625.6215.17.camel@roc-laptop> <20071015182743.GA2529@Ahmed> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2501 Lines: 62 On 10/16/07, Ahmed S. Darwish wrote: > On Mon, Oct 15, 2007 at 11:48:17AM -0400, Dmitry Torokhov wrote: > > Hi Bryan, > > > > On 10/15/07, Bryan Wu wrote: > > > + > > > +static int ad7142_thread(void *nothing) > > > +{ > > > + do { > > > + wait_for_completion(&ad7142_completion); > > > + ad7142_decode(); > > > + enable_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX); > > > + } while (!kthread_should_stop()); > > > + > > > > No, this is not going to work well: > > - you at least need to reinitialize the completion before enabling > > IRQ, otherwise you will spin in a very tight loop > > - if noone would touch the joystick ad7142_clsoe would() block > > infinitely because noone would signal the completion and > > ad7142_thread() would never stop. > > > > Completion is just not a good abstraction here... Please use work > > abstraction and possibly a separate workqueue. > > > > Bryan, I'm very interested in the technical advantage of using a completion > here. > You are welcome, I'd like to discuss these things here. > In my _not-experienced_ opinion, I remember completions was created mainly for > "create_task, wait till task got finished, go on" case. Why using it in a > different context while workqueues was created for a similar situation to > ad7142 one (non-irq context bottom-half) ? I like completion because it is simple to use and understand. Your understanding is right. But there is no limit for using different context with completion. completion is a wrapper of waitqueue+done flag. For some drivers, in process context call wait_for_completetion(), then schedule out and in irq handler call complete(). This is very simple and helpful for driver design (For example, you call dma function to transfer data, then you schedule out and then DMA IRQ handler will call complete() to wakeup you). But in this driver, a) can not call ad7142_decode() in IRQ handler, because it will sleep in IRQ context by calling some i2c API, b) in original design, creating a new kthread and using some waitqueue API is the same way as using workqueue, c) cannot use completion as Dmitry said. I am going to use workqueue here. Any idea? Thanks -Bryan Wu - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/