i2c bus multiplexing (Version 2).

i2c bus multiplexing (Version 2).

[Rodolfo Giometti]

This patch set adds to the I2C core the ability to handle multiplexed
I2C bus topologies by presenting each multiplexed segment as a I2C
adapter.

As you can see from the changelog I had to update i2c-core since when
a real adapter (not multiplexer) driver is inserted into the kernel
_after_ an i2c multiplxer one, function i2c_add_adapter() is called
recursively (and the same for i2c_del_adapter() during module removal)
causing a kernel hang due the i2c's "core_lock" mutex.

Changelog
=========

Version 1 -> 2:
---------------

* Fixes a bug related to recursive calls of i2c_(add|del)_adapter()
  functions when main adapter is inserted or removed _after_ slave
  adapetrs (i2c multiplexers).

  This goal has been achieved by:

  a) using rwsem instead of mutex for accessing board info data struct,

  b) ignoring any active client detaching faults (see relative patch for
     further info),

  c) removing "core_lock" mutex into adapters (de)registration code.

Rodolfo

--

 b/drivers/i2c/Kconfig         |   10 +
 b/drivers/i2c/Makefile        |    3 
 b/drivers/i2c/i2c-boardinfo.c |    6 
 b/drivers/i2c/i2c-core.c      |    4 
 b/drivers/i2c/i2c-core.h      |    2 
 b/drivers/i2c/i2c-mux.c       |  180 +++++++++++++++++++++++
 b/drivers/i2c/muxes/Kconfig   |    8 +
 b/drivers/i2c/muxes/Makefile  |    6
 b/drivers/i2c/muxes/pca954x.c |  325 ++++++++++++++++++++++++++++++++++++++++++
 b/include/linux/i2c-id.h      |    3
 b/include/linux/i2c.h         |   14 +
 b/include/linux/i2c/pca954x.h |   13 +
 drivers/i2c/i2c-core.c        |   78 ++++------
 drivers/i2c/muxes/Kconfig     |    9 +
 drivers/i2c/muxes/Makefile    |    2
 15 files changed, 615 insertions(+), 48 deletions(-)

[PATCH 1/6] i2c: put driver_unregister() out of core_lock protection.

[Rodolfo Giometti]

As for i2c_register_driver() during driver registration only
class_for_each_device() call is protected by the i2c "core_lock" so we
can (hopefully) do the same during driver deletion.

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/i2c-core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index b1c9abe..3454878 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -799,10 +799,10 @@ void i2c_del_driver(struct i2c_driver *driver)
 	class_for_each_device(&i2c_adapter_class, NULL, driver,
 			      __detach_adapter);
 
+	mutex_unlock(&core_lock);
+
 	driver_unregister(&driver->driver);
 	pr_debug("i2c-core: driver [%s] unregistered\n", driver->driver.name);
-
-	mutex_unlock(&core_lock);
 }
 EXPORT_SYMBOL(i2c_del_driver);
 
-- 
1.5.6.3

[PATCH 2/6] i2c: use rwsem instead of mutex.

[Rodolfo Giometti]

By using rwsem we can easily manage recursive calls of
i2c_scan_static_board_info() function without breaking the locking.

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/i2c-boardinfo.c |    6 +++---
 drivers/i2c/i2c-core.c      |    4 ++--
 drivers/i2c/i2c-core.h      |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/i2c-boardinfo.c b/drivers/i2c/i2c-boardinfo.c
index ffb35f0..981cdd8 100644
--- a/drivers/i2c/i2c-boardinfo.c
+++ b/drivers/i2c/i2c-boardinfo.c
@@ -25,7 +25,7 @@
 /* These symbols are exported ONLY FOR the i2c core.
  * No other users will be supported.
  */
-DEFINE_MUTEX(__i2c_board_lock);
+DECLARE_RWSEM(__i2c_board_lock);
 EXPORT_SYMBOL_GPL(__i2c_board_lock);
 
 LIST_HEAD(__i2c_board_list);
@@ -63,7 +63,7 @@ i2c_register_board_info(int busnum,
 {
 	int status;
 
-	mutex_lock(&__i2c_board_lock);
+	down_write(&__i2c_board_lock);
 
 	/* dynamic bus numbers will be assigned after the last static one */
 	if (busnum >= __i2c_first_dynamic_bus_num)
@@ -84,7 +84,7 @@ i2c_register_board_info(int busnum,
 		list_add_tail(&devinfo->list, &__i2c_board_list);
 	}
 
-	mutex_unlock(&__i2c_board_lock);
+	up_write(&__i2c_board_lock);
 
 	return status;
 }
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3454878..74b89a9 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -408,7 +408,7 @@ static void i2c_scan_static_board_info(struct i2c_adapter *adapter)
 {
 	struct i2c_devinfo	*devinfo;
 
-	mutex_lock(&__i2c_board_lock);
+	down_read(&__i2c_board_lock);
 	list_for_each_entry(devinfo, &__i2c_board_list, list) {
 		if (devinfo->busnum == adapter->nr
 				&& !i2c_new_device(adapter,
@@ -417,7 +417,7 @@ static void i2c_scan_static_board_info(struct i2c_adapter *adapter)
 				i2c_adapter_id(adapter),
 				devinfo->board_info.addr);
 	}
-	mutex_unlock(&__i2c_board_lock);
+	up_read(&__i2c_board_lock);
 }
 
 static int i2c_do_add_adapter(struct device_driver *d, void *data)
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
index cd5bff8..e6af064 100644
--- a/drivers/i2c/i2c-core.h
+++ b/drivers/i2c/i2c-core.h
@@ -25,7 +25,7 @@ struct i2c_devinfo {
 /* board_lock protects board_list and first_dynamic_bus_num.
  * only i2c core components are allowed to use these symbols.
  */
-extern struct mutex	__i2c_board_lock;
+extern struct rw_semaphore	__i2c_board_lock;
 extern struct list_head	__i2c_board_list;
 extern int		__i2c_first_dynamic_bus_num;
 
-- 
1.5.6.3

[PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

When we are removing an adapter we decide to ignore any active client
detaching faults for two reasons:

1) one (or more) active client may be switched off, so it cannot
   replay to the "adapter removed" event.

2) it shouldn't happen, and even if it happens it may be due a bus
   fault which can be resolved by resetting the adapter (most of them
   can be resetted simply by rmmod and then insmod the module again).

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/i2c-core.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 74b89a9..5d3646f 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -593,11 +593,13 @@ static int i2c_do_del_adapter(struct device_driver *d, void *data)
 
 	if (!driver->detach_adapter)
 		return 0;
+
+	/* We ignore the return code; if it fails, too bad */
 	res = driver->detach_adapter(adapter);
 	if (res)
 		dev_err(&adapter->dev, "detach_adapter failed (%d) "
 			"for driver [%s]\n", res, driver->driver.name);
-	return res;
+	return 0;
 }
 
 /**
@@ -611,7 +613,7 @@ static int i2c_do_del_adapter(struct device_driver *d, void *data)
 int i2c_del_adapter(struct i2c_adapter *adap)
 {
 	struct i2c_client *client, *_n;
-	int res = 0;
+	int res = 0, dummy;
 
 	mutex_lock(&core_lock);
 
@@ -624,13 +626,10 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 	}
 
 	/* Tell drivers about this removal */
-	res = bus_for_each_drv(&i2c_bus_type, NULL, adap,
+	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
 			       i2c_do_del_adapter);
-	if (res)
-		goto out_unlock;
 
-	/* detach any active clients. This must be done first, because
-	 * it can fail; in which case we give up. */
+	/* Detach any active clients */
 	list_for_each_entry_safe_reverse(client, _n, &adap->clients, list) {
 		struct i2c_driver	*driver;
 
@@ -643,12 +642,10 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 		}
 
 		/* legacy drivers create and remove clients themselves */
-		if ((res = driver->detach_client(client))) {
+		if (driver->detach_client(client))
 			dev_err(&adap->dev, "detach_client failed for client "
 				"[%s] at address 0x%02x\n", client->name,
 				client->addr);
-			goto out_unlock;
-		}
 	}
 
 	/* clean up the sysfs representation */
-- 
1.5.6.3

[PATCH 4/6] i2c: free adapters (de)registration from i2c "core_lock" mutex.

[Rodolfo Giometti]

The adapters (de)registration are now protectd by using a dedicated
mutex instead of using global "core_lock" one. This allow multiple
(and recursive) (de)registration of several adapters (or i2c
multiplexers acting as i2c virtual adapters).

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/i2c-core.c |   57 +++++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5d3646f..4b088ef 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -40,6 +40,7 @@
 
 
 static DEFINE_MUTEX(core_lock);
+static DEFINE_MUTEX(idr_adapter_lock);
 static DEFINE_IDR(i2c_adapter_idr);
 
 #define is_newstyle_driver(d) ((d)->probe || (d)->remove || (d)->detect)
@@ -438,7 +439,7 @@ static int i2c_do_add_adapter(struct device_driver *d, void *data)
 
 static int i2c_register_adapter(struct i2c_adapter *adap)
 {
-	int res = 0, dummy;
+	int res, dummy;
 
 	/* Can't register until after driver model init */
 	if (unlikely(WARN_ON(!i2c_bus_type.p)))
@@ -448,8 +449,6 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	mutex_init(&adap->clist_lock);
 	INIT_LIST_HEAD(&adap->clients);
 
-	mutex_lock(&core_lock);
-
 	/* Add the adapter to the driver core.
 	 * If the parent pointer is not set up,
 	 * we add this adapter to the host bus.
@@ -463,8 +462,13 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	adap->dev.release = &i2c_adapter_dev_release;
 	adap->dev.class = &i2c_adapter_class;
 	res = device_register(&adap->dev);
-	if (res)
-		goto out_list;
+	if (res) {
+		mutex_lock(&idr_adapter_lock);
+		idr_remove(&i2c_adapter_idr, adap->nr);
+		mutex_unlock(&idr_adapter_lock);
+
+		return res;
+	}
 
 	dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
 
@@ -476,13 +480,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
 				 i2c_do_add_adapter);
 
-out_unlock:
-	mutex_unlock(&core_lock);
-	return res;
-
-out_list:
-	idr_remove(&i2c_adapter_idr, adap->nr);
-	goto out_unlock;
+	return 0;
 }
 
 /**
@@ -506,11 +504,11 @@ retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
 
-	mutex_lock(&core_lock);
+	mutex_lock(&idr_adapter_lock);
 	/* "above" here means "above or equal to", sigh */
 	res = idr_get_new_above(&i2c_adapter_idr, adapter,
 				__i2c_first_dynamic_bus_num, &id);
-	mutex_unlock(&core_lock);
+	mutex_unlock(&idr_adapter_lock);
 
 	if (res < 0) {
 		if (res == -EAGAIN)
@@ -555,7 +553,7 @@ retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
 
-	mutex_lock(&core_lock);
+	mutex_lock(&idr_adapter_lock);
 	/* "above" here means "above or equal to", sigh;
 	 * we need the "equal to" result to force the result
 	 */
@@ -564,7 +562,7 @@ retry:
 		status = -EBUSY;
 		idr_remove(&i2c_adapter_idr, id);
 	}
-	mutex_unlock(&core_lock);
+	mutex_unlock(&idr_adapter_lock);
 	if (status == -EAGAIN)
 		goto retry;
 
@@ -612,19 +610,25 @@ static int i2c_do_del_adapter(struct device_driver *d, void *data)
  */
 int i2c_del_adapter(struct i2c_adapter *adap)
 {
+	struct i2c_adapter *ptr;
 	struct i2c_client *client, *_n;
-	int res = 0, dummy;
+	int dummy;
 
-	mutex_lock(&core_lock);
+	mutex_lock(&idr_adapter_lock);
 
 	/* First make sure that this adapter was ever added */
-	if (idr_find(&i2c_adapter_idr, adap->nr) != adap) {
+	ptr = idr_find(&i2c_adapter_idr, adap->nr);
+	if (ptr != adap) {
 		pr_debug("i2c-core: attempting to delete unregistered "
 			 "adapter [%s]\n", adap->name);
-		res = -EINVAL;
-		goto out_unlock;
+		mutex_unlock(&idr_adapter_lock);
+		return -EINVAL;
 	}
 
+	/* ... then remove adapter ID */
+	idr_remove(&i2c_adapter_idr, adap->nr);
+	mutex_unlock(&idr_adapter_lock);
+
 	/* Tell drivers about this removal */
 	dummy = bus_for_each_drv(&i2c_bus_type, NULL, adap,
 			       i2c_do_del_adapter);
@@ -655,18 +659,13 @@ int i2c_del_adapter(struct i2c_adapter *adap)
 	/* wait for sysfs to drop all references */
 	wait_for_completion(&adap->dev_released);
 
-	/* free bus id */
-	idr_remove(&i2c_adapter_idr, adap->nr);
-
 	dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name);
 
 	/* Clear the device structure in case this adapter is ever going to be
 	   added again */
 	memset(&adap->dev, 0, sizeof(adap->dev));
 
- out_unlock:
-	mutex_unlock(&core_lock);
-	return res;
+	return 0;
 }
 EXPORT_SYMBOL(i2c_del_adapter);
 
@@ -1497,12 +1496,12 @@ struct i2c_adapter* i2c_get_adapter(int id)
 {
 	struct i2c_adapter *adapter;
 
-	mutex_lock(&core_lock);
+	mutex_lock(&idr_adapter_lock);
 	adapter = (struct i2c_adapter *)idr_find(&i2c_adapter_idr, id);
 	if (adapter && !try_module_get(adapter->owner))
 		adapter = NULL;
 
-	mutex_unlock(&core_lock);
+	mutex_unlock(&idr_adapter_lock);
 	return adapter;
 }
 EXPORT_SYMBOL(i2c_get_adapter);
-- 
1.5.6.3

[PATCH 5/6] i2c: Multiplexed I2C bus core support.

[Rodolfo Giometti]

Add to the I2C core the ability to handle multiplexed I2C bus
topologies by presenting each multiplexed segment as a I2C adapter.

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/Kconfig        |   10 +++
 drivers/i2c/Makefile       |    3 +-
 drivers/i2c/i2c-mux.c      |  180 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/muxes/Kconfig  |    8 ++
 drivers/i2c/muxes/Makefile |    6 ++
 include/linux/i2c-id.h     |    3 +
 include/linux/i2c.h        |   14 ++++
 7 files changed, 223 insertions(+), 1 deletions(-)
 create mode 100644 drivers/i2c/i2c-mux.c
 create mode 100644 drivers/i2c/muxes/Kconfig
 create mode 100644 drivers/i2c/muxes/Makefile

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 711ca08..78dc245 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -27,6 +27,16 @@ config I2C_BOARDINFO
 	boolean
 	default 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.
+
+source drivers/i2c/muxes/Kconfig
+
 config I2C_CHARDEV
 	tristate "I2C device interface"
 	help
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index ba26e6c..281e2a7 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -4,8 +4,9 @@
 
 obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
 obj-$(CONFIG_I2C)		+= i2c-core.o
+obj-$(CONFIG_I2C_MUX)		+= i2c-mux.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
-obj-y				+= busses/ chips/ algos/
+obj-y				+= busses/ chips/ muxes/ algos/
 
 ifeq ($(CONFIG_I2C_DEBUG_CORE),y)
 EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
new file mode 100644
index 0000000..390dbc3
--- /dev/null
+++ b/drivers/i2c/i2c-mux.c
@@ -0,0 +1,180 @@
+/*
+ * 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 a virtual 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 Copyright (c) 2004 Google, Inc. (Ken Harrenstien)
+ *    i2c-virtual.c from Brian Kuschak <bkuschak-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
+ * which was:
+ *    Adapted from i2c-adap-ibm_ocp.c
+ *    Original file Copyright 2000-2002 MontaVista Software Inc.
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/i2c-id.h>
+
+struct i2c_mux_priv {
+	struct i2c_adapter adap;
+	struct i2c_algorithm algo;
+
+	struct i2c_adapter *parent_adap;
+	void *client;			/* The mux chip/device */
+	u32 id;				/* the mux id */
+
+	int (*select)(struct i2c_adapter *, void *, u32);
+	int (*deselect)(struct i2c_adapter *, void *, u32);
+};
+
+#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_adap;
+	int ret;
+
+	/* Grab the lock for the parent adapter. We already hold the lock for
+	 * the virtual adapter. Then select the right mux port and perform
+	 * the transfer.
+	 */
+
+	mutex_lock_nested(&parent->bus_lock, SINGLE_DEPTH_NESTING);
+
+	ret = priv->select(parent, priv->client, priv->id);
+	if (ret >= 0)
+		ret = parent->algo->master_xfer(parent, msgs, num);
+	priv->deselect(parent, priv->client, priv->id);
+
+	mutex_unlock(&parent->bus_lock);
+
+	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_adap;
+	int ret;
+
+	/* Grab the lock for the parent adapter.  We already hold the lock for
+	 * the virtual adapter.  Then select the right mux port and perform
+	 * the transfer.
+	 */
+
+	mutex_lock_nested(&parent->bus_lock, SINGLE_DEPTH_NESTING);
+
+	ret = priv->select(parent, priv->client, priv->id);
+	if (ret == 0)
+		ret = parent->algo->smbus_xfer(parent, addr, flags,
+					   read_write, command, size, data);
+	priv->deselect(parent, priv->client, priv->id);
+
+	mutex_unlock(&parent->bus_lock);
+
+	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_adap;
+
+	return parent->algo->functionality(parent);
+}
+
+struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
+				void *client, u32 force_nr, u32 mux_val,
+				int (*select_cb) (struct i2c_adapter *,
+						void *, u32),
+				int (*deselect_cb) (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_adap = parent;
+	priv->client = client;
+	priv->id = mux_val;
+	priv->select = select_cb;
+	priv->deselect = deselect_cb;
+
+	/* 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 (mux %02x)",
+			i2c_adapter_id(parent), mux_val);
+	priv->adap.owner = THIS_MODULE;
+	priv->adap.id = I2C_HW_VIRT | i2c_adapter_id(parent);
+	priv->adap.algo = &priv->algo;
+	priv->adap.algo_data = priv;
+	priv->adap.timeout = VIRT_TIMEOUT;
+	priv->adap.retries = VIRT_RETRIES;
+	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) {
+		kfree(priv);
+		return NULL;
+	}
+
+	dev_info(&parent->dev, "i2c-mux-%d: Virtual I2C bus - "
+		"physical bus i2c-%d, port %d\n",
+		i2c_adapter_id(&priv->adap), i2c_adapter_id(parent), mux_val);
+
+	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 <giometti-k2GhghHVRtY@public.gmane.org>");
+MODULE_DESCRIPTION("Virtual I2C driver for multiplexed I2C busses");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
new file mode 100644
index 0000000..95b9bde
--- /dev/null
+++ b/drivers/i2c/muxes/Kconfig
@@ -0,0 +1,8 @@
+#
+# Multiplexer I2C chip drivers configuration
+#
+
+menu "Multiplexer I2C Chip support"
+	depends on I2C && I2C_MUX && EXPERIMENTAL
+
+endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
new file mode 100644
index 0000000..5e70bde
--- /dev/null
+++ b/drivers/i2c/muxes/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for multiplexer I2C chip drivers.
+
+ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
index 01d67ba..d4f97b6 100644
--- a/include/linux/i2c-id.h
+++ b/include/linux/i2c-id.h
@@ -162,4 +162,7 @@
 #define I2C_HW_SAA7146		0x060000 /* SAA7146 video decoder bus */
 #define I2C_HW_SAA7134		0x090000 /* SAA7134 video decoder bus */
 
+/* --- Virtual adapter mark */
+#define I2C_HW_VIRT            0x80000000
+
 #endif /* LINUX_I2C_ID_H */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 20873d4..748fc22 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -588,6 +588,20 @@ union i2c_smbus_data {
 #define I2C_SMBUS_BLOCK_PROC_CALL   7		/* SMBus 2.0 */
 #define I2C_SMBUS_I2C_BLOCK_DATA    8
 
+/*
+ * Called to create a 'virtual' i2c bus which represents a multiplexed bus
+ * segment.  The client and mux_val 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 *client, u32 force_nr, u32 mux_val,
+				int (*select_cb) (struct i2c_adapter *,
+						void *, u32),
+				int (*deselect_cb) (struct i2c_adapter *,
+						void *, u32));
+
+int i2c_del_mux_adapter(struct i2c_adapter *adap);
+
 
 #ifdef __KERNEL__
 
-- 
1.5.6.3

[PATCH 6/6] i2c: driver for PCA954x I2C multiplexer series.

[Rodolfo Giometti]

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/muxes/Kconfig   |    9 ++
 drivers/i2c/muxes/Makefile  |    2 +
 drivers/i2c/muxes/pca954x.c |  325 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/pca954x.h |   13 ++
 4 files changed, 349 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/muxes/pca954x.c
 create mode 100644 include/linux/i2c/pca954x.h

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 95b9bde..72c7055 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -5,4 +5,13 @@
 menu "Multiplexer I2C Chip support"
 	depends on I2C && I2C_MUX && EXPERIMENTAL
 
+config I2CMUX_PCA954x
+	tristate "Philips PCA953x I2C Mux/switches"
+	help
+	  If you say yes here you get support for the Philips PCA954x
+	  I2C mux/switch devices.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called pca954x.
+
 endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 5e70bde..f752576 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -1,6 +1,8 @@
 #
 # Makefile for multiplexer I2C chip drivers.
 
+obj-$(CONFIG_I2CMUX_PCA954x)	+= pca954x.o
+
 ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
 EXTRA_CFLAGS += -DDEBUG
 endif
diff --git a/drivers/i2c/muxes/pca954x.c b/drivers/i2c/muxes/pca954x.c
new file mode 100644
index 0000000..5643563
--- /dev/null
+++ b/drivers/i2c/muxes/pca954x.c
@@ -0,0 +1,325 @@
+/*
+ * I2C multiplexer
+ *
+ * Copyright (c) 2008-2009 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
+ * Copyright (c) 2008-2009 Eurotech S.p.A. <info-ymFC7zkAqBI1GQ1Ptb7lUw@public.gmane.org>
+ *
+ * This module supports the PCA954x series of I2C multiplexer/switch chips
+ * made by Philips Semiconductors.
+ * This includes the:
+ *     PCA9540, PCA9542, PCA9543, PCA9544, PCA9545, PCA9546, PCA9547
+ *     and PCA9548.
+ *
+ * These chips are all controlled via the I2C bus itself, and all have a
+ * single 8-bit register (normally at 0x70).  The upstream "parent" bus fans
+ * out to two, four, or eight downstream busses or channels; which of these
+ * are selected is determined by the chip type and register contents.  A
+ * mux can select only one sub-bus at a time; a switch can select any
+ * combination simultaneously.
+ *
+ * Based on:
+ *    pca954x.c from Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
+ * Copyright (C) 2006
+ *
+ * Based on:
+ *    pca954x.c from Ken Harrenstien
+ * Copyright (C) 2004 Google, Inc. (Ken Harrenstien)
+ *
+ * Based on:
+ *    i2c-virtual_cb.c from Brian Kuschak <bkuschak-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
+ * and
+ *    pca9540.c from Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>, which was
+ *    based on pcf8574.c from the same project by Frodo Looijaard,
+ *    Philip Edelbrock, Dan Eaton and Aurelien Jarno.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+
+#include <linux/i2c/pca954x.h>
+
+#define PCA954X_MAX_NCHANS 8
+
+enum pca_type {
+	pca_9540,
+	pca_9542,
+	pca_9543,
+	pca_9544,
+	pca_9545,
+	pca_9546,
+	pca_9547,
+	pca_9548,
+};
+
+struct pca954x {
+	struct i2c_client *client;
+	struct i2c_client dev;
+	unsigned int type;
+	struct i2c_adapter *virt_adaps[PCA954X_MAX_NCHANS];
+
+	u8 last_chan;
+	unsigned int deselect_on_exit:1;
+	int (*is_chan_connected)(struct i2c_adapter *adap,
+			struct i2c_client *client, u32 chan);
+};
+
+struct chip_desc {
+	u8 nchans;
+	u8 enable;		/* used for muxes only */
+	enum muxtype {
+		pca954x_ismux = 0,
+		pca954x_isswi
+	} muxtype;
+};
+
+/* Provide specs for the PCA954x types we know about */
+static struct chip_desc chips[] = {
+	[pca_9540] = {
+		.nchans = 2,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
+	[pca_9542] = {
+		.nchans = 2,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
+	[pca_9543] = {
+		.nchans = 2,
+		.enable = 0x0,
+		.muxtype = pca954x_isswi,
+	},
+	[pca_9544] = {
+		.nchans = 4,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
+	[pca_9542] = {
+		.nchans = 4,
+		.enable = 0x0,
+		.muxtype = pca954x_isswi,
+	},
+	[pca_9542] = {
+		.nchans = 4,
+		.enable = 0x0,
+		.muxtype = pca954x_isswi,
+	},
+	[pca_9542] = {
+		.nchans = 8,
+		.enable = 0x8,
+		.muxtype = pca954x_ismux,
+	},
+	[pca_9542] = {
+		.nchans = 8,
+		.enable = 0x0,
+		.muxtype = pca954x_isswi,
+	},
+};
+
+static const struct i2c_device_id pca954x_id[] = {
+	{ "pca9540", pca_9540 },
+	{ "pca9542", pca_9542 },
+	{ "pca9543", pca_9543 },
+	{ "pca9544", pca_9544 },
+	{ "pca9545", pca_9545 },
+	{ "pca9546", pca_9546 },
+	{ "pca9547", pca_9547 },
+	{ "pca9548", pca_9548 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pca954x_id);
+
+static int pca954x_xfer(struct i2c_adapter *adap,
+			struct i2c_client *client, int read_write, u8 *val)
+{
+	int ret = -ENODEV;
+
+	if (adap->algo->master_xfer) {
+		struct i2c_msg msg;
+		char buf[1];
+
+		msg.addr = client->addr;
+		msg.flags = (read_write == I2C_SMBUS_READ ? I2C_M_RD : 0);
+		msg.len = 1;
+		buf[0] = *val;
+		msg.buf = buf;
+		ret = adap->algo->master_xfer(adap, &msg, 1);
+	} else if (adap->algo->smbus_xfer) {
+		union i2c_smbus_data data;
+		ret = adap->algo->smbus_xfer(adap, client->addr,
+						client->flags & I2C_M_TEN,
+						read_write,
+						*val, I2C_SMBUS_BYTE, &data);
+	}
+
+	return ret;
+}
+
+static int pca954x_select_chan(struct i2c_adapter *adap,
+				void *client, u32 chan)
+{
+	struct pca954x *data = i2c_get_clientdata((struct i2c_client *) client);
+	struct chip_desc *chip = &chips[data->type];
+	u8 regval = 0;
+	int ret = 0;
+
+	/* Callback to check for bus connection */
+	if (data->is_chan_connected) {
+		ret = data->is_chan_connected(adap, client, chan);
+		if (!ret)
+			return -EBUSY;
+	}
+
+	/* we make switches look like muxes, not sure how to be smarter */
+	if (chip->muxtype == pca954x_ismux)
+		regval = chan | chip->enable;
+	else
+		regval = 1 << chan;
+
+	/* Only select the channel if its different from the last channel */
+	if (data->last_chan != chan) {
+		ret = pca954x_xfer(adap, client, I2C_SMBUS_WRITE, &regval);
+		data->last_chan = chan;
+	}
+
+	return ret;
+}
+
+static int pca954x_deselect_mux(struct i2c_adapter *adap,
+				void *client, u32 chan)
+{
+	struct pca954x *data = i2c_get_clientdata((struct i2c_client *) client);
+	struct chip_desc *chip = &chips[data->type];
+	u8 regval = 0;
+
+	/* Check if we have to stay on the last channel we selected */
+	if (!data->deselect_on_exit && (chip->muxtype == pca954x_ismux))
+		return 0;
+
+	/* Signal we are deselecting active channel */
+	data->last_chan = -1;
+
+	regval = chan | ~chip->enable;
+	return pca954x_xfer(adap, client, I2C_SMBUS_WRITE, &regval);
+}
+
+/*
+ * I2C init/probing/exit functions
+ */
+
+static int pca954x_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
+	struct pca954x_platform_data *pdata = client->dev.platform_data;
+	int i, n, f;
+	struct pca954x *data;
+	int ret = -ENODEV;
+
+	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
+		goto err;
+
+	data = kzalloc(sizeof(struct pca954x), GFP_KERNEL);
+	if (!data) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	i2c_set_clientdata(client, data);
+
+	/* Read the mux register at addr.  This does two things: it verifies
+	 * that the mux is in fact present, and fetches its current
+	 * contents for possible use with a future deselect algorithm.
+	 */
+	i = i2c_smbus_read_byte(client);
+	if (i < 0) {
+		dev_warn(&client->dev, "probe failed\n");
+		goto exit_free;
+	}
+
+	data->type = id->driver_data;
+	data->last_chan = -1;			/* force the first selection */
+
+	/* Now create virtual busses and adapters for them */
+	for (i = 0; i < chips[data->type].nchans; i++) {
+		f = 0;				/* dynamic adap number */
+		if (pdata && (i < pdata->num_modes))
+			f = pdata->modes[i].adap_id; /* force static number */
+
+		data->virt_adaps[i] = i2c_add_mux_adapter(adap, client, f, i,
+						&pca954x_select_chan,
+						&pca954x_deselect_mux);
+		if (data->virt_adaps[i] == NULL) {
+			ret = -ENODEV;
+			goto virt_reg_failed;
+		}
+
+		data->deselect_on_exit = pdata->modes[i].deselect_on_exit;
+		data->is_chan_connected = pdata->modes[i].is_chan_connected;
+	}
+
+	dev_info(&client->dev, "registered %d virtual busses for I2C %s %s\n",
+			i, chips[data->type].muxtype == pca954x_ismux ?
+					"mux" : "switch", client->name);
+
+	return 0;
+
+virt_reg_failed:
+	for (n = 0; n < i; n++)
+		i2c_del_mux_adapter(data->virt_adaps[n]);
+	i2c_detach_client(client);
+exit_free:
+	kfree(data);
+err:
+	return ret;
+}
+
+static int __devexit pca954x_remove(struct i2c_client *client)
+{
+	struct pca954x *data = i2c_get_clientdata(client);
+	struct chip_desc *chip = &chips[data->type];
+	int i, err;
+
+	for (i = 0; i < chip->nchans; ++i)
+		if (data->virt_adaps[i]) {
+			err = i2c_del_mux_adapter(data->virt_adaps[i]);
+			if (err)
+				return err;
+			data->virt_adaps[i] = NULL;
+		}
+
+	kfree(data);
+	return 0;
+}
+
+static struct i2c_driver pca954x_driver = {
+	.driver = {
+		.name	= "pca954x",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= pca954x_probe,
+	.remove	= __devexit_p(pca954x_remove),
+	.id_table = pca954x_id,
+};
+
+static int __init pca954x_init(void)
+{
+	return i2c_add_driver(&pca954x_driver);
+}
+
+static void __exit pca954x_exit(void)
+{
+	i2c_del_driver(&pca954x_driver);
+}
+
+module_init(pca954x_init);
+module_exit(pca954x_exit);
+
+MODULE_AUTHOR("Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>");
+MODULE_DESCRIPTION("PCA954x I2C mux/switch driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c/pca954x.h b/include/linux/i2c/pca954x.h
new file mode 100644
index 0000000..18d5e79
--- /dev/null
+++ b/include/linux/i2c/pca954x.h
@@ -0,0 +1,13 @@
+/* Platform data for the PCA954x I2C multiplexers */
+
+struct pca954x_platform_mode {
+	int		adap_id;
+	unsigned int	deselect_on_exit:1;
+	int		(*is_chan_connected)(struct i2c_adapter *adap,
+					struct i2c_client *client, u32 chan);
+};
+
+struct pca954x_platform_data {
+	struct pca954x_platform_mode *modes;
+	int num_modes;
+};
-- 
1.5.6.3

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[David Brownell]

On Thursday 05 February 2009, Rodolfo Giometti wrote:
> 
> When we are removing an adapter we decide to ignore any active client
> detaching faults for two reasons:
> 
> 1) one (or more) active client may be switched off, so it cannot
>    replay to the "adapter removed" event.
> 
> 2) it shouldn't happen, and even if it happens it may be due a bus
>    fault which can be resolved by resetting the adapter (most of them
>    can be resetted simply by rmmod and then insmod the module again).

This bothers me.  If the client can't detach, it can't;
there may be all kinds of chaos introduced by trying
to fake success in such cases.  Like resources that
suddenly just vanish, leaving breakage in their wake.

If this is a shutdown() path, where it's basically
just a polite driver notification, that's one thing.

But I2C gets used as a system control bus, so this
sort of "ignore the errors" stuff worries me a lot.

- Dave

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

On Thu, Feb 05, 2009 at 04:02:43PM -0800, David Brownell wrote:
> On Thursday 05 February 2009, Rodolfo Giometti wrote:
> > 
> > When we are removing an adapter we decide to ignore any active client
> > detaching faults for two reasons:
> > 
> > 1) one (or more) active client may be switched off, so it cannot
> >    replay to the "adapter removed" event.
> > 
> > 2) it shouldn't happen, and even if it happens it may be due a bus
> >    fault which can be resolved by resetting the adapter (most of them
> >    can be resetted simply by rmmod and then insmod the module again).
> 
> This bothers me.  If the client can't detach, it can't;
> there may be all kinds of chaos introduced by trying
> to fake success in such cases.  Like resources that
> suddenly just vanish, leaving breakage in their wake.
> 
> If this is a shutdown() path, where it's basically
> just a polite driver notification, that's one thing.
> 
> But I2C gets used as a system control bus, so this
> sort of "ignore the errors" stuff worries me a lot.

If so I suppose we should take in account the
adapter->client_unregister() return value also, which is currently
ignored in i2c_unregister_device() function...

However, since I2c is a bus it could be possible that a slave device
stops functioning (or the user decides to turn it off), so aborting
the adapter removal in these cases can be not right... moreover if the
adapter should be logically separated from the i2c clients I suppose
is better allowing the user to remove (and eventually replace) it and
then resolving i2c client's stale states into the relative driver.

I suppose this is the same behaviour of USB bus: we can remove an USB
host adapter independently from USB devices.

Ciao,

Rodolfo

-- 

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

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Jean Delvare]

On Fri, 6 Feb 2009 11:22:21 +0100, Rodolfo Giometti wrote:
> On Thu, Feb 05, 2009 at 04:02:43PM -0800, David Brownell wrote:
> > On Thursday 05 February 2009, Rodolfo Giometti wrote:
> > > 
> > > When we are removing an adapter we decide to ignore any active client
> > > detaching faults for two reasons:
> > > 
> > > 1) one (or more) active client may be switched off, so it cannot
> > >    replay to the "adapter removed" event.
> > > 
> > > 2) it shouldn't happen, and even if it happens it may be due a bus
> > >    fault which can be resolved by resetting the adapter (most of them
> > >    can be resetted simply by rmmod and then insmod the module again).
> > 
> > This bothers me.  If the client can't detach, it can't;
> > there may be all kinds of chaos introduced by trying
> > to fake success in such cases.  Like resources that
> > suddenly just vanish, leaving breakage in their wake.
> > 
> > If this is a shutdown() path, where it's basically
> > just a polite driver notification, that's one thing.
> > 
> > But I2C gets used as a system control bus, so this
> > sort of "ignore the errors" stuff worries me a lot.
> 
> If so I suppose we should take in account the
> adapter->client_unregister() return value also, which is currently
> ignored in i2c_unregister_device() function...

Taking the old, deprecated, notoriously broken i2c model as an example
doesn't necessarily serve your cause ;)

> However, since I2c is a bus it could be possible that a slave device
> stops functioning (or the user decides to turn it off), so aborting
> the adapter removal in these cases can be not right... moreover if the
> adapter should be logically separated from the i2c clients I suppose
> is better allowing the user to remove (and eventually replace) it and
> then resolving i2c client's stale states into the relative driver.
> 
> I suppose this is the same behaviour of USB bus: we can remove an USB
> host adapter independently from USB devices.

The big difference, I presume, is that USB is never a system bus. But
even for USB, devices can't exist without the host. If you kill the
host and resurrect it later, you also kill the devices and resurrect
them later.

The debate about drivers failing device removal is an old one, not
specific to i2c. My opinion is that .remove() should succeed as much as
possible. It should really only fail if the problem is so serious that
the system's state would otherwise become a problem (e.g. freeing
memory which is still referenced.) This should be a rare case.

-- 
Jean Delvare

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

On Fri, Feb 06, 2009 at 01:15:20PM +0100, Jean Delvare wrote:
> 
> Taking the old, deprecated, notoriously broken i2c model as an example
> doesn't necessarily serve your cause ;)

I see, but till this part is in we should consider it, shouldn't we?
Eheheh :D

> The big difference, I presume, is that USB is never a system bus. But
> even for USB, devices can't exist without the host. If you kill the
> host and resurrect it later, you also kill the devices and resurrect
> them later.

But this can be done with i2c devices too! Let's think about leds
drivers with uses an i2c GPIOs expander, they can exist without any
adapter, even if they are not functional of course. :)

> The debate about drivers failing device removal is an old one, not
> specific to i2c. My opinion is that .remove() should succeed as much as
> possible. It should really only fail if the problem is so serious that
> the system's state would otherwise become a problem (e.g. freeing
> memory which is still referenced.) This should be a rare case.

I think that i2c clients devices should be really separated by adapter
ones, that is we should be free to add and remove any adapter without
considering the clients state. Then each clients device drivers should
free each allocated resource they use and the adapters shouldn't know
about it.

Doing like this will semply the code a lot and it allows i2c core to
be more flexible.

Ciao,

Rodolfo

-- 

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

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Jean Delvare]

On Fri, 6 Feb 2009 13:59:07 +0100, Rodolfo Giometti wrote:
> On Fri, Feb 06, 2009 at 01:15:20PM +0100, Jean Delvare wrote:
> > 
> > Taking the old, deprecated, notoriously broken i2c model as an example
> > doesn't necessarily serve your cause ;)
> 
> I see, but till this part is in we should consider it, shouldn't we?
> Eheheh :D

No. As I've said before, ideally I wanted legacy binding style to die
_before_ we add support for multiplexing. But if we don't want to wait,
I still want legacy binding and multiplexing to be mutually exclusive.

> > The big difference, I presume, is that USB is never a system bus. But
> > even for USB, devices can't exist without the host. If you kill the
> > host and resurrect it later, you also kill the devices and resurrect
> > them later.
> 
> But this can be done with i2c devices too! Let's think about leds
> drivers with uses an i2c GPIOs expander, they can exist without any
> adapter, even if they are not functional of course. :)

That not the question. We all agree that some i2c chip drivers don't
need to be able to prevent the removal of the underlying bus driver.
The open point is whether some i2c chip drivers do. Honestly, I don't
know, as such cases would happen on embedded systems, which are out of
my realm. But maybe David has examples in mind.

> > The debate about drivers failing device removal is an old one, not
> > specific to i2c. My opinion is that .remove() should succeed as much as
> > possible. It should really only fail if the problem is so serious that
> > the system's state would otherwise become a problem (e.g. freeing
> > memory which is still referenced.) This should be a rare case.
> 
> I think that i2c clients devices should be really separated by adapter
> ones, that is we should be free to add and remove any adapter without
> considering the clients state. Then each clients device drivers should
> free each allocated resource they use and the adapters shouldn't know
> about it.

How does your description differ from the current implementation? I
don't follow you.

> Doing like this will semply the code a lot and it allows i2c core to
> be more flexible.

Patches welcome... but I have no clue what you want to do at this point.

-- 
Jean Delvare

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

On Fri, Feb 06, 2009 at 10:15:19PM +0100, Jean Delvare wrote:
> 
> No. As I've said before, ideally I wanted legacy binding style to die
> _before_ we add support for multiplexing. But if we don't want to wait,
> I still want legacy binding and multiplexing to be mutually exclusive.

Ok.

> > > The big difference, I presume, is that USB is never a system bus. But
> > > even for USB, devices can't exist without the host. If you kill the
> > > host and resurrect it later, you also kill the devices and resurrect
> > > them later.
> > 
> > But this can be done with i2c devices too! Let's think about leds
> > drivers with uses an i2c GPIOs expander, they can exist without any
> > adapter, even if they are not functional of course. :)
> 
> That not the question. We all agree that some i2c chip drivers don't
> need to be able to prevent the removal of the underlying bus driver.
> The open point is whether some i2c chip drivers do. Honestly, I don't
> know, as such cases would happen on embedded systems, which are out of
> my realm. But maybe David has examples in mind.

I see. But in this case we should dig into the code and verify what
each i2c drivers wish to do during driver removal and eventually fix
it, is that right?

> > > The debate about drivers failing device removal is an old one, not
> > > specific to i2c. My opinion is that .remove() should succeed as much as
> > > possible. It should really only fail if the problem is so serious that
> > > the system's state would otherwise become a problem (e.g. freeing
> > > memory which is still referenced.) This should be a rare case.
> > 
> > I think that i2c clients devices should be really separated by adapter
> > ones, that is we should be free to add and remove any adapter without
> > considering the clients state. Then each clients device drivers should
> > free each allocated resource they use and the adapters shouldn't know
> > about it.
> 
> How does your description differ from the current implementation? I
> don't follow you.

Currently an i2c driver may block adpater removal procedure, my
implementation, instead, considers i2c adapter completely separated
from i2c clients so that no one can't interfere with adapter
insertion/removal code.

> > Doing like this will semply the code a lot and it allows i2c core to
> > be more flexible.
> 
> Patches welcome... but I have no clue what you want to do at this point.

If you agree I can provide a branch tree from current vanilla and
describe my solution into linux-i2c wiki, so users may test it and
propose enhancements without touching current stable code.

Ciao,

Rodolfo

-- 

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

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Jean Delvare]

On Mon, 9 Feb 2009 14:06:42 +0100, Rodolfo Giometti wrote:
> On Fri, Feb 06, 2009 at 10:15:19PM +0100, Jean Delvare wrote:
> > That not the question. We all agree that some i2c chip drivers don't
> > need to be able to prevent the removal of the underlying bus driver.
> > The open point is whether some i2c chip drivers do. Honestly, I don't
> > know, as such cases would happen on embedded systems, which are out of
> > my realm. But maybe David has examples in mind.
> 
> I see. But in this case we should dig into the code and verify what
> each i2c drivers wish to do during driver removal and eventually fix
> it, is that right?

I simply don't know. As I have written above, I do not have any example
of an I2C device which should be allowed to prevent bus removal. But I
am a PC person and the i2c subsystem is used in a very different way on
embedded systems. So better discuss this with people who know these
systems.

> > (...)
> > How does your description differ from the current implementation? I
> > don't follow you.
> 
> Currently an i2c driver may block adpater removal procedure, my
> implementation, instead, considers i2c adapter completely separated
> from i2c clients so that no one can't interfere with adapter
> insertion/removal code.

As far as I can see this only happens for legacy i2c chip drivers,
which implement driver->detach_client() or driver->detach_adapter().
The plan is to get rid of the last few users of this model as soon as
possible, so that's not really relevant.

Do you see any other place in i2c-core where we check for error on
device removal and do something based on that, in particular for
new-style i2c chip drivers?

> > Patches welcome... but I have no clue what you want to do at this point.
> 
> If you agree I can provide a branch tree from current vanilla and
> describe my solution into linux-i2c wiki, so users may test it and
> propose enhancements without touching current stable code.

Well, you can always do that if you want, but please first clearly
define the practical problem you are trying to solve. We are not going
to change the i2c core without a good reason.

Oh, and if you have time to spend on i2c, then helping getting rid of
the remaining legacy drivers would be greatly appreciated. As long as
we have to support both models, there are many core cleanups and
improvements that are simply not possible (or so risky that nobody
wants to try.)

-- 
Jean Delvare

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

On Tue, Feb 10, 2009 at 12:08:39PM +0100, Jean Delvare wrote:
> On Mon, 9 Feb 2009 14:06:42 +0100, Rodolfo Giometti wrote:
> > 
> > If you agree I can provide a branch tree from current vanilla and
> > describe my solution into linux-i2c wiki, so users may test it and
> > propose enhancements without touching current stable code.
> 
> Well, you can always do that if you want, but please first clearly
> define the practical problem you are trying to solve. We are not going
> to change the i2c core without a good reason.

I created a new page here:

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

Please, take a look and report everything should be fixed. :)

Hope I did exactly what you requested...

Ciao,

Rodolfo

-- 

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

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Jean Delvare]

Hi Rodolfo,

Sorry for the late answer, I missed this post in the middle of the
thread.

On Wed, 18 Feb 2009 17:48:13 +0100, Rodolfo Giometti wrote:
> On Tue, Feb 10, 2009 at 12:08:39PM +0100, Jean Delvare wrote:
> > Well, you can always do that if you want, but please first clearly
> > define the practical problem you are trying to solve. We are not going
> > to change the i2c core without a good reason.
> 
> I created a new page here:
> 
>    http://i2c.wiki.kernel.org/index.php/I2C_bus_multiplexing

Thanks for doing this!

> Please, take a look and report everything should be fixed. :)

Random comments:

* I really would like to get rid of the "virtual" term that has been
  repeatedly abused. Bus segments behind multiplexers are very real.

* I am surprised that you use the term "trunk" for all segments, be
  they in front of or behind multiplexers. When I think of a
  multiplexer, I designate the segment between the controller and the
  multiplexer as the trunk, and the segments behind the multiplexer as
  branches.

Other than that I am rather happy with your page, which explains the
problem and the proposed solution clearly.

-- 
Jean Delvare

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

On Thu, May 28, 2009 at 09:50:54AM +0200, Jean Delvare wrote:
> Hi Rodolfo,
> 
> Sorry for the late answer, I missed this post in the middle of the
> thread.

Don't worry about it! :)

> On Wed, 18 Feb 2009 17:48:13 +0100, Rodolfo Giometti wrote:
> > On Tue, Feb 10, 2009 at 12:08:39PM +0100, Jean Delvare wrote:
> > > Well, you can always do that if you want, but please first clearly
> > > define the practical problem you are trying to solve. We are not going
> > > to change the i2c core without a good reason.
> > 
> > I created a new page here:
> > 
> >    http://i2c.wiki.kernel.org/index.php/I2C_bus_multiplexing
> 
> Thanks for doing this!
> 
> > Please, take a look and report everything should be fixed. :)
> 
> Random comments:
> 
> * I really would like to get rid of the "virtual" term that has been
>   repeatedly abused. Bus segments behind multiplexers are very real.

Actually I use the term "virtual" regarding to the adapters not to the
bus segments. In fact the new devices created are not "real" adapters
but "virtual" ones.

> * I am surprised that you use the term "trunk" for all segments, be
>   they in front of or behind multiplexers. When I think of a
>   multiplexer, I designate the segment between the controller and the
>   multiplexer as the trunk, and the segments behind the multiplexer as
>   branches.

Ok, this can be corrected easily. :)

> Other than that I am rather happy with your page, which explains the
> problem and the proposed solution clearly.

Thanks a lot!

Ciao,

Rodolfo

-- 

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

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Jean Delvare]

On Thu, 28 May 2009 09:58:36 +0200, Rodolfo Giometti wrote:
> On Thu, May 28, 2009 at 09:50:54AM +0200, Jean Delvare wrote:
> > * I really would like to get rid of the "virtual" term that has been
> >   repeatedly abused. Bus segments behind multiplexers are very real.
> 
> Actually I use the term "virtual" regarding to the adapters not to the
> bus segments. In fact the new devices created are not "real" adapters
> but "virtual" ones.

The confusion comes from the fact that struct i2c_adapter should have
been named struct i2c_segment, as this is what it really represents.
But back when it was decided, multiplexing wasn't considered so the
developers thought "controller", "adapter" and "segment" were the same
thing.

Ideally we should rename i2c_adapter to i2c_segment, but this seems a
little intrusive for a rather thin benefit.

-- 
Jean Delvare

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Rodolfo Giometti]

On Thu, May 28, 2009 at 09:50:54AM +0200, Jean Delvare wrote:
> Hi Rodolfo,
> 
> Sorry for the late answer, I missed this post in the middle of the
> thread.
> 
> On Wed, 18 Feb 2009 17:48:13 +0100, Rodolfo Giometti wrote:
> > On Tue, Feb 10, 2009 at 12:08:39PM +0100, Jean Delvare wrote:
> > > Well, you can always do that if you want, but please first clearly
> > > define the practical problem you are trying to solve. We are not going
> > > to change the i2c core without a good reason.
> > 
> > I created a new page here:
> > 
> >    http://i2c.wiki.kernel.org/index.php/I2C_bus_multiplexing
> 
> Thanks for doing this!
> 
> > Please, take a look and report everything should be fixed. :)
> 
> Random comments:
> 
> * I really would like to get rid of the "virtual" term that has been
>   repeatedly abused. Bus segments behind multiplexers are very real.
> 
> * I am surprised that you use the term "trunk" for all segments, be
>   they in front of or behind multiplexers. When I think of a
>   multiplexer, I designate the segment between the controller and the
>   multiplexer as the trunk, and the segments behind the multiplexer as
>   branches.

Both fixed. Please review the page again. :)

Ciao,

Rodolfo

-- 

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

Re: [PATCH 3/6] i2c: ignore active clients detaching during adapter removal.

[Jean Delvare]

On Fri, 29 May 2009 09:28:35 +0200, Rodolfo Giometti wrote:
> On Thu, May 28, 2009 at 09:50:54AM +0200, Jean Delvare wrote:
> > Random comments:
> > 
> > * I really would like to get rid of the "virtual" term that has been
> >   repeatedly abused. Bus segments behind multiplexers are very real.
> > 
> > * I am surprised that you use the term "trunk" for all segments, be
> >   they in front of or behind multiplexers. When I think of a
> >   multiplexer, I designate the segment between the controller and the
> >   multiplexer as the trunk, and the segments behind the multiplexer as
> >   branches.
> 
> Both fixed. Please review the page again. :)

Looks good, thanks!

-- 
Jean Delvare

Re: [PATCH 2/6] i2c: use rwsem instead of mutex.

[Jean Delvare]

Hi Rodolfo,

On Thu,  5 Feb 2009 14:36:09 +0100, Rodolfo Giometti wrote:
> By using rwsem we can easily manage recursive calls of
> i2c_scan_static_board_info() function without breaking the locking.
> 
> Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
> ---
>  drivers/i2c/i2c-boardinfo.c |    6 +++---
>  drivers/i2c/i2c-core.c      |    4 ++--
>  drivers/i2c/i2c-core.h      |    2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-boardinfo.c b/drivers/i2c/i2c-boardinfo.c
> index ffb35f0..981cdd8 100644
> --- a/drivers/i2c/i2c-boardinfo.c
> +++ b/drivers/i2c/i2c-boardinfo.c
> @@ -25,7 +25,7 @@
>  /* These symbols are exported ONLY FOR the i2c core.
>   * No other users will be supported.
>   */
> -DEFINE_MUTEX(__i2c_board_lock);
> +DECLARE_RWSEM(__i2c_board_lock);
>  EXPORT_SYMBOL_GPL(__i2c_board_lock);
>  
>  LIST_HEAD(__i2c_board_list);
> @@ -63,7 +63,7 @@ i2c_register_board_info(int busnum,
>  {
>  	int status;
>  
> -	mutex_lock(&__i2c_board_lock);
> +	down_write(&__i2c_board_lock);
>  
>  	/* dynamic bus numbers will be assigned after the last static one */
>  	if (busnum >= __i2c_first_dynamic_bus_num)
> @@ -84,7 +84,7 @@ i2c_register_board_info(int busnum,
>  		list_add_tail(&devinfo->list, &__i2c_board_list);
>  	}
>  
> -	mutex_unlock(&__i2c_board_lock);
> +	up_write(&__i2c_board_lock);
>  
>  	return status;
>  }
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 3454878..74b89a9 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -408,7 +408,7 @@ static void i2c_scan_static_board_info(struct i2c_adapter *adapter)
>  {
>  	struct i2c_devinfo	*devinfo;
>  
> -	mutex_lock(&__i2c_board_lock);
> +	down_read(&__i2c_board_lock);
>  	list_for_each_entry(devinfo, &__i2c_board_list, list) {
>  		if (devinfo->busnum == adapter->nr
>  				&& !i2c_new_device(adapter,
> @@ -417,7 +417,7 @@ static void i2c_scan_static_board_info(struct i2c_adapter *adapter)
>  				i2c_adapter_id(adapter),
>  				devinfo->board_info.addr);
>  	}
> -	mutex_unlock(&__i2c_board_lock);
> +	up_read(&__i2c_board_lock);
>  }
>  
>  static int i2c_do_add_adapter(struct device_driver *d, void *data)
> diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h
> index cd5bff8..e6af064 100644
> --- a/drivers/i2c/i2c-core.h
> +++ b/drivers/i2c/i2c-core.h
> @@ -25,7 +25,7 @@ struct i2c_devinfo {
>  /* board_lock protects board_list and first_dynamic_bus_num.
>   * only i2c core components are allowed to use these symbols.
>   */
> -extern struct mutex	__i2c_board_lock;
> +extern struct rw_semaphore	__i2c_board_lock;
>  extern struct list_head	__i2c_board_list;
>  extern int		__i2c_first_dynamic_bus_num;
>  

Patch applied, thank you. I've add missing includes of <linux/rwsem.h>
in all 3 files.

-- 
Jean Delvare

Re: i2c bus multiplexing (Version 2).

[Muralidharan Karicheri]

> This patch set adds to the I2C core the ability to handle multiplexed
> I2C bus topologies by presenting each multiplexed segment as a I2C
> adapter.

Hi Rodolfo,

Thanks for this patch. I learned about this patch through linux-media
mailing list. 

1)I have a sensor board header that uses pca9543a i2c switch
which is addressed by this patch. In my board, I have another
i2c chip that share the same i2c address with the sensor.
Currently I cannot have both these drivers co-exists due to
this. Please let me know if this can work once I use this patch.

2)Which kernel version you use for this patch?

3) Where can I download your patch so that I can give it a
shot. Is there any platform data I need to setup in my arch/
folder to use this patch?

4) When do you think this will be merged to mainline kernel?

Thanks

Murali

Re: i2c bus multiplexing (Version 2).

[Jean Delvare]

Hi Murali,

On Fri, 11 Dec 2009 22:41:33 +0000 (UTC), Muralidharan Karicheri wrote:
> 
> > This patch set adds to the I2C core the ability to handle multiplexed
> > I2C bus topologies by presenting each multiplexed segment as a I2C
> > adapter.
> 
> Hi Rodolfo,
> 
> Thanks for this patch. I learned about this patch through linux-media
> mailing list. 
> 
> 1)I have a sensor board header that uses pca9543a i2c switch
> which is addressed by this patch. In my board, I have another
> i2c chip that share the same i2c address with the sensor.
> Currently I cannot have both these drivers co-exists due to
> this. Please let me know if this can work once I use this patch.

Yes, this is the idea. With bus multiplexing support, we can finally
handle bus topologies with multiple chips using the same I2C address.

> 2)Which kernel version you use for this patch?
> 
> 3) Where can I download your patch so that I can give it a
> shot. Is there any platform data I need to setup in my arch/
> folder to use this patch?
> 
> 4) When do you think this will be merged to mainline kernel?

I am reviewing the patches at the moment, after Michael sends an
updated version addressing my comments, we should be ready to go. So we
currently aim at merging i2c multiplexing support into kernel 2.6.35.
At least a first version... more features can be added later.

-- 
Jean Delvare