[PATCH v3 0/2] i2c/of: Populate multiplexed i2c busses from the device tree.

[PATCH v3 0/2] i2c/of: Populate multiplexed i2c busses from the device tree.

[David Daney]

From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

v3: Integrate changes from Lars-Peter Clausen to make better use of
    the of_*() infrastructure.  Get rid of ugly #ifdefs.

v2: Update bindings to use "reg" insutead of "cell-index"

v1: Unchanged from the original RFC where I said:

  We need to populate our I2C devices from the device tree, but some
  of our systems have multiplexers which currently break this process.

  The basic idea is to have the generic i2c-mux framework propagate
  the device_node for the child bus into the corresponding
  i2c_adapter, and then call of_i2c_register_devices().  This means
  that the device tree bindings for *all* i2c muxes must have some
  common properties.  I try to document these in mux.txt.

This is now tested against 3.4-rc2 and is still working well.

David Daney (2):
  i2c: Add a struct device * parameter to i2c_add_mux_adapter()
  i2c/of: Automatically populate i2c mux busses from device tree data.

 Documentation/devicetree/bindings/i2c/mux.txt |   60 +++++++++++++++++++++++++
 drivers/i2c/i2c-mux.c                         |   43 ++++++++++++++----
 drivers/i2c/muxes/gpio-i2cmux.c               |    3 +-
 drivers/i2c/muxes/pca9541.c                   |    3 +-
 drivers/i2c/muxes/pca954x.c                   |    2 +-
 include/linux/i2c-mux.h                       |    3 +-
 6 files changed, 101 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/mux.txt

Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
-- 
1.7.2.3

[PATCH v3 1/2] i2c: Add a struct device * parameter to i2c_add_mux_adapter()

[David Daney]

From: David Daney <david.daney@cavium.com>

And adjust all callers.

The new device parameter is used in the next patch to initialize the
mux's of_node so that its children may be automatically populated.

Signed-off-by: David Daney <david.daney@cavium.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/i2c/i2c-mux.c           |   19 ++++++++++---------
 drivers/i2c/muxes/gpio-i2cmux.c |    3 ++-
 drivers/i2c/muxes/pca9541.c     |    3 ++-
 drivers/i2c/muxes/pca954x.c     |    2 +-
 include/linux/i2c-mux.h         |    3 ++-
 5 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index d7a4833..26ab31d 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -31,11 +31,11 @@ struct i2c_mux_priv {
 	struct i2c_algorithm algo;
 
 	struct i2c_adapter *parent;
-	void *mux_dev;	/* the mux chip/device */
+	void *mux_priv;	/* the mux chip/device */
 	u32  chan_id;	/* the channel id */
 
-	int (*select)(struct i2c_adapter *, void *mux_dev, u32 chan_id);
-	int (*deselect)(struct i2c_adapter *, void *mux_dev, u32 chan_id);
+	int (*select)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
+	int (*deselect)(struct i2c_adapter *, void *mux_priv, u32 chan_id);
 };
 
 static int i2c_mux_master_xfer(struct i2c_adapter *adap,
@@ -47,11 +47,11 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
 
 	/* Switch to the right mux port and perform the transfer. */
 
-	ret = priv->select(parent, priv->mux_dev, priv->chan_id);
+	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
 	if (ret >= 0)
 		ret = parent->algo->master_xfer(parent, msgs, num);
 	if (priv->deselect)
-		priv->deselect(parent, priv->mux_dev, priv->chan_id);
+		priv->deselect(parent, priv->mux_priv, priv->chan_id);
 
 	return ret;
 }
@@ -67,12 +67,12 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
 
 	/* Select the right mux port and perform the transfer. */
 
-	ret = priv->select(parent, priv->mux_dev, priv->chan_id);
+	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
 	if (ret >= 0)
 		ret = parent->algo->smbus_xfer(parent, addr, flags,
 					read_write, command, size, data);
 	if (priv->deselect)
-		priv->deselect(parent, priv->mux_dev, priv->chan_id);
+		priv->deselect(parent, priv->mux_priv, priv->chan_id);
 
 	return ret;
 }
@@ -87,7 +87,8 @@ static u32 i2c_mux_functionality(struct i2c_adapter *adap)
 }
 
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
-				void *mux_dev, u32 force_nr, u32 chan_id,
+				struct device *mux_dev,
+				void *mux_priv, u32 force_nr, u32 chan_id,
 				int (*select) (struct i2c_adapter *,
 					       void *, u32),
 				int (*deselect) (struct i2c_adapter *,
@@ -102,7 +103,7 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 
 	/* Set up private adapter data */
 	priv->parent = parent;
-	priv->mux_dev = mux_dev;
+	priv->mux_priv = mux_priv;
 	priv->chan_id = chan_id;
 	priv->select = select;
 	priv->deselect = deselect;
diff --git a/drivers/i2c/muxes/gpio-i2cmux.c b/drivers/i2c/muxes/gpio-i2cmux.c
index e5fa695..fc5c1ef 100644
--- a/drivers/i2c/muxes/gpio-i2cmux.c
+++ b/drivers/i2c/muxes/gpio-i2cmux.c
@@ -105,7 +105,8 @@ static int __devinit gpiomux_probe(struct platform_device *pdev)
 	for (i = 0; i < pdata->n_values; i++) {
 		u32 nr = pdata->base_nr ? (pdata->base_nr + i) : 0;
 
-		mux->adap[i] = i2c_add_mux_adapter(parent, mux, nr, i,
+		mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux,
+						   nr, i,
 						   gpiomux_select, deselect);
 		if (!mux->adap[i]) {
 			ret = -ENODEV;
diff --git a/drivers/i2c/muxes/pca9541.c b/drivers/i2c/muxes/pca9541.c
index e0df9b6..8aacde1 100644
--- a/drivers/i2c/muxes/pca9541.c
+++ b/drivers/i2c/muxes/pca9541.c
@@ -353,7 +353,8 @@ static int pca9541_probe(struct i2c_client *client,
 	force = 0;
 	if (pdata)
 		force = pdata->modes[0].adap_id;
-	data->mux_adap = i2c_add_mux_adapter(adap, client, force, 0,
+	data->mux_adap = i2c_add_mux_adapter(adap, &client->dev, client,
+					     force, 0,
 					     pca9541_select_chan,
 					     pca9541_release_chan);
 
diff --git a/drivers/i2c/muxes/pca954x.c b/drivers/i2c/muxes/pca954x.c
index 0e37ef2..f2dfe0d 100644
--- a/drivers/i2c/muxes/pca954x.c
+++ b/drivers/i2c/muxes/pca954x.c
@@ -226,7 +226,7 @@ static int pca954x_probe(struct i2c_client *client,
 		}
 
 		data->virt_adaps[num] =
-			i2c_add_mux_adapter(adap, client,
+			i2c_add_mux_adapter(adap, &client->dev, client,
 				force, num, pca954x_select_chan,
 				(pdata && pdata->modes[num].deselect_on_exit)
 					? pca954x_deselect_mux : NULL);
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 747f0cd..c790838 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -34,7 +34,8 @@
  * mux control.
  */
 struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
-				void *mux_dev, u32 force_nr, u32 chan_id,
+				struct device *mux_dev,
+				void *mux_priv, u32 force_nr, u32 chan_id,
 				int (*select) (struct i2c_adapter *,
 					       void *mux_dev, u32 chan_id),
 				int (*deselect) (struct i2c_adapter *,
-- 
1.7.2.3

[PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.

[David Daney]

From: David Daney <david.daney@cavium.com>

For 'normal' i2c bus drivers, we can call of_i2c_register_devices()
and have the device tree framework automatically populate the bus with
the devices specified in the device tree.

This patch adds a common code to the i2c mux framework to have the mux
sub-busses be populated by the of_i2c_register_devices() too.  If the
mux device has an of_node, we populate the sub-bus' of_node so that
the subsequent call to of_i2c_register_devices() will find the
corresponding devices.

It seemed better to put this logic in i2c_add_mux_adapter() rather
than the individual mux drivers, as they will all probably want to do
the same thing.

Signed-off-by: David Daney <david.daney@cavium.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
---
 Documentation/devicetree/bindings/i2c/mux.txt |   60 +++++++++++++++++++++++++
 drivers/i2c/i2c-mux.c                         |   24 ++++++++++
 2 files changed, 84 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/mux.txt

diff --git a/Documentation/devicetree/bindings/i2c/mux.txt b/Documentation/devicetree/bindings/i2c/mux.txt
new file mode 100644
index 0000000..af84cce
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/mux.txt
@@ -0,0 +1,60 @@
+Common i2c bus multiplexer/switch properties.
+
+An i2c bus multiplexer/switch will have several child busses that are
+numbered uniquely in a device dependent manner.  The nodes for an i2c bus
+multiplexer/switch will have one child node for each child
+bus.
+
+Required properties:
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Required properties for child nodes:
+- #address-cells = <1>;
+- #size-cells = <0>;
+- reg : The sub-bus number.
+
+Optional properties for child nodes:
+- Other properties specific to the multiplexer/switch hardware.
+- Child nodes conforming to i2c bus binding
+
+
+Example :
+
+	/*
+	   An NXP pca9548 8 channel I2C multiplexer at address 0x70
+	   with two NXP pca8574 GPIO expanders attached, one each to
+	   ports 3 and 4.
+	 */
+
+	mux@70 {
+		compatible = "nxp,pca9548";
+		reg = <0x70>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+
+			gpio1: gpio@38 {
+				compatible = "nxp,pca8574";
+				reg = <0x38>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+		};
+		i2c@4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <4>;
+
+			gpio2: gpio@38 {
+				compatible = "nxp,pca8574";
+				reg = <0x38>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+		};
+	};
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 26ab31d..609794b 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -24,6 +24,8 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
+#include <linux/of.h>
+#include <linux/of_i2c.h>
 
 /* multiplexer per channel data */
 struct i2c_mux_priv {
@@ -125,6 +127,26 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 	priv->adap.algo_data = priv;
 	priv->adap.dev.parent = &parent->dev;
 
+	/*
+	 * Try to get populate the mux adapter's of_node, expands to
+	 * nothing if !CONFIG_OF.
+	 */
+	if (mux_dev->of_node) {
+		struct device_node *child;
+		u32 reg;
+		int ret;
+
+		for_each_child_of_node(mux_dev->of_node, child) {
+			ret = of_property_read_u32(child, "reg", &reg);
+			if (ret)
+				continue;
+			if (chan_id == reg) {
+				priv->adap.dev.of_node = child;
+				break;
+			}
+		}
+	}
+
 	if (force_nr) {
 		priv->adap.nr = force_nr;
 		ret = i2c_add_numbered_adapter(&priv->adap);
@@ -142,6 +164,8 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
 	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
 		 i2c_adapter_id(&priv->adap));
 
+	of_i2c_register_devices(&priv->adap);
+
 	return &priv->adap;
 }
 EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
-- 
1.7.2.3

Re: [PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.

[Wolfram Sang]

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

On Thu, Apr 12, 2012 at 02:14:23PM -0700, David Daney wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> For 'normal' i2c bus drivers, we can call of_i2c_register_devices()
> and have the device tree framework automatically populate the bus with
> the devices specified in the device tree.
> 
> This patch adds a common code to the i2c mux framework to have the mux
> sub-busses be populated by the of_i2c_register_devices() too.  If the
> mux device has an of_node, we populate the sub-bus' of_node so that
> the subsequent call to of_i2c_register_devices() will find the
> corresponding devices.
> 
> It seemed better to put this logic in i2c_add_mux_adapter() rather
> than the individual mux drivers, as they will all probably want to do
> the same thing.

Both patches looking mostly good, two things here:

> +	/*
> +	 * Try to get populate the mux adapter's of_node, expands to

"get populate"? I'd think you mean "populate" only, but am not sure
enough to fix it myself.

> +	 * nothing if !CONFIG_OF.
> +	 */
> +	if (mux_dev->of_node) {
> +		struct device_node *child;
> +		u32 reg;
> +		int ret;

We have a "ret" already in this function.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.

[David Daney]

On 04/22/2012 09:20 AM, Wolfram Sang wrote:
> On Thu, Apr 12, 2012 at 02:14:23PM -0700, David Daney wrote:
>> From: David Daney<david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>
>> For 'normal' i2c bus drivers, we can call of_i2c_register_devices()
>> and have the device tree framework automatically populate the bus with
>> the devices specified in the device tree.
>>
>> This patch adds a common code to the i2c mux framework to have the mux
>> sub-busses be populated by the of_i2c_register_devices() too.  If the
>> mux device has an of_node, we populate the sub-bus' of_node so that
>> the subsequent call to of_i2c_register_devices() will find the
>> corresponding devices.
>>
>> It seemed better to put this logic in i2c_add_mux_adapter() rather
>> than the individual mux drivers, as they will all probably want to do
>> the same thing.
>
> Both patches looking mostly good, two things here:
>
>> +	/*
>> +	 * Try to get populate the mux adapter's of_node, expands to
>
> "get populate"? I'd think you mean "populate" only, but am not sure
> enough to fix it myself.

You are correct.

>
>> +	 * nothing if !CONFIG_OF.
>> +	 */
>> +	if (mux_dev->of_node) {
>> +		struct device_node *child;
>> +		u32 reg;
>> +		int ret;
>
> We have a "ret" already in this function.
>

Good catch.  That definition of "ret" could (and should) be removed. 
The code works correctly, but fails the elegance test.

I can send a new patch set with those corrections, or if you prefer, you 
could commit these making the changes yourself.

Which do you prefer?

David Daney

Re: [PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.

[Wolfram Sang]

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

> I can send a new patch set with those corrections, or if you prefer,
> you could commit these making the changes yourself.

I'll do it, probably faster for both of us :)

Thanks.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.

[David Daney]

On 04/23/2012 09:58 AM, Wolfram Sang wrote:
>> I can send a new patch set with those corrections, or if you prefer,
>> you could commit these making the changes yourself.
>
> I'll do it, probably faster for both of us :)
>

Great!

Thank you for reviewing this,
David Daney

Re: [PATCH v3 2/2] i2c/of: Automatically populate i2c mux busses from device tree data.

[Stephen Warren]

On 04/12/2012 03:14 PM, David Daney wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> For 'normal' i2c bus drivers, we can call of_i2c_register_devices()
> and have the device tree framework automatically populate the bus with
> the devices specified in the device tree.
> 
> This patch adds a common code to the i2c mux framework to have the mux
> sub-busses be populated by the of_i2c_register_devices() too.  If the
> mux device has an of_node, we populate the sub-bus' of_node so that
> the subsequent call to of_i2c_register_devices() will find the
> corresponding devices.
> 
> It seemed better to put this logic in i2c_add_mux_adapter() rather
> than the individual mux drivers, as they will all probably want to do
> the same thing.
> 
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Cc: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>

Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

Re: [PATCH v3 0/2] i2c/of: Populate multiplexed i2c busses from the device tree.

[Lars-Peter Clausen]

On 04/12/2012 11:14 PM, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> v3: Integrate changes from Lars-Peter Clausen to make better use of
>     the of_*() infrastructure.  Get rid of ugly #ifdefs.
> 
> v2: Update bindings to use "reg" insutead of "cell-index"
> 
> v1: Unchanged from the original RFC where I said:
> 
>   We need to populate our I2C devices from the device tree, but some
>   of our systems have multiplexers which currently break this process.
> 
>   The basic idea is to have the generic i2c-mux framework propagate
>   the device_node for the child bus into the corresponding
>   i2c_adapter, and then call of_i2c_register_devices().  This means
>   that the device tree bindings for *all* i2c muxes must have some
>   common properties.  I try to document these in mux.txt.
> 
> This is now tested against 3.4-rc2 and is still working well.
> 

I've been using these patches with a pca9548 and a pca9546 for a while now.
Both:

Tested-by: Lars-Peter Clausen <lars@metafoo.de>

Re: [PATCH v3 0/2] i2c/of: Populate multiplexed i2c busses from the device tree.

[Wolfram Sang]

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

On Fri, Apr 13, 2012 at 01:09:20PM +0200, Lars-Peter Clausen wrote:
> On 04/12/2012 11:14 PM, David Daney wrote:
> > From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > 
> > v3: Integrate changes from Lars-Peter Clausen to make better use of
> >     the of_*() infrastructure.  Get rid of ugly #ifdefs.
> > 
> > v2: Update bindings to use "reg" insutead of "cell-index"
> > 
> > v1: Unchanged from the original RFC where I said:
> > 
> >   We need to populate our I2C devices from the device tree, but some
> >   of our systems have multiplexers which currently break this process.
> > 
> >   The basic idea is to have the generic i2c-mux framework propagate
> >   the device_node for the child bus into the corresponding
> >   i2c_adapter, and then call of_i2c_register_devices().  This means
> >   that the device tree bindings for *all* i2c muxes must have some
> >   common properties.  I try to document these in mux.txt.
> > 
> > This is now tested against 3.4-rc2 and is still working well.
> > 
> 
> I've been using these patches with a pca9548 and a pca9546 for a while now.
> Both:
> 
> Tested-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>

Both applied to next, thanks to all involved.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]