Re: [RFC] i2c: designware: Avoid initcall and initialize the driver like a regular one

[Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>]

[-- Attachment #1: Type: text/plain, Size: 2750 bytes --]



On 12/23/2014 12:26 PM, Wolfram Sang wrote:
> 
>>>> This guarantees it will probe after GPIOs drivers.
> 
> BTW this is not true to the best of my knowledge. It will make that
> "very likely" but not "guarantee" anything. So, the race window is
> smaller but it is still there. You need a proper fix anyhow.
> 

Right.

>>>> Platforms based on devicetree won't be affected by this change.
>>>
>>> Huh, why is that?
>>>
>>
>> Unless I'm missing something here, our beloved DeviceTree guarantees to
>> model the dependency between I2C slaves devices and the I2C master their
>> connected to.
> 
> Frankly, you are missing quite some things here. The I2C core registers
> the clients when a master gets registered. No difference between
> platform and DT here.
> 
>> So, a machine fully-based on DeviceTree would never attempt to use the I2C
>> bus without first registering the master, right?
> 
> Neither would platform, that would be quite a bug.
> 
>> This means there won't be any early users of the I2C platform driver in this
>> scenario.
> 
> There won't be with platform as well.

Oh, OK. Then maybe you can clarify why all those i2c busses need to be
registered with initcall in the first place?

> But I think you are missing the
> point. We are a *consumer* of GPIOs here. All of the above has nothing
> to do with GPIO controllers being already available.
> 

Hm, true. I was missing the fact that probe call order does not
guarantee a succesful probe order.

>>>> Legacy platforms, relying on the I2C being available early, might need
>>>> to implement proper defered mechanisms to overcome potential problems.
>>>
>>> NAK. We can't say "Let's cause a regression to force people to fix
>>> things that used to work" IMO. You exactly pointed out the problem that using
>>> subsys_initcall() creates.
>>>
>>> What about fixing the drivers you use to support deferred probing when
>>> acquitin the irq?
>>>
>>
>> Maybe we can fix the legacy ones instead. However, a quick look shows there
>> aren't any!
>>
>> $ git grep i2c_designware
>> drivers/i2c/busses/i2c-designware-pcidrv.c:MODULE_ALIAS("i2c_designware-pci");
>> drivers/i2c/busses/i2c-designware-platdrv.c:MODULE_ALIAS("platform:i2c_designware");
>> drivers/i2c/busses/i2c-designware-platdrv.c:            .name   = "i2c_designware",
>>
>> Looks like this patch is pretty harmless.
> 
> In-tree you are right. Out-of-tree, you probably aren't. I wouldn't care
> about the latter if that would block a real bugfix. But since the above
> patch is not the proper fix IMO, I prefer being stable here.
> 

Fair enough.

Thanks for the feedback,
-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]