[PATCH 1/3] i2c: virtual i2c adapter support.

[PATCH 1/3] i2c: virtual i2c adapter support.

[Rodolfo Giometti]

Simplifies access to complex multiplexed I2C bus topologies, by
presenting each multiplexed bus segment as a virtual I2C adapter. In
this manner I2C devices "after" the multiplexer can ba managed as
usual.

Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
---
 drivers/i2c/Kconfig    |    9 +++
 drivers/i2c/Makefile   |    1 +
 drivers/i2c/i2c-virt.c |  188 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c-id.h |    3 +
 include/linux/i2c.h    |   17 +++++
 5 files changed, 218 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/i2c-virt.c

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 9686734..053fe2f 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -27,6 +27,15 @@ config I2C_BOARDINFO
 	boolean
 	default y
 
+config I2C_VIRT
+	tristate "I2C virtual adapter support"
+	depends on I2C
+	help
+	  Say Y here if you want the I2C core to support the ability to have
+	  virtual adapters. Virtual adapters are useful 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..db72ed9 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
 obj-$(CONFIG_I2C)		+= i2c-core.o
+obj-$(CONFIG_I2C_VIRT)		+= i2c-virt.o
 obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
 obj-y				+= busses/ chips/ algos/
 
diff --git a/drivers/i2c/i2c-virt.c b/drivers/i2c/i2c-virt.c
new file mode 100644
index 0000000..ed58f64
--- /dev/null
+++ b/drivers/i2c/i2c-virt.c
@@ -0,0 +1,188 @@
+/*
+ * i2c-virt.c - Virtual I2C bus driver.
+ *
+ * Copyright (c) 2008 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
+ * Copyright (c) 2008 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_virt_priv {
+	struct i2c_adapter *parent_adap;
+	struct i2c_client *client;	/* The mux chip/device */
+
+	u32 id;				/* the mux id */
+
+	int (*select)(struct i2c_adapter *, struct i2c_client *, u32);
+					/* Enable the mux */
+	int (*deselect)(struct i2c_adapter *, struct i2c_client *, u32);
+					/* Disable the mux */
+};
+
+#define VIRT_TIMEOUT		(HZ/2)
+#define VIRT_RETRIES		3
+
+static int i2c_virt_master_xfer(struct i2c_adapter *adap,
+				struct i2c_msg msgs[], int num)
+{
+	struct i2c_virt_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(&parent->bus_lock);
+
+	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_virt_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_virt_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(&parent->bus_lock);
+
+	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_virt_functionality(struct i2c_adapter *adap)
+{
+	struct i2c_virt_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->parent_adap;
+
+	return parent->algo->functionality(parent);
+}
+
+struct i2c_adapter *i2c_add_virt_adapter(struct i2c_adapter *parent,
+				struct i2c_client *client,
+				u32 force_nr, u32 mux_val,
+				int (*select_cb) (struct i2c_adapter *,
+						struct i2c_client *, u32),
+				int (*deselect_cb) (struct i2c_adapter *,
+						struct i2c_client *, u32))
+{
+	struct i2c_adapter *adap;
+	struct i2c_virt_priv *priv;
+	struct i2c_algorithm *algo;
+	int ret;
+
+	adap = kzalloc(sizeof(struct i2c_adapter)
+			+ sizeof(struct i2c_virt_priv)
+			+ sizeof(struct i2c_algorithm), GFP_KERNEL);
+	if (!adap)
+		return NULL;
+
+	priv = (struct i2c_virt_priv *)(adap + 1);
+	algo = (struct i2c_algorithm *)(priv + 1);
+
+	/* 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.
+	 */
+	algo->master_xfer = (parent->algo->master_xfer
+					? i2c_virt_master_xfer : NULL);
+	algo->smbus_xfer = (parent->algo->smbus_xfer
+					? i2c_virt_smbus_xfer : NULL);
+	algo->functionality = i2c_virt_functionality;
+
+	/* Now fill out new adapter structure */
+	snprintf(adap->name, sizeof(adap->name),
+			"i2c-%d-virt (mux %02x:%02x)",
+			i2c_adapter_id(parent), client->addr, mux_val);
+	adap->id = I2C_HW_VIRT | i2c_adapter_id(parent);
+	adap->algo = algo;
+	adap->algo_data = priv;
+	adap->timeout = VIRT_TIMEOUT;
+	adap->retries = VIRT_RETRIES;
+	adap->dev.parent = &parent->dev;
+
+	if (force_nr) {
+		adap->nr = force_nr;
+		ret = i2c_add_numbered_adapter(adap);
+	} else
+		ret = i2c_add_adapter(adap);
+	if (ret < 0) {
+		kfree(adap);
+		return NULL;
+	}
+
+	dev_info(&parent->dev, "i2c-virt-%d: Virtual I2C bus - "
+		"physical bus i2c-%d, multiplexer 0x%02x port %d\n",
+		i2c_adapter_id(adap), i2c_adapter_id(parent),
+		client->addr, mux_val);
+
+	return adap;
+}
+EXPORT_SYMBOL_GPL(i2c_add_virt_adapter);
+
+int i2c_del_virt_adapter(struct i2c_adapter *adap)
+{
+	int ret;
+
+	ret = i2c_del_adapter(adap);
+	if (ret < 0)
+		return ret;
+	kfree(adap);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_del_virt_adapter);
+
+MODULE_AUTHOR("Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org, " \
+		"Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>");
+MODULE_DESCRIPTION("Virtual I2C driver for multiplexed I2C busses");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
index 580acc9..ba19ef9 100644
--- a/include/linux/i2c-id.h
+++ b/include/linux/i2c-id.h
@@ -176,4 +176,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 fb9af6a..968e8f3 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -553,6 +553,23 @@ 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.
+ *
+ * The caller is expected to have the core_lists lock
+ */
+struct i2c_adapter *i2c_add_virt_adapter(struct i2c_adapter *parent,
+				struct i2c_client *client,
+				u32 force_nr, u32 mux_val,
+				int (*select_cb) (struct i2c_adapter *,
+						struct i2c_client *, u32),
+				int (*deselect_cb) (struct i2c_adapter *,
+						struct i2c_client *, u32));
+
+int i2c_del_virt_adapter(struct i2c_adapter *adap);
+
 
 #ifdef __KERNEL__
 
-- 
1.5.4.3


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support.

[Ben Dooks]

On Wed, Jun 18, 2008 at 03:06:03PM +0200, Rodolfo Giometti wrote:
> Simplifies access to complex multiplexed I2C bus topologies, by
> presenting each multiplexed bus segment as a virtual I2C adapter. In
> this manner I2C devices "after" the multiplexer can ba managed as
> usual.
> 
> Signed-off-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
> ---
>  drivers/i2c/Kconfig    |    9 +++
>  drivers/i2c/Makefile   |    1 +
>  drivers/i2c/i2c-virt.c |  188 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c-id.h |    3 +
>  include/linux/i2c.h    |   17 +++++
>  5 files changed, 218 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/i2c-virt.c
> 
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index 9686734..053fe2f 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -27,6 +27,15 @@ config I2C_BOARDINFO
>  	boolean
>  	default y
>  
> +config I2C_VIRT
> +	tristate "I2C virtual adapter support"
> +	depends on I2C
> +	help
> +	  Say Y here if you want the I2C core to support the ability to have
> +	  virtual adapters. Virtual adapters are useful 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..db72ed9 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -4,6 +4,7 @@
>  
>  obj-$(CONFIG_I2C_BOARDINFO)	+= i2c-boardinfo.o
>  obj-$(CONFIG_I2C)		+= i2c-core.o
> +obj-$(CONFIG_I2C_VIRT)		+= i2c-virt.o
>  obj-$(CONFIG_I2C_CHARDEV)	+= i2c-dev.o
>  obj-y				+= busses/ chips/ algos/
>  
> diff --git a/drivers/i2c/i2c-virt.c b/drivers/i2c/i2c-virt.c
> new file mode 100644
> index 0000000..ed58f64
> --- /dev/null
> +++ b/drivers/i2c/i2c-virt.c
> @@ -0,0 +1,188 @@
> +/*
> + * i2c-virt.c - Virtual I2C bus driver.
> + *
> + * Copyright (c) 2008 Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>
> + * Copyright (c) 2008 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_virt_priv {
> +	struct i2c_adapter *parent_adap;
> +	struct i2c_client *client;	/* The mux chip/device */
> +
> +	u32 id;				/* the mux id */
> +
> +	int (*select)(struct i2c_adapter *, struct i2c_client *, u32);
> +					/* Enable the mux */
> +	int (*deselect)(struct i2c_adapter *, struct i2c_client *, u32);
> +					/* Disable the mux */
> +};
> +
> +#define VIRT_TIMEOUT		(HZ/2)
> +#define VIRT_RETRIES		3
> +
> +static int i2c_virt_master_xfer(struct i2c_adapter *adap,
> +				struct i2c_msg msgs[], int num)
> +{
> +	struct i2c_virt_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(&parent->bus_lock);
> +
> +	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;
> +}

Out of interest, is it going to be better to hide the parent
away completely from the system? I suppose if clients have
already bound to it then we're probably going to just have to
live with it being available.

> +static int i2c_virt_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_virt_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(&parent->bus_lock);
> +
> +	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_virt_functionality(struct i2c_adapter *adap)
> +{
> +	struct i2c_virt_priv *priv = adap->algo_data;
> +	struct i2c_adapter *parent = priv->parent_adap;
> +
> +	return parent->algo->functionality(parent);
> +}
> +
> +struct i2c_adapter *i2c_add_virt_adapter(struct i2c_adapter *parent,
> +				struct i2c_client *client,
> +				u32 force_nr, u32 mux_val,
> +				int (*select_cb) (struct i2c_adapter *,
> +						struct i2c_client *, u32),
> +				int (*deselect_cb) (struct i2c_adapter *,
> +						struct i2c_client *, u32))
> +{
> +	struct i2c_adapter *adap;
> +	struct i2c_virt_priv *priv;
> +	struct i2c_algorithm *algo;
> +	int ret;
> +
> +	adap = kzalloc(sizeof(struct i2c_adapter)
> +			+ sizeof(struct i2c_virt_priv)
> +			+ sizeof(struct i2c_algorithm), GFP_KERNEL);
> +	if (!adap)
> +		return NULL;
> +
> +	priv = (struct i2c_virt_priv *)(adap + 1);
> +	algo = (struct i2c_algorithm *)(priv + 1);

you shouldn't need force-typecast here.

you could always make your i2c_virt_priv data contain the i2c_adapter
and i2c_algorithm structure so you can easily go between them.

> +	/* 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.
> +	 */
> +	algo->master_xfer = (parent->algo->master_xfer
> +					? i2c_virt_master_xfer : NULL);
> +	algo->smbus_xfer = (parent->algo->smbus_xfer
> +					? i2c_virt_smbus_xfer : NULL);
> +	algo->functionality = i2c_virt_functionality;

you're using kzalloc, so this boils down to the neater:

	if (parent->algo->master_xfer)
		algo->master_xfer = i2c_virt_master_xfer;
etc.

> +	/* Now fill out new adapter structure */
> +	snprintf(adap->name, sizeof(adap->name),
> +			"i2c-%d-virt (mux %02x:%02x)",
> +			i2c_adapter_id(parent), client->addr, mux_val);
> +	adap->id = I2C_HW_VIRT | i2c_adapter_id(parent);
> +	adap->algo = algo;
> +	adap->algo_data = priv;
> +	adap->timeout = VIRT_TIMEOUT;
> +	adap->retries = VIRT_RETRIES;
> +	adap->dev.parent = &parent->dev;

How about creating a struct with the callbacks in, and the data
for timeout, retries and any other data that would be needed?

> +	if (force_nr) {
> +		adap->nr = force_nr;
> +		ret = i2c_add_numbered_adapter(adap);
> +	} else
> +		ret = i2c_add_adapter(adap);
> +	if (ret < 0) {
> +		kfree(adap);
> +		return NULL;
> +	}
> +
> +	dev_info(&parent->dev, "i2c-virt-%d: Virtual I2C bus - "
> +		"physical bus i2c-%d, multiplexer 0x%02x port %d\n",
> +		i2c_adapter_id(adap), i2c_adapter_id(parent),
> +		client->addr, mux_val);
> +
> +	return adap;
> +}
> +EXPORT_SYMBOL_GPL(i2c_add_virt_adapter);
> +
> +int i2c_del_virt_adapter(struct i2c_adapter *adap)
> +{
> +	int ret;
> +
> +	ret = i2c_del_adapter(adap);
> +	if (ret < 0)
> +		return ret;
> +	kfree(adap);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_del_virt_adapter);
> +
> +MODULE_AUTHOR("Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org, " \
> +		"Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>");
> +MODULE_DESCRIPTION("Virtual I2C driver for multiplexed I2C busses");
> +MODULE_LICENSE("GPL");

"GPL v2" is the definition you probably want here.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support.

[Rodolfo Giometti]

[-- Attachment #1.1: Type: text/plain, Size: 4974 bytes --]

On Wed, Jun 18, 2008 at 08:00:08PM +0100, Ben Dooks wrote:

> > +static int i2c_virt_master_xfer(struct i2c_adapter *adap,
> > +				struct i2c_msg msgs[], int num)
> > +{
> > +	struct i2c_virt_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(&parent->bus_lock);
> > +
> > +	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;
> > +}
> 
> Out of interest, is it going to be better to hide the parent
> away completely from the system? I suppose if clients have
> already bound to it then we're probably going to just have to
> live with it being available.

I'm sorry but I don't understand what you mean. =:-o

The parent is strictly connected with the children virtual adapter, how
can I hide it?

> > +struct i2c_adapter *i2c_add_virt_adapter(struct i2c_adapter *parent,
> > +				struct i2c_client *client,
> > +				u32 force_nr, u32 mux_val,
> > +				int (*select_cb) (struct i2c_adapter *,
> > +						struct i2c_client *, u32),
> > +				int (*deselect_cb) (struct i2c_adapter *,
> > +						struct i2c_client *, u32))
> > +{
> > +	struct i2c_adapter *adap;
> > +	struct i2c_virt_priv *priv;
> > +	struct i2c_algorithm *algo;
> > +	int ret;
> > +
> > +	adap = kzalloc(sizeof(struct i2c_adapter)
> > +			+ sizeof(struct i2c_virt_priv)
> > +			+ sizeof(struct i2c_algorithm), GFP_KERNEL);
> > +	if (!adap)
> > +		return NULL;
> > +
> > +	priv = (struct i2c_virt_priv *)(adap + 1);
> > +	algo = (struct i2c_algorithm *)(priv + 1);
> 
> you shouldn't need force-typecast here.
> 
> you could always make your i2c_virt_priv data contain the i2c_adapter
> and i2c_algorithm structure so you can easily go between them.

Ok.

> > +	/* 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.
> > +	 */
> > +	algo->master_xfer = (parent->algo->master_xfer
> > +					? i2c_virt_master_xfer : NULL);
> > +	algo->smbus_xfer = (parent->algo->smbus_xfer
> > +					? i2c_virt_smbus_xfer : NULL);
> > +	algo->functionality = i2c_virt_functionality;
> 
> you're using kzalloc, so this boils down to the neater:
> 
> 	if (parent->algo->master_xfer)
> 		algo->master_xfer = i2c_virt_master_xfer;
> etc.

Ok.

> > +	/* Now fill out new adapter structure */
> > +	snprintf(adap->name, sizeof(adap->name),
> > +			"i2c-%d-virt (mux %02x:%02x)",
> > +			i2c_adapter_id(parent), client->addr, mux_val);
> > +	adap->id = I2C_HW_VIRT | i2c_adapter_id(parent);
> > +	adap->algo = algo;
> > +	adap->algo_data = priv;
> > +	adap->timeout = VIRT_TIMEOUT;
> > +	adap->retries = VIRT_RETRIES;
> > +	adap->dev.parent = &parent->dev;
> 
> How about creating a struct with the callbacks in, and the data
> for timeout, retries and any other data that would be needed?

There are few assignments... :)

> > +	if (force_nr) {
> > +		adap->nr = force_nr;
> > +		ret = i2c_add_numbered_adapter(adap);
> > +	} else
> > +		ret = i2c_add_adapter(adap);
> > +	if (ret < 0) {
> > +		kfree(adap);
> > +		return NULL;
> > +	}
> > +
> > +	dev_info(&parent->dev, "i2c-virt-%d: Virtual I2C bus - "
> > +		"physical bus i2c-%d, multiplexer 0x%02x port %d\n",
> > +		i2c_adapter_id(adap), i2c_adapter_id(parent),
> > +		client->addr, mux_val);
> > +
> > +	return adap;
> > +}
> > +EXPORT_SYMBOL_GPL(i2c_add_virt_adapter);
> > +
> > +int i2c_del_virt_adapter(struct i2c_adapter *adap)
> > +{
> > +	int ret;
> > +
> > +	ret = i2c_del_adapter(adap);
> > +	if (ret < 0)
> > +		return ret;
> > +	kfree(adap);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(i2c_del_virt_adapter);
> > +
> > +MODULE_AUTHOR("Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org, " \
> > +		"Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>");
> > +MODULE_DESCRIPTION("Virtual I2C driver for multiplexed I2C busses");
> > +MODULE_LICENSE("GPL");
> 
> "GPL v2" is the definition you probably want here.

Ok.

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support

[Felix Radensky]

Hi, Rodolfo

I've tried your patches with linux-2.6.27 kernel running
on custom board with 460EX PowerPC. This board has
two i2c buses and pca9548 switch on bus 0.

The 954x driver (compiled into the kernel) creates 8
virtual buses just fine.  However devices behind the
mux are not detected if I use the "recommended" way
of doing things.

I have RTC (ds1338) and temp sensor (ds1625) on mux
channel 4 (i.e virtual bus 6). What I did initially to add
the following code to board init code:

struct board_i2c_info {
    int bus;
    int addr;
    char *id;
};

static struct board_i2c_info board_i2c_devices[] = {
    /* Bus  Addr       ID  */
    {6,     0x68,    "ds1338"},
    {6,     0x49,    "ds1625"},
};

static int board_i2c_init(void)
{
    int i;

    for (i = 0; i < ARRAY_SIZE(board_i2c_devices); i++) {
        struct i2c_board_info info = {};

        info.irq = -1;
        info.addr = board_i2c_devices[i].addr;
        if (strlcpy(info.type, board_i2c_devices[i].id,
                I2C_NAME_SIZE) >= I2C_NAME_SIZE)
            return -ENOMEM;

        i2c_register_board_info(board_i2c_devices[i].bus, &info, 1);
    }
}
arch_initcall(board_i2c_init);

This code is invoked by kernel before i2c bus and mux detection,
but, as I said, RTC and sensor are not identified.

If I invoke the following code  after creation of virtual
adapters, things start working, but I don't like this solution

static int board_i2c_init(void)
{
    int i;
   
    for (i = 0; i < ARRAY_SIZE(board_i2c_devices); i++) {
        struct i2c_board_info info = {};
        struct i2c_adapter *adap;
 
       info.irq = -1;
        info.addr = board_i2c_devices[i].addr;
        if (strlcpy(info.type, board_i2c_devices[i].id,
                I2C_NAME_SIZE) >= I2C_NAME_SIZE)
            return -ENOMEM;

        adap = i2c_get_adapter(board_i2c_devices[i].bus);
        i2c_new_device(adap, &info);
    }
}

My question is: how can I register board i2c devices behind
the mux in board init code, and make them properly identified
when relevant drivers are loaded.

Thanks a lot.

Felix.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support

[Rodolfo Giometti]

[-- Attachment #1.1: Type: text/plain, Size: 1962 bytes --]

On Sun, Oct 26, 2008 at 11:53:55PM +0200, Felix Radensky wrote:
> Hi, Rodolfo
>
> I've tried your patches with linux-2.6.27 kernel running
> on custom board with 460EX PowerPC. This board has
> two i2c buses and pca9548 switch on bus 0.

Please, consider thay my patch has been *not* accepted into main line
due a mutex problem... :'(

> My question is: how can I register board i2c devices behind
> the mux in board init code, and make them properly identified
> when relevant drivers are loaded.

I have just defined the i2c devices behind the mux as connected to the
new virtual adapters:

static int wr1100_adap_ids[] = {
        2, 3,
};

static struct pca954x_platform_data wr1100_i2cmux_adap_ids = {
        .adap_ids       = wr1100_adap_ids,
        .len            = ARRAY_SIZE(wr1100_adap_ids),
};

static struct i2c_board_info __initdata wr1100_i2c_devices[] = {
        {
                I2C_BOARD_INFO("pca9540", 0x70),
                .platform_data = &wr1100_i2cmux_adap_ids,
        },
};

static struct i2c_board_info __initdata wr1100_i2c_mux_devices[] = {
        {
                I2C_BOARD_INFO("bq27200", 0x55),
        },
};

Then into machine startup code:

        i2c_register_board_info(0, wr1100_i2c_devices,
                                        ARRAY_SIZE(wr1100_i2c_devices));
        i2c_register_board_info(2, wr1100_i2c_mux_devices,
                                        ARRAY_SIZE(wr1100_i2c_mux_devices));
        i2c_register_board_info(3, wr1100_i2c_mux_devices,
                                        ARRAY_SIZE(wr1100_i2c_mux_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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support

[Felix Radensky]

Hi, Rodolfo

Rodolfo Giometti wrote:
> On Sun, Oct 26, 2008 at 11:53:55PM +0200, Felix Radensky wrote:
>   
>> Hi, Rodolfo
>>
>> I've tried your patches with linux-2.6.27 kernel running
>> on custom board with 460EX PowerPC. This board has
>> two i2c buses and pca9548 switch on bus 0.
>>     
>
> Please, consider thay my patch has been *not* accepted into main line
> due a mutex problem... :'(
>
>   
I didn't find any discussion on the mailing list. What mutex problem 
exactly ?

I find your patches very useful, and the concept of virtual buses very 
elegant.
Hopefully you can rework your patches and get them into mainline. I'm 
willing
to test new patch versions.
>> My question is: how can I register board i2c devices behind
>> the mux in board init code, and make them properly identified
>> when relevant drivers are loaded.
>>     
>
> I have just defined the i2c devices behind the mux as connected to the
> new virtual adapters:
>
> static int wr1100_adap_ids[] = {
>         2, 3,
> };
>
> static struct pca954x_platform_data wr1100_i2cmux_adap_ids = {
>         .adap_ids       = wr1100_adap_ids,
>         .len            = ARRAY_SIZE(wr1100_adap_ids),
> };
>
> static struct i2c_board_info __initdata wr1100_i2c_devices[] = {
>         {
>                 I2C_BOARD_INFO("pca9540", 0x70),
>                 .platform_data = &wr1100_i2cmux_adap_ids,
>         },
> };
>
> static struct i2c_board_info __initdata wr1100_i2c_mux_devices[] = {
>         {
>                 I2C_BOARD_INFO("bq27200", 0x55),
>         },
> };
>
> Then into machine startup code:
>
>         i2c_register_board_info(0, wr1100_i2c_devices,
>                                         ARRAY_SIZE(wr1100_i2c_devices));
>         i2c_register_board_info(2, wr1100_i2c_mux_devices,
>                                         ARRAY_SIZE(wr1100_i2c_mux_devices));
>         i2c_register_board_info(3, wr1100_i2c_mux_devices,
>                                         ARRAY_SIZE(wr1100_i2c_mux_devices));
>
>   
That works fine in my setup too. Thanks a lot for your help !!!
> Ciao,
>
> Rodolfo
>
>   
Felix.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support

[Felix Radensky]

Hi, Rodolfo

Rodolfo Giometti wrote:
> On Sun, Oct 26, 2008 at 11:53:55PM +0200, Felix Radensky wrote:
>   
>> Hi, Rodolfo
>>
>> I've tried your patches with linux-2.6.27 kernel running
>> on custom board with 460EX PowerPC. This board has
>> two i2c buses and pca9548 switch on bus 0.
>>     
>
> Please, consider thay my patch has been *not* accepted into main line
> due a mutex problem... :'(
>
>   
I didn't find any discussion on the mailing list. What mutex problem 
exactly ?

I find your patches very useful, and the concept of virtual buses very 
elegant.
Hopefully you can rework your patches and get them into mainline. I'm 
willing
to test new patch versions.
>> My question is: how can I register board i2c devices behind
>> the mux in board init code, and make them properly identified
>> when relevant drivers are loaded.
>>     
>
> I have just defined the i2c devices behind the mux as connected to the
> new virtual adapters:
>
> static int wr1100_adap_ids[] = {
>         2, 3,
> };
>
> static struct pca954x_platform_data wr1100_i2cmux_adap_ids = {
>         .adap_ids       = wr1100_adap_ids,
>         .len            = ARRAY_SIZE(wr1100_adap_ids),
> };
>
> static struct i2c_board_info __initdata wr1100_i2c_devices[] = {
>         {
>                 I2C_BOARD_INFO("pca9540", 0x70),
>                 .platform_data = &wr1100_i2cmux_adap_ids,
>         },
> };
>
> static struct i2c_board_info __initdata wr1100_i2c_mux_devices[] = {
>         {
>                 I2C_BOARD_INFO("bq27200", 0x55),
>         },
> };
>
> Then into machine startup code:
>
>         i2c_register_board_info(0, wr1100_i2c_devices,
>                                         ARRAY_SIZE(wr1100_i2c_devices));
>         i2c_register_board_info(2, wr1100_i2c_mux_devices,
>                                         ARRAY_SIZE(wr1100_i2c_mux_devices));
>         i2c_register_board_info(3, wr1100_i2c_mux_devices,
>                                         ARRAY_SIZE(wr1100_i2c_mux_devices));
>
>   
That works fine in my setup too. Thanks a lot for your help !!!
> Ciao,
>
> Rodolfo
>
>   
Felix.

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [PATCH 1/3] i2c: virtual i2c adapter support

[Rodolfo Giometti]

[-- Attachment #1.1: Type: text/plain, Size: 885 bytes --]

On Mon, Oct 27, 2008 at 01:31:25PM +0200, Felix Radensky wrote:
>>   
> I didn't find any discussion on the mailing list. What mutex problem  
> exactly ?

See this thread:

   http://www.mail-archive.com/i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org/msg01535.html

> I find your patches very useful, and the concept of virtual buses very  
> elegant.
> Hopefully you can rework your patches and get them into mainline. I'm  
> willing
> to test new patch versions.

No time to dedicate to... :'( maybe you can try to solve the issue! :P

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c