| Message ID | 1409236538-21274-7-git-send-email-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h > index 79b674d..c8240e5 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -125,7 +125,8 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, > * struct i2c_driver - represent an I2C device driver > * @class: What kind of i2c device we instantiate (for detect) > * @attach_adapter: Callback for bus addition (deprecated) > - * @probe: Callback for device binding > + * @probe: Callback for device binding - soon to be deprecated > + * @probe2: New callback for device binding I don't like the naming. What about 'simplified_probe' instead of 'probe2'? The old 'probe' would be deprecated immediately, I guess. Also, this needs more information in the kernel docs. Come to think of it, I'd like to see some more documentation in general. A document in Documentation/i2c should explain the differences between the probe-functions, the reason why they are there, the intended path to migrate over and examples how to convert drivers and what to keep in mind doing so. > * @remove: Callback for device unbinding > * @shutdown: Callback for device shutdown > * @suspend: Callback for device suspend > @@ -170,6 +171,11 @@ struct i2c_driver { > int (*probe)(struct i2c_client *, const struct i2c_device_id *); > int (*remove)(struct i2c_client *); > > + /* New driver model interface to aid the seamless removal of the > + * current probe()'s, more commonly unused than used second parameter. "largely unused second parameter"? Maybe more readable? At least, I needed to read this sentence more than once to get it. BTW have you measured how often it was used/unused? > + */ > + int (*probe2)(struct i2c_client *); > + > /* driver model interfaces that don't relate to enumeration */ > void (*shutdown)(struct i2c_client *); > int (*suspend)(struct i2c_client *, pm_message_t mesg); What I like in general: After your series I2C does more like what other subsystems (especially SPI) do. What I dislike about this in general: Putting to the drivers that they should find out about the matching. I mean the core already did the matching, so why should a driver do that again? Especially since it can get very cumbersome to do so with platform_data, DT and ACPI around which need to be checked. This is why I stopped my implementation to fix this issue. I felt the need for a helper function to assist the drivers how they were matched. Ideally by just retrieving the information which the subsystem cores already gathered. Making such a thing generic was where I stopped working on that. Has this been discussed somewhere already?
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 7c21f49..5c74bd0 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -270,8 +270,6 @@ static int i2c_device_probe(struct device *dev) return 0; driver = to_i2c_driver(dev->driver); - if (!driver->probe) - return -EINVAL; /* * An I2C ID table is not mandatory, if and only if, a suitable Device @@ -291,7 +289,15 @@ static int i2c_device_probe(struct device *dev) return status; acpi_dev_pm_attach(&client->dev, true); - status = driver->probe(client, i2c_match_id(driver->id_table, client)); + + /* When there are no more users of probe(), rename probe2 to probe. */ + if (driver->probe2) + status = driver->probe2(client); + else if (driver->probe) + status = driver->probe(client, + i2c_match_id(driver->id_table, client)); + else + return -EINVAL; if (status) acpi_dev_pm_detach(&client->dev, true); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 79b674d..c8240e5 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -125,7 +125,8 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client, * struct i2c_driver - represent an I2C device driver * @class: What kind of i2c device we instantiate (for detect) * @attach_adapter: Callback for bus addition (deprecated) - * @probe: Callback for device binding + * @probe: Callback for device binding - soon to be deprecated + * @probe2: New callback for device binding * @remove: Callback for device unbinding * @shutdown: Callback for device shutdown * @suspend: Callback for device suspend @@ -170,6 +171,11 @@ struct i2c_driver { int (*probe)(struct i2c_client *, const struct i2c_device_id *); int (*remove)(struct i2c_client *); + /* New driver model interface to aid the seamless removal of the + * current probe()'s, more commonly unused than used second parameter. + */ + int (*probe2)(struct i2c_client *); + /* driver model interfaces that don't relate to enumeration */ void (*shutdown)(struct i2c_client *); int (*suspend)(struct i2c_client *, pm_message_t mesg);
This will aid the seamless removal of the current probe()'s, more commonly unused than used second parameter. Most I2C drivers can simply switch over to the new interface, others which have DT support can use its own matching instead and others can call i2c_match_id() themselves. This brings I2C's device probe method into line with other similar interfaces in the kernel and prevents the requirement to pass an i2c_device_id table. Suggested-by: Grant Likely <grant.likely@linaro.org> Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/i2c/i2c-core.c | 12 +++++++++--- include/linux/i2c.h | 8 +++++++- 2 files changed, 16 insertions(+), 4 deletions(-)