Skip to contentSkip to conversation

ARM: I2C: Add device tree bindings to i2c-mux-gpio

PATCH Superseded Maxime Ripard 4 participants 7 messages Sep 21, 2012 – Sep 22, 2012

Subsystems (5)

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

cover ARM: I2C: Add device tree bindings to i2c-mux-gpio

Hi everyone,

This patchset adds the device tree entry to the CFA-10049 board of its i2c muxer. This muxer controls sub-buses that contains three Nuvoton NAU7802 ADCs and a NXP PCA955 GPIO expander. Support for these will be added eventually.

Thanks,
Maxime

Maxime Ripard (3):
i2c: i2c-mux-gpio: Use devm_kzalloc instead of kzalloc
i2c: mux: Add dt support to i2c-mux-gpio driver
ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049

 .../devicetree/bindings/i2c/i2c-mux-gpio.txt       |   86 ++++++++++
 arch/arm/boot/dts/imx28-cfa10049.dts               |   33 ++++
 drivers/i2c/muxes/i2c-mux-gpio.c                   |  168 +++++++++++++++-----
 3 files changed, 244 insertions(+), 43 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/3 i2c: i2c-mux-gpio: Use devm_kzalloc instead of kzalloc

Use the devm_kzalloc managed function to stripdown the error and remove code.

---
 drivers/i2c/muxes/i2c-mux-gpio.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/i2c/muxes/i2c-mux-gpio.c +5 -9
index 68b1f8e..fbc400b 100644
@@ -71,7 +71,7 @@ static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux) {
 		ret = -ENOMEM;
 		goto alloc_failed;
@@ -79,11 +79,12 @@ static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
 
 	mux->parent = parent;
 	mux->data = *pdata;
-	mux->adap = kzalloc(sizeof(struct i2c_adapter *) * pdata->n_values,
-			    GFP_KERNEL);
+	mux->adap = devm_kzalloc(&pdev->dev,
+				sizeof(struct i2c_adapter *) * pdata->n_values,
+				GFP_KERNEL);
 	if (!mux->adap) {
 		ret = -ENOMEM;
-		goto alloc_failed2;
+		goto alloc_failed;
 	}
 
 	if (pdata->idle != I2C_MUX_GPIO_NO_IDLE) {
@@ -128,9 +129,6 @@ add_adapter_failed:
 err_request_gpio:
 	for (; i > 0; i--)
 		gpio_free(pdata->gpios[i - 1]);
-	kfree(mux->adap);
-alloc_failed2:
-	kfree(mux);
 alloc_failed:
 	i2c_put_adapter(parent);
 
@@ -150,8 +148,6 @@ static int __devexit i2c_mux_gpio_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 	i2c_put_adapter(mux->parent);
-	kfree(mux->adap);
-	kfree(mux);
 
 	return 0;
 }
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 1/3 i2c: i2c-mux-gpio: Use devm_kzalloc instead of kzalloc

Quoted 22 lines · Maxime Ripard

Use the devm_kzalloc managed function to stripdown the error and remove code.

---
 drivers/i2c/muxes/i2c-mux-gpio.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/i2c/muxes/i2c-mux-gpio.c +1 -1
index 68b1f8e..fbc400b 100644
@@ -71,7 +71,7 @@ static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL);
 	if (!mux) {
 		ret = -ENOMEM;
 		goto alloc_failed;

drivers/i2c/muxes/i2c-mux-gpio.c static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev) · line 84

Quoted 8 lines
drivers/i2c/muxes/i2c-mux-gpio.c +3 -2
@@ -79,11 +79,12 @@ static int __devinit i2c_mux_gpio_probe(struct platform_device *pdev)
 
 	mux->parent = parent;
 	mux->data = *pdata;
-	mux->adap = kzalloc(sizeof(struct i2c_adapter *) * pdata->n_values,
-			    GFP_KERNEL);
+	mux->adap = devm_kzalloc(&pdev->dev,
+				sizeof(struct i2c_adapter *) * pdata->n_values,
+				GFP_KERNEL);

Alignment is off by one here.

Quoted 18 lines
drivers/i2c/muxes/i2c-mux-gpio.c +1 -4
 	if (!mux->adap) {
 		ret = -ENOMEM;
-		goto alloc_failed2;
+		goto alloc_failed;
 	}
 
 	if (pdata->idle != I2C_MUX_GPIO_NO_IDLE) {
@@ -128,9 +129,6 @@ add_adapter_failed:
 err_request_gpio:
 	for (; i > 0; i--)
 		gpio_free(pdata->gpios[i - 1]);
-	kfree(mux->adap);
-alloc_failed2:
-	kfree(mux);
 alloc_failed:
 	i2c_put_adapter(parent);
 

drivers/i2c/muxes/i2c-mux-gpio.c static int __devexit i2c_mux_gpio_remove(struct platform_device *pdev) · line 153

Quoted 8 lines
drivers/i2c/muxes/i2c-mux-gpio.c +0 -2
@@ -150,8 +148,6 @@ static int __devexit i2c_mux_gpio_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 	i2c_put_adapter(mux->parent);
-	kfree(mux->adap);
-	kfree(mux);
 
 	return 0;
 }

Other than this it looks OK.

I don't know a thing about DT and ARM so I won't review the following patches of this series. For this reason I will not pick this one in my tree either, to avoid inter-tree dependencies.

Signature · 1 line
Jean Delvare
#3 lore raw

PATCH 1/3 i2c: i2c-mux-gpio: Use devm_kzalloc instead of kzalloc

In reply to Maxime Ripard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Maxime" == Maxime Ripard <[email protected]> writes:

Maxime> Use the devm_kzalloc managed function to stripdown the error and remove Maxime> code.

Maxime> Signed-off-by: Maxime Ripard <[email protected]>

Besides the comment of Jean -

Signature · 1 line
Bye, Peter Korsgaard
#4 lore raw

PATCH 2/3 i2c: mux: Add dt support to i2c-mux-gpio driver

In reply to Maxime Ripard

Allow the i2c-mux-gpio to be used by a device tree enabled device. The bindings are inspired by the one found in the i2c-mux-pinctrl driver.

---
 .../devicetree/bindings/i2c/i2c-mux-gpio.txt       |   86 +++++++++++
 drivers/i2c/muxes/i2c-mux-gpio.c                   |  158 +++++++++++++++-----
 2 files changed, 208 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +86 -0
new file mode 100644
index 0000000..f2bd120
@@ -0,0 +1,86 @@
+GPIO-based I2C Bus Mux
+
+This binding describes an I2C bus multiplexer that uses GPIOs to
+route the I2C signals.
+
+                                 +-----+  +-----+
+                                 | dev |  | dev |
+    +------------------------+   +-----+  +-----+
+    | SoC                    |      |        |
+    |                   /----|------+--------+
+    |   +---+   +------+     | child bus A, on GPIO value set to 0
+    |   |I2C|---| Mux  |     |
+    |   +---+   +--+---+     | child bus B, on GPIO value set to 1
+    |              |    \----|------+--------+--------+
+    |   +------+   |         |      |        |        |
+    |   | GPIO |---+         |  +-----+  +-----+  +-----+
+    |   +------+             |  | dev |  | dev |  | dev |
+    +------------------------+  +-----+  +-----+  +-----+
+
+Required properties:
+- compatible: i2c-mux-gpio
+- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
+  port is connected to.
+
+Also required are:
+
+- muxer-gpios: list of gpios to use to control the muxer
+
+* Standard I2C mux properties. See mux.txt in this directory.
+
+* I2C child bus nodes. See mux.txt in this directory.
+
+For each i2c child nodes, an I2C child bus will be created. They will
+be numbered based on the reg property of each nodes.
+
+Whenever an access is made to a device on a child bus, the value set
+in the revelant node's reg property will be outputed using the list of
+GPIOs, the first in the list holding the most-significant value.
+
+If an idle state is defined, using the idle-state (optional) property,
+whenever an access is not being made to a device on a child bus, the
+idle value will be programmed into the GPIOs.
+
+If an idle state is not defined, the most recently used value will be
+left programmed into hardware whenever no access is being made of a
+device on a child bus.
+
+Example:
+	i2cmux {
+		compatible = "i2c-mux-gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		muxer-gpios = <&gpio1 22 0 &gpio1 23 0>;
+		i2c-parent = <&i2c1>;
+
+		i2c at 0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c at 1 {
+		      reg = <1>;
+		      #address-cells = <1>;
+		      #size-cells = <0>;
+		};
+
+		i2c at 2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c at 3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555 at 20 {
+				compatible = "nxp,pca9555";
+				gpio-controller;
+				#gpio-cells = <2>;
+				reg = <0x20>;
+			};
+		};
+	};
drivers/i2c/muxes/i2c-mux-gpio.c +122 -36

257 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#5 lore raw

PATCH 2/3 i2c: mux: Add dt support to i2c-mux-gpio driver

Quoted 15 lines · Maxime Ripard

Allow the i2c-mux-gpio to be used by a device tree enabled device. The bindings are inspired by the one found in the i2c-mux-pinctrl driver.

+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +8 -0
@@ -0,0 +1,86 @@
+GPIO-based I2C Bus Mux
+
+This binding describes an I2C bus multiplexer that uses GPIOs to
+route the I2C signals.
+
+                                 +-----+  +-----+
+                                 | dev |  | dev |
+    +------------------------+   +-----+  +-----+

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 18

Quoted 10 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +10 -0
+    | SoC                    |      |        |
+    |                   /----|------+--------+
+    |   +---+   +------+     | child bus A, on GPIO value set to 0
+    |   |I2C|---| Mux  |     |
+    |   +---+   +--+---+     | child bus B, on GPIO value set to 1
+    |              |    \----|------+--------+--------+
+    |   +------+   |         |      |        |        |
+    |   | GPIO |---+         |  +-----+  +-----+  +-----+
+    |   +------+             |  | dev |  | dev |  | dev |
+    +------------------------+  +-----+  +-----+  +-----+

The "Mux" box should be outside the SoC, since it isn't part of the SoC itself but something external.

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 27

Quoted 8 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +8 -0
+Required properties:
+- compatible: i2c-mux-gpio
+- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
+  port is connected to.
+
+Also required are:
+
+- muxer-gpios: list of gpios to use to control the muxer

Perhaps just "mux-gpios"? Bikeshedding, I know...

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 34

Quoted 6 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +6 -0
+* Standard I2C mux properties. See mux.txt in this directory.
+
+* I2C child bus nodes. See mux.txt in this directory.
+
+For each i2c child nodes, an I2C child bus will be created. They will
+be numbered based on the reg property of each nodes.

s/nodes/node/ in both of the two lines above.

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 37

Quoted 2 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +2 -0
+Whenever an access is made to a device on a child bus, the value set
+in the revelant node's reg property will be outputed using the list of

s/outputed/output/

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 40

Quoted 3 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +3 -0
+GPIOs, the first in the list holding the most-significant value.
+
+If an idle state is defined, using the idle-state (optional) property,

The idle-state property isn't documented in the list of properties above.

Quoted 16 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +16 -0
+whenever an access is not being made to a device on a child bus, the
+idle value will be programmed into the GPIOs.
+
+If an idle state is not defined, the most recently used value will be
+left programmed into hardware whenever no access is being made of a
+device on a child bus.
+
+Example:
+	i2cmux {
+		compatible = "i2c-mux-gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		muxer-gpios = <&gpio1 22 0 &gpio1 23 0>;
+		i2c-parent = <&i2c1>;
+
+		i2c at 0 {

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 66

Quoted 10 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +10 -0
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c at 1 {
+		      reg = <1>;
+		      #address-cells = <1>;
+		      #size-cells = <0>;
+		};

The indentation above is a mix of TABs and spaces, and is inconsistent between nodes; just TABs would be best.

Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt line 72

Quoted 6 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +6 -0
+
+		i2c at 2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};

I'm not sure it's a good idea to have example bus nodes that are empty; why not leave out two of the options, and put some device on each of the buses that is defined?

Quoted 15 lines
Documentation/devicetree/bindings/i2c/i2c-mux-gpio.txt new file +13 -0
+		i2c at 3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555 at 20 {
+				compatible = "nxp,pca9555";
+				gpio-controller;
+				#gpio-cells = <2>;
+				reg = <0x20>;
+			};
+		};
+	};

drivers/i2c/muxes/i2c-mux-gpio.c static int i2c_mux_gpio_deselect(struct i2c_adapter *adap, void *data, u32 chan) · line 89

Quoted 8 lines
drivers/i2c/muxes/i2c-mux-gpio.c +6 -0
 
+#ifdef CONFIG_OF
+static int __devinit i2c_mux_gpio_probe_dt(struct gpiomux *mux,
+					struct platform_device *pdev)
 
+	mux->data->n_values = of_get_child_count(np);
+
+	values = devm_kzalloc(&pdev->dev, sizeof(*mux->data->values), GFP_KERNEL);

Don't you need to multiply the size by mux->data->n_values?

drivers/i2c/muxes/i2c-mux-gpio.c static int i2c_mux_gpio_deselect(struct i2c_adapter *adap, void *data, u32 chan) · line 110

Quoted 1 line
drivers/i2c/muxes/i2c-mux-gpio.c +1 -0
+	gpios = devm_kzalloc(&pdev->dev, mux->data->n_gpios, GFP_KERNEL);

Don't you need to multiple the size by sizeof(*gpios) here?

#6 lore raw

PATCH 3/3 ARM: dts: cfa10049: Add the i2c muxer buses to the CFA-10049

In reply to Maxime Ripard

This will allow to add the 3 Nuvoton NAU7802 ADCs and the NXP PCA9555 GPIO expander eventually.

---
 arch/arm/boot/dts/imx28-cfa10049.dts |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
arch/arm/boot/dts/imx28-cfa10049.dts +33 -0
index a9288fb..bd6b938 100644
@@ -76,6 +76,39 @@
 				status = "okay";
 			};
 
+			i2cmux {
+				compatible = "i2c-mux-gpio";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				muxer-gpios = <&gpio1 22 0 &gpio1 23 0>;
+				i2c-parent = <&i2c1>;
+
+				i2c at 0 {
+					reg = <0>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+				};
+
+				i2c at 1 {
+				      reg = <1>;
+				      #address-cells = <1>;
+				      #size-cells = <0>;
+				};
+
+				i2c at 2 {
+					reg = <2>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+				};
+
+				i2c at 3 {
+					reg = <3>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+				};
+			};
+
 			usbphy1: usbphy at 8007e000 {
 				status = "okay";
 			};
Signature · 1 line
1.7.9.5