i2c/gpio: add DT support

PATCH Superseded Jean-Christophe PLAGNIOL-VILLARD 5 participants 21 messages Mar 8, 2012 – Mar 13, 2012

Subsystems (6)

Device tree bindings Arm (32-bit) GPIO I2C subsystem host drivers +2 more subsystems

lore

Share & tools

lore

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 08:50 UTC #0 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc linux-i2c at vger.kernel.org

Cc devicetree-discuss at lists.ozlabs.org

---
v3:

update i2c binding (Rob comments)

Jean can I have a ack to apply it?

Best Regards,
J.

 .../devicetree/bindings/gpio/gpio_i2c.txt          |   32 +++++++
 drivers/i2c/busses/i2c-gpio.c                      |   94 +++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt

Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +32 -0

new file mode 100644
index 0000000..4f8ec94
@@ -0,0 +1,32 @@
+Device-Tree bindings for i2c gpio driver
+
+Required properties:
+	- compatible = "i2c-gpio";
+	- gpios: sda and scl gpio
+
+
+Optional properties:
+	- i2c-gpio,sda-open-drain: sda as open drain
+	- i2c-gpio,scl-open-drain: scl as open drain
+	- i2c-gpio,scl-output-only: scl as output only
+	- i2c-gpio,delay-us: delay between GPIO operations (may depend on each platform)
+	- i2c-gpio,timeout-ms: timeout to get data
+
+Example nodes:
+
+i2c at 0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2 at 56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};

drivers/i2c/busses/i2c-gpio.c +74 ~~-20~~

153 lines not on this page · raw message

Signature · 1 line

1.7.7

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-13 07:07 UTC #1 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

Quoted 11 lines · Jean-Christophe PLAGNIOL-VILLARD

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc linux-i2c at vger.kernel.org
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Jean can I have a ack to apply it?

I update with last comment can one of hte i2c maintainer give a ack to apply via at91 as I've long list of depedency

Best Regards,
J.

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 08:50 UTC #2 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc devicetree-discuss at lists.ozlabs.org

---
v3:

update i2c binding (Rob comments)

Best Regards,
J.

 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

arch/arm/boot/dts/at91sam9g20.dtsi +13 -0

index 4b0dc99..072b2da 100644
@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };

Signature · 1 line

1.7.7

Rob Herring

2012-03-08 17:08 UTC #3 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

Quoted 20 lines · Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
arch/arm/boot/dts/at91sam9g20.dtsi +0 -0
index 4b0dc99..072b2da 100644

arch/arm/boot/dts/at91sam9g20.dtsi line 193

Quoted 5 lines

arch/arm/boot/dts/at91sam9g20.dtsi +2 -0

@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {

You updated the example, but not all the actual uses to i2c at 0.

Rob

Quoted 12 lines

arch/arm/boot/dts/at91sam9g20.dtsi +11 -0

+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-09 09:53 UTC #4 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

Quoted 16 lines · Rob Herring

Quoted 15 lines · Jean-Christophe PLAGNIOL-VILLARD
For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

Quoted 11 lines

arch/arm/boot/dts/at91sam9g20.dtsi line 192
Quoted 9 lines
arch/arm/boot/dts/at91sam9g20.dtsi +1 -0
index 4b0dc99..072b2da 100644
@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+> > +	i2c-gpio at 0 {
You updated the example, but not all the actual uses to i2c at 0.

on purpose as on some soc we will have HW and soft binding

on the board you put the alias

Best Regards,
J.

Quoted 21 lines

Rob
arch/arm/boot/dts/at91sam9g20.dtsi line 205
Quoted 12 lines
arch/arm/boot/dts/at91sam9g20.dtsi +11 -0
+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Rob Herring

2012-03-09 13:59 UTC #5 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

Quoted 28 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 16 lines · Rob Herring
Quoted 15 lines · Jean-Christophe PLAGNIOL-VILLARD
For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
Quoted 11 lines
arch/arm/boot/dts/at91sam9g20.dtsi line 192
Quoted 9 lines
arch/arm/boot/dts/at91sam9g20.dtsi +1 -0
index 4b0dc99..072b2da 100644
@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+> > +	i2c-gpio at 0 {
You updated the example, but not all the actual uses to i2c at 0.

Quoted 4 lines

on purpose as on some soc we will have HW and soft binding
on the board you put the alias

I'm not sure I follow.

If you use the h/w block, then you would have i2c@<addr> while for gpio it's basically an index.

Also, I don't think you need an alias for i2c. You should not care about bus numbering as devices should be sub nodes.

Rob

Quoted 23 lines

Best Regards,
J.
Quoted 21 lines
Rob
arch/arm/boot/dts/at91sam9g20.dtsi line 205
Quoted 12 lines
arch/arm/boot/dts/at91sam9g20.dtsi +11 -0
+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-13 07:08 UTC #6 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

Quoted 34 lines · Rob Herring

Quoted 28 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 16 lines · Rob Herring
Quoted 15 lines · Jean-Christophe PLAGNIOL-VILLARD
For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
Quoted 11 lines
arch/arm/boot/dts/at91sam9g20.dtsi line 192
Quoted 9 lines
arch/arm/boot/dts/at91sam9g20.dtsi +1 -0
index 4b0dc99..072b2da 100644
@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+> > +	i2c-gpio at 0 {
You updated the example, but not all the actual uses to i2c at 0.
Quoted 4 lines
on purpose as on some soc we will have HW and soft binding
on the board you put the alias

Quoted 7 lines

I'm not sure I follow.
If you use the h/w block, then you would have i2c@<addr> while for gpio it's basically an index.
Also, I don't think you need an alias for i2c. You should not care about bus numbering as devices should be sub nodes.

forget about it tired when I reply

Best Regards,
J.

Grant Likely

2012-03-09 05:26 UTC #7 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

Quoted 20 lines · Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
arch/arm/boot/dts/at91sam9g20.dtsi +0 -0
index 4b0dc99..072b2da 100644

arch/arm/boot/dts/at91sam9g20.dtsi line 193

Quoted 5 lines

arch/arm/boot/dts/at91sam9g20.dtsi +2 -0

@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {

i2c at 0

arch/arm/boot/dts/at91sam9g20.dtsi line 204

Quoted 11 lines

arch/arm/boot/dts/at91sam9g20.dtsi +11 -0

+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};

gpio-driven i2c busses are inherently board-specific. Why is this in the SoC include file?

Otherwise I think this patch series looks okay.

Quoted 8 lines

arch/arm/boot/dts/at91sam9g20.dtsi +0 -0

};

Signature · 6 lines

1.7.7

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Signature · 2 lines

Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-09 09:55 UTC #8 lore raw

PATCH v3 2/5 ARM: at91: sam9g20 add i2c DT support

Quoted 29 lines · Grant Likely

Quoted 20 lines · Jean-Christophe PLAGNIOL-VILLARD
For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9g20.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
arch/arm/boot/dts/at91sam9g20.dtsi +0 -0
index 4b0dc99..072b2da 100644
arch/arm/boot/dts/at91sam9g20.dtsi line 193
Quoted 5 lines
arch/arm/boot/dts/at91sam9g20.dtsi +2 -0
@@ -189,4 +189,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
i2c at 0

Quoted 14 lines

arch/arm/boot/dts/at91sam9g20.dtsi line 204
Quoted 11 lines
arch/arm/boot/dts/at91sam9g20.dtsi +11 -0
+		compatible = "i2c-gpio";
+		gpios = <&pioA 23 0 /* sda */
+			 &pioA 24 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
gpio-driven i2c busses are inherently board-specific. Why is this in the SoC include file?

because they use the hw pin and all board will use the same as on some soc (old) we can not use the hw IP

Quoted 2 lines

Otherwise I think this patch series looks okay.

Best Regards,
J.

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 08:50 UTC #9 lore raw

PATCH v3 3/5 ARM: at91: usb_a9g20 add DT i2c support

In reply to Jean-Christophe PLAGNIOL-VILLARD

Use i2c-gpio and enable rv3029 RTC.

Enable the rtc in the sam9g20 defconfig.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc devicetree-discuss at lists.ozlabs.org

---
 arch/arm/boot/dts/usb_a9g20.dts        |    9 +++++++++
 arch/arm/configs/at91sam9g20_defconfig |    3 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

arch/arm/boot/dts/usb_a9g20.dts +9 -0

index 6ab7e7a..07012ca 100644
@@ -97,4 +97,13 @@
 			gpio-key,wakeup;
 		};
 	};
+
+	i2c-gpio at 0 {
+		status = "okay";
+
+		rv3029c2 at 56 {
+			compatible = "rv3029c2";
+			reg = <0x56>;
+		};
+	};
 };

arch/arm/configs/at91sam9g20_defconfig +3 -0

index 9123568..994d331 100644
@@ -74,6 +74,8 @@ CONFIG_LEGACY_PTY_COUNT=16
 CONFIG_SERIAL_ATMEL=y
 CONFIG_SERIAL_ATMEL_CONSOLE=y
 CONFIG_HW_RANDOM=y
+CONFIG_I2C=y
+CONFIG_I2C_GPIO=y
 CONFIG_SPI=y
 CONFIG_SPI_ATMEL=y
 CONFIG_SPI_SPIDEV=y

@@ -105,6 +107,7 @@ CONFIG_LEDS_TRIGGERS=y
 CONFIG_LEDS_TRIGGER_TIMER=y
 CONFIG_LEDS_TRIGGER_HEARTBEAT=y
 CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_RV3029C2=y
 CONFIG_RTC_DRV_AT91SAM9=y
 CONFIG_EXT2_FS=y
 CONFIG_MSDOS_FS=y

Signature · 1 line

1.7.7

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 08:50 UTC #10 lore raw

PATCH v3 4/5 ARM: at91: sam9g45 add i2c DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc devicetree-discuss at lists.ozlabs.org

---
v3:

update i2c binding (Rob comments)

Best Regards,
J.

 arch/arm/boot/dts/at91sam9g45.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

arch/arm/boot/dts/at91sam9g45.dtsi +13 -0

index d79021b..4a8a36d 100644
@@ -197,4 +197,17 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 20 0 /* sda */
+			 &pioA 21 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <5>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };

Signature · 1 line

1.7.7

Karol Lewandowski

2012-03-08 10:52 UTC #11 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

Quoted 14 lines · Jean-Christophe PLAGNIOL-VILLARD

drivers/i2c/busses/i2c-gpio.c +14 -0

+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (pdata->sda_pin < 0 || pdata->scl_pin < 0) {
+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);

drivers/i2c/busses/i2c-gpio.c static int i2c_gpio_getscl(void *data)

Quoted 10 lines

drivers/i2c/busses/i2c-gpio.c +7 -0

+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
 
 
 
+	if (of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);

I've already said (two times!) that of_property_read_u32() returns nonzero (negative) error code on _error_.

Don't you see obvious error in above code fragment?

You assign to pdata->timeout on _error_.

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 16:53 UTC #12 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

Quoted 31 lines · Karol Lewandowski

Quoted 14 lines · Jean-Christophe PLAGNIOL-VILLARD

drivers/i2c/busses/i2c-gpio.c +14 -0

+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (pdata->sda_pin < 0 || pdata->scl_pin < 0) {
+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);

drivers/i2c/busses/i2c-gpio.c static int i2c_gpio_getscl(void *data)

Quoted 10 lines

drivers/i2c/busses/i2c-gpio.c +7 -0

+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
 
 
 
+	if (of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);

I've already said (two times!) that of_property_read_u32() returns nonzero (negative) error code on _error_.

Quoted 3 lines

Don't you see obvious error in above code fragment?
You assign to pdata->timeout on _error_.

yeah forget this one

Jean can I have you are ack so I send the v4 and the pull today

Best Regards,
J.

Rob Herring

2012-03-08 17:16 UTC #13 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

Quoted 7 lines · Jean-Christophe PLAGNIOL-VILLARD

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc linux-i2c at vger.kernel.org
Cc devicetree-discuss at lists.ozlabs.org
---
v3:

Quoted 6 lines

update i2c binding (Rob comments)
Jean can I have a ack to apply it?
Best Regards,
J.

For this patch:

Acked-by Rob Herring <[email protected]>

Quoted 205 lines

 .../devicetree/bindings/gpio/gpio_i2c.txt          |   32 +++++++
 drivers/i2c/busses/i2c-gpio.c                      |   94 +++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt

Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +32 -0

new file mode 100644
index 0000000..4f8ec94
@@ -0,0 +1,32 @@
+Device-Tree bindings for i2c gpio driver
+
+Required properties:
+	- compatible = "i2c-gpio";
+	- gpios: sda and scl gpio
+
+
+Optional properties:
+	- i2c-gpio,sda-open-drain: sda as open drain
+	- i2c-gpio,scl-open-drain: scl as open drain
+	- i2c-gpio,scl-output-only: scl as output only
+	- i2c-gpio,delay-us: delay between GPIO operations (may depend on each platform)
+	- i2c-gpio,timeout-ms: timeout to get data
+
+Example nodes:
+
+i2c at 0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2 at 56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};

drivers/i2c/busses/i2c-gpio.c +74 ~~-20~~

index a651779..98ae85b 100644
@@ -14,8 +14,15 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_i2c.h>
 
-#include <asm/gpio.h>
+struct i2c_gpio_private_data {
+	struct i2c_adapter adap;
+	struct i2c_algo_bit_data bit_data;
+	struct i2c_gpio_platform_data pdata;
+};
 
 /* Toggle SDA by changing the direction of the pin */
 static void i2c_gpio_setsda_dir(void *data, int state)

@@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data)
 	return gpio_get_value(pdata->scl_pin);
 }
 
+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)
+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (pdata->sda_pin < 0 || pdata->scl_pin < 0) {
+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);
+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
+	if (of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		!!of_property_read_bool(np, "i2c-gpio,sda-open-drain");
+	pdata->scl_is_open_drain =
+		!!of_property_read_bool(np, "i2c-gpio,scl-open-drain");
+	pdata->scl_is_output_only =
+		!!of_property_read_bool(np, "i2c-gpio,scl-output-only");
+
+	return 0;
+}
+
 static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_algo_bit_data *bit_data;
 	struct i2c_adapter *adap;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata)
-		return -ENXIO;
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	adap = &priv->adap;
+	bit_data = &priv->bit_data;
+	pdata = &priv->pdata;
 
-	ret = -ENOMEM;
-	adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (!adap)
-		goto err_alloc_adap;
-	bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
-	if (!bit_data)
-		goto err_alloc_bit_data;
+	if (pdev->dev.of_node) {
+		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+		if (ret)
+			return ret;
+	} else {
+		if (!pdev->dev.platform_data)
+			return -ENXIO;
+		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+	}
 
 	ret = gpio_request(pdata->sda_pin, "sda");
 	if (ret)

@@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->algo_data = bit_data;
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
 
 	/*
 	 * If "dev->id" is negative we consider it as zero.

@@ -154,7 +200,9 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_bus;
 
-	platform_set_drvdata(pdev, adap);
+	of_i2c_register_devices(adap);
+
+	platform_set_drvdata(pdev, priv);
 
 	dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n",
 		 pdata->sda_pin, pdata->scl_pin,

@@ -168,34 +216,40 @@ err_add_bus:
 err_request_scl:
 	gpio_free(pdata->sda_pin);
 err_request_sda:
-	kfree(bit_data);
-err_alloc_bit_data:
-	kfree(adap);
-err_alloc_adap:
 	return ret;
 }
 
 static int __devexit i2c_gpio_remove(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_adapter *adap;
 
-	adap = platform_get_drvdata(pdev);
-	pdata = pdev->dev.platform_data;
+	priv = platform_get_drvdata(pdev);
+	adap = &priv->adap;
+	pdata = &priv->pdata;
 
 	i2c_del_adapter(adap);
 	gpio_free(pdata->scl_pin);
 	gpio_free(pdata->sda_pin);
-	kfree(adap->algo_data);
-	kfree(adap);
 
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id i2c_gpio_dt_ids[] = {
+	{ .compatible = "i2c-gpio", },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);
+#endif
+
 static struct platform_driver i2c_gpio_driver = {
 	.driver		= {
 		.name	= "i2c-gpio",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(i2c_gpio_dt_ids),
 	},
 	.probe		= i2c_gpio_probe,
 	.remove		= __devexit_p(i2c_gpio_remove),

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 08:50 UTC #14 lore raw

PATCH v3 5/5 ARM: at91: sam9x5 add i2c DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc devicetree-discuss at lists.ozlabs.org

---
v3:

update i2c binding (Rob comments)

Best Regards,
J.

 arch/arm/boot/dts/at91sam9x5.dtsi |   39 +++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

arch/arm/boot/dts/at91sam9x5.dtsi +39 -0

index c294657..fdd1ac6 100644
@@ -188,4 +188,43 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 30 0 /* sda */
+			 &pioA 31 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 1 {
+		compatible = "i2c-gpio";
+		gpios = <&pioC 0 0 /* sda */
+			 &pioC 1 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 2 {
+		compatible = "i2c-gpio";
+		gpios = <&pioB 4 0 /* sda */
+			 &pioB 5 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };

Signature · 1 line

1.7.7

Rob Herring

2012-03-08 17:12 UTC #15 lore raw

PATCH v3 5/5 ARM: at91: sam9x5 add i2c DT support

Quoted 52 lines · Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc devicetree-discuss at lists.ozlabs.org

---
v3:

update i2c binding (Rob comments)

Best Regards,
J.

 arch/arm/boot/dts/at91sam9x5.dtsi |   39 +++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

arch/arm/boot/dts/at91sam9x5.dtsi +29 -0

index c294657..fdd1ac6 100644
@@ -188,4 +188,43 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 30 0 /* sda */
+			 &pioA 31 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 1 {
+		compatible = "i2c-gpio";
+		gpios = <&pioC 0 0 /* sda */
+			 &pioC 1 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 2 {
+		compatible = "i2c-gpio";

arch/arm/boot/dts/at91sam9x5.dtsi line 229

Quoted 10 lines

arch/arm/boot/dts/at91sam9x5.dtsi +10 -0

+		gpios = <&pioB 4 0 /* sda */
+			 &pioB 5 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};

If these are just any random gpio lines, it seems strange to define these in a SOC dtsi and then disable them.

Rob

Quoted 1 line

arch/arm/boot/dts/at91sam9x5.dtsi +0 -0
 };

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-08 17:52 UTC #16 lore raw

PATCH v3 5/5 ARM: at91: sam9x5 add i2c DT support

Quoted 58 lines · Rob Herring

Quoted 57 lines · Jean-Christophe PLAGNIOL-VILLARD

For now on use i2c-gpio driver on the same pin as the hardware IP.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Nicolas Ferre <[email protected]>

Cc devicetree-discuss at lists.ozlabs.org

---
v3:

update i2c binding (Rob comments)

Best Regards,
J.

 arch/arm/boot/dts/at91sam9x5.dtsi |   39 +++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

arch/arm/boot/dts/at91sam9x5.dtsi +34 -0

index c294657..fdd1ac6 100644
@@ -188,4 +188,43 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 30 0 /* sda */
+			 &pioA 31 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 1 {
+		compatible = "i2c-gpio";
+		gpios = <&pioC 0 0 /* sda */
+			 &pioC 1 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 2 {
+		compatible = "i2c-gpio";
+		gpios = <&pioB 4 0 /* sda */
+			 &pioB 5 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;

Quoted 8 lines

arch/arm/boot/dts/at91sam9x5.dtsi line 203
Quoted 5 lines
arch/arm/boot/dts/at91sam9x5.dtsi +5 -0
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
If these are just any random gpio lines, it seems strange to define these in a SOC dtsi and then disable them.

they are not, they are the same pin as the i2c hardware IP.

Today the hw driver is not ready or will not work correctly (IP issue)

On hardware design we use the I2C pin but can not use the hw driver

Best Regards,
J.

Rob Herring

2012-03-08 20:36 UTC #17 lore raw

PATCH v3 5/5 ARM: at91: sam9x5 add i2c DT support

Quoted 67 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 58 lines · Rob Herring
Quoted 57 lines · Jean-Christophe PLAGNIOL-VILLARD
For now on use i2c-gpio driver on the same pin as the hardware IP.
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc devicetree-discuss at lists.ozlabs.org
---
v3:
update i2c binding (Rob comments)
Best Regards,
J.
 arch/arm/boot/dts/at91sam9x5.dtsi |   39 +++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
arch/arm/boot/dts/at91sam9x5.dtsi +34 -0
index c294657..fdd1ac6 100644
@@ -188,4 +188,43 @@
 			status = "disabled";
 		};
 	};
+
+	i2c-gpio at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&pioA 30 0 /* sda */
+			 &pioA 31 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 1 {
+		compatible = "i2c-gpio";
+		gpios = <&pioC 0 0 /* sda */
+			 &pioC 1 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
+	i2c-gpio at 2 {
+		compatible = "i2c-gpio";
+		gpios = <&pioB 4 0 /* sda */
+			 &pioB 5 0 /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
Quoted 8 lines
arch/arm/boot/dts/at91sam9x5.dtsi line 203
Quoted 5 lines
arch/arm/boot/dts/at91sam9x5.dtsi +5 -0
+		i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
If these are just any random gpio lines, it seems strange to define these in a SOC dtsi and then disable them.

Quoted 6 lines

they are not, they are the same pin as the i2c hardware IP.
Today the hw driver is not ready or will not work correctly (IP issue)
On hardware design we use the I2C pin but can not use the hw driver

Okay. Then other than the i2c-gpio node name, ack for all of these.

Rob

Wolfram Sang

2012-03-13 07:37 UTC #18 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

In reply to Jean-Christophe PLAGNIOL-VILLARD

Quoted 4 lines · Jean-Christophe PLAGNIOL-VILLARD

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc linux-i2c at vger.kernel.org
Cc devicetree-discuss at lists.ozlabs.org

Your patch description says "add DT support" but you are doing a bit more. This should at least be described here.

Quoted 20 lines

---
v3:
update i2c binding (Rob comments)
Jean can I have a ack to apply it?
Best Regards,
J.
 .../devicetree/bindings/gpio/gpio_i2c.txt          |   32 +++++++
 drivers/i2c/busses/i2c-gpio.c                      |   94 +++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt
Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +0 -0
new file mode 100644
index 0000000..4f8ec94

Documentation/devicetree/bindings/gpio/gpio_i2c.txt line 4

Quoted 4 lines

Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +4 -0
@@ -0,0 +1,32 @@
+Device-Tree bindings for i2c gpio driver
+
+Required properties:
+	- compatible = "i2c-gpio";

I assume people at "devicetree-discuss" have taken care of that, yet I wonder that this breaks the "vendor,product" syntax I know of for compatible entries.

Quoted 55 lines

Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +28 -0

+	- gpios: sda and scl gpio
+
+
+Optional properties:
+	- i2c-gpio,sda-open-drain: sda as open drain
+	- i2c-gpio,scl-open-drain: scl as open drain
+	- i2c-gpio,scl-output-only: scl as output only
+	- i2c-gpio,delay-us: delay between GPIO operations (may depend on each platform)
+	- i2c-gpio,timeout-ms: timeout to get data
+
+Example nodes:
+
+i2c at 0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2 at 56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};

drivers/i2c/busses/i2c-gpio.c +10 -1

index a651779..98ae85b 100644
@@ -14,8 +14,15 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_i2c.h>
 
-#include <asm/gpio.h>
+struct i2c_gpio_private_data {
+	struct i2c_adapter adap;
+	struct i2c_algo_bit_data bit_data;
+	struct i2c_gpio_platform_data pdata;
+};
 
 /* Toggle SDA by changing the direction of the pin */
 static void i2c_gpio_setsda_dir(void *data, int state)

@@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data)
 	return gpio_get_value(pdata->scl_pin);
 }
 
+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)

drivers/i2c/busses/i2c-gpio.c static int i2c_gpio_getscl(void *data) · line 99

Quoted 10 lines

drivers/i2c/busses/i2c-gpio.c +10 -0

+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (pdata->sda_pin < 0 || pdata->scl_pin < 0) {

gpio_is_valid()?

Quoted 115 lines

drivers/i2c/busses/i2c-gpio.c +51 ~~-19~~

+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);
+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
+	if (of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		!!of_property_read_bool(np, "i2c-gpio,sda-open-drain");
+	pdata->scl_is_open_drain =
+		!!of_property_read_bool(np, "i2c-gpio,scl-open-drain");
+	pdata->scl_is_output_only =
+		!!of_property_read_bool(np, "i2c-gpio,scl-output-only");
+
+	return 0;
+}
+
 static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_algo_bit_data *bit_data;
 	struct i2c_adapter *adap;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata)
-		return -ENXIO;
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	adap = &priv->adap;
+	bit_data = &priv->bit_data;
+	pdata = &priv->pdata;
 
-	ret = -ENOMEM;
-	adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (!adap)
-		goto err_alloc_adap;
-	bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
-	if (!bit_data)
-		goto err_alloc_bit_data;
+	if (pdev->dev.of_node) {
+		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+		if (ret)
+			return ret;
+	} else {
+		if (!pdev->dev.platform_data)
+			return -ENXIO;
+		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+	}
 
 	ret = gpio_request(pdata->sda_pin, "sda");
 	if (ret)

@@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->algo_data = bit_data;
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
 
 	/*
 	 * If "dev->id" is negative we consider it as zero.

@@ -154,7 +200,9 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_bus;
 
-	platform_set_drvdata(pdev, adap);
+	of_i2c_register_devices(adap);
+
+	platform_set_drvdata(pdev, priv);
 
 	dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n",
 		 pdata->sda_pin, pdata->scl_pin,

@@ -168,34 +216,40 @@ err_add_bus:
 err_request_scl:
 	gpio_free(pdata->sda_pin);
 err_request_sda:
-	kfree(bit_data);
-err_alloc_bit_data:
-	kfree(adap);
-err_alloc_adap:
 	return ret;
 }
 
 static int __devexit i2c_gpio_remove(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_adapter *adap;
 
-	adap = platform_get_drvdata(pdev);
-	pdata = pdev->dev.platform_data;
+	priv = platform_get_drvdata(pdev);
+	adap = &priv->adap;
+	pdata = &priv->pdata;
 
 	i2c_del_adapter(adap);
 	gpio_free(pdata->scl_pin);
 	gpio_free(pdata->sda_pin);
-	kfree(adap->algo_data);
-	kfree(adap);
 
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id i2c_gpio_dt_ids[] = {
+	{ .compatible = "i2c-gpio", },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);

drivers/i2c/busses/i2c-gpio.c err_add_bus: · line 255

Quoted 10 lines

drivers/i2c/busses/i2c-gpio.c +3 -0

+#endif
+
 static struct platform_driver i2c_gpio_driver = {
 	.driver		= {
 		.name	= "i2c-gpio",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(i2c_gpio_dt_ids),
 	},
 	.probe		= i2c_gpio_probe,
 	.remove		= __devexit_p(i2c_gpio_remove),

Otherwise looks good in general. What was your test scenario?

Thanks,

Wolfram

Signature · 9 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120313/2b483ec8/attachment.sig>

Jean-Christophe PLAGNIOL-VILLARD

author

2012-03-13 09:11 UTC #19 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

Quoted 23 lines · Wolfram Sang

Quoted 4 lines · Jean-Christophe PLAGNIOL-VILLARD
Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
Cc Nicolas Ferre <[email protected]>
Cc linux-i2c at vger.kernel.org
Cc devicetree-discuss at lists.ozlabs.org
Your patch description says "add DT support" but you are doing a bit more. This should at least be described here.
Quoted 14 lines
---
v3:
update i2c binding (Rob comments)
Jean can I have a ack to apply it?
Best Regards,
J.
 .../devicetree/bindings/gpio/gpio_i2c.txt          |   32 +++++++
 drivers/i2c/busses/i2c-gpio.c                      |   94 +++++++++++++++----
 2 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_i2c.txt

Quoted 13 lines

Documentation/devicetree/bindings/gpio/gpio_i2c.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +4 -0
new file mode 100644
index 0000000..4f8ec94
@@ -0,0 +1,32 @@
+Device-Tree bindings for i2c gpio driver
+
+Required properties:
+	- compatible = "i2c-gpio";
I assume people at "devicetree-discuss" have taken care of that, yet I wonder that this breaks the "vendor,product" syntax I know of for compatible entries.

yes but there no vendor here
get ack from Grant and Rob

Quoted 188 lines

Quoted 56 lines

Documentation/devicetree/bindings/gpio/gpio_i2c.txt new file +28 -0

+	- gpios: sda and scl gpio
+
+
+Optional properties:
+	- i2c-gpio,sda-open-drain: sda as open drain
+	- i2c-gpio,scl-open-drain: scl as open drain
+	- i2c-gpio,scl-output-only: scl as output only
+	- i2c-gpio,delay-us: delay between GPIO operations (may depend on each platform)
+	- i2c-gpio,timeout-ms: timeout to get data
+
+Example nodes:
+
+i2c at 0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2 at 56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};

drivers/i2c/busses/i2c-gpio.c +10 -1

index a651779..98ae85b 100644
@@ -14,8 +14,15 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/of_i2c.h>
 
-#include <asm/gpio.h>
+struct i2c_gpio_private_data {
+	struct i2c_adapter adap;
+	struct i2c_algo_bit_data bit_data;
+	struct i2c_gpio_platform_data pdata;
+};
 
 /* Toggle SDA by changing the direction of the pin */
 static void i2c_gpio_setsda_dir(void *data, int state)

@@ -78,24 +85,62 @@ static int i2c_gpio_getscl(void *data)
 	return gpio_get_value(pdata->scl_pin);
 }
 
+static int __devinit of_i2c_gpio_probe(struct device_node *np,
+			     struct i2c_gpio_platform_data *pdata)

drivers/i2c/busses/i2c-gpio.c static int i2c_gpio_getscl(void *data) · line 99

Quoted 10 lines

drivers/i2c/busses/i2c-gpio.c +10 -0

+{
+	u32 reg;
+
+	if (of_gpio_count(np) < 2)
+		return -ENODEV;
+
+	pdata->sda_pin = of_get_gpio(np, 0);
+	pdata->scl_pin = of_get_gpio(np, 1);
+
+	if (pdata->sda_pin < 0 || pdata->scl_pin < 0) {

gpio_is_valid()?

Quoted 119 lines

drivers/i2c/busses/i2c-gpio.c +53 ~~-19~~

+		pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
+		       np->full_name, pdata->sda_pin, pdata->scl_pin);
+		return -ENODEV;
+	}
+
+	of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
+
+	if (of_property_read_u32(np, "i2c-gpio,timeout-ms", &reg))
+		pdata->timeout = msecs_to_jiffies(reg);
+
+	pdata->sda_is_open_drain =
+		!!of_property_read_bool(np, "i2c-gpio,sda-open-drain");
+	pdata->scl_is_open_drain =
+		!!of_property_read_bool(np, "i2c-gpio,scl-open-drain");
+	pdata->scl_is_output_only =
+		!!of_property_read_bool(np, "i2c-gpio,scl-output-only");
+
+	return 0;
+}
+
 static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_algo_bit_data *bit_data;
 	struct i2c_adapter *adap;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata)
-		return -ENXIO;
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	adap = &priv->adap;
+	bit_data = &priv->bit_data;
+	pdata = &priv->pdata;
 
-	ret = -ENOMEM;
-	adap = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (!adap)
-		goto err_alloc_adap;
-	bit_data = kzalloc(sizeof(struct i2c_algo_bit_data), GFP_KERNEL);
-	if (!bit_data)
-		goto err_alloc_bit_data;
+	if (pdev->dev.of_node) {
+		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+		if (ret)
+			return ret;
+	} else {
+		if (!pdev->dev.platform_data)
+			return -ENXIO;
+		memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+	}
 
 	ret = gpio_request(pdata->sda_pin, "sda");
 	if (ret)

@@ -143,6 +188,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->algo_data = bit_data;
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;
 
 	/*
 	 * If "dev->id" is negative we consider it as zero.

@@ -154,7 +200,9 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_add_bus;
 
-	platform_set_drvdata(pdev, adap);
+	of_i2c_register_devices(adap);
+
+	platform_set_drvdata(pdev, priv);
 
 	dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n",
 		 pdata->sda_pin, pdata->scl_pin,

@@ -168,34 +216,40 @@ err_add_bus:
 err_request_scl:
 	gpio_free(pdata->sda_pin);
 err_request_sda:
-	kfree(bit_data);
-err_alloc_bit_data:
-	kfree(adap);
-err_alloc_adap:
 	return ret;
 }
 
 static int __devexit i2c_gpio_remove(struct platform_device *pdev)
 {
+	struct i2c_gpio_private_data *priv;
 	struct i2c_gpio_platform_data *pdata;
 	struct i2c_adapter *adap;
 
-	adap = platform_get_drvdata(pdev);
-	pdata = pdev->dev.platform_data;
+	priv = platform_get_drvdata(pdev);
+	adap = &priv->adap;
+	pdata = &priv->pdata;
 
 	i2c_del_adapter(adap);
 	gpio_free(pdata->scl_pin);
 	gpio_free(pdata->sda_pin);
-	kfree(adap->algo_data);
-	kfree(adap);
 
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id i2c_gpio_dt_ids[] = {
+	{ .compatible = "i2c-gpio", },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, i2c_gpio_dt_ids);
+#endif
+
 static struct platform_driver i2c_gpio_driver = {
 	.driver		= {

Quoted 8 lines

drivers/i2c/busses/i2c-gpio.c err_add_bus: · line 255
Quoted 6 lines
drivers/i2c/busses/i2c-gpio.c +1 -0
 		.name	= "i2c-gpio",
 		.owner	= THIS_MODULE,
+		.of_match_table	= of_match_ptr(i2c_gpio_dt_ids),
 	},
 	.probe		= i2c_gpio_probe,
 	.remove		= __devexit_p(i2c_gpio_remove),
Otherwise looks good in general. What was your test scenario?

AT91 support on 3 different SoC

will update and send it via AT91 with your ack

Best Regards,
J.

Quoted 8 lines

Thanks,

Wolfram

Signature · 2 lines

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

Wolfram Sang

2012-03-13 10:22 UTC #20 lore raw

PATCH v3 1/5 i2c/gpio: add DT support

Quoted 2 lines

will update and send it via AT91 with your ack

Please wait with my ack until I saw the update.

Signature · 9 lines

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120313/65cd860a/attachment.sig>