Re: [PATCH 15/27] regulator: fan53555: Export I2C module alias information

[Javier Martinez Canillas <javier@osg.samsung.com>]

Hello Paul,

On 08/04/2015 12:08 PM, Paul Bolle wrote:
> Hi Javier,
> 
> On ma, 2015-08-03 at 16:29 +0200, Javier Martinez Canillas wrote:
>> On 08/03/2015 01:43 PM, Paul Bolle wrote:
>>> On do, 2015-07-30 at 18:18 +0200, Javier Martinez Canillas wrote:
> 
>> I2C is a little special in that it uses the id_table again to match
>> in i2c_device_probe() and pass a i2c_device_id to the I2C driver's probe
>> function. That is what I meant by matching but maybe I could had been more
>> precise.
> 
> (So is what I2C currently does comparable to what, say, USB does (ie,
> using usb_device_id for the match and also passing it to the driver's
> probe function) or is it more complicated?)
>

Yes, is similar to what USB does. Now the difference is that USB is an
enumerable bus so you have a single id_table in USB drivers. But I2C
is not an enumerable bus so you need a i2c_device_id table and in the
case of Device Trees, also a of_device_id table.

So USB does the same but has it easy since the match is by USB idVendor,
idProduct, etc while I2C could need both the I2C and OF tables.

Currently for OF, I2C core uses the OF table in i2c_device_match() when
calling of_driver_match_device() and the I2C table in i2c_device_probe()
when calling i2c_match_id().

That's one of the reasons why OF only drivers needs an I2C table. The
other reason is due MODALIAS=i2c:<foo> uevents always been reported.

To make even more complicated, the struct i2c_device_id has a kernel_ulong_t
to store the driver data while the struct of_device_id has a const void * so
some casting would be necessary if the OF table is also used on probe in
the future to avoid the need of an I2C table in OF only drivers.
 
>>> But I'm guessing that parsing a device tree blob that contains
>>> strings like
>>>     compatible = "silergy,syr828"
>>>
>>> would add strings like
>>>     MODALIAS=of:N[...]T[...]Csilergy,syr828
>>
>> That would be the correct behavior and is what the RFC patch #27 does.
>>  
>>> to the related uevents. (Likewise for the two other aliases.) Doesn't
>>> that happen here?
>>
>> No, that is exactly the problem.
> 
> Which also explains how these
>    MODULE_DEVICE_TABLE(of, ...);
> 
> lines, which have no effect for the drivers at hand, added to my confusion.
>

Yes, it is confusing indeed and even more since this series are adding these
for the missing drivers. But the rationale for that is that once all drivers
have a proper MODULE_DEVICE_TABLE({i2c,of},...) then the I2C core can be
changed to report a proper MODALIAS=of:N*T*C* uevent.

>> Take a look to i2c_device_uevent() [0],
>> it just does:
>>
>> add_uevent_var(env, "MODALIAS=%s%s", I2C_MODULE_PREFIX, client->name))
>>
>> So if you have a i2c_device_id table but no 
>> MODULE_DEVICE_TABLE(i2c,...),
>> then module autoload won't work.
> 
> Thanks for taking the time to explain all this to me.
>

You are welcome and thanks a lot for the interest in the series
and feedback.

> 
> Paul Bolle
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America