[PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

Add multiplexed bus core support. I2C multiplexer and switches
like pca954x instantiate new adapters per output.

Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
---
Based on kernel 2.6.33 +
[PATCH] i2c-core: Use per-adapter userspace device lists
by Jean Delware <20100424201914.08b3f008-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
<http://article.gmane.org/gmane.linux.drivers.i2c/5994/>

From 1d17617d8845f2268d4123ec0cf5310743303d45 Mon Sep 17 00:00:00 2001
From: Michael Lawnick <demx1175@demmc3qc.(none)>
Date: Tue, 4 May 2010 13:34:52 +0200
Subject: [PATCH] mux_core_support

---
 drivers/i2c/Kconfig     |    8 ++
 drivers/i2c/Makefile    |    1 +
 drivers/i2c/i2c-core.c  |   93 ++++++++++++++++++++++--
 drivers/i2c/i2c-dev.c   |   46 ++++++++++++-
 drivers/i2c/i2c-mux.c   |  184
+++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-mux.h |   46 ++++++++++++
 include/linux/i2c.h     |   11 +++
 7 files changed, 381 insertions(+), 8 deletions(-)
 create mode 100755 drivers/i2c/i2c-mux.c
 create mode 100755 include/linux/i2c-mux.h

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 8d8a00e..2cd6d78 100755
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -36,6 +36,14 @@ config I2C_COMPAT
 	  other user-space package which expects i2c adapters to be class
 	  devices. If you don't know, say Y.

+config I2C_MUX
+	tristate "I2C bus multiplexing support"
+	depends on I2C
+	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.
+
 config I2C_CHARDEV
 	tristate "I2C device interface"
 	help
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index ba26e6c..a488e8b 100755
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
 obj-$(CONFIG_I2C)		+= i2c-core.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
+obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
 obj-y				+= busses/ chips/ algos/

 ifeq ($(CONFIG_I2C_DEBUG_CORE),y)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index fc93e28..682966a 100755
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -20,7 +20,10 @@
 /* With some changes from Kyösti Mälkki <kmalkki@cc.hut.fi>.
    All SMBus-related things are written by Frodo Looijaard <frodol@dds.nl>
    SMBus 2.0 support by Mark Studebaker <mdsxyz123-/E1597aS9LQAvxtiuMwx3w@public.gmane.org> and
-   Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> */
+   Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
+   Mux support by Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org> and
+   Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org> */
+

 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -288,6 +291,12 @@ struct i2c_client *i2c_verify_client(struct device
*dev)
 }
 EXPORT_SYMBOL(i2c_verify_client);

+static int i2c_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
+{
+	return adapter->dev.parent != NULL
+		&& adapter->dev.parent->bus == &i2c_bus_type;
+}
+

 /**
  * i2c_new_device - instantiate an i2c device
@@ -624,6 +633,7 @@ static int i2c_register_adapter(struct i2c_adapter
*adap)

 	rt_mutex_init(&adap->bus_lock);
 	INIT_LIST_HEAD(&adap->userspace_clients);
+	INIT_LIST_HEAD(&adap->mux_list_head);

 	/* Set default timeout to 1 second if not already set */
 	if (adap->timeout == 0)
@@ -949,9 +959,78 @@ static int __i2c_check_addr(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);
+
+	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_childs(struct i2c_adapter *adapter, int addr)
+{
+	int			result = 0;
+	struct i2c_adapter	*child, *next;
+
+	list_for_each_entry_safe(child, next, &adapter->mux_list_head,
+							 mux_list) {
+		result = device_for_each_child(&child->dev, &addr,
+							     __i2c_check_addr);
+		if (result)
+			break;
+		result = i2c_check_mux_childs(child, addr);
+	}
+	return result;
+}
+
 static int i2c_check_addr(struct i2c_adapter *adapter, int addr)
 {
-	return device_for_each_child(&adapter->dev, &addr, __i2c_check_addr);
+	int	result;
+
+	result = i2c_check_mux_parents(adapter, addr);
+
+	if (!result)
+		result = i2c_check_mux_childs(adapter, addr);
+
+	return result;
+}
+
+static void i2c_mux_tree_lock(struct i2c_adapter *adap)
+{
+	if (i2c_parent_is_i2c_adapter(adap))
+		i2c_mux_tree_lock(to_i2c_adapter(adap->dev.parent));
+	else
+		i2c_lock_adapter(adap);
+}
+
+static int i2c_mux_tree_trylock(struct i2c_adapter *adap)
+{
+	int			ret;
+	struct i2c_adapter	*parent;
+
+	if (i2c_parent_is_i2c_adapter(adap)) {
+		parent = to_i2c_adapter(adap->dev.parent);
+		ret = i2c_mux_tree_trylock(parent);
+	} else {
+		ret = i2c_trylock_adapter(adap);
+	}
+
+	return ret;
+}
+
+static void i2c_mux_tree_unlock(struct i2c_adapter *adap)
+{
+	if (i2c_parent_is_i2c_adapter(adap))
+		i2c_mux_tree_unlock(to_i2c_adapter(adap->dev.parent));
+	else
+		i2c_unlock_adapter(adap);
 }

 /**
@@ -1104,12 +1183,12 @@ int i2c_transfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
 #endif

 		if (in_atomic() || irqs_disabled()) {
-			ret = rt_mutex_trylock(&adap->bus_lock);
+			ret = i2c_mux_tree_trylock(adap);
 			if (!ret)
 				/* I2C activity is ongoing. */
 				return -EAGAIN;
 		} else {
-			rt_mutex_lock(&adap->bus_lock);
+			i2c_mux_tree_lock(adap);
 		}

 		/* Retry automatically on arbitration loss */
@@ -1121,7 +1200,7 @@ int i2c_transfer(struct i2c_adapter *adap, struct
i2c_msg *msgs, int num)
 			if (time_after(jiffies, orig_jiffies + adap->timeout))
 				break;
 		}
-		rt_mutex_unlock(&adap->bus_lock);
+		i2c_mux_tree_unlock(adap);

 		return ret;
 	} else {
@@ -1857,7 +1936,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter,
u16 addr, unsigned short flags,
 	flags &= I2C_M_TEN | I2C_CLIENT_PEC;

 	if (adapter->algo->smbus_xfer) {
-		rt_mutex_lock(&adapter->bus_lock);
+		i2c_mux_tree_lock(adapter);

 		/* Retry automatically on arbitration loss */
 		orig_jiffies = jiffies;
@@ -1871,7 +1950,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter,
u16 addr, unsigned short flags,
 				       orig_jiffies + adapter->timeout))
 				break;
 		}
-		rt_mutex_unlock(&adapter->bus_lock);
+		i2c_mux_tree_unlock(adapter);
 	} else
 		res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write,
 					      command, protocol, data);
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index f4110aa..da5327f 100755
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -193,12 +193,56 @@ static int i2cdev_check(struct device *dev, void
*addrp)
 	return dev->driver ? -EBUSY : 0;
 }

+static int i2cdev_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
+{
+	return adapter->dev.parent != NULL
+		&& adapter->dev.parent->bus == &i2c_bus_type;
+}
+
+/* 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 && i2cdev_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_childs(struct i2c_adapter *adapter, int addr)
+{
+	int result = 0;
+	struct i2c_adapter *child, *next;
+
+	list_for_each_entry_safe(child, next, &adapter->mux_list_head,
+				 mux_list) {
+		result = device_for_each_child(&child->dev, &addr,
+					       i2cdev_check);
+		if (result)
+			break;
+		result = i2cdev_check_mux_childs(child, addr);
+	}
+	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;
+
+	result = i2cdev_check_mux_parents(adapter, addr);
+
+	if (!result)
+		result = i2cdev_check_mux_childs(adapter, addr);
+
+	return result;
 }

 static noinline int i2cdev_ioctl_rdrw(struct i2c_client *client,
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
new file mode 100755
index 0000000..f88d29a
--- /dev/null
+++ b/drivers/i2c/i2c-mux.c
@@ -0,0 +1,184 @@
+/*
+ * Multiplexed I2C bus driver.
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
+ *
+ * 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 <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
+ *	i2c-virtual.c from Ken Harrenstien, Copyright (c) 2004 Google, Inc.
+ *	i2c-virtual.c from Brian Kuschak <bkuschak-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
+ *
+ * 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.
+ */
+
+/* Adapted to kernel 2.6.33 by Michael Lawnick
<michael.lawnick.ext-OYasijW0DpE@public.gmane.org> */
+
+#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 *adap, void *mux_dev, u32 chan_id);
+	int (*deselect)(struct i2c_adapter *adap, void *mux_dev, u32 chan_id);
+};
+
+#define VIRT_TIMEOUT		(HZ/2)
+#define VIRT_RETRIES		3
+
+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 for the virtual adapter */
+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;
+	}
+
+	list_add_tail(&priv->adap.mux_list, &parent->mux_list_head);
+
+	dev_info(&parent->dev, "i2c-mux-%d: Multiplexed I2C bus on "
+		"parent bus i2c-%d, chan_id %d\n",
+		i2c_adapter_id(&priv->adap), i2c_adapter_id(parent), chan_id);
+
+	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;
+	struct i2c_adapter *child, *next, *parent;
+
+	if (adap->dev.parent != NULL
+		&& adap->dev.parent->bus == &i2c_bus_type) {
+		parent = to_i2c_adapter(adap->dev.parent);
+		list_for_each_entry_safe(child, next, &parent->mux_list_head,
+					 mux_list) {
+			if (child == adap) {
+				list_del(&child->mux_list);
+				break;
+			}
+		}
+	}
+
+	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 <giometti-k2GhghHVRtY@public.gmane.org>");
+MODULE_DESCRIPTION("I2C driver for multiplexed I2C busses");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
new file mode 100755
index 0000000..0d5d6c8
--- /dev/null
+++ b/include/linux/i2c-mux.h
@@ -0,0 +1,46 @@
+/*
+ *
+ * i2c-mux.h - functions for the i2c-bus mux support
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
+ * Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org>
+ *
+ * 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_H */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 1fd7b12..898a8c1 100755
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -349,6 +349,8 @@ struct i2c_adapter {
 	struct completion dev_released;

 	struct list_head userspace_clients;
+	struct list_head mux_list_head;
+	struct list_head mux_list;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)

@@ -380,6 +382,15 @@ static inline void i2c_unlock_adapter(struct
i2c_adapter *adapter)
 	rt_mutex_unlock(&adapter->bus_lock);
 }

+/**
+ * i2c_trylock_adapter - Try to prevent access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline int i2c_trylock_adapter(struct i2c_adapter *adapter)
+{
+	return rt_mutex_trylock(&adapter->bus_lock);
+}
+
 /*flags for the client struct: */
 #define I2C_CLIENT_PEC	0x04		/* Use Packet Error Checking */
 #define I2C_CLIENT_TEN	0x10		/* we have a ten bit chip address */

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Rodolfo Giometti]

On Tue, May 04, 2010 at 02:29:21PM +0200, Michael Lawnick wrote:
> Add multiplexed bus core support. I2C multiplexer and switches
> like pca954x instantiate new adapters per output.
> 
> Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>

Acked-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>

-- 

GNU/Linux Solutions                  e-mail: giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org
Linux Device Driver                          giometti-k2GhghHVRtY@public.gmane.org
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

Seems like Thunderbird isn't usable for sending patches :-(
Trying again with attachment.

[PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: 0001-mux_core_support.patch --]
[-- Type: text/plain, Size: 16889 bytes --]

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

Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>

---
Based on kernel 2.6.33 +
[PATCH] i2c-core: Use per-adapter userspace device lists
by Jean Delware <20100424201914.08b3f008-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
<http://article.gmane.org/gmane.linux.drivers.i2c/5994/>

 drivers/i2c/Kconfig     |    8 ++
 drivers/i2c/Makefile    |    1 +
 drivers/i2c/i2c-core.c  |   93 ++++++++++++++++++++++--
 drivers/i2c/i2c-dev.c   |   46 ++++++++++++-
 drivers/i2c/i2c-mux.c   |  184 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-mux.h |   46 ++++++++++++
 include/linux/i2c.h     |   11 +++
 7 files changed, 381 insertions(+), 8 deletions(-)
 create mode 100755 drivers/i2c/i2c-mux.c
 create mode 100755 include/linux/i2c-mux.h

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 8d8a00e..2cd6d78 100755
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -36,6 +36,14 @@ config I2C_COMPAT
 	  other user-space package which expects i2c adapters to be class
 	  devices. If you don't know, say Y.
 
+config I2C_MUX
+	tristate "I2C bus multiplexing support"
+	depends on I2C
+	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.
+
 config I2C_CHARDEV
 	tristate "I2C device interface"
 	help
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index ba26e6c..a488e8b 100755
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
 obj-$(CONFIG_I2C)		+= i2c-core.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
+obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
 obj-y				+= busses/ chips/ algos/
 
 ifeq ($(CONFIG_I2C_DEBUG_CORE),y)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index fc93e28..682966a 100755
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -20,7 +20,10 @@
 /* With some changes from Kyösti Mälkki <kmalkki-kf+aQKke1yb1KXRcyAk9cg@public.gmane.org>.
    All SMBus-related things are written by Frodo Looijaard <frodol-B0qZmFHriGg@public.gmane.org>
    SMBus 2.0 support by Mark Studebaker <mdsxyz123-/E1597aS9LQAvxtiuMwx3w@public.gmane.org> and
-   Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> */
+   Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
+   Mux support by Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org> and
+   Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org> */
+
 
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -288,6 +291,12 @@ struct i2c_client *i2c_verify_client(struct device *dev)
 }
 EXPORT_SYMBOL(i2c_verify_client);
 
+static int i2c_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
+{
+	return adapter->dev.parent != NULL
+		&& adapter->dev.parent->bus == &i2c_bus_type;
+}
+
 
 /**
  * i2c_new_device - instantiate an i2c device
@@ -624,6 +633,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 
 	rt_mutex_init(&adap->bus_lock);
 	INIT_LIST_HEAD(&adap->userspace_clients);
+	INIT_LIST_HEAD(&adap->mux_list_head);
 
 	/* Set default timeout to 1 second if not already set */
 	if (adap->timeout == 0)
@@ -949,9 +959,78 @@ static int __i2c_check_addr(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);
+
+	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_childs(struct i2c_adapter *adapter, int addr)
+{
+	int			result = 0;
+	struct i2c_adapter	*child, *next;
+
+	list_for_each_entry_safe(child, next, &adapter->mux_list_head,
+							 mux_list) {
+		result = device_for_each_child(&child->dev, &addr,
+							     __i2c_check_addr);
+		if (result)
+			break;
+		result = i2c_check_mux_childs(child, addr);
+	}
+	return result;
+}
+
 static int i2c_check_addr(struct i2c_adapter *adapter, int addr)
 {
-	return device_for_each_child(&adapter->dev, &addr, __i2c_check_addr);
+	int	result;
+
+	result = i2c_check_mux_parents(adapter, addr);
+
+	if (!result)
+		result = i2c_check_mux_childs(adapter, addr);
+
+	return result;
+}
+
+static void i2c_mux_tree_lock(struct i2c_adapter *adap)
+{
+	if (i2c_parent_is_i2c_adapter(adap))
+		i2c_mux_tree_lock(to_i2c_adapter(adap->dev.parent));
+	else
+		i2c_lock_adapter(adap);
+}
+
+static int i2c_mux_tree_trylock(struct i2c_adapter *adap)
+{
+	int			ret;
+	struct i2c_adapter	*parent;
+
+	if (i2c_parent_is_i2c_adapter(adap)) {
+		parent = to_i2c_adapter(adap->dev.parent);
+		ret = i2c_mux_tree_trylock(parent);
+	} else {
+		ret = i2c_trylock_adapter(adap);
+	}
+
+	return ret;
+}
+
+static void i2c_mux_tree_unlock(struct i2c_adapter *adap)
+{
+	if (i2c_parent_is_i2c_adapter(adap))
+		i2c_mux_tree_unlock(to_i2c_adapter(adap->dev.parent));
+	else
+		i2c_unlock_adapter(adap);
 }
 
 /**
@@ -1104,12 +1183,12 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 #endif
 
 		if (in_atomic() || irqs_disabled()) {
-			ret = rt_mutex_trylock(&adap->bus_lock);
+			ret = i2c_mux_tree_trylock(adap);
 			if (!ret)
 				/* I2C activity is ongoing. */
 				return -EAGAIN;
 		} else {
-			rt_mutex_lock(&adap->bus_lock);
+			i2c_mux_tree_lock(adap);
 		}
 
 		/* Retry automatically on arbitration loss */
@@ -1121,7 +1200,7 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 			if (time_after(jiffies, orig_jiffies + adap->timeout))
 				break;
 		}
-		rt_mutex_unlock(&adap->bus_lock);
+		i2c_mux_tree_unlock(adap);
 
 		return ret;
 	} else {
@@ -1857,7 +1936,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
 	flags &= I2C_M_TEN | I2C_CLIENT_PEC;
 
 	if (adapter->algo->smbus_xfer) {
-		rt_mutex_lock(&adapter->bus_lock);
+		i2c_mux_tree_lock(adapter);
 
 		/* Retry automatically on arbitration loss */
 		orig_jiffies = jiffies;
@@ -1871,7 +1950,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
 				       orig_jiffies + adapter->timeout))
 				break;
 		}
-		rt_mutex_unlock(&adapter->bus_lock);
+		i2c_mux_tree_unlock(adapter);
 	} else
 		res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write,
 					      command, protocol, data);
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index f4110aa..da5327f 100755
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -193,12 +193,56 @@ static int i2cdev_check(struct device *dev, void *addrp)
 	return dev->driver ? -EBUSY : 0;
 }
 
+static int i2cdev_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
+{
+	return adapter->dev.parent != NULL
+		&& adapter->dev.parent->bus == &i2c_bus_type;
+}
+
+/* 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 && i2cdev_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_childs(struct i2c_adapter *adapter, int addr)
+{
+	int result = 0;
+	struct i2c_adapter *child, *next;
+
+	list_for_each_entry_safe(child, next, &adapter->mux_list_head,
+				 mux_list) {
+		result = device_for_each_child(&child->dev, &addr,
+					       i2cdev_check);
+		if (result)
+			break;
+		result = i2cdev_check_mux_childs(child, addr);
+	}
+	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;
+
+	result = i2cdev_check_mux_parents(adapter, addr);
+
+	if (!result)
+		result = i2cdev_check_mux_childs(adapter, addr);
+
+	return result;
 }
 
 static noinline int i2cdev_ioctl_rdrw(struct i2c_client *client,
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
new file mode 100755
index 0000000..f88d29a
--- /dev/null
+++ b/drivers/i2c/i2c-mux.c
@@ -0,0 +1,184 @@
+/*
+ * Multiplexed I2C bus driver.
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
+ *
+ * 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 <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
+ *	i2c-virtual.c from Ken Harrenstien, Copyright (c) 2004 Google, Inc.
+ *	i2c-virtual.c from Brian Kuschak <bkuschak-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
+ *
+ * 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.
+ */
+
+/* Adapted to kernel 2.6.33 by Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org> */
+
+#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 *adap, void *mux_dev, u32 chan_id);
+	int (*deselect)(struct i2c_adapter *adap, void *mux_dev, u32 chan_id);
+};
+
+#define VIRT_TIMEOUT		(HZ/2)
+#define VIRT_RETRIES		3
+
+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 for the virtual adapter */
+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;
+	}
+
+	list_add_tail(&priv->adap.mux_list, &parent->mux_list_head);
+
+	dev_info(&parent->dev, "i2c-mux-%d: Multiplexed I2C bus on "
+		"parent bus i2c-%d, chan_id %d\n",
+		i2c_adapter_id(&priv->adap), i2c_adapter_id(parent), chan_id);
+
+	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;
+	struct i2c_adapter *child, *next, *parent;
+
+	if (adap->dev.parent != NULL
+		&& adap->dev.parent->bus == &i2c_bus_type) {
+		parent = to_i2c_adapter(adap->dev.parent);
+		list_for_each_entry_safe(child, next, &parent->mux_list_head,
+					 mux_list) {
+			if (child == adap) {
+				list_del(&child->mux_list);
+				break;
+			}
+		}
+	}
+
+	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 <giometti-k2GhghHVRtY@public.gmane.org>");
+MODULE_DESCRIPTION("I2C driver for multiplexed I2C busses");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
new file mode 100755
index 0000000..0d5d6c8
--- /dev/null
+++ b/include/linux/i2c-mux.h
@@ -0,0 +1,46 @@
+/*
+ *
+ * i2c-mux.h - functions for the i2c-bus mux support
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
+ * Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org>
+ *
+ * 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_H */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 1fd7b12..898a8c1 100755
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -349,6 +349,8 @@ struct i2c_adapter {
 	struct completion dev_released;
 
 	struct list_head userspace_clients;
+	struct list_head mux_list_head;
+	struct list_head mux_list;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
@@ -380,6 +382,15 @@ static inline void i2c_unlock_adapter(struct i2c_adapter *adapter)
 	rt_mutex_unlock(&adapter->bus_lock);
 }
 
+/**
+ * i2c_trylock_adapter - Try to prevent access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline int i2c_trylock_adapter(struct i2c_adapter *adapter)
+{
+	return rt_mutex_trylock(&adapter->bus_lock);
+}
+
 /*flags for the client struct: */
 #define I2C_CLIENT_PEC	0x04		/* Use Packet Error Checking */
 #define I2C_CLIENT_TEN	0x10		/* we have a ten bit chip address */
-- 
1.6.2.1

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Rodolfo Giometti]

On Tue, May 04, 2010 at 02:46:57PM +0200, Michael Lawnick wrote:

> Add multiplexed bus core support. I2C multiplexer and switches
> like pca954x get instantiated as new adapters per output.
> 
> Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>

Acked-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>

-- 

GNU/Linux Solutions                  e-mail: giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org
Linux Device Driver                          giometti-k2GhghHVRtY@public.gmane.org
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Yegor Yefremov]

On Tue, May 4, 2010 at 2:37 PM, Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org> wrote:
> Seems like Thunderbird isn't usable for sending patches :-(
> Trying again with attachment.

Have you tried External Editor with VIM:
http://globs.org/articles.php?pg=2&lng=en

For me it is working like a charm.

Regards,
Yegor

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

Michael Lawnick said the following:
> Add multiplexed bus core support.
...

Ping

Jean, still any chance to get that done into .35?

-- 
KR
Michael Lawnick

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Jean Delvare]

Hi Michael,

On Mon, 17 May 2010 09:36:32 +0200, Michael Lawnick wrote:
> Michael Lawnick said the following:
> > Add multiplexed bus core support.
> ...
> 
> Ping
> 
> Jean, still any chance to get that done into .35?

I hope so, yes. I am busy with the w83795 driver right now, hope to get
some time to review your updated patches this week. Stay tuned.

-- 
Jean Delvare

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Jean Delvare]

Hi Michael,

On Tue, 04 May 2010 14:46:57 +0200, Michael Lawnick wrote:
> Add multiplexed bus core support. I2C multiplexer and switches
> like pca954x get instantiated as new adapters per output.
> 
> Signed-off-by: Michael Lawnick <ml.lawnick-Mmb7MZpHnFY@public.gmane.org>
> 
> ---
> Based on kernel 2.6.33 +
> [PATCH] i2c-core: Use per-adapter userspace device lists
> by Jean Delware <20100424201914.08b3f008-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
> <http://article.gmane.org/gmane.linux.drivers.i2c/5994/>

Future patches should be based on 2.6.34 at least... 2.6.35-rc2 would
be even better.

Review follows. Need some more work I fear, but not that much... Please
let me know if you are going to submit another iteration, or if you
prefer to let me fix things the way I like (in which case I'll still
ask you to test my version of the patch when I'm done.)

> 
>  drivers/i2c/Kconfig     |    8 ++
>  drivers/i2c/Makefile    |    1 +
>  drivers/i2c/i2c-core.c  |   93 ++++++++++++++++++++++--
>  drivers/i2c/i2c-dev.c   |   46 ++++++++++++-
>  drivers/i2c/i2c-mux.c   |  184 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c-mux.h |   46 ++++++++++++
>  include/linux/i2c.h     |   11 +++
>  7 files changed, 381 insertions(+), 8 deletions(-)
>  create mode 100755 drivers/i2c/i2c-mux.c
>  create mode 100755 include/linux/i2c-mux.h
> 
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index 8d8a00e..2cd6d78 100755
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -36,6 +36,14 @@ config I2C_COMPAT
>  	  other user-space package which expects i2c adapters to be class
>  	  devices. If you don't know, say Y.
>  
> +config I2C_MUX
> +	tristate "I2C bus multiplexing support"
> +	depends on I2C

Dependency on I2C isn't needed: this configuration item is already
in an "if I2C" block. A dependency on EXPERIMENTAL might be a good idea
for the time being though.

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

For support that can be build as a module, we usually add a sentence
saying so and giving the name of the module.

> +

I would move config I2C_MUX below config I2C_CHARDEV, to make it
consistent with Makefile, and also because users expect new options to
be added after existing options.

>  config I2C_CHARDEV
>  	tristate "I2C device interface"
>  	help
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index ba26e6c..a488e8b 100755
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -5,6 +5,7 @@
>  obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
>  obj-$(CONFIG_I2C)		+= i2c-core.o
>  obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
> +obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
>  obj-y				+= busses/ chips/ algos/
>  
>  ifeq ($(CONFIG_I2C_DEBUG_CORE),y)
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index fc93e28..682966a 100755
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -20,7 +20,10 @@
>  /* With some changes from Kyösti Mälkki <kmalkki-kf+aQKke1yb1KXRcyAk9cg@public.gmane.org>.
>     All SMBus-related things are written by Frodo Looijaard <frodol@dds.nl>
>     SMBus 2.0 support by Mark Studebaker <mdsxyz123-/E1597aS9LQAvxtiuMwx3w@public.gmane.org> and
> -   Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> */
> +   Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> +   Mux support by Rodolfo Giometti <giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org> and
> +   Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org> */
> +

Extra blank line.

>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> @@ -288,6 +291,12 @@ struct i2c_client *i2c_verify_client(struct device *dev)
>  }
>  EXPORT_SYMBOL(i2c_verify_client);
>  
> +static int i2c_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
> +{
> +	return adapter->dev.parent != NULL
> +		&& adapter->dev.parent->bus == &i2c_bus_type;
> +}
> +
>  
>  /**
>   * i2c_new_device - instantiate an i2c device
> @@ -624,6 +633,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>  
>  	rt_mutex_init(&adap->bus_lock);
>  	INIT_LIST_HEAD(&adap->userspace_clients);
> +	INIT_LIST_HEAD(&adap->mux_list_head);
>  
>  	/* Set default timeout to 1 second if not already set */
>  	if (adap->timeout == 0)
> @@ -949,9 +959,78 @@ static int __i2c_check_addr(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);
> +
> +	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_childs(struct i2c_adapter *adapter, int addr)

"childs" doesn't exist in proper English, the right work is "children" ;)

> +{
> +	int			result = 0;
> +	struct i2c_adapter	*child, *next;
> +
> +	list_for_each_entry_safe(child, next, &adapter->mux_list_head,
> +							 mux_list) {
> +		result = device_for_each_child(&child->dev, &addr,
> +							     __i2c_check_addr);
> +		if (result)
> +			break;
> +		result = i2c_check_mux_childs(child, addr);

This value is ignored, except for the last child. I think you should
break here too.

> +	}
> +	return result;
> +}
> +
>  static int i2c_check_addr(struct i2c_adapter *adapter, int addr)
>  {
> -	return device_for_each_child(&adapter->dev, &addr, __i2c_check_addr);
> +	int	result;
> +
> +	result = i2c_check_mux_parents(adapter, addr);
> +
> +	if (!result)
> +		result = i2c_check_mux_childs(adapter, addr);
> +
> +	return result;
> +}

I am worried about (the lack of) locking when the above function is
called. The problem isn't new with your patch, but your patch makes it
worse: beforehand, the driver core would complain if we hit a race
here, because we would try to create two clients with the same bus_id as
a result. But now, we may create 2 conflicting clients with different
bus_id, so the driver core will not complain.

I'll think about it some more, and add locking if needed.

> +static void i2c_mux_tree_lock(struct i2c_adapter *adap)
> +{
> +	if (i2c_parent_is_i2c_adapter(adap))
> +		i2c_mux_tree_lock(to_i2c_adapter(adap->dev.parent));
> +	else
> +		i2c_lock_adapter(adap);
> +}
> +
> +static int i2c_mux_tree_trylock(struct i2c_adapter *adap)
> +{
> +	int			ret;
> +	struct i2c_adapter	*parent;
> +
> +	if (i2c_parent_is_i2c_adapter(adap)) {
> +		parent = to_i2c_adapter(adap->dev.parent);
> +		ret = i2c_mux_tree_trylock(parent);
> +	} else {
> +		ret = i2c_trylock_adapter(adap);
> +	}
> +
> +	return ret;
> +}
> +
> +static void i2c_mux_tree_unlock(struct i2c_adapter *adap)
> +{
> +	if (i2c_parent_is_i2c_adapter(adap))
> +		i2c_mux_tree_unlock(to_i2c_adapter(adap->dev.parent));
> +	else
> +		i2c_unlock_adapter(adap);
>  }
>  
>  /**
> @@ -1104,12 +1183,12 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  #endif
>  
>  		if (in_atomic() || irqs_disabled()) {
> -			ret = rt_mutex_trylock(&adap->bus_lock);
> +			ret = i2c_mux_tree_trylock(adap);
>  			if (!ret)
>  				/* I2C activity is ongoing. */
>  				return -EAGAIN;
>  		} else {
> -			rt_mutex_lock(&adap->bus_lock);
> +			i2c_mux_tree_lock(adap);
>  		}
>  
>  		/* Retry automatically on arbitration loss */
> @@ -1121,7 +1200,7 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  			if (time_after(jiffies, orig_jiffies + adap->timeout))
>  				break;
>  		}
> -		rt_mutex_unlock(&adap->bus_lock);
> +		i2c_mux_tree_unlock(adap);
>  
>  		return ret;
>  	} else {
> @@ -1857,7 +1936,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>  	flags &= I2C_M_TEN | I2C_CLIENT_PEC;
>  
>  	if (adapter->algo->smbus_xfer) {
> -		rt_mutex_lock(&adapter->bus_lock);
> +		i2c_mux_tree_lock(adapter);
>  
>  		/* Retry automatically on arbitration loss */
>  		orig_jiffies = jiffies;
> @@ -1871,7 +1950,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>  				       orig_jiffies + adapter->timeout))
>  				break;
>  		}
> -		rt_mutex_unlock(&adapter->bus_lock);
> +		i2c_mux_tree_unlock(adapter);
>  	} else
>  		res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write,
>  					      command, protocol, data);

I'm worried that this approach is risky. We now use i2c_mux_tree_*lock
internally, but still expose the i2c_*lock_adapter() API externally.
This API is _not_ mux-aware, and will do the wrong thing if called on a
bus segment behind a mux.

I'd rather not have two APIs when we really only need one. Let's just
update the current public API to be mux-aware, and use that everywhere.

> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index f4110aa..da5327f 100755
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -193,12 +193,56 @@ static int i2cdev_check(struct device *dev, void *addrp)
>  	return dev->driver ? -EBUSY : 0;
>  }
>  
> +static int i2cdev_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
> +{
> +	return adapter->dev.parent != NULL
> +		&& adapter->dev.parent->bus == &i2c_bus_type;
> +}

This is an exact duplicate of the same function in i2c-core. I'd rather
make it an inline function in <linux/i2c.h>, or export the one from
i2c-core.c. We have enough duplicate code in i2c-dev.c already :(

> +
> +/* 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 && i2cdev_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_childs(struct i2c_adapter *adapter, int addr)

children

> +{
> +	int result = 0;
> +	struct i2c_adapter *child, *next;
> +
> +	list_for_each_entry_safe(child, next, &adapter->mux_list_head,
> +				 mux_list) {
> +		result = device_for_each_child(&child->dev, &addr,
> +					       i2cdev_check);
> +		if (result)
> +			break;
> +		result = i2cdev_check_mux_childs(child, addr);

Same as in the i2c-core variant, this value is ignored, I think this
isn't correct.

> +	}
> +	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;
> +
> +	result = i2cdev_check_mux_parents(adapter, addr);
> +
> +	if (!result)
> +		result = i2cdev_check_mux_childs(adapter, addr);
> +
> +	return result;
>  }
>  
>  static noinline int i2cdev_ioctl_rdrw(struct i2c_client *client,
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> new file mode 100755
> index 0000000..f88d29a
> --- /dev/null
> +++ b/drivers/i2c/i2c-mux.c
> @@ -0,0 +1,184 @@
> +/*
> + * Multiplexed I2C bus driver.
> + *
> + * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
> + * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
> + *
> + * 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 <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> + *	i2c-virtual.c from Ken Harrenstien, Copyright (c) 2004 Google, Inc.
> + *	i2c-virtual.c from Brian Kuschak <bkuschak-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
> + *
> + * 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.
> + */
> +
> +/* Adapted to kernel 2.6.33 by Michael Lawnick <michael.lawnick.ext@nsn.com> */

I'd rather have you add your copyright with the other ones already
present above.

> +
> +#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 *adap, void *mux_dev, u32 chan_id);
> +	int (*deselect)(struct i2c_adapter *adap, void *mux_dev, u32 chan_id);

The two "adap" can go away... I insisted on having parameter names for
the other two; for structures, it's usually pretty clear what we're
talking about.

> +};
> +
> +#define VIRT_TIMEOUT		(HZ/2)
> +#define VIRT_RETRIES		3

These two are no longer used and can thus be deleted.

> +
> +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.*/

Missing space at end of comment.

> +
> +	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.*/

Ditto.

> +
> +	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);

Strange alignment.

> +	if (priv->deselect)
> +		priv->deselect(parent, priv->mux_dev, priv->chan_id);
> +
> +	return ret;
> +}
> +
> +/* Return the parent's functionality for the virtual adapter */

Please avoid referring to "virtual adapters". The fact here is that all
bus segments of a given tree share the same functionality, because it
is a characteristic of the controller - nothing virtual here.

> +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);

I'm only half happy with this name. One problem is that it isn't
unique: if you have 2 mux chips on the same root segment, this will
lead to duplicate segments with the same name. Another problem is that
it completely hides the name of the controller (which, I admit, we may
recover by walking sysfs, but still...)

This isn't a blocker point, but something to keep in mind for further
improvements. It might even make sense to let the caller provide the
format for the segment names.

> +	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;
> +	}
> +
> +	list_add_tail(&priv->adap.mux_list, &parent->mux_list_head);

The driver core maintains a device tree already, do you really need to
keep your own? We've tried hard to remove all redundancy between
i2c-core and the driver core in the last couple years, so I really would
like us to not add such redundancy back now.

If you have a good reason to have your own list, please explain. If
not, please get rid of it.

> +
> +	dev_info(&parent->dev, "i2c-mux-%d: Multiplexed I2C bus on "
> +		"parent bus i2c-%d, chan_id %d\n",
> +		i2c_adapter_id(&priv->adap), i2c_adapter_id(parent), chan_id);

Have you looked at the output? I expect some redundancy, as you use
dev_info() on the parent, i2c_adapter_id(parent) is already included. I
think I'd rather call dev_info() on the newly spawned child, so you can
get rid of the leading "i2c-mux-%d". BTW, please use dev_name() instead
of hard-coding i2c-%d for the parent.

> +
> +	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;
> +	struct i2c_adapter *child, *next, *parent;

Would look nicer with all structs in front and ret last, IMHO.

> +
> +	if (adap->dev.parent != NULL
> +		&& adap->dev.parent->bus == &i2c_bus_type) {

This is a copy of i2c_parent_is_i2c_adapter(), isn't it? That's one
more reason to export it from i2c-core.

> +		parent = to_i2c_adapter(adap->dev.parent);

Note that we could have i2c_parent_is_i2c_adapter() return the parent
adapter, if it proves useful in practice.

> +		list_for_each_entry_safe(child, next, &parent->mux_list_head,
> +					 mux_list) {
> +			if (child == adap) {
> +				list_del(&child->mux_list);
> +				break;
> +			}
> +		}
> +	}

That being said... Using the driver core's list would save you the
hassle altogether.

> +
> +	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 <giometti-k2GhghHVRtY@public.gmane.org>");
> +MODULE_DESCRIPTION("I2C driver for multiplexed I2C busses");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
> new file mode 100755
> index 0000000..0d5d6c8
> --- /dev/null
> +++ b/include/linux/i2c-mux.h
> @@ -0,0 +1,46 @@
> +/*
> + *
> + * i2c-mux.h - functions for the i2c-bus mux support
> + *
> + * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
> + * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
> + * Michael Lawnick <michael.lawnick.ext-OYasijW0DpE@public.gmane.org>
> + *
> + * 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_H */

You actually mean /* _LINUX_I2C_MUX_H */.

> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 1fd7b12..898a8c1 100755
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -349,6 +349,8 @@ struct i2c_adapter {
>  	struct completion dev_released;
>  
>  	struct list_head userspace_clients;
> +	struct list_head mux_list_head;
> +	struct list_head mux_list;

As said before, I would much prefer if we didn't have to introduce
these.

>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>  
> @@ -380,6 +382,15 @@ static inline void i2c_unlock_adapter(struct i2c_adapter *adapter)
>  	rt_mutex_unlock(&adapter->bus_lock);
>  }
>  
> +/**
> + * i2c_trylock_adapter - Try to prevent access to an I2C bus segment
> + * @adapter: Target I2C bus segment
> + */
> +static inline int i2c_trylock_adapter(struct i2c_adapter *adapter)
> +{
> +	return rt_mutex_trylock(&adapter->bus_lock);
> +}
> +
>  /*flags for the client struct: */
>  #define I2C_CLIENT_PEC	0x04		/* Use Packet Error Checking */
>  #define I2C_CLIENT_TEN	0x10		/* we have a ten bit chip address */

Thanks,
-- 
Jean Delvare

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

Hi,

beside all the other issues,

Jean Delvare said the following:
> Hi Michael,
> 
>> +	list_add_tail(&priv->adap.mux_list, &parent->mux_list_head);
> 
> The driver core maintains a device tree already, do you really need to
> keep your own? We've tried hard to remove all redundancy between
> i2c-core and the driver core in the last couple years, so I really would
> like us to not add such redundancy back now.
> 
> If you have a good reason to have your own list, please explain. If
> not, please get rid of it.
> 
this is a core part of mux management. You need it for checking whether
the current client that is to be added is already present.
The problem is to efficiently parse down a mux tree towards the leaves.
Sub-muxes are registered as i2c clients to their upper bus. I found no way
to separate muxes from normal clients. So there were two ideas:
invent a client-type field in i2c-client or to hold a separate list.
The first one e.g. would have the possibility to provide basic info in sysFs
but I feared a general debate. So I decided to go the other way which is
hidden to the outer world.

If you don't like this way, I'd need some ideas how to do it.

-- 
KR
Michael

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Jean Delvare]

Hi Michael,

On Fri, 11 Jun 2010 08:31:59 +0200, Michael Lawnick wrote:
> Jean Delvare said the following:
> > Hi Michael,
> > 
> >> +	list_add_tail(&priv->adap.mux_list, &parent->mux_list_head);
> > 
> > The driver core maintains a device tree already, do you really need to
> > keep your own? We've tried hard to remove all redundancy between
> > i2c-core and the driver core in the last couple years, so I really would
> > like us to not add such redundancy back now.
> > 
> > If you have a good reason to have your own list, please explain. If
> > not, please get rid of it.
> > 
> this is a core part of mux management. You need it for checking whether
> the current client that is to be added is already present.
> The problem is to efficiently parse down a mux tree towards the leaves.
> Sub-muxes are registered as i2c clients to their upper bus.

This is correct, but irrelevant, as far as I can see. When walking an
I2C segment tree towards the leaves, you care about the segments
themselves (struct i2c_adapter), not the mux chips. In your
architecture, muxed segments are direct children of the root segment
from the device tree perspective, not children of the mux chip.

> I found no way to separate muxes from normal clients.

If you have to, it is easy, the driver core has support for this
already. Search for i2c_client_type and i2c_adapter_type in i2c-core.c.
We could add i2c_mux_type easily. But as I wrote above, I don't think
we need this.

> So there were two ideas:
> invent a client-type field in i2c-client or to hold a separate list.
> The first one e.g. would have the possibility to provide basic info in sysFs
> but I feared a general debate. So I decided to go the other way which is
> hidden to the outer world.
> 
> If you don't like this way, I'd need some ideas how to do it.

I would walk the device tree maintained by the driver core (using
device_for_each_child) and filtering out irrelevant devices. Basically
filtering on dev->type == &i2c_adapter_type should do the trick,
methinks. Is there anything I am missing?

Thanks,
-- 
Jean Delvare

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

Hi Jean,

Jean Delvare said the following:
...
>>  	if (adapter->algo->smbus_xfer) {
>> -		rt_mutex_lock(&adapter->bus_lock);
>> +		i2c_mux_tree_lock(adapter);
>>
>>  		/* Retry automatically on arbitration loss */
>>  		orig_jiffies = jiffies;
>> @@ -1871,7 +1950,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>>  				       orig_jiffies + adapter->timeout))
>>  				break;
>>  		}
>> -		rt_mutex_unlock(&adapter->bus_lock);
>> +		i2c_mux_tree_unlock(adapter);
>>  	} else
>>  		res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write,
>>  					      command, protocol, data);
>
> I'm worried that this approach is risky. We now use i2c_mux_tree_*lock
> internally, but still expose the i2c_*lock_adapter() API externally.
> This API is _not_ mux-aware, and will do the wrong thing if called on a
> bus segment behind a mux.
>
> I'd rather not have two APIs when we really only need one. Let's just
> update the current public API to be mux-aware, and use that everywhere.

Agree, but I'd vote to do it not inline as its now. Otherwise we start to export
mux implementation details in i2c.h

>
>> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
>> index f4110aa..da5327f 100755
>> --- a/drivers/i2c/i2c-dev.c
>> +++ b/drivers/i2c/i2c-dev.c
>> @@ -193,12 +193,56 @@ static int i2cdev_check(struct device *dev, void *addrp)
>>  	return dev->driver ? -EBUSY : 0;
>>  }
>>
>> +static int i2cdev_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
>> +{
>> +	return adapter->dev.parent != NULL
>> +		&& adapter->dev.parent->bus == &i2c_bus_type;
>> +}
>
> This is an exact duplicate of the same function in i2c-core. I'd rather
> make it an inline function in <linux/i2c.h>, or export the one from
> i2c-core.c. We have enough duplicate code in i2c-dev.c already :(

Must this and the above be done in an extra patch or can it be mixed into this mux patch?

-- 
KR
Michael

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Michael Lawnick]

Hi Jean,

Jean Delvare said the following:
> > Hi Michael,
...
> > I would walk the device tree maintained by the driver core (using
> > device_for_each_child) and filtering out irrelevant devices. Basically
> > filtering on dev->type == &i2c_adapter_type should do the trick,
> > methinks.
> >
You are right, I missed that somehow when investigating for a solution.
Until your post I was convinced that the adapters are all registered to
bus only. I now rechecked by some small code and found that I can throw
away these awefull lists.

Iteration will follow.

-- KR Michael

Re: [PATCH v2 1/2] i2c: Multiplexed I2C bus core support

[Jean Delvare]

Hi Michael,

On Tue, 15 Jun 2010 10:52:49 +0200, Michael Lawnick wrote:
> Hi Jean,
> 
> Jean Delvare said the following:
> ...
> >>  	if (adapter->algo->smbus_xfer) {
> >> -		rt_mutex_lock(&adapter->bus_lock);
> >> +		i2c_mux_tree_lock(adapter);
> >>
> >>  		/* Retry automatically on arbitration loss */
> >>  		orig_jiffies = jiffies;
> >> @@ -1871,7 +1950,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
> >>  				       orig_jiffies + adapter->timeout))
> >>  				break;
> >>  		}
> >> -		rt_mutex_unlock(&adapter->bus_lock);
> >> +		i2c_mux_tree_unlock(adapter);
> >>  	} else
> >>  		res = i2c_smbus_xfer_emulated(adapter,addr,flags,read_write,
> >>  					      command, protocol, data);
> >
> > I'm worried that this approach is risky. We now use i2c_mux_tree_*lock
> > internally, but still expose the i2c_*lock_adapter() API externally.
> > This API is _not_ mux-aware, and will do the wrong thing if called on a
> > bus segment behind a mux.
> >
> > I'd rather not have two APIs when we really only need one. Let's just
> > update the current public API to be mux-aware, and use that everywhere.
> 
> Agree, but I'd vote to do it not inline as its now. Otherwise we start to export
> mux implementation details in i2c.h

Of course. In fact I am preparing a patch which moves these functions
to i2c-core, uninlines them, and uses them as appropriate. Your
multiplexing patch would stack on top of it, so it is nice and clean
and self-contained.

> >> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> >> index f4110aa..da5327f 100755
> >> --- a/drivers/i2c/i2c-dev.c
> >> +++ b/drivers/i2c/i2c-dev.c
> >> @@ -193,12 +193,56 @@ static int i2cdev_check(struct device *dev, void *addrp)
> >>  	return dev->driver ? -EBUSY : 0;
> >>  }
> >>
> >> +static int i2cdev_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
> >> +{
> >> +	return adapter->dev.parent != NULL
> >> +		&& adapter->dev.parent->bus == &i2c_bus_type;
> >> +}
> >
> > This is an exact duplicate of the same function in i2c-core. I'd rather
> > make it an inline function in <linux/i2c.h>, or export the one from
> > i2c-core.c. We have enough duplicate code in i2c-dev.c already :(
> 
> Must this and the above be done in an extra patch or can it be mixed into this mux patch?

Whatever is easier for you, both are fine with me.

-- 
Jean Delvare