>-----Original Message----- >From: uclinux-dist-devel-bounces@xxxxxxxxxxxxxxxxxxxx >[mailto:uclinux-dist-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On >Behalf Of David Brownell >Sent: Wednesday, September 09, 2009 2:49 AM >To: Barry Song >Cc: 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, Barry Song wrote: >> +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); >> + if (ret) >> + goto err; >> + ret = i2c_add_driver(&ad714x_i2c_driver); >> + if (ret) >> + spi_unregister_driver(&ad714x_spi_driver); >> +err: >> + return ret; >> +#endif >> +} > >Ugly and error prone. >You should have only one block of code not four ... > >Absolutely the cleanest solution adds inlined NOP stubs >for I2C and SPI driver register/unregister calls, and >gets rid of all that #ifdeffery; there are other solutions. > The aim is that the module can work at: 1. only spi bus enabled (probe spi devices) 2. only i2c bus enabled (probe i2c devices) 3. both spi and i2c bus enabled. (probe both i2c/spi devices) So the module depends on SPI || I2C not SPI && I2C. I used the #ifdeffery to avoid compiling error due to symbols losing of I2C or SPI while kernel configuration doesn't enable both SPI and I2C. If kernel only enables one of the two buses, the driver only probes the enabled bus. 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 = { .driver = { .name = "ad714x_captouch", }, .probe = ad714x_i2c_probe, .remove = __devexit_p(ad714x_i2c_remove), .suspend = ad714x_i2c_suspend, .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); } #else static inline int ad714x_i2c_add_driver(struct i2c_driver *i2c_drv) { return 0; } #define ad714x_i2c_driver NULL #endif static int __init ad714x_init(void) { ... ad714x_i2c_add_driver(&ad714x_i2c_driver); ... } 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? > > >_______________________________________________ >Uclinux-dist-devel mailing list >Uclinux-dist-devel@xxxxxxxxxxxxxxxxxxxx >https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel > -- 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
