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

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

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
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

diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
new file mode 100644
index 0000000..4f8ec94
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
@@ -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>;
+	};
+};
diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index a651779..98ae85b 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -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),
-- 
1.7.7

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

[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 <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
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(-)

diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
index 4b0dc99..072b2da 100644
--- a/arch/arm/boot/dts/at91sam9g20.dtsi
+++ b/arch/arm/boot/dts/at91sam9g20.dtsi
@@ -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";
+	};
 };
-- 
1.7.7

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

[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 <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
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(-)

diff --git a/arch/arm/boot/dts/usb_a9g20.dts b/arch/arm/boot/dts/usb_a9g20.dts
index 6ab7e7a..07012ca 100644
--- a/arch/arm/boot/dts/usb_a9g20.dts
+++ b/arch/arm/boot/dts/usb_a9g20.dts
@@ -97,4 +97,13 @@
 			gpio-key,wakeup;
 		};
 	};
+
+	i2c-gpio at 0 {
+		status = "okay";
+
+		rv3029c2 at 56 {
+			compatible = "rv3029c2";
+			reg = <0x56>;
+		};
+	};
 };
diff --git a/arch/arm/configs/at91sam9g20_defconfig b/arch/arm/configs/at91sam9g20_defconfig
index 9123568..994d331 100644
--- a/arch/arm/configs/at91sam9g20_defconfig
+++ b/arch/arm/configs/at91sam9g20_defconfig
@@ -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
-- 
1.7.7

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

[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 <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
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(-)

diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index d79021b..4a8a36d 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -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";
+	};
 };
-- 
1.7.7

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

[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 <plagnioj@jcrosoft.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
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(-)

diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index c294657..fdd1ac6 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -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";
+	};
 };
-- 
1.7.7

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

[Karol Lewandowski]

On 08.03.2012 09:50, Jean-Christophe PLAGNIOL-VILLARD wrote:

> +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);


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_.

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:52 Thu 08 Mar     , Karol Lewandowski wrote:
> On 08.03.2012 09:50, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > +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);
> 
> 
> 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_.
yeah forget this one

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

Best Regards,
J.

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

[Rob Herring]

On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> For now on use i2c-gpio driver on the same pin as the hardware IP.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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(-)
> 
> diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> index 4b0dc99..072b2da 100644
> --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> @@ -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

> +		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";
> +	};
>  };

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

[Rob Herring]

On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> For now on use i2c-gpio driver on the same pin as the hardware IP.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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(-)
> 
> diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
> index c294657..fdd1ac6 100644
> --- a/arch/arm/boot/dts/at91sam9x5.dtsi
> +++ b/arch/arm/boot/dts/at91sam9x5.dtsi
> @@ -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";
> +	};

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

Rob


>  };

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

[Rob Herring]

On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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.

For this patch:

Acked-by: Rob Herring <rob.herring@calxeda.com>


>  .../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
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
> new file mode 100644
> index 0000000..4f8ec94
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
> @@ -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>;
> +	};
> +};
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> index a651779..98ae85b 100644
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -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),

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:12 Thu 08 Mar     , Rob Herring wrote:
> On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > For now on use i2c-gpio driver on the same pin as the hardware IP.
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > 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(-)
> > 
> > diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
> > index c294657..fdd1ac6 100644
> > --- a/arch/arm/boot/dts/at91sam9x5.dtsi
> > +++ b/arch/arm/boot/dts/at91sam9x5.dtsi
> > @@ -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";
> > +	};
> 
> 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.

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

[Rob Herring]

On 03/08/2012 11:52 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:12 Thu 08 Mar     , Rob Herring wrote:
>> On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> For now on use i2c-gpio driver on the same pin as the hardware IP.
>>>
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> 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(-)
>>>
>>> diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
>>> index c294657..fdd1ac6 100644
>>> --- a/arch/arm/boot/dts/at91sam9x5.dtsi
>>> +++ b/arch/arm/boot/dts/at91sam9x5.dtsi
>>> @@ -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";
>>> +	};
>>
>> 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
> 

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

Rob

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

[Grant Likely]

On Thu,  8 Mar 2012 09:50:32 +0100, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> For now on use i2c-gpio driver on the same pin as the hardware IP.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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(-)
> 
> diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> index 4b0dc99..072b2da 100644
> --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> @@ -189,4 +189,17 @@
>  			status = "disabled";
>  		};
>  	};
> +
> +	i2c-gpio at 0 {

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>;
> +		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.

g.

>  };
> -- 
> 1.7.7
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

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

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:08 Thu 08 Mar     , Rob Herring wrote:
> On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > For now on use i2c-gpio driver on the same pin as the hardware IP.
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > 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(-)
> > 
> > diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> > index 4b0dc99..072b2da 100644
> > --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> > +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> > @@ -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.
> 
> Rob
> 
> > +		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

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 22:26 Thu 08 Mar     , Grant Likely wrote:
> On Thu,  8 Mar 2012 09:50:32 +0100, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> > For now on use i2c-gpio driver on the same pin as the hardware IP.
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > 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(-)
> > 
> > diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> > index 4b0dc99..072b2da 100644
> > --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> > +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> > @@ -189,4 +189,17 @@
> >  			status = "disabled";
> >  		};
> >  	};
> > +
> > +	i2c-gpio at 0 {
> 
> 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>;
> > +		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
> 
> Otherwise I think this patch series looks okay.

Best Regards,
J.

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

[Rob Herring]

On 03/09/2012 03:53 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:08 Thu 08 Mar     , Rob Herring wrote:
>> On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> For now on use i2c-gpio driver on the same pin as the hardware IP.
>>>
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> 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(-)
>>>
>>> diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
>>> index 4b0dc99..072b2da 100644
>>> --- a/arch/arm/boot/dts/at91sam9g20.dtsi
>>> +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
>>> @@ -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
> 

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

> Best Regards,
> J.
>>
>> Rob
>>
>>> +		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

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:50 Thu 08 Mar     , Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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.

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 07:59 Fri 09 Mar     , Rob Herring wrote:
> On 03/09/2012 03:53 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 11:08 Thu 08 Mar     , Rob Herring wrote:
> >> On 03/08/2012 02:50 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>> For now on use i2c-gpio driver on the same pin as the hardware IP.
> >>>
> >>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> >>> 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(-)
> >>>
> >>> diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> >>> index 4b0dc99..072b2da 100644
> >>> --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> >>> +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> >>> @@ -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
> > 
> 
> 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.

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

[Wolfram Sang]

On Thu, Mar 08, 2012 at 09:50:31AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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.

> ---
> 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
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
> new file mode 100644
> index 0000000..4f8ec94
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
> @@ -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.

> +	- 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>;
> +	};
> +};
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> index a651779..98ae85b 100644
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -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) {

gpio_is_valid()?

> +		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),

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

Thanks,

   Wolfram

-- 
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>

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 08:37 Tue 13 Mar     , Wolfram Sang wrote:
> On Thu, Mar 08, 2012 at 09:50:31AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > 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.
> 
> > ---
> > 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
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio_i2c.txt b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
> > new file mode 100644
> > index 0000000..4f8ec94
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio_i2c.txt
> > @@ -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
> 
> > +	- 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>;
> > +	};
> > +};
> > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> > index a651779..98ae85b 100644
> > --- a/drivers/i2c/busses/i2c-gpio.c
> > +++ b/drivers/i2c/busses/i2c-gpio.c
> > @@ -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) {
> 
> gpio_is_valid()?
> 
> > +		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),
> 
> 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.
> 
> Thanks,
> 
>    Wolfram
> 
> -- 
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[Wolfram Sang]

> will update and send it via AT91  with your ack

Please wait with my ack until I saw the update.

-- 
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>