From patchwork Wed Sep 9 03:33:44 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Song, Barry" X-Patchwork-Id: 46317 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n893Yp5V022175 for ; Wed, 9 Sep 2009 03:34:52 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752812AbZIIDeI (ORCPT ); Tue, 8 Sep 2009 23:34:08 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752508AbZIIDeI (ORCPT ); Tue, 8 Sep 2009 23:34:08 -0400 Received: from nwd2mail10.analog.com ([137.71.25.55]:46074 "EHLO nwd2mail10.analog.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752812AbZIIDeH convert rfc822-to-8bit (ORCPT ); Tue, 8 Sep 2009 23:34:07 -0400 To: "Song, Barry" , "David Brownell" , From: "Song, Barry" X-IronPort-AV: E=Sophos;i="4.44,356,1249272000"; d="scan'208";a="4093070" Received: from nwd2hubcas2.ad.analog.com ([10.64.73.30]) by nwd2mail10.analog.com with ESMTP; 08 Sep 2009 23:34:09 -0400 Received: from nwd2exm5.ad.analog.com (10.64.51.20) by NWD2HUBCAS2.ad.analog.com (10.64.73.30) with Microsoft SMTP Server id 8.1.358.0; Tue, 8 Sep 2009 23:34:09 -0400 Received: from nwd2exm20.ad.analog.com ([10.64.73.20]) by nwd2exm5.ad.analog.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 8 Sep 2009 23:34:09 -0400 Received: from chinexm1.ad.analog.com ([10.99.27.42]) by nwd2exm20.ad.analog.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 8 Sep 2009 23:33:48 -0400 X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-Class: urn:content-classes:message MIME-Version: 1.0 Subject: RE: [Uclinux-dist-devel] [PATCH v2]add analog devices AD714Xcaptouch input driver Date: Wed, 9 Sep 2009 11:33:44 +0800 Message-ID: <0F1B54C89D5F954D8535DB252AF412FA04B118A5@chinexm1.ad.analog.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [Uclinux-dist-devel] [PATCH v2]add analog devices AD714Xcaptouch input driver Thread-Index: Acow9W+ZiEOCTAnUTDiWv0gDGhjN2QABCEBQAAA7j1A= References: <1252401564-5291-1-git-send-email-21cnbao@gmail.com> <200909081149.28942.david-b@pacbell.net> <0F1B54C89D5F954D8535DB252AF412FA04B11725@chinexm1.ad.analog.com> <200909081929.57935.david-b@pacbell.net> CC: "Barry Song" <21cnbao@gmail.com>, , X-OriginalArrivalTime: 09 Sep 2009 03:33:48.0424 (UTC) FILETIME=[57ACA880:01CA30FE] Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org >-----Original Message----- >From: Song, Barry >Sent: Wednesday, September 09, 2009 11:00 AM >To: 'David Brownell'; 'dmitry.torokhov@gmail.com' >Cc: Barry Song; uclinux-dist-devel@blackfin.uclinux.org; >linux-input@vger.kernel.org >Subject: RE: [Uclinux-dist-devel] [PATCH v2]add analog devices >AD714Xcaptouch input driver > > > >>-----Original Message----- >>From: David Brownell [mailto:david-b@pacbell.net] >>Sent: Wednesday, September 09, 2009 10:30 AM >>To: Song, Barry >>Cc: Barry Song; uclinux-dist-devel@blackfin.uclinux.org; >>linux-input@vger.kernel.org >>Subject: Re: [Uclinux-dist-devel] [PATCH v2]add analog devices >>AD714Xcaptouch input driver >> >>On Tuesday 08 September 2009, Song, Barry wrote: >>> I don't know what the exact meaning of "inlined NOP stubs", >>do you mean things like the following? >>> #if (defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)) >>> ... >>> static struct i2c_driver ad714x_i2c_driver = { >>>         .. >>> }; >> >>You _should_ be able to always declare that and let GCC >>optimize it away ... Even though I can always declare it, the functions in ad714x_i2c_driver still access i2c symbols. For that, I still need to use #ifdef to make the compiling pass. >> >> >>> static inline int ad714x_i2c_add_driver(struct i2c_driver *i2c_drv) >>> { >>>         return i2c_add_driver(i2c_drv); >>> } >>> #else >>> static inline int ad714x_i2c_add_driver(struct i2c_driver *i2c_drv) >>> { >>>         return 0; >>> } >> >>That'd be OK as a quick fix; ideally i2c_add_driver() itself >>would be what the #ifdef covers (in and likewise >>in ). And you'd need to cover driver remove >>paths too. >> >> >>> #define ad714x_i2c_driver NULL >> >>Ditto: shouldn't need to #define that Even though GCC can optimize unused variants/functions, there are still compiling error since field functions of ad714x_i2c_driver/ad714x_spi_driver and other functions will access other spi/i2c symbols. So maybe the best fix is: >> >> >>> #endif >>> >>> static int __init ad714x_init(void) >>> { >>>         ... >>>         ad714x_i2c_add_driver(&ad714x_i2c_driver); >> >>Yes; but do check the value! >> >>>         ... >>> } >>> >>> Then ad714x_init can use a unified ad714x_i2c_add_driver, >>> ad714x_spi_register_driver to avoid #ifdeffery used? >>> It seems that doesn't make the codes more pretty? >> >>Way less #ifdeffery and *ONE* non-#ifdeffed chunk of >>code in driver init/exit code. >Fixed >> >>- Dave >> >> >> > --- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: ad714x.c =================================================================== --- ad714x.c (revision 7274) +++ ad714x.c (working copy) @@ -1478,9 +1478,22 @@ .suspend = ad714x_spi_suspend, .resume = ad714x_spi_resume, }; + +static inline int ad714x_spi_register_driver(struct spi_driver *spi_drv) +{ + return spi_register_driver(spi_drv); +} + +static inline void ad714x_spi_unregister_driver(struct spi_driver *spi_drv) +{ + spi_unregister_driver(spi_drv); +} + +#else +#define ad714x_spi_register_driver(p) 0 +#define ad714x_spi_unregister_driver(p) #endif - #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) static int ad714x_i2c_write(struct device *dev, unsigned short reg, unsigned short data) @@ -1595,43 +1608,40 @@ .resume = ad714x_i2c_resume, .id_table = ad714x_id, }; + +static inline int ad714x_i2c_add_driver(struct i2c_driver *i2c_drv) +{ + return i2c_add_driver(i2c_drv); +} + +static inline void ad714x_i2c_del_driver(struct i2c_driver *i2c_drv) +{ + i2c_del_driver(i2c_drv); +} + +#else +#define ad714x_i2c_add_driver(p) 0 +#define ad714x_i2c_del_driver(p) #endif static int __init ad714x_init(void) { -#if (defined(CONFIG_SPI) || defined(CONFIG_SPI_MODULE)) && \ - !(defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)) - return spi_register_driver(&ad714x_spi_driver); -#endif -#if (defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)) && \ - !(defined(CONFIG_SPI) || defined(CONFIG_SPI_MODULE)) - return i2c_add_driver(&ad714x_i2c_driver); -#endif - -#if (defined(CONFIG_SPI) || defined(CONFIG_SPI_MODULE)) && \ - (defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)) int ret = 0; - ret = spi_register_driver(&ad714x_spi_driver); + ret = ad714x_spi_register_driver(&ad714x_spi_driver); if (ret) goto err; - ret = i2c_add_driver(&ad714x_i2c_driver); + ret = ad714x_i2c_add_driver(&ad714x_i2c_driver); if (ret) - spi_unregister_driver(&ad714x_spi_driver); + ad714x_spi_unregister_driver(&ad714x_spi_driver); err: return ret; -#endif } static void __exit ad714x_exit(void) { -#if defined(CONFIG_SPI) || defined(CONFIG_SPI_MODULE) - spi_unregister_driver(&ad714x_spi_driver); -#endif - -#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE) - i2c_del_driver(&ad714x_i2c_driver); -#endif + ad714x_spi_unregister_driver(&ad714x_spi_driver); + ad714x_i2c_del_driver(&ad714x_i2c_driver); } module_init(ad714x_init);