Re: i2c-remove-redundant-i2c_client-list.patch

[David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]

On Thursday 17 January 2008, David Brownell wrote:
> > (d) Meanwhile, come up with a different solution to the deadlock
> >     observed with i2c_adapter.clist ... which for some unknown
> >     reason has *NOT* shown up for me with lockdep.
> 
> This should suffice.  Could be merged with the patch above,
> or even made to not depend on it.
> 
> And maybe the list_add should be moved after device_register()
> so the locking is only needed on that one path.  That'd be a
> net code shrink and simplification...

Grr.  Disregard that previous patch.  This version fixes bugs
in that one, and includes those cleanups too.  To make it not
depend on the previous patch, make it "__i2c_check_addr".

========= SNIP!
This goes on top of the patch removing most i2c_adapter.clients usage,
updating i2c_attach_client:

 - Don't call device_register() while holding clist_lock.  This
   removes a self-deadlock when on the i2c_driver.probe() path,
   for drivers that need to attach new devices (e.g. dummies).

 - Remove a redundant address check.  The driver model core does
   this as a consequence of guaranteeing unique names.

 - Move the "device registered" diagnostic so that it never lies;
   previously, on error paths it would falsely report success.

---
 drivers/i2c/i2c-core.c |   22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

--- at91.orig/drivers/i2c/i2c-core.c	2008-01-17 10:07:38.000000000 -0800
+++ at91/drivers/i2c/i2c-core.c	2008-01-17 11:53:01.000000000 -0800
@@ -752,13 +752,6 @@ int i2c_attach_client(struct i2c_client 
 	struct i2c_adapter *adapter = client->adapter;
 	int res = 0;
 
-	mutex_lock(&adapter->clist_lock);
-	if (i2c_check_addr(client->adapter, client->addr)) {
-		res = -EBUSY;
-		goto out_unlock;
-	}
-	list_add_tail(&client->list,&adapter->clients);
-
 	client->usage_count = 0;
 
 	client->dev.parent = &client->adapter->dev;
@@ -775,13 +768,17 @@ int i2c_attach_client(struct i2c_client 
 
 	snprintf(&client->dev.bus_id[0], sizeof(client->dev.bus_id),
 		"%d-%04x", i2c_adapter_id(adapter), client->addr);
-	dev_dbg(&adapter->dev, "client [%s] registered with bus id %s\n",
-		client->name, client->dev.bus_id);
 	res = device_register(&client->dev);
 	if (res)
-		goto out_list;
+		goto out_err;
+
+	mutex_lock(&adapter->clist_lock);
+	list_add_tail(&client->list,&adapter->clients);
 	mutex_unlock(&adapter->clist_lock);
 
+	dev_dbg(&adapter->dev, "client [%s] registered with bus id %s\n",
+		client->name, client->dev.bus_id);
+
 	if (adapter->client_register)  {
 		if (adapter->client_register(client)) {
 			dev_dbg(&adapter->dev, "client_register "
@@ -792,12 +789,9 @@ int i2c_attach_client(struct i2c_client 
 
 	return 0;
 
-out_list:
-	list_del(&client->list);
+out_err:
 	dev_err(&adapter->dev, "Failed to attach i2c client %s at 0x%02x "
 		"(%d)\n", client->name, client->addr, res);
-out_unlock:
-	mutex_unlock(&adapter->clist_lock);
 	return res;
 }
 EXPORT_SYMBOL(i2c_attach_client);


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c