Re: [PATCH RFC] i2c: Use ID table to detect ACPI I2C devices

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Wed, May 20, 2015 at 01:49:02PM +0300, Robert Dolca wrote:
> On Wed, May 20, 2015 at 12:48 PM, Mika Westerberg wrote:
> > On Wed, May 20, 2015 at 12:39:22PM +0300, Robert Dolca wrote:
> >> Currently, if the name used for DT (in dts) matches one of the names
> >> specified in the id table you will have a match. Isn't that an
> >> intended behavior?
> >
> > I thought one needs to put IDs to the driver .of_match_table. This is
> > also what i2c_device_match() is expecting, if I read it right.
> 
> If you put the DT id in of_match_table it will match here:
> 
> i2c_device_match
>         /* Attempt an OF style match */
>         if (of_driver_match_device(dev, drv))
>                 return 1;
> 
> If you don't specify of_match_table and you put the same ID in
> i2c_device_id table it wil match here:
> 
> i2c_device_match
>         driver = to_i2c_driver(drv);
>         /* match on an id table if there is one */
>         if (driver->id_table)
>                 return i2c_match_id(driver->id_table, client) != NULL;
> 
> This is happening because the name from dts is used for client->name.
> i2c_match_id does the matching based on the client name.

OK.

> > BTW, how modules are supposed to be matched if we allow putting ACPI
> > identifiers to i2c_device_id table?
> 
> My aproach was like this: if the driver specifies .acpi_match table it
> will work like before.
> 
> i2c_device_match
>         /* Then ACPI style match */
>         if (acpi_driver_match_device(dev, drv))
>                 return 1;
> 
> If the driver does not specify .acpi_match table the i2c core will
> atempt to match against the  i2c_match_id table (the same way it does
> for DT). In the ACPI case the client->name has that :nn suffix and
> what the patch does is to ignore that when i2c_match_id is called.
> 
> i2c_device_match
>         driver = to_i2c_driver(drv);
>         /* match on an id table if there is one */
>         if (driver->id_table)
>                 return i2c_match_id(driver->id_table, client) != NULL;

Yeah but when you have device with modalias of "acpi:FOO:" how
udev/modprobe is supposed find the correct module?

> The final goal is to simplify the driver and remove redundant code.

IMHO mixing ACPI identifiers with I2C device identifiers does not
simplify anything. And since you need to stick the ACPI ID somewhere
anyway I don't get the point of removing redundant code either.