Re: [PATCH v3] add analog devices AD714Xcaptouchinput driver

["Song, Barry" <Barry.Song-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>]

 

>-----Original Message-----
>From: uclinux-dist-devel-bounces-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org 
>[mailto:uclinux-dist-devel-bounces-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org] On 
>Behalf Of Mike Frysinger
>Sent: Friday, October 09, 2009 4:56 PM
>To: Hennerich, Michael
>Cc: David Brownell; uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org; 
>dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: Re: [Uclinux-dist-devel] [PATCH v3] add analog 
>devices AD714Xcaptouchinput driver
>
>On Fri, Sep 11, 2009 at 09:19, Hennerich, Michael wrote:
>> On Friday 11 September 2009, Song, Barry wrote:
>> +static int __init ad714x_init(void)
>> +{
>> +
>> +       int ret = 0;
>> +       ret = ad714x_spi_register_driver(&ad714x_spi_driver);
>> +       if (ret)
>> +               goto err;
>> +       ret = ad714x_i2c_add_driver(&ad714x_i2c_driver);
>> +       if (ret)
>> +               ad714x_spi_unregister_driver(&ad714x_spi_driver);
>> +err:
>> +       return ret;
>> +}
>> +
>> +static void __exit ad714x_exit(void)
>> +{
>> +       ad714x_spi_unregister_driver(&ad714x_spi_driver);
>> +       ad714x_i2c_del_driver(&ad714x_i2c_driver);
>> +}
>>
>> This doesn't make much sense!
>> Assuming we have two AD714x in a system. One connected by 
>SPI the other
>> by I2C.
>> Why would I remove the SPI one in case the other I2C fails, 
>or not even
>> try the SPI one if the I2C fails?
>
>is this a realistic problem ?  we arent talking about failure to probe
>the device, we're talking about failure to register the driver as
>available.  i.e. the only way the module_init step should fail is if
>i2c_add_driver() or spi_register_driver() fails.  and if either of
>those fail, it's most likely a pretty bad situation.

I agree with this. It is clear the failure of spi_register_driver or i2c_add_driver means a kernel issue not a device probe issue. That maybe means no memory or other serious problems.
But anyway, I check in some codes to support the capacity that the other bus can still continue to register even after the first one fails, maybe the new codes are ugly.
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
}

>
>failure to probe/detect/talk to the device are a different issue and a
>problem with one interface will not affect the other.
>
>> Who says that a driver can't have two module_init()?
>
>Mr. C Code says it wont work.  if ad714x is built as a module and both
>I2C and SPI support are enabled, we get:
>drivers/input/misc/ad714x.c:1621: error: redefinition of '__inittest'
>drivers/input/misc/ad714x.c:1492: error: previous definition of
>'__inittest' was here
>
>these lines correspond to the module_init() macro
>
>linux/init.h says:
>/* Each module must use one module_init(). */
>#define module_init(initfn)                 \
>    static inline initcall_t __inittest(void)       \
>    { return initfn; }                  \
>    int init_module(void) __attribute__((alias(#initfn)));
>
>and indeed, the error makes sense -- you cant have multiple functions
>named the same thing (in this case, we'd have two __inittest() and two
>init_module())
>-mike
>_______________________________________________
>Uclinux-dist-devel mailing list
>Uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org
>https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>