Re: [PATCH] Input: tca6416-keypad: Change to module_init()

[Magnus Damm <magnus.damm@gmail.com>]

On Wed, Mar 23, 2011 at 12:32 AM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Wed, Mar 23, 2011 at 12:22:05AM +0900, Magnus Damm wrote:
>> On Tue, Mar 22, 2011 at 11:33 PM, Paul Mundt <lethal@linux-sh.org> wrote:
>> > On Tue, Mar 22, 2011 at 02:28:55PM +0000, Mark Brown wrote:
>> >> On Tue, Mar 22, 2011 at 11:26:19PM +0900, Magnus Damm wrote:
>> >>
>> >> > The tca6416 driver makes use of the I2C bus for chatting
>> >> > with the actual hardware device. Without this patch both
>> >> > the I2C bus driver and the tca6416 driver are initialized
>> >> > at the subsys_initcall() level. This may lead to problems
>> >> > with the tca6416 driver being initialized before the I2C
>> >> > bus driver.
>> >>
>> >> While this change seems reasonable I'm curious what the problems caused
>> >> by out of order registration are?
>> >
>> > I'm also curious as to why link order isn't a sufficient gaurantee like
>> > it is for everyone else?
>>
>> I believe all other i2c keyboard drivers use module_init().
>>
> We do not change initcall ordering around unless there is a reason to do
> so, as it's assumed that a given initcall has been chosen for a reason.

Yes, obviously this driver is special and all other keypad drivers are wrong.

It would be interesting to hear why subsys_initcall() was put there in
the first place.

> You have hit upon a bug or at least something timing related causing you
> a delay and so have elected to push it down a level. That is of course
> fine, but none of that is anywhere in your commit text leaving us to try
> and figure out what exactly the point of this exercise is.

The keypad driver tries to use the I2C bus before the I2C bus driver
is initialized. Isn't that a pretty good reason to change the initcall
order?

> Usually "because everyone else is doing it" and another driver is not,
> there's a reason for that driver doing things differently. There are
> certainly enough cases where initcall and link ordering is strongly
> ordered for a reason that cosmetic/janitorial fixes are best rejected out
> of hand.

So let's hear what other people have to say about this.

> You had a reason, great. Next time put it in your commit text.

Whatever. Let me know which lines you'd like to add and I'll send a V2.

/ magnus