[i2c] i2c-dev and I2C address business

[Jean Delvare]

Hi David,

With the introduction of the new-style i2c drivers, we changed the
definition of what a busy I2C address is for i2c-dev. Beforehand,
having an i2c_client registered at a given address meant that this
address was busy. Now, what makes an address busy is having an
i2c_client registered _and_ a driver bound to it.

We can't just change the condition in __i2c_check_addr, because the
other address checks in i2c-core would be affected. So I added a
dedicated address checking function in i2c-dev:

Signed-off-by: Jean Delvare <khali at linux-fr.org>
---
 drivers/i2c/i2c-dev.c |   26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

--- linux-2.6.22-rc3.orig/drivers/i2c/i2c-dev.c	2007-06-04 10:09:38.000000000 +0200
+++ linux-2.6.22-rc3/drivers/i2c/i2c-dev.c	2007-06-05 09:01:33.000000000 +0200
@@ -154,6 +154,29 @@ static ssize_t i2cdev_write (struct file
 	return ret;
 }
 
+/* This address checking function differs from the one in i2c-core
+   in that it considers an address with a registered device, but no
+   bounded driver, as NOT busy. */
+static int i2cdev_check_addr(struct i2c_adapter *adapter, unsigned int addr)
+{
+	struct list_head *item;
+	struct i2c_client *client;
+	int res = 0;
+
+	mutex_lock(&adapter->clist_lock);
+	list_for_each(item, &adapter->clients) {
+		client = list_entry(item, struct i2c_client, list);
+		if (client->addr == addr) {
+			if (client->driver)
+				res = -EBUSY;
+			break;
+		}
+	}
+	mutex_unlock(&adapter->clist_lock);
+
+	return res;
+}
+
 static int i2cdev_ioctl(struct inode *inode, struct file *file,
 		unsigned int cmd, unsigned long arg)
 {
@@ -175,8 +198,9 @@ static int i2cdev_ioctl(struct inode *in
 		if ((arg > 0x3ff) ||
 		    (((client->flags & I2C_M_TEN) == 0) && arg > 0x7f))
 			return -EINVAL;
-		if ((cmd == I2C_SLAVE) && i2c_check_addr(client->adapter,arg))
+		if (cmd == I2C_SLAVE && i2cdev_check_addr(client->adapter, arg))
 			return -EBUSY;
+		/* REVISIT: address could become busy later */
 		client->addr = arg;
 		return 0;
 	case I2C_TENBIT:

Now this made me realize (hence the comment) that this i2c-dev code
isn't safe at all, because it does not register the i2c_client it is
using. Another i2c-dev instance could access the same address without a
notice. Or a driver could later be bound to the I2C device in question.
While this is probably acceptable for I2C_SLAVE_FORCE, as it is unsafe
by design, it isn't acceptable for I2C_SLAVE. It sounds like the right
time to think about it and address the issue.

I see two different cases here. First case, we have an address without
a device. We should instantiate a device, and have a driver ready for
it. Shouldn't be too hard. Second case, we have an address with a
device already declared, but no driver bound to it. We need to somehow
force our driver to attach to this device. The problem is that I don't
know how to do that. I'm not even sure if the device driver model
allows this. David, can you please help with this?

Thanks,
-- 
Jean Delvare