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

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

On Wednesday 09 January 2008, David Brownell wrote:
> (a) Start phasing out users of i2c_client.list and its lock, ASAP;
>     merging those DVB driver patches, and some i2c-core changes.

Posted:  http://marc.info/?l=i2c&m=120034972711126&w=2

> (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...

- Dave

====== CUT HERE
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 |   19 +++++++++----------
 1 file changed, 9 insertions(+), 10 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 10:11:04.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;
@@ -773,14 +766,18 @@ int i2c_attach_client(struct i2c_client 
 	} else
 		client->dev.release = i2c_client_dev_release;
 
+	mutex_lock(&adapter->clist_lock);
+	list_add_tail(&client->list,&adapter->clients);
+	mutex_unlock(&adapter->clist_lock);
+
 	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;
-	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)) {
@@ -793,7 +790,9 @@ int i2c_attach_client(struct i2c_client 
 	return 0;
 
 out_list:
+	mutex_lock(&adapter->clist_lock);
 	list_del(&client->list);
+	mutex_unlock(&adapter->clist_lock);
 	dev_err(&adapter->dev, "Failed to attach i2c client %s at 0x%02x "
 		"(%d)\n", client->name, client->addr, res);
 out_unlock:



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