>-----Original Message----- >From: Song, Barry >Sent: Wednesday, September 09, 2009 11:00 AM >To: 'David Brownell'; 'dmitry.torokhov@xxxxxxxxx' >Cc: Barry Song; uclinux-dist-devel@xxxxxxxxxxxxxxxxxxxx; >linux-input@xxxxxxxxxxxxxxx >Subject: RE: [Uclinux-dist-devel] [PATCH v2]add analog devices >AD714Xcaptouch input driver > > > >>-----Original Message----- >>From: David Brownell [mailto:david-b@xxxxxxxxxxx] >>Sent: Wednesday, September 09, 2009 10:30 AM >>To: Song, Barry >>Cc: Barry Song; uclinux-dist-devel@xxxxxxxxxxxxxxxxxxxx; >>linux-input@xxxxxxxxxxxxxxx >>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 <linux/i2c.h> and likewise >>in <linux/spi/spi.h>). 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: 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); >> >> >>> #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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
