On Tuesday 27 of August 2013 14:00:24 Archit Taneja wrote: > Hi, > > On Tuesday 27 August 2013 01:44 PM, Tomasz Figa wrote: > > Hi Laurent, > > > > On Tuesday 27 of August 2013 10:02:39 Laurent Pinchart wrote: > >> Add DT bindings for the pcf857x-compatible chips and parse the device > >> tree node in the driver. > >> > >> Signed-off-by: Laurent Pinchart > >> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> --- > >> > >> .../devicetree/bindings/gpio/gpio-pcf857x.txt | 71 > >> > >> ++++++++++++++++++++++ drivers/gpio/gpio-pcf857x.c > >> > >> | 44 +++++++++++--- 2 files changed, 107 insertions(+), 8 > >> > >> deletions(-) > >> > >> create mode 100644 > >> > >> Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt > >> > >> Changes since v4: > >> > >> - Don't try to get ngpio from of_device_id data, we already get it > >> from > >> > >> i2c_device_id > > > > Hmm, I'm not sure how this is supposed to work. > > > > How does the I2C core resolve OF compatible name to particular entry in > > id_table? I believe it simply passes NULL as the second argument of > > .probe() if the device is instantiated based on OF compatible string > > and > > not one in the legacy ID table. > > It doesn't pass the second argument as NULL. If you look at > i2c_device_probe() in drivers/i2c/i2c-core.c, the second argument to > probe is passed as: i2c_match_id(driver->id_table, client) > > This will extract the i2c_device_id pointer from the id_table. Yes, there is a chance that it will not return NULL, but I think that relying on this is rather flawed. If you look at the whole code path, you can see that it's only a coincidence that this works. See the execution flow: - I2C adapter driver calls of_i2c_register_devices(), - of_i2c_register_devices() calls of_modalias_node() for every device on this bus, - of_modalias_node() stores the second substring of compatible string separated by a comma, if there is one or the whole compatible otherwise to the output buffer (type field of i2c_board_info struct, as passed by of_i2c_register_devices()), - of_i2c_register_devices() then calls i2c_new_device() with the resulting info struct, - i2c_new_device() takes info->type and copies its contents to client- >name, - then a bit later, I2C core calls i2c_match_id(), which does matching of client->name against id_table of the driver and the resulting i2c_device_id entry (or NULL) is then passed to driver's .probe() callback. So if it happens that compatible is not equal to "<vendor>,<ID from legacy I2C table>", then the matching will fail and NULL will be passed. [CCing Wolfram and Grant, as they should now more about this behavior and whether it's intentional or no] Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html