Re: [PATCH v3] add analog devices AD714X captouchinput driver

From: Barry Song <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Dmitry Torokhov
	<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org,
	"Hennerich,
	Michael"
	<Michael.Hennerich-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v3] add analog devices AD714X captouchinput driver
Date: Sat, 10 Oct 2009 10:24:40 +0800	[thread overview]
Message-ID: <3c17e3570910091924p68a248e6q3dd2640acc17c90d@mail.gmail.com> (raw)
In-Reply-To: <20091009162219.GB1092-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>

On Sat, Oct 10, 2009 at 12:22 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Oct 09, 2009 at 06:11:45AM -0400, Mike Frysinger wrote:
>> On Fri, Oct 9, 2009 at 05:18, Mike Frysinger <vapier.adi@gmail.com> wrote:
>> > we could have three modules here: ad714x, ad714x-i2c, ad714x-spi
>>
>> are you ok with this approach Dmitry ?  this way people can
>> load/unload a specific bus without affecting the core or the other
>> bus, and errors with loading one bus type wouldnt affect loading of
>> the other.
>
> Yes, I think this is the most clean way.
That will decrease the cohesion and increase coupling. It's tedious
for one driver with single feature to have three modules. To fulfill
that, ad714x need to expose data structures and export symbols to
ad714x-i2c and ad714x-spi. But those data structures and symbols are
more like local stuff. And I2C and SPI are just bottom level
communication ways, they should be the bottom level of ad714x, but
after splitting to 3 modules, it seems they become the top level of
ad714x.
How about we still have one module, but let users to select which bus
should be probed according to their boards.

config INPUT_AD714X
        tristate "Analog Devices AD7142/7147 Capacitance Touch Sensor"
        help
          Say Y here if you want to support an AD7142/7147 touch sensor.

          To compile this driver as a module, choose M here: the
          module will be called ad714x.

if INPUT_AD714X

config AD714X_SCAN_SPI
        bool "scan AD7142/7147 devices on SPI bus"
        default y
        select SPI
        ---help---
          Say Y here if you have AD7142/AD7147 hooked on SPI bus.
          If unsure, say N.

config AD714X_SCAN_I2C
        bool "scan AD7142/7147 devices on I2C bus"
        default y
        select I2C
        ---help---
          Say Y here if you have AD7142/AD7147 hooked on I2C bus.
          If unsure, say N.

endif

While AD714X_SCAN_SPI or AD714X_SCAN_I2C is selected, we let
corresponding codes compiled.  And we use variables to maintain the
registers of SPI/I2C to permit the other one working even after one
fails to register.

static int spi_sta = -1, i2c_sta = -1;

static __init int ad714x_init(void)
{
#if defined(CONFIG_AD714X_SCAN_SPI)
        spi_sta = spi_register_driver(&ad714x_spi_driver);
#endif

#if defined(CONFIG_AD714X_SCAN_I2C)
        i2c_sta = i2c_add_driver(&ad714x_i2c_driver);
#endif

        /* If anyone of spi and i2c init successfully, we permit it to work */
        if ((spi_sta && i2c_sta) == 0)
                return 0;
        else
                return -ENODEV;
}

static __init void ad714x_exit(void)
{
#if defined(CONFIG_AD714X_SCAN_SPI)
        if (!spi_sta)
                spi_unregister_driver(&ad714x_spi_driver);
#endif

#if defined(CONFIG_AD714X_SCAN_I2C)
        if (!i2c_sta)
                i2c_del_driver(&ad714x_i2c_driver);
#endif
}
module_init(ad714x_init);
module_exit(ad714x_exit);

>
> --
> Dmitry
> _______________________________________________
> Uclinux-dist-devel mailing list
> Uclinux-dist-devel@blackfin.uclinux.org
> https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>
_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel@blackfin.uclinux.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel