[i2c] [patch 2.6.21-rc3-git +i2c 4/4] remove i2c_client.list

David Brownell david-b at pacbell.net
Mon Mar 12 19:38:25 CET 2007


More I2C core duplication elimination:

  - We don't need a list coupling each i2c_adapter to its i2c_client
    children, the driver model tracks that for us.

  - Neither do we need the lock protecting that list, since the driver
    model protects its list;

  - Or an extra check for address uniqueness when instantiating
    an i2c_client node, same reason.

Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>

---
Not really tested at all... likely not yet correct or mergeable.

 drivers/i2c/busses/i2c-isa.c |   27 +----
 drivers/i2c/i2c-core.c       |  202 ++++++++++++++++++++-----------------------
 include/linux/i2c.h          |    4 
 3 files changed, 106 insertions(+), 127 deletions(-)

Index: at91/include/linux/i2c.h
===================================================================
--- at91.orig/include/linux/i2c.h	2007-03-12 11:16:09.000000000 -0700
+++ at91/include/linux/i2c.h	2007-03-12 11:21:24.000000000 -0700
@@ -38,6 +38,7 @@
 /* --- For i2c-isa ---------------------------------------------------- */
 
 extern void i2c_adapter_dev_release(struct device *dev);
+extern int i2c_do_detach_drv_client(struct device *dev, void *_drv);
 extern struct class i2c_adapter_class;
 extern struct bus_type i2c_bus_type;
 
@@ -171,7 +172,6 @@ struct i2c_client {
 					/* to the client		*/
 	struct device dev;		/* the device structure		*/
 	int irq;			/* irq issued by device (or -1) */
-	struct list_head list;
 	const char *driver_name;
 	struct completion released;
 };
@@ -297,14 +297,12 @@ struct i2c_adapter {
 	/* data fields that are valid for all devices	*/
 	u8 level; 			/* nesting level for lockdep */
 	struct mutex bus_lock;
-	struct mutex clist_lock;
 
 	int timeout;
 	int retries;
 	struct device dev;		/* the adapter device */
 
 	int nr;
-	struct list_head clients;
 	char name[50];
 	struct completion dev_released;
 };
Index: at91/drivers/i2c/i2c-core.c
===================================================================
--- at91.orig/drivers/i2c/i2c-core.c	2007-03-12 11:17:01.000000000 -0700
+++ at91/drivers/i2c/i2c-core.c	2007-03-12 11:24:45.000000000 -0700
@@ -253,19 +253,9 @@ void i2c_unregister_device(struct i2c_cl
 		return;
 	}
 
-	/* unbind driver */
-	if (driver) {
-		/* REVISIT driver model core would handle this for us, and
-		 * once i2c_detach_client() stops updating lists, it should.
-		 */
-		status = i2c_device_remove(&client->dev);
-		if (status < 0) {
-			dev_err(&client->dev, "unbind failed (%d)\n", status);
-			return;
-		}
-	}
+	/* REVISIT hmm, refcounting is still goofed ... */
 
-	/* now remove the device, and free its memory */
+	/* now remove the device (unbinding driver), and free its memory */
 	status = i2c_detach_client(client);
 	if (status < 0)
 		dev_err(&adap->dev, "detach failed (%d) for client [%s] "
@@ -330,8 +320,6 @@ static int i2c_register_adapter(struct i
 	struct i2c_driver  *driver;
 
 	mutex_init(&adap->bus_lock);
-	mutex_init(&adap->clist_lock);
-	INIT_LIST_HEAD(&adap->clients);
 
 	mutex_lock(&core_lists);
 
@@ -462,11 +450,34 @@ retry:
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
+static int do_remove_client(struct device *dev, void *_drv)
+{
+	struct i2c_client	*client;
+	struct i2c_driver	*driver;
+	int			res;
+
+	if (dev->bus != &i2c_bus_type || !dev->driver)
+		return 0;
+	client = to_i2c_client(dev);
+	driver = to_i2c_driver(dev->driver);
+
+	/* new style, follow standard driver model */
+	if (is_newstyle_driver(driver)) {
+		i2c_unregister_device(client);
+		return 0;
+	}
+
+	/* legacy drivers create and remove clients themselves */
+	if ((res = driver->detach_client(client)))
+		dev_err(dev->parent, "detach_client failed for client "
+			"[%s] at address 0x%02x\n", client->name,
+			client->addr);
+	return res;
+}
+
 int i2c_del_adapter(struct i2c_adapter *adap)
 {
-	struct list_head  *item, *_n;
 	struct i2c_driver *driver;
-	struct i2c_client *client;
 	int res = 0;
 
 	mutex_lock(&core_lists);
@@ -479,8 +490,7 @@ int i2c_del_adapter(struct i2c_adapter *
 		goto out_unlock;
 	}
 
-	list_for_each(item,&drivers) {
-		driver = list_entry(item, struct i2c_driver, list);
+	list_for_each_entry(driver, &drivers, list) {
 		if (driver->detach_adapter)
 			if ((res = driver->detach_adapter(adap))) {
 				dev_err(&adap->dev, "detach_adapter failed "
@@ -492,28 +502,9 @@ int i2c_del_adapter(struct i2c_adapter *
 
 	/* detach any active clients. This must be done first, because
 	 * it can fail; in which case we give up. */
-	list_for_each_safe(item, _n, &adap->clients) {
-		struct i2c_driver	*driver;
-
-		client = list_entry(item, struct i2c_client, list);
-		driver = client->dev.driver
-			? to_i2c_driver(client->dev.driver)
-			: NULL;
-
-		/* new style, follow standard driver model */
-		if (!driver || is_newstyle_driver(driver)) {
-			i2c_unregister_device(client);
-			continue;
-		}
-
-		/* legacy drivers create and remove clients themselves */
-		if ((res = driver->detach_client(client))) {
-			dev_err(&adap->dev, "detach_client failed for client "
-				"[%s] at address 0x%02x\n", client->name,
-				client->addr);
-			goto out_unlock;
-		}
-	}
+	res = device_for_each_child(&adap->dev, NULL, do_remove_client);
+	if (res)
+		goto out_unlock;
 
 	/* clean up the sysfs representation */
 	init_completion(&adap->dev_released);
@@ -590,14 +581,34 @@ int i2c_register_driver(struct module *o
 }
 EXPORT_SYMBOL(i2c_register_driver);
 
+int i2c_do_detach_drv_client(struct device *dev, void *_drv)
+{
+	struct i2c_driver	*driver = _drv;
+	struct i2c_client	*client;
+	int			res;
+
+	if (dev->driver != &driver->driver || !driver->detach_client)
+		return 0;
+
+	client = to_i2c_client(dev);
+	dev_dbg(dev->parent, "detaching client [%s] at 0x%02x\n",
+		client->name, client->addr);
+
+	res = driver->detach_client(client);
+	if (res != 0)
+		dev_err(dev->parent,
+			"detach_client failed for client [%s] at 0x%02x\n",
+			client->name, client->addr);
+	return res;
+}
+EXPORT_SYMBOL_GPL(i2c_do_detach_drv_client);	// ONLY FOR I2C-ISA!!!
+
 /**
  * i2c_del_driver - unregister I2C driver
  * @driver: the driver being unregistered
  */
 int i2c_del_driver(struct i2c_driver *driver)
 {
-	struct list_head   *item2, *_n;
-	struct i2c_client  *client;
 	struct i2c_adapter *adap;
 
 	int res = 0;
@@ -621,22 +632,10 @@ int i2c_del_driver(struct i2c_driver *dr
 				goto out_unlock;
 			}
 		} else {
-			list_for_each_safe(item2, _n, &adap->clients) {
-				client = list_entry(item2, struct i2c_client,
-						list);
-				if (client->dev.driver != &driver->driver)
-					continue;
-				dev_dbg(&adap->dev, "detaching client [%s] "
-					"at 0x%02x\n", client->name,
-					client->addr);
-				if ((res = driver->detach_client(client))) {
-					dev_err(&adap->dev, "detach_client "
-						"failed for client [%s] at "
-						"0x%02x\n", client->name,
-						client->addr);
-					goto out_unlock;
-				}
-			}
+			res = device_for_each_child(&adap->dev, driver,
+					i2c_do_detach_drv_client);
+			if (res != 0)
+				goto out_unlock;
 		}
 	}
 
@@ -653,28 +652,20 @@ EXPORT_SYMBOL(i2c_del_driver);
 
 /* ------------------------------------------------------------------------- */
 
-static int __i2c_check_addr(struct i2c_adapter *adapter, unsigned int addr)
+static int do_check_addr(struct device *dev, void *_addr)
 {
-	struct list_head   *item;
-	struct i2c_client  *client;
+	unsigned int		addr = *(int *)_addr;
+	struct i2c_client	*client;
 
-	list_for_each(item,&adapter->clients) {
-		client = list_entry(item, struct i2c_client, list);
-		if (client->addr == addr)
-			return -EBUSY;
-	}
-	return 0;
+	if (dev->bus != &i2c_bus_type)
+		return 0;
+	client = to_i2c_client(dev);
+	return (client->addr == addr) ? -EBUSY : 0;
 }
 
 int i2c_check_addr(struct i2c_adapter *adapter, int addr)
 {
-	int rval;
-
-	mutex_lock(&adapter->clist_lock);
-	rval = __i2c_check_addr(adapter, addr);
-	mutex_unlock(&adapter->clist_lock);
-
-	return rval;
+	return device_for_each_child(&adapter->dev, &addr, do_check_addr);
 }
 EXPORT_SYMBOL(i2c_check_addr);
 
@@ -687,12 +678,6 @@ int i2c_attach_client(struct i2c_client 
 		return -ENOENT;
 
 	adapter = to_i2c_adapter(client->dev.parent);
-	mutex_lock(&adapter->clist_lock);
-	if (__i2c_check_addr(adapter, client->addr)) {
-		res = -EBUSY;
-		goto out_unlock;
-	}
-	list_add_tail(&client->list, &adapter->clients);
 
 	client->usage_count = 0;
 
@@ -703,10 +688,11 @@ int i2c_attach_client(struct i2c_client 
 		"%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);
+
+	/* registration fails if the address is already in use */
 	res = device_register(&client->dev);
 	if (res)
 		goto out_list;
-	mutex_unlock(&adapter->clist_lock);
 
 	if (adapter->client_register)  {
 		if (adapter->client_register(client)) {
@@ -719,11 +705,8 @@ int i2c_attach_client(struct i2c_client 
 	return 0;
 
 out_list:
-	list_del(&client->list);
 	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);
@@ -749,11 +732,8 @@ int i2c_detach_client(struct i2c_client 
 		}
 	}
 
-	mutex_lock(&adapter->clist_lock);
-	list_del(&client->list);
 	init_completion(&client->released);
 	device_unregister(&client->dev);
-	mutex_unlock(&adapter->clist_lock);
 	wait_for_completion(&client->released);
 
  out:
@@ -812,27 +792,39 @@ int i2c_release_client(struct i2c_client
 }
 EXPORT_SYMBOL(i2c_release_client);
 
+struct driver_cmd {
+	unsigned int	cmd;
+	void		*arg;
+};
+
+static int do_client_command(struct device *dev, void *_cmd)
+{
+	struct i2c_driver	*driver;
+	struct i2c_client	*client;
+	struct driver_cmd	*cmd;
+
+	if (dev->bus != &i2c_bus_type || !dev->driver)
+		return 0;
+
+	driver = to_i2c_driver(dev->driver);
+	if (!driver->command)
+		return 0;
+	if (!try_module_get(driver->driver.owner))
+		return 0;
+
+	client = to_i2c_client(dev);
+	cmd = _cmd;
+	driver->command(client, cmd->cmd, cmd->arg);
+
+	module_put(driver->driver.owner);
+	return 0;
+}
+
 void i2c_clients_command(struct i2c_adapter *adap, unsigned int cmd, void *arg)
 {
-	struct list_head  *item;
-	struct i2c_client *client;
+	struct driver_cmd c = { .cmd = cmd, .arg = arg, };
 
-	mutex_lock(&adap->clist_lock);
-	list_for_each(item,&adap->clients) {
-		struct i2c_driver *driver;
-
-		client = list_entry(item, struct i2c_client, list);
-		driver = to_i2c_driver(client->dev.driver);
-		if (!try_module_get(driver->driver.owner))
-			continue;
-		if (NULL != driver->command) {
-			mutex_unlock(&adap->clist_lock);
-			driver->command(client,cmd,arg);
-			mutex_lock(&adap->clist_lock);
-		}
-		module_put(driver->driver.owner);
-       }
-       mutex_unlock(&adap->clist_lock);
+	device_for_each_child(&adap->dev, &c, do_client_command);
 }
 EXPORT_SYMBOL(i2c_clients_command);
 
Index: at91/drivers/i2c/busses/i2c-isa.c
===================================================================
--- at91.orig/drivers/i2c/busses/i2c-isa.c	2007-03-12 10:54:06.000000000 -0700
+++ at91/drivers/i2c/busses/i2c-isa.c	2007-03-12 11:21:24.000000000 -0700
@@ -100,26 +100,18 @@ int i2c_isa_add_driver(struct i2c_driver
 	return res;
 }
 
+
+/* NOTE:  this is identical to i2c_del_driver(), except that applies
+ * to all adapters but this one is restricted to the ISA adapter.
+ */
 int i2c_isa_del_driver(struct i2c_driver *driver)
 {
-	struct list_head *item, *_n;
-	struct i2c_client *client;
 	int res;
 
-	/* Detach all clients belonging to this one driver */
-	list_for_each_safe(item, _n, &isa_adapter.clients) {
-		client = list_entry(item, struct i2c_client, list);
-		if (client->dev.driver != &driver->driver)
-			continue;
-		dev_dbg(&isa_adapter.dev, "Detaching client %s at 0x%x\n",
-			client->name, client->addr);
-		if ((res = driver->detach_client(client))) {
-			dev_err(&isa_adapter.dev, "Failed, driver "
-				"%s not unregistered!\n",
-				driver->driver.name);
-			return res;
-		}
-	}
+	res = device_for_each_child(&isa_adapter.dev, driver,
+			i2c_do_detach_drv_client);
+	if (res != 0)
+		return res;
 
 	/* Get the driver off the core list */
 	driver_unregister(&driver->driver);
@@ -133,9 +125,6 @@ static int __init i2c_isa_init(void)
 {
 	int err;
 
-	mutex_init(&isa_adapter.clist_lock);
-	INIT_LIST_HEAD(&isa_adapter.clients);
-
 	isa_adapter.nr = ANY_I2C_ISA_BUS;
 	isa_adapter.dev.parent = &platform_bus;
 	sprintf(isa_adapter.dev.bus_id, "i2c-%d", isa_adapter.nr);



More information about the i2c mailing list