-----Original Message-----
From: uclinux-dist-devel-bounces@blackfin.uclinux.org 
[mailto:uclinux-dist-devel-bounces@blackfin.uclinux.org] On 
Behalf Of David Brownell
Sent: Wednesday, September 09, 2009 2:49 AM
To: Barry Song
Cc: 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, Barry Song wrote:

quoted

+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@blackfin.uclinux.org
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html