i2c: Keep client->driver and client->dev.driver in sync
Subsystems (2)
PATCH 1/1 i2c: Keep client->driver and client->dev.driver in sync
From: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/[email protected]>
Ensures that client->driver is set to NULL if the probe() returns an error (this keeps client->driver and client->dev.driver in sync).
---
Hans, I've updated your patch a bit to make i2c_device_probe() look more like i2c_device_remove(); hope you don't mind.
David, this fix looks correct to me, I'll send it to Linus quickly if you have no objection.
drivers/i2c/i2c-core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/i2c/i2c-core.c +5 -2
@@ -90,12 +90,15 @@ static int i2c_device_probe(struct devic {struct i2c_client *client =to_i2c_client (dev);struct i2c_driver *driver =to_i2c_driver (dev->driver); +int status;if (!driver->probe)return -ENODEV ; - client->driver = driver;dev_dbg (dev,"probe\n" ); -return driver->probe (client); + status = driver->probe (client); +if (status ==0 ) + client->driver = driver; +return status; }static int i2c_device_remove (struct device *dev)
Signature · 6 lines
Jean Delvare _______________________________________________ i2c mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 5 lines · Jean Delvare
From: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/[email protected]>
Ensures that
client->driveris set to NULL if theprobe()returns an error (this keepsclient->driverandclient->dev.driverin sync).
Quoted 8 lines
Signed-off-by Jean Delvare <[email protected]>---
Hans, I've updated your patch a bit to makei2c_device_probe()look more likei2c_device_remove(); hope you don't mind.David, this fix looks correct to me, I'll send it to Linus quickly if you have no objection.
The only issue I have is that code which the driver probe() calls may expect client->driver to be set, but it won't be...
While I'd rather see client->driver vanish, in this case I'd just suggest leaving the early assignment, and nulling it out on error (instead of assigning it on success).
Quoted 28 lines
drivers/i2c/i2c-core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/i2c/i2c-core.c +5
-2@@ -90,12 +90,15 @@ static int i2c_device_probe(struct devic {struct i2c_client *client =to_i2c_client (dev);struct i2c_driver *driver =to_i2c_driver (dev->driver); +int status;if (!driver->probe)return -ENODEV ; - client->driver = driver;dev_dbg (dev,"probe\n" ); -return driver->probe (client); + status = driver->probe (client); +if (status ==0 ) + client->driver = driver; +return status; }static int i2c_device_remove (struct device *dev)Signature · 2 lines
-- Jean Delvare
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): 200803081312.36524.david-b-yBeKhBN/[email protected]
Hi David,
Quoted 6 lines · David Brownell
The only issue I have is that code which the driver
probe()calls may expectclient->driverto be set, but it won't be...While I'd rather see
client->drivervanish, in this case I'd just suggest leaving the early assignment, and nulling it out on error (instead of assigning it on success).
Oh well, that's exactly what Hans' patch did originally, I changed it to make the code more simple, but if you think it is better that way, I'll revert this change of mine. Here you go:
From: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/[email protected]>
Ensure that client->driver is set to NULL if the probe() returns an error (this keeps client->driver and client->dev.driver in sync).
--- drivers/i2c/i2c-core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/i2c/i2c-core.c +5 -1
@@ -90,12 +90,16 @@ static int i2c_device_probe(struct devic {struct i2c_client *client =to_i2c_client (dev);struct i2c_driver *driver =to_i2c_driver (dev->driver); +int status;if (!driver->probe)return -ENODEV ; client->driver = driver;dev_dbg (dev,"probe\n" ); -return driver->probe (client); + status = driver->probe (client); +if (status) + client->driver =NULL ; +return status; }static int i2c_device_remove (struct device *dev)
Signature · 6 lines
Jean Delvare _______________________________________________ i2c mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/i2c
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 6 lines · Jean Delvare
Hi David,
Quoted 3 lines · David Brownell
The only issue I have is that code which the driver
probe()calls may expectclient->driverto be set, but it won't be...
Quoted 7 lines
Quoted 3 lines
While I'd rather see
client->drivervanish, in this case I'd just suggest leaving the early assignment, and nulling it out on error (instead of assigning it on success).Oh well, that's exactly what Hans' patch did originally, I changed it to make the code more simple, but if you think it is better that way, I'll revert this change of mine. Here you go:
Yeah, this has much less chance to break something inadvertently.
- Dave
Quoted 37 lines
From: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/[email protected]>
Ensure that
client->driveris set to NULL if theprobe()returns an error (this keepsclient->driverandclient->dev.driverin sync).Signed-off-by Jean Delvare <[email protected]>--- drivers/i2c/i2c-core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)drivers/i2c/i2c-core.c +5
-1@@ -90,12 +90,16 @@ static int i2c_device_probe(struct devic {struct i2c_client *client =to_i2c_client (dev);struct i2c_driver *driver =to_i2c_driver (dev->driver); +int status;if (!driver->probe)return -ENODEV ; client->driver = driver;dev_dbg (dev,"probe\n" ); -return driver->probe (client); + status = driver->probe (client); +if (status) + client->driver =NULL ; +return status; }static int i2c_device_remove (struct device *dev)Signature · 2 lines
-- Jean Delvare
_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c