i2c: Multiplexed I2C bus core support

Add multiplexed bus core support. I2C multiplexer and switches like pca954x get instantiated as new adapters per port.

Signed-off-by Michael Lawnick <demx1175@wbit01lx.(none)>

Cc Jean Delvare <[email protected]>

---
Based on kernel 2.6.35.rc2 +
[PATCH] i2c: Move adapter locking helpers to i2c-core
by Jean Delware <[email protected]>
<http://article.gmane.org/gmane.linux.drivers.i2c/6226/match=i2c+move+adapter+locking+helpers+i2c+core>

 drivers/i2c/Kconfig         |   11 +++
 drivers/i2c/Makefile        |    1 +
 drivers/i2c/i2c-core.c      |   80 +++++++++++++++++----
 drivers/i2c/i2c-dev.c       |   44 +++++++++++-
 drivers/i2c/i2c-mux.c       |  165

+++++++++++++++++++++++++++++++++++++++++++

 include/linux/i2c-mux.h     |   46 ++++++++++++
 include/linux/i2c.h         |    8 ++
 7 files changed, 341 insertions(+), 14 deletions(-)
 create mode 100755 drivers/i2c/i2c-mux.c
 create mode 100755 include/linux/i2c-mux.h

drivers/i2c/Kconfig +11 -0

index d06083f..efb48ad
@@ -47,6 +47,17 @@ config I2C_CHARDEV
 	  This support is also available as a module.  If so, the module
 	  will be called i2c-dev.
 
+config I2C_MUX
+	tristate "I2C bus multiplexing support"
+	depends on EXPERIMENTAL
+	help
+	  Say Y here if you want the I2C core to support the ability to
+	  handle multiplexed I2C bus topologies, by presenting each
+	  multiplexed segment as a I2C adapter.
+
+	  This support is also available as a module.  If so, the module
+	  will be called i2c-mux.
+
 config I2C_HELPER_AUTO
 	bool "Autoselect pertinent helper modules"
 	default y

drivers/i2c/Makefile +1 -0

index a7d9b4b..f363258
@@ -6,6 +6,7 @@ obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
 obj-$(CONFIG_I2C)		+= i2c-core.o
 obj-$(CONFIG_I2C_SMBUS)		+= i2c-smbus.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
+obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
 obj-y				+= algos/ busses/
 
 ifeq ($(CONFIG_I2C_DEBUG_CORE),y)

drivers/i2c/i2c-core.c +35 -3

index 05a147a..d46fb3e
@@ -20,7 +20,9 @@
 /* With some changes from Kyösti Mälkki <[email protected]>.
    All SMBus-related things are written by Frodo Looijaard <[email protected]>
    SMBus 2.0 support by Mark Studebaker <mdsxyz123-/[email protected]> and
-   Jean Delvare <[email protected]> */
+   Jean Delvare <[email protected]>
+   Mux support by Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/[email protected]> and
+   Michael Lawnick <[email protected]> */
 
 #include <linux/module.h>
 #include <linux/kernel.h>

@@ -417,10 +419,52 @@ static int __i2c_check_addr_busy(struct device
 *dev, void *addrp)
 	return 0;
 }
 
+/* walk up mux tree */
+static int i2c_check_mux_parents(struct i2c_adapter *adapter, int addr)
+{
+	int	result;
+
+	result = device_for_each_child(&adapter->dev, &addr,
+					__i2c_check_addr_busy);
+
+	if (!result && i2c_parent_is_i2c_adapter(adapter))
+		result = i2c_check_mux_parents(
+				    to_i2c_adapter(adapter->dev.parent), addr);
+
+	return result;
+}
+
+/* recurse down mux tree */
+static int i2c_check_mux_children(struct device *dev, void *addrp)
+{
+	int result;
+
+	if (dev->type == &i2c_adapter_type)
+		result = device_for_each_child(dev, addrp,
+						i2c_check_mux_children);
+	else
+		result = __i2c_check_addr_busy(dev, addrp);
+
+	return result;
+}
+
 static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
 {
-	return device_for_each_child(&adapter->dev, &addr,
-				     __i2c_check_addr_busy);
+	int	result = 0;
+
+	i2c_lock_adapter(adapter);

drivers/i2c/i2c-core.c +2 -0

+
+	return result;
 }
 

/**

drivers/i2c/i2c-core.c +12 -3

@@ -429,7 +473,10 @@ static int i2c_check_addr_busy(struct i2c_adapter
 *adapter, int addr)
  */
 void i2c_lock_adapter(struct i2c_adapter *adapter)
 {
-	rt_mutex_lock(&adapter->bus_lock);
+	if (i2c_parent_is_i2c_adapter(adapter))
+		i2c_lock_adapter(to_i2c_adapter(adapter->dev.parent));
+	else
+		rt_mutex_lock(&adapter->bus_lock);
 }
 EXPORT_SYMBOL_GPL(i2c_lock_adapter);

@@ -439,7 +486,10 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
  */
 static int i2c_trylock_adapter(struct i2c_adapter *adapter)
 {
-	return rt_mutex_trylock(&adapter->bus_lock);
+	if (i2c_parent_is_i2c_adapter(adapter))
+		return i2c_trylock_adapter(to_i2c_adapter(adapter->dev.parent));
+	else
+		return rt_mutex_trylock(&adapter->bus_lock);
 }
 
 /**

@@ -448,7 +498,10 @@ static int i2c_trylock_adapter(struct i2c_adapter
 *adapter)
  */
 void i2c_unlock_adapter(struct i2c_adapter *adapter)
 {
-	rt_mutex_unlock(&adapter->bus_lock);
+	if (i2c_parent_is_i2c_adapter(adapter))
+		i2c_unlock_adapter(to_i2c_adapter(adapter->dev.parent));
+	else
+		rt_mutex_unlock(&adapter->bus_lock);
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);

@@ -656,9 +709,9 @@ i2c_sysfs_new_device(struct device *dev, struct
 device_attribute *attr,

drivers/i2c/i2c-core.c +1 -1

@@ -697,7 +750,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 
 	/* Make sure the device was added through sysfs */
 	res = -ENOENT;
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_for_each_entry_safe(client, next, &adap->userspace_clients,
 				 detected) {

drivers/i2c/i2c-core.c +1 -1

 		if (client->addr == addr) {

@@ -710,7 +763,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 			break;
 		}
 	}
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 
 	if (res < 0)

drivers/i2c/i2c-core.c +2 -1

 		dev_err(dev, "%s: Can't find device in list\n",

@@ -737,10 +790,11 @@ static const struct attribute_group
 *i2c_adapter_attr_groups[] = {
 	NULL
 };
 
-static struct device_type i2c_adapter_type = {
+struct device_type i2c_adapter_type = {
 	.groups		= i2c_adapter_attr_groups,
 	.release	= i2c_adapter_dev_release,
 };
+EXPORT_SYMBOL_GPL(i2c_adapter_type);
 
 #ifdef CONFIG_I2C_COMPAT

static struct class_compat *i2c_adapter_compat_class;

drivers/i2c/i2c-core.c +2 -2

@@ -995,7 +1049,7 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 		return res;
 
 	/* Remove devices instantiated from sysfs */
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_for_each_entry_safe(client, next, &adap->userspace_clients,
 				 detected) {
 		dev_dbg(&adap->dev, "Removing %s at 0x%x\n", client->name,

@@ -1003,7 +1057,7 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 		list_del(&client->detected);
 		i2c_unregister_device(client);
 	}
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 
 	/* Detach any active clients. This can't fail, thus we do not
 	   checking the returned value. */

drivers/i2c/i2c-dev.c +43 -1

index e0694e4..6717bba
@@ -193,12 +193,54 @@ static int i2cdev_check(struct device *dev, void
 *addrp)
 	return dev->driver ? -EBUSY : 0;
 }
 
+/* walk up mux tree */
+static int i2cdev_check_mux_parents(struct i2c_adapter *adapter, int addr)
+{
+	int	result;
+
+	result = device_for_each_child(&adapter->dev, &addr, i2cdev_check);
+
+	if (!result && i2c_parent_is_i2c_adapter(adapter))
+		result = i2cdev_check_mux_parents(
+				    to_i2c_adapter(adapter->dev.parent), addr);
+
+	return result;
+}
+
+/* recurse down mux tree */
+static int i2cdev_check_mux_children(struct device *dev, void *addrp)
+{
+	int result;
+
+	if (dev->type == &i2c_adapter_type)
+		result = device_for_each_child(dev, addrp,
+						i2cdev_check_mux_children);
+	else
+		result = i2cdev_check(dev, addrp);
+
+	return result;
+}
+
 /* This address checking function differs from the one in i2c-core
    in that it considers an address with a registered device, but no
    driver bound to it, as NOT busy. */
 static int i2cdev_check_addr(struct i2c_adapter *adapter, unsigned int
 addr)
 {
-	return device_for_each_child(&adapter->dev, &addr, i2cdev_check);
+	int	result = 0;
+
+	i2c_lock_adapter(adapter);
+
+	if (i2c_parent_is_i2c_adapter(adapter))
+		result = i2cdev_check_mux_parents(
+				    to_i2c_adapter(adapter->dev.parent), addr);
+
+	if (!result)
+		result = device_for_each_child(&adapter->dev, &addr,
+						i2cdev_check_mux_children);
+
+	i2c_unlock_adapter(adapter);
+
+	return result;
 }

static noinline int i2cdev_ioctl_rdrw(struct i2c_client *client,

drivers/i2c/i2c-mux.c new file +8 -0

new file mode 100755
index 0000000..d32a484
@@ -0,0 +1,165 @@
+/*
+ * Multiplexed I2C bus driver.
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <[email protected]>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <[email protected]>
+ * Copyright (c) 2009-2010 NSN GmbH & Co KG <[email protected]>
+ *
+ * Simplifies access to complex multiplexed I2C bus topologies, by

presenting
+ * each multiplexed bus segment as an additional I2C adapter.
+ * Supports multi-level mux'ing (mux behind a mux).
+ *
+ * Based on:
+ * i2c-virt.c from Kumar Gala <[email protected]>
+ * i2c-virtual.c from Ken Harrenstien, Copyright (c) 2004 Google, Inc.
+ * i2c-virtual.c from Brian Kuschak <bkuschak-/[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */

drivers/i2c/i2c-mux.c new file +145 -0

+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+
+/* multiplexer per channel data */
+struct i2c_mux_priv {
+	struct i2c_adapter adap;
+	struct i2c_algorithm algo;
+
+	struct i2c_adapter *parent;
+	void *mux_dev;	/* the mux chip/device */
+	u32  chan_id;	/* the channel id */
+
+	int (*select)(struct i2c_adapter *, void *mux_dev, u32 chan_id);
+	int (*deselect)(struct i2c_adapter *, void *mux_dev, u32 chan_id);
+};
+
+static int i2c_mux_master_xfer(struct i2c_adapter *adap,
+			       struct i2c_msg msgs[], int num)
+{
+	struct i2c_mux_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->parent;
+	int ret;
+
+	/* Switch to the right mux port and perform the transfer. */
+
+	ret = priv->select(parent, priv->mux_dev, priv->chan_id);
+	if (ret >= 0)
+		ret = parent->algo->master_xfer(parent, msgs, num);
+	if (priv->deselect)
+		priv->deselect(parent, priv->mux_dev, priv->chan_id);
+
+	return ret;
+}
+
+static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
+			      u16 addr, unsigned short flags,
+			      char read_write, u8 command,
+			      int size, union i2c_smbus_data *data)
+{
+	struct i2c_mux_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->parent;
+	int ret;
+
+	/* Select the right mux port and perform the transfer. */
+
+	ret = priv->select(parent, priv->mux_dev, priv->chan_id);
+	if (ret >= 0)
+		ret = parent->algo->smbus_xfer(parent, addr, flags,
+					read_write, command, size, data);
+	if (priv->deselect)
+		priv->deselect(parent, priv->mux_dev, priv->chan_id);
+
+	return ret;
+}
+
+/* Return the parent's functionality */
+static u32 i2c_mux_functionality(struct i2c_adapter *adap)
+{
+	struct i2c_mux_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->parent;
+
+	return parent->algo->functionality(parent);
+}
+
+struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+				void *mux_dev, u32 force_nr, u32 chan_id,
+				int (*select) (struct i2c_adapter *,
+					       void *, u32),
+				int (*deselect) (struct i2c_adapter *,
+						 void *, u32))
+{
+	struct i2c_mux_priv *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(struct i2c_mux_priv), GFP_KERNEL);
+	if (!priv)
+		return NULL;
+
+	/* Set up private adapter data */
+	priv->parent = parent;
+	priv->mux_dev = mux_dev;
+	priv->chan_id = chan_id;
+	priv->select = select;
+	priv->deselect = deselect;
+
+	/* Need to do algo dynamically because we don't know ahead
+	 * of time what sort of physical adapter we'll be dealing with.
+	 */
+	if (parent->algo->master_xfer)
+		priv->algo.master_xfer = i2c_mux_master_xfer;
+	if (parent->algo->smbus_xfer)
+		priv->algo.smbus_xfer = i2c_mux_smbus_xfer;
+	priv->algo.functionality = i2c_mux_functionality;
+
+	/* Now fill out new adapter structure */
+	snprintf(priv->adap.name, sizeof(priv->adap.name),
+		 "i2c-%d-mux (chan_id %d)", i2c_adapter_id(parent), chan_id);
+	priv->adap.owner = THIS_MODULE;
+	priv->adap.id = parent->id;
+	priv->adap.algo = &priv->algo;
+	priv->adap.algo_data = priv;
+	priv->adap.dev.parent = &parent->dev;
+
+	if (force_nr) {
+		priv->adap.nr = force_nr;
+		ret = i2c_add_numbered_adapter(&priv->adap);
+	} else {
+		ret = i2c_add_adapter(&priv->adap);
+	}
+	if (ret < 0) {
+		dev_err(&parent->dev,
+			"failed to add mux-adapter (error=%d)\n",
+			ret);
+		kfree(priv);
+		return NULL;
+	}
+
+	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
+		 i2c_adapter_id(&priv->adap));
+
+	return &priv->adap;
+}
+EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
+
+int i2c_del_mux_adapter(struct i2c_adapter *adap)
+{
+	struct i2c_mux_priv *priv = adap->algo_data;
+	int ret;
+
+	ret = i2c_del_adapter(adap);
+	if (ret < 0)
+		return ret;
+	kfree(priv);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_del_mux_adapter);
+
+MODULE_AUTHOR("Rodolfo Giometti <[email protected]>");
+MODULE_DESCRIPTION("I2C driver for multiplexed I2C busses");
+MODULE_LICENSE("GPL v2");

include/linux/i2c-mux.h new file +46 -0

new file mode 100755
index 0000000..4c52ee1
@@ -0,0 +1,46 @@
+/*
+ *
+ * i2c-mux.h - functions for the i2c-bus mux support
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <[email protected]>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <[email protected]>
+ * Michael Lawnick <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef _LINUX_I2C_MUX_H
+#define _LINUX_I2C_MUX_H
+
+#ifdef __KERNEL__
+
+/*
+ * Called to create a i2c bus on a multiplexed bus segment.
+ * The mux_dev and chan_id parameters are passed to the select
+ * and deselect callback functions to perform hardware-specific
+ * mux control.
+ */
+struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+				void *mux_dev, u32 force_nr, u32 chan_id,
+				int (*select) (struct i2c_adapter *adap,
+				void *mux_dev, u32 chan_id),
+				int (*deselect) (struct i2c_adapter *,
+				void *mux_dev, u32 chan_id));
+
+int i2c_del_mux_adapter(struct i2c_adapter *adap);
+
+#endif /* __KERNEL__ */
+
+#endif /* _LINUX_I2C_MUX_H */

include/linux/i2c.h +8 -0

index f2d36bd..8f2b83a
@@ -37,6 +37,7 @@
 #include <linux/of.h>		/* for struct device_node */
 
 extern struct bus_type i2c_bus_type;
+extern struct device_type i2c_adapter_type;
 
 /* --- General options ------------------------------------------------	*/
 

@@ -374,6 +375,13 @@ static inline void i2c_set_adapdata(struct
 i2c_adapter *dev, void *data)
 	dev_set_drvdata(&dev->dev, data);
 }
 
+static inline int i2c_parent_is_i2c_adapter(const struct i2c_adapter
 *adapter)
+{
+	return adapter->dev.parent != NULL
+		&& adapter->dev.parent->bus == &i2c_bus_type
+		&& adapter->dev.parent->type == &i2c_adapter_type;
+}
+
 /* Adapter locking functions, exported for shared pin cases */

void i2c_lock_adapter(struct i2c_adapter *);
void i2c_unlock_adapter(struct i2c_adapter *);

Hi Michael,

Quoted 2 lines

 drivers/i2c/i2c-mux.c       |  165

Quoted 1 line

+++++++++++++++++++++++++++++++++++++++++++

As you can see, your e-mail client wrapped long lines again. I had to manually fix the patch so that I could apply it...

Review i2c_sysfs_new_device(struct device *dev, struct · line 717

Quoted 13 lines

(quoted diff) +2 -2

@@ -656,9 +709,9 @@ i2c_sysfs_new_device(struct device *dev, struct
 device_attribute *attr,
 		return -EINVAL;
 
 	/* Keep track of the added device */
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_add_tail(&client->detected, &adap->userspace_clients);
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 	dev_info(dev, "%s: Instantiated device %s at 0x%02hx\n", "new_device",
 		 info.type, info.addr);

I expected us to just use i2c_lock/unlock_adapter() everywhere for simplicity. Now I have to agree that using the segment's mutex works too, as the operation is both local to the mutex and unrelated to the other use cases of i2c_lock/unlock_adapter(). But it becomes a little tricky, so it should be all documented clearly (which I will do, don't worry.)

Quoted 18 lines

(quoted diff) +1 -1

@@ -697,7 +750,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 
 	/* Make sure the device was added through sysfs */
 	res = -ENOENT;
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_for_each_entry_safe(client, next, &adap->userspace_clients,
 				 detected) {

(quoted diff) +1 -1

 		if (client->addr == addr) {

@@ -710,7 +763,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 			break;
 		}
 	}
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 

Quoted 2 lines

(quoted diff) +0 -0

 	if (res < 0)

dev_err(dev, "%s: Can't find device in list\n",

Jean Delvare said the following:

Quoted 39 lines

Review i2c_sysfs_new_device(struct device *dev, struct · line 717

Quoted 13 lines

(quoted diff) +2 -2

@@ -656,9 +709,9 @@ i2c_sysfs_new_device(struct device *dev, struct
 device_attribute *attr,
 		return -EINVAL;
 
 	/* Keep track of the added device */
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_add_tail(&client->detected, &adap->userspace_clients);
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 	dev_info(dev, "%s: Instantiated device %s at 0x%02hx\n", "new_device",
 		 info.type, info.addr);

I expected us to just use i2c_lock/unlock_adapter() everywhere for simplicity. Now I have to agree that using the segment's mutex works too, as the operation is both local to the mutex and unrelated to the other use cases of i2c_lock/unlock_adapter(). But it becomes a little tricky, so it should be all documented clearly (which I will do, don't worry.)

Quoted 18 lines

(quoted diff) +1 -1

@@ -697,7 +750,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 
 	/* Make sure the device was added through sysfs */
 	res = -ENOENT;
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_for_each_entry_safe(client, next, &adap->userspace_clients,
 				 detected) {

(quoted diff) +1 -1

 		if (client->addr == addr) {

@@ -710,7 +763,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 			break;
 		}
 	}
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 

Quoted 2 lines

Quoted 2 lines

(quoted diff) +0 -0

 	if (res < 0)

dev_err(dev, "%s: Can't find device in list\n",

In i2c_sysfs_delete_device you need a local lock, otherwise you'll get a deadlock on removing sub-clients/tree. This in turn brings the local lock to i2c_sysfs_new_device().

Thank you for your review.
ToDo after release: Inventing mux-adapter-name definition in mux-client code instead of mux-code. You were right in your last review, this is terribly necessary.

Quoted 19 lines

Jean Delvare said the following:

Quoted 18 lines

Quoted 18 lines

(quoted diff) +1 -1

@@ -697,7 +750,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 
 	/* Make sure the device was added through sysfs */
 	res = -ENOENT;
-	i2c_lock_adapter(adap);
+	rt_mutex_lock(&adap->bus_lock);
 	list_for_each_entry_safe(client, next, &adap->userspace_clients,
 				 detected) {

(quoted diff) +1 -1

 		if (client->addr == addr) {

@@ -710,7 +763,7 @@ i2c_sysfs_delete_device(struct device *dev, struct
 device_attribute *attr,
 			break;
 		}
 	}
-	i2c_unlock_adapter(adap);
+	rt_mutex_unlock(&adap->bus_lock);
 

Quoted 6 lines

Quoted 2 lines

Quoted 2 lines

(quoted diff) +0 -0

 	if (res < 0)

dev_err(dev, "%s: Can't find device in list\n",

In i2c_sysfs_delete_device you need a local lock, otherwise you'll get a deadlock on removing sub-clients/tree. This in turn brings the local lock to i2c_sysfs_new_device().

This is only relevant if the device instantiated / removed from user-space is an I2C mux chip, right?

Please remember that i2c_lock_adapter() and rt_mutex_lock() might do exactly the same, if applied to the root segment of an I2C tree. So if i2c_lock_adapter() would deadlock, I fear that a simple rt_mutex_lock() might deadlock too. So in the end we might have to introduce another mutex dedicated to protecting the adapter->userspace_clients list. Maybe we should have done this from the beginning...

Add multiplexed bus core support. I2C multiplexer and switches like pca954x get instantiated as new adapters per port.

Signed-off-by Michael Lawnick <demx1175@wbit01lx.(none)>

Cc Jean Delvare <[email protected]>

---
Based on kernel 2.6.35.rc2 +
[PATCH] i2c: Move adapter locking helpers to i2c-core
by Jean Delware <[email protected]>
<http://article.gmane.org/gmane.linux.drivers.i2c/6226/match=i2c+move+adapter+locking+helpers+i2c+core>

 drivers/i2c/Kconfig         |   11 +++
 drivers/i2c/Makefile        |    1 +
 drivers/i2c/i2c-core.c      |   80 +++++++++++++++++----
 drivers/i2c/i2c-dev.c       |   44 +++++++++++-
 drivers/i2c/i2c-mux.c       |  165 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-mux.h     |   46 ++++++++++++
 include/linux/i2c.h         |    8 ++
 7 files changed, 341 insertions(+), 14 deletions(-)
 create mode 100755 drivers/i2c/i2c-mux.c
 create mode 100755 include/linux/i2c-mux.h

Hi Michael,

Patch applied, with two changes:
* Reverted changes to i2c_sysfs_new_device(), i2c_sysfs_delete_device() and i2c_del_adapter() - no longer needed after I added a dedicated mutex for userspace client lists.
* Removed locking in i2c_check_addr_busy() and i2cdev_check_addr(). It was insufficient anyway, I'll think of something better and add it later.

Rodoflo, I've added your Acked-by, I hope this is fine.

Resulting patch can be seen here:
ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-multiplexing-core-support.patch

Thanks again for your contribution and patience :)

Michael, maybe you wish updating the linux-i2c wiki page regarding this issue?

https://i2c.wiki.kernel.org/index.php/I2C_bus_multiplexing

Quoted 4 lines · Rodolfo Giometti

Michael, maybe you wish updating the linux-i2c wiki page regarding this issue?

https://i2c.wiki.kernel.org/index.php/I2C_bus_multiplexing

Or maybe you could do it, Rodolfo? Michael is apparently busy and so am I.

Quoted 4 lines · Rodolfo Giometti

Quoted 3 lines

Michael, maybe you wish updating the linux-i2c wiki page regarding this issue?

Quoted 4 lines

Quoted 1 line

https://i2c.wiki.kernel.org/index.php/I2C_bus_multiplexing

Or maybe you could do it, Rodolfo? Michael is apparently busy and so am I.

It's ok for me. I just suggested Michael since he did the last (hard) work but if you agree I have no problem in doing it. :)

Add multiplexed bus core support. I2C multiplexer and switches like pca954x get instantiated as new adapters per port.

Signed-off-by Michael Lawnick<demx1175@wbit01lx.(none)>

Cc Jean Delvare<khali-PUYAD+kWke1g9hUCZPvPmw-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>

Hi everyone,

Quoted 5 lines · Michael Lawnick

Add multiplexed bus core support. I2C multiplexer and switches like pca954x get instantiated as new adapters per port.

Signed-off-by Michael Lawnick<demx1175@wbit01lx.(none)>

Cc Jean Delvare<khali-PUYAD+kWke1g9hUCZPvPmw-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>

I noticed that when you have multiple muxes connected to the same branch, implementation hides some information about the complete i2c topology.

So for instance if I have something like (notice there are three PCA9544's "in parallel", each with its own address, which I assume is a bit unusual):

# CPU (i2c-0)
# '--I2C Mux0 (PCA9547) @ 0x70

#      +--Port0 (i2c-1)
#      +--Port1 (i2c-2)
#      +--Port2 (i2c-3)
#      +--Port3 (i2c-4)
#      +--Port4 (i2c-5)
#      |   '--- I2C Mux1 (PCA9544) @ 0x74
#      |   |     +--Port0 (i2c-9)
#      |   |     +--Port1 (i2c-10)
#      |   |     +--Port2 (i2c-11)
#      |   |     '--Port3 (i2c-12)
#      |   +--- I2C Mux2 (PCA9544) @ 0x71
#      |   |     +--Port0 (i2c-13)
#      |   |     +--Port1 (i2c-14)
#      |   |     +--Port2 (i2c-15)
#      |   |     '--Port3 (i2c-16)

which only reveals i2c-9 is the bus segment connected to channel 0 of a mux behind i2c-5, but since there's two of them, I have no way to tell which one...

So I guess the easiest thing to do would be to add this information within the .name attribute, though I fear this may somehow turn out not as trivial at it looks.

devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-0(=>...i2c-9) devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-1(=>...i2c-10) devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-2(=>...i2c-11) devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-3(=>...i2c-12)

On my project I do no automatic i2c detection, but all is controlled via user space. So I always know where and which devices and buses get connected.

Quoted 4 lines

devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-0(=>...i2c-9) devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-1(=>...i2c-10) devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-2(=>...i2c-11) devices/platform/i2c-gpio/i2c-0/0-0070/chan-4(=>...i2c-5)/5-0071/chan-3(=>...i2c-12)

Introducing links to sub-buses, named with respective channel id sounds good for me. Beside that your link example does not reflect your ASCII graphics of course ;-)

Hi Michael,

first of all thanks for your answer :-)

On 02/18/2013 11:19 AM, Michael Lawnick wrote:

Quoted 3 lines

On my project I do no automatic i2c detection, but all is controlled via user space. So I always know where and which devices and buses get connected.

You mean you add devices through sysfs? That's another approach I was thinking I'd follow, except it still doesn't solve my problem... meaning I have no reliable way of telling the numbers assigned to the busses added.