From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yong Wang Subject: Re: [PATCH] eeepc-wmi: Add backlight support Date: Thu, 8 Apr 2010 06:50:05 +0800 Message-ID: <20100407225005.GA30445@ywang-moblin2.bj.intel.com> References: <20100407141750.GA16896@ywang-moblin2.bj.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mga06.intel.com ([134.134.136.21]:46220 "EHLO orsmga101.jf.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755476Ab0DGXAF (ORCPT ); Wed, 7 Apr 2010 19:00:05 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Corentin Chary Cc: Matthew Garrett , Dmitry Torokhov , Richard Purdie , platform-driver-x86@vger.kernel.org, linux-input@vger.kernel.org On Wed, Apr 07, 2010 at 05:05:41PM +0200, Corentin Chary wrote: > On Wed, Apr 7, 2010 at 4:17 PM, Yong Wang wrote: > > Add backlight support for WMI based Eee PC laptops. In addition, start > > to use a platform device to manage all functional devices as more > > features will be implemented later. > > > > +static struct platform_device *eeepc_wmi_platform_device; > > ?static struct input_dev *eeepc_wmi_input_dev; > > +struct backlight_device *eeepc_wmi_backlight_device; > > Instead of using static variables, you should really use a struct > eeepc_wmi, store stuff inside it, > and make all these functions reentrant. Alan did it for eeepc-laptop, > and I did it for asus-laptop, > see http://git.iksaif.net/?p=acpi4asus.git;a=commit;h=854c78363f37f03e30e2856ef17d7eefc62e0d06 > . > The driver would really be cleaner and easier to review with that. And > it would be more coherent with > eeepc-laptop's code. > Thanks for your review, Corentin. Could you please explain a bit more what you mean by "make all these functions reentrant"? Thanks -Yong