Re: [PATCH v2 2/2] IIO: st_accel_i2c.c: Use probe_new() instead of probe()

From: Nikolaus Voss
Date: Wed Jul 04 2018 - 07:46:31 EST

Hi Javier,

On Wed, 4 Jul 2018, Javier Martinez Canillas wrote:
Hi Nikolaus,

On Wed, Jul 4, 2018 at 1:15 PM, Nikolaus Voss
<nikolaus.voss@xxxxxxxxxxxxxxxxxxxxx> wrote:
On Wed, 4 Jul 2018, Javier Martinez Canillas wrote:


I think Nikolaus is correct, assuming that the driver can be used on
platforms that register the I2C devices using board files / platform data.
In that case you still need a I2C device ID table for (a) and (c) as he

I don't buy on (b) though, that's a bug in my opinion. If you register an
device via DT then you must have a OF device ID entry that matches the
and the same for ACPI. You can't rely on the I2C device table to do the

Ok, in my opinion it is an elegant way of not bloating the driver when no
explicit handling (e.g. reading DT properties) is needed. Just adding an
of_device_id doesn't change any driver functionality then but only maps to
an already existing i2c_table_id.

I disagree, in the case of OF for example a compatible string is
composed of both a vendor a device, the complete tuple is what should
be matched.

But with the fallback, only the device portion would be used so both
<foo,bar> and <baz,bar> will match against the i2c device with id
"bar". It may or may not be correct but the vendor portion is very
important to disambiguate.

Disambiguation via DT implies there is already a name collision in i2c modalias name space, so adding an of_id would work around this collision for DT enumeration. I2c_board_info driver binding would still be broken. The right fix would be to remove the name collision.

I would also remove the struct i2c_device_id .driver_data fields from the
device ID table, since are not used and just makes reading the code
(only the struct i2c_device_id .name is used as far as I can see).

Valid point, thanks. I will change that.

Javier, just a summary of the above. Nikolaus switched one driver to
use ->probe_new() hook and left i2c ID table at the same time.
My understanding that this table is not anymore in use.

But I have to admit I didn't see entire picture of this. Can you shed a

So to shed some light, in the past even {OF,ACPI}-only drivers needed an
table because: 1) the .probe callback had a struct i2c_device_id *
and 2) the I2C core always reported a modalias of the form i2c:<foo> even
devices registered via OF.

It could have been a null pointer and device driver binding (and
auto-loading) done just via

I'm not sure I understood this comment.

What I was trying to say is that if the i2c_device_id table information was not needed (i.d. only one single id), even the old probe() could be used without defining an i2c_device_id table, the i2c_device_id* argument to probe() being a nullptr.