Re: [PATCHv2 3/3] gpio: msm: Add device tree and irqdomain supportfor gpio-msm-v2

[Grant Likely]

On Fri, May 31, 2013 at 9:01 PM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Thu, May 23, 2013 at 3:29 AM, Rohit Vaswani <rvaswani@xxxxxxxxxxxxxx> wrote:
>> This cleans up the gpio-msm-v2 driver of all the global define usage.
>> The number of gpios are now defined in the device tree. This enables
>> adding irqdomain support as well.
>
> Besides the other comments, I have one important here.
>
>> -static int __init msm_gpio_init(void)
>> -{
>> -       int rc;
>> -
>> -       rc = platform_driver_register(&msm_gpio_driver);
>> -       if (!rc) {
>> -               rc = platform_device_register(&msm_device_gpio);
>> -               if (rc)
>> -                       platform_driver_unregister(&msm_gpio_driver);
>> -       }
>> -
>> -       return rc;
>> -}
>> -
>> -static void __exit msm_gpio_exit(void)
>> -{
>> -       platform_device_unregister(&msm_device_gpio);
>> -       platform_driver_unregister(&msm_gpio_driver);
>> -}
>> -
>> -postcore_initcall(msm_gpio_init);
>> -module_exit(msm_gpio_exit);
>> +module_platform_driver(msm_gpio_driver)
>
> It's a really big mistake to do this.
> GPIO nowadays is used in many hardware subsystems to support early
> stages of booting and initializing.
>
> postcore_initcall is early enough to support mostly anything that
> hardware wants from GPIO (for example, ACPI events, though it seems
> not a case here).

Actually, we're going the other way in the kernel. I do agree that for
existing drivers you need to be careful and make sure that switching
the initcall level won't cause any breakage with existing systems, but
new platform support must not rely on the initcall order for setting
up gpios. That's what deferred probe is intended to solve.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/