[PATCH v2 0/3] Add device tree support for Samsung's I2C driver

[PATCH v2 0/3] Add device tree support for Samsung's I2C driver

[Thomas Abraham]

This patchset adds device tree support for Samsung's I2C driver. The
Exynos4 DT enabled machine is updated to support device tree based probe
for I2C instance 0 and 1.

Changes since v1:

- Addressed all comments from Grant Likely
    - s3c24xx_i2c_is2440 function is simpler now.
    - Consolidated all the scatterd dt support code into a single function.
    - but the dependency on i2c pinmux handler function passed from the
      platform data is retained. It would be removed eventually when the
      new pinmux api is finalized and the i2c driver is modified to
      support the pinmux settings using the pinmux api.

Thomas Abraham (3):
  i2c: s3c2410: Keep a copy of platform data and use it.
  i2c: s3c2410: Add device tree support
  arm: dt: Add device tree support for i2c instance 0 and 1 on exynos4 dt machine

 .../devicetree/bindings/i2c/samsung-i2c.txt        |   44 +++++++++++++++
 arch/arm/boot/dts/exynos4-smdkv310.dts             |   45 +++++++++++++++
 arch/arm/mach-exynos4/Kconfig                      |    2 +
 arch/arm/mach-exynos4/mach-exynos4-dt.c            |   23 ++++++++
 drivers/i2c/busses/i2c-s3c2410.c                   |   58 +++++++++++++++++++-
 5 files changed, 169 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt

[PATCH v2 1/3] i2c: s3c2410: Keep a copy of platform data and use it.

[Thomas Abraham]

The platform data is copied into driver's private data and the copy is
used for all access to the platform data. This simpifies the addition
of device tree support for the i2c-s3c2410 driver.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 6c00c10..e132168 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -78,6 +78,7 @@ struct s3c24xx_i2c {
 	struct resource		*ioarea;
 	struct i2c_adapter	adap;
 
+	struct s3c2410_platform_i2c	pdata;
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 #endif
@@ -626,7 +627,7 @@ static int s3c24xx_i2c_calcdivisor(unsigned long clkin, unsigned int wanted,
 
 static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
 {
-	struct s3c2410_platform_i2c *pdata = i2c->dev->platform_data;
+	struct s3c2410_platform_i2c *pdata = &i2c->pdata;
 	unsigned long clkin = clk_get_rate(i2c->clk);
 	unsigned int divs, div1;
 	unsigned long target_frequency;
@@ -755,7 +756,7 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
 
 	/* get the plafrom data */
 
-	pdata = i2c->dev->platform_data;
+	pdata = &i2c->pdata;
 
 	/* inititalise the gpio */
 
@@ -810,6 +811,8 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	memcpy(&i2c->pdata, pdata, sizeof(*pdata));
+
 	strlcpy(i2c->adap.name, "s3c2410-i2c", sizeof(i2c->adap.name));
 	i2c->adap.owner   = THIS_MODULE;
 	i2c->adap.algo    = &s3c24xx_i2c_algorithm;
-- 
1.6.6.rc2

[PATCH v2 2/3] i2c: s3c2410: Add device tree support

[Thomas Abraham]

Add device tree probe support for Samsung's s3c2410 i2c driver.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 .../devicetree/bindings/i2c/samsung-i2c.txt        |   44 +++++++++++++++++
 drivers/i2c/busses/i2c-s3c2410.c                   |   51 +++++++++++++++++++-
 2 files changed, 94 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt

diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
new file mode 100644
index 0000000..4e1a2ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -0,0 +1,44 @@
+* Samsung's I2C controller
+
+The Samsung's I2C controller is used to interface with I2C devices.
+
+Required properties:
+  - compatible: value should be either of the following.
+      (a) "samsung, s3c2410-i2c", for i2c compatible with s3c2410 i2c.
+      (b) "samsung, s3c2440-i2c", for i2c compatible with s3c2440 i2c.
+
+  - reg: physical base address of the controller and length of memory mapped
+    region.
+
+  - interrupts: interrupt number to the cpu.
+
+  - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
+
+  - gpios: The order of the gpios should be in the following order.
+       <SDA, SCL>
+
+Optional properties:
+  - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
+    specified, default value is 0.
+
+  - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
+    specified, the default value in Hz is 100000.
+
+Example:
+
+	i2c@13870000 {
+		compatible = "samsung,s3c2440-i2c";
+		reg = <0x13870000 0x100>;
+		interrupts = <345>;
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <100000>;
+		gpios = <&gpd1 2 0 /* SDA */
+			 &gpd1 3 0 /* SCL */>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		wm8994@1a {
+			compatible = "wlf,wm8994";
+			reg = <0x1a>;
+		};
+	};
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index e132168..40264b0 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -35,6 +35,7 @@
 #include <linux/cpufreq.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/of_i2c.h>
 
 #include <asm/irq.h>
 
@@ -96,6 +97,10 @@ static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
 	struct platform_device *pdev = to_platform_device(i2c->dev);
 	enum s3c24xx_i2c_type type;
 
+	if (i2c->dev->of_node)
+		return of_device_is_compatible(i2c->dev->of_node,
+						"samsung,s3c2440-i2c");
+
 	type = platform_get_device_id(pdev)->driver_data;
 	return type == TYPE_S3C2440;
 }
@@ -787,6 +792,34 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+
+/* s3c24xx_i2c_parse_dt
+ *
+ * Parse the device tree node and retreive the platform data.
+*/
+
+static void
+s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
+{
+	struct s3c2410_platform_i2c *pdata = &i2c->pdata;
+
+	if (!np)
+		return;
+
+	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
+	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
+	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
+				(u32 *)&pdata->frequency);
+}
+#else
+static void
+s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
+{
+	return;
+}
+#endif
+
 /* s3c24xx_i2c_probe
  *
  * called by the bus driver when a suitable device is found
@@ -812,6 +845,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	}
 
 	memcpy(&i2c->pdata, pdata, sizeof(*pdata));
+	s3c24xx_i2c_parse_dt(pdev->dev.of_node, i2c);
 
 	strlcpy(i2c->adap.name, "s3c2410-i2c", sizeof(i2c->adap.name));
 	i2c->adap.owner   = THIS_MODULE;
@@ -908,13 +942,16 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	 */
 
 	i2c->adap.nr = pdata->bus_num;
+	i2c->adap.dev.of_node = pdev->dev.of_node;
 
-	ret = i2c_add_numbered_adapter(&i2c->adap);
+	ret = (pdev->dev.of_node) ? i2c_add_adapter(&i2c->adap) :
+					i2c_add_numbered_adapter(&i2c->adap);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add bus to i2c core\n");
 		goto err_cpufreq;
 	}
 
+	of_i2c_register_devices(&i2c->adap);
 	platform_set_drvdata(pdev, i2c);
 
 	dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
@@ -1016,6 +1053,17 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
 };
 MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
 
+#ifdef CONFIG_OF
+static const struct of_device_id s3c24xx_i2c_match[] = {
+	{ .compatible = "samsung,s3c2410-i2c" },
+	{ .compatible = "samsung,s3c2440-i2c" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
+#else
+#define s3c24xx_i2c_match NULL
+#endif
+
 static struct platform_driver s3c24xx_i2c_driver = {
 	.probe		= s3c24xx_i2c_probe,
 	.remove		= s3c24xx_i2c_remove,
@@ -1024,6 +1072,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "s3c-i2c",
 		.pm	= S3C24XX_DEV_PM_OPS,
+		.of_match_table	= s3c24xx_i2c_match,
 	},
 };
 
-- 
1.6.6.rc2

[PATCH v2 3/3] arm: dt: Add device tree support for i2c instance 0 and 1 on exynos4 dt machine

[Thomas Abraham]

Add device node for i2c instance 0 and 1 and list all its connected slave
devices.

Signed-off-by: Thomas Abraham <thomas.abraham at linaro.org>
---
 arch/arm/boot/dts/exynos4-smdkv310.dts  |   45 +++++++++++++++++++++++++++++++
 arch/arm/mach-exynos4/Kconfig           |    2 +
 arch/arm/mach-exynos4/mach-exynos4-dt.c |   23 ++++++++++++++++
 3 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
index d65c18c..6fd2032 100644
--- a/arch/arm/boot/dts/exynos4-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
@@ -41,6 +41,12 @@
 			irq-start = <61>;
 		};
 
+		gpd1: gpio-controller@114000C0 {
+			compatible = "samsung,exynos4-gpio-gpd1","samsung,exynos4-gpio";
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
+
 		watchdog@10060000 {
 			compatible = "samsung,s3c2410-wdt";
 			reg = <0x10060000 0x400>;
@@ -64,5 +70,44 @@
 			samsung,sdhci-cd-type = <0>;
 			samsung,sdhci-clkdiv-external;
 		};
+
+		i2c@13860000 {
+			compatible = "samsung,s3c2440-i2c";
+			reg = <0x13860000 0x100>;
+			interrupts = <344>;
+			samsung,i2c-sda-delay = <100>;
+			samsung,i2c-max-bus-freq = <20000>;
+			gpios = <&gpd1 0 0 /* SDA */
+				 &gpd1 1 0 /* SCL */>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			eeprom@50 {
+				compatible = "samsung,24ad0xd1";
+				reg = <0x50>;
+			};
+
+			eeprom@52 {
+				compatible = "samsung,24ad0xd1";
+				reg = <0x51>;
+			};
+		};
+
+		i2c@13870000 {
+			compatible = "samsung,s3c2440-i2c";
+			reg = <0x13870000 0x100>;
+			interrupts = <345>;
+			samsung,i2c-sda-delay = <100>;
+			samsung,i2c-max-bus-freq = <100000>;
+			gpios = <&gpd1 2 0 /* SDA */
+				 &gpd1 3 0 /* SCL */>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			wm8994@1a {
+				compatible = "wlf,wm8994";
+				reg = <0x1a>;
+			};
+		};
 	};
 };
diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index bb97b7e..06b43ca 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -193,6 +193,8 @@ config MACH_EXYNOS4_DT
 	select S3C_DEV_HSMMC
 	select S3C_DEV_HSMMC2
 	select EXYNOS4_SETUP_SDHCI
+	select EXYNOS4_SETUP_I2C0
+	select EXYNOS4_SETUP_I2C1
 	help
 	  Machine support for Samsung Exynos4 machine with device tree enabled.
 
diff --git a/arch/arm/mach-exynos4/mach-exynos4-dt.c b/arch/arm/mach-exynos4/mach-exynos4-dt.c
index 120665a..8b0cf8f 100644
--- a/arch/arm/mach-exynos4/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos4/mach-exynos4-dt.c
@@ -23,7 +23,10 @@
 #include <plat/regs-serial.h>
 #include <plat/exynos4.h>
 #include <plat/cpu.h>
+#include <plat/devs.h>
 #include <plat/sdhci.h>
+#include <plat/iic.h>
+#include <plat/iic-core.h>
 
 #include <mach/map.h>
 
@@ -63,6 +66,22 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
 };
 
 /*
+ * The i2c driver does not handle the pinmux settings. Instead, the
+ * platform specific callback function is passed through the platform
+ * data that can handle the pinmux settings for the i2c module.
+ * Eventually, when the new pinmux api is merged, the i2c driver can
+ * be modified to handle the pinmux settings. This is temporary for
+ * now.
+ */
+static struct s3c2410_platform_i2c exynos4_dt_i2c_data0 __initdata = {
+	.cfg_gpio	= s3c_i2c0_cfg_gpio,
+};
+
+static struct s3c2410_platform_i2c exynos4_dt_i2c_data1 __initdata = {
+	.cfg_gpio	= s3c_i2c1_cfg_gpio,
+};
+
+/*
  * The following lookup table is used to override device names when devices
  * are registered from device tree. Optionally, the platform data can also
  * to supplied. The sdhci driver requires the device name to be overridden
@@ -75,6 +94,10 @@ static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
 				"s3c-sdhci.2", &s3c_hsmmc2_def_platdata),
 	OF_DEV_AUXDATA("samsung,s3c6410-sdhci", EXYNOS4_PA_HSMMC(0),
 				"s3c-sdhci.0", &s3c_hsmmc0_def_platdata),
+	OF_DEV_AUXDATA("samsung,s3c2440-i2c", EXYNOS4_PA_IIC(0),
+				"s3c2440-i2c.0", &exynos4_dt_i2c_data0),
+	OF_DEV_AUXDATA("samsung,s3c2440-i2c", EXYNOS4_PA_IIC(1),
+				"s3c2440-i2c.1", &exynos4_dt_i2c_data1),
 	{},
 };
 
-- 
1.6.6.rc2

Re: [PATCH v2 3/3] arm: dt: Add device tree support for i2c instance 0 and 1 on exynos4 dt machine

[Grant Likely]

On Fri, Jul 22, 2011 at 03:48:38PM +0530, Thomas Abraham wrote:
> Add device node for i2c instance 0 and 1 and list all its connected slave
> devices.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham at linaro.org>
> ---
>  arch/arm/boot/dts/exynos4-smdkv310.dts  |   45 +++++++++++++++++++++++++++++++
>  arch/arm/mach-exynos4/Kconfig           |    2 +
>  arch/arm/mach-exynos4/mach-exynos4-dt.c |   23 ++++++++++++++++
>  3 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
> index d65c18c..6fd2032 100644
> --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
> +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
> @@ -41,6 +41,12 @@
>  			irq-start = <61>;
>  		};
>  
> +		gpd1: gpio-controller@114000C0 {
> +			compatible = "samsung,exynos4-gpio-gpd1","samsung,exynos4-gpio";
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
> +
>  		watchdog@10060000 {
>  			compatible = "samsung,s3c2410-wdt";
>  			reg = <0x10060000 0x400>;
> @@ -64,5 +70,44 @@
>  			samsung,sdhci-cd-type = <0>;
>  			samsung,sdhci-clkdiv-external;
>  		};
> +
> +		i2c@13860000 {
> +			compatible = "samsung,s3c2440-i2c";
> +			reg = <0x13860000 0x100>;
> +			interrupts = <344>;
> +			samsung,i2c-sda-delay = <100>;
> +			samsung,i2c-max-bus-freq = <20000>;
> +			gpios = <&gpd1 0 0 /* SDA */
> +				 &gpd1 1 0 /* SCL */>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			eeprom@50 {
> +				compatible = "samsung,24ad0xd1";
> +				reg = <0x50>;
> +			};
> +
> +			eeprom@52 {
> +				compatible = "samsung,24ad0xd1";
> +				reg = <0x51>;
> +			};
> +		};
> +
> +		i2c@13870000 {
> +			compatible = "samsung,s3c2440-i2c";
> +			reg = <0x13870000 0x100>;
> +			interrupts = <345>;
> +			samsung,i2c-sda-delay = <100>;
> +			samsung,i2c-max-bus-freq = <100000>;
> +			gpios = <&gpd1 2 0 /* SDA */
> +				 &gpd1 3 0 /* SCL */>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			wm8994@1a {
> +				compatible = "wlf,wm8994";
> +				reg = <0x1a>;
> +			};
> +		};
>  	};
>  };
> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
> index bb97b7e..06b43ca 100644
> --- a/arch/arm/mach-exynos4/Kconfig
> +++ b/arch/arm/mach-exynos4/Kconfig
> @@ -193,6 +193,8 @@ config MACH_EXYNOS4_DT
>  	select S3C_DEV_HSMMC
>  	select S3C_DEV_HSMMC2
>  	select EXYNOS4_SETUP_SDHCI
> +	select EXYNOS4_SETUP_I2C0
> +	select EXYNOS4_SETUP_I2C1

Hmmm, there should be a better way do do this; but I don't have any
objections.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

>  	help
>  	  Machine support for Samsung Exynos4 machine with device tree enabled.
>  
> diff --git a/arch/arm/mach-exynos4/mach-exynos4-dt.c b/arch/arm/mach-exynos4/mach-exynos4-dt.c
> index 120665a..8b0cf8f 100644
> --- a/arch/arm/mach-exynos4/mach-exynos4-dt.c
> +++ b/arch/arm/mach-exynos4/mach-exynos4-dt.c
> @@ -23,7 +23,10 @@
>  #include <plat/regs-serial.h>
>  #include <plat/exynos4.h>
>  #include <plat/cpu.h>
> +#include <plat/devs.h>
>  #include <plat/sdhci.h>
> +#include <plat/iic.h>
> +#include <plat/iic-core.h>
>  
>  #include <mach/map.h>
>  
> @@ -63,6 +66,22 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
>  };
>  
>  /*
> + * The i2c driver does not handle the pinmux settings. Instead, the
> + * platform specific callback function is passed through the platform
> + * data that can handle the pinmux settings for the i2c module.
> + * Eventually, when the new pinmux api is merged, the i2c driver can
> + * be modified to handle the pinmux settings. This is temporary for
> + * now.
> + */
> +static struct s3c2410_platform_i2c exynos4_dt_i2c_data0 __initdata = {
> +	.cfg_gpio	= s3c_i2c0_cfg_gpio,
> +};
> +
> +static struct s3c2410_platform_i2c exynos4_dt_i2c_data1 __initdata = {
> +	.cfg_gpio	= s3c_i2c1_cfg_gpio,
> +};
> +
> +/*
>   * The following lookup table is used to override device names when devices
>   * are registered from device tree. Optionally, the platform data can also
>   * to supplied. The sdhci driver requires the device name to be overridden
> @@ -75,6 +94,10 @@ static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
>  				"s3c-sdhci.2", &s3c_hsmmc2_def_platdata),
>  	OF_DEV_AUXDATA("samsung,s3c6410-sdhci", EXYNOS4_PA_HSMMC(0),
>  				"s3c-sdhci.0", &s3c_hsmmc0_def_platdata),
> +	OF_DEV_AUXDATA("samsung,s3c2440-i2c", EXYNOS4_PA_IIC(0),
> +				"s3c2440-i2c.0", &exynos4_dt_i2c_data0),
> +	OF_DEV_AUXDATA("samsung,s3c2440-i2c", EXYNOS4_PA_IIC(1),
> +				"s3c2440-i2c.1", &exynos4_dt_i2c_data1),
>  	{},
>  };
>  
> -- 
> 1.6.6.rc2
> 

Re: [PATCH v2 3/3] arm: dt: Add device tree support for i2c instance 0 and 1 on exynos4 dt machine

[Thomas Abraham]

Hi Grant,

On 31 July 2011 04:55, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Fri, Jul 22, 2011 at 03:48:38PM +0530, Thomas Abraham wrote:
>> Add device node for i2c instance 0 and 1 and list all its connected slave
>> devices.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham at linaro.org>
>> ---
>>  arch/arm/boot/dts/exynos4-smdkv310.dts  |   45 +++++++++++++++++++++++++++++++
>>  arch/arm/mach-exynos4/Kconfig           |    2 +
>>  arch/arm/mach-exynos4/mach-exynos4-dt.c |   23 ++++++++++++++++
>>  3 files changed, 70 insertions(+), 0 deletions(-)
>>

[...]

>> diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
>> index bb97b7e..06b43ca 100644
>> --- a/arch/arm/mach-exynos4/Kconfig
>> +++ b/arch/arm/mach-exynos4/Kconfig
>> @@ -193,6 +193,8 @@ config MACH_EXYNOS4_DT
>>       select S3C_DEV_HSMMC
>>       select S3C_DEV_HSMMC2
>>       select EXYNOS4_SETUP_SDHCI
>> +     select EXYNOS4_SETUP_I2C0
>> +     select EXYNOS4_SETUP_I2C1
>
> Hmmm, there should be a better way do do this; but I don't have any
> objections.

These config options could go away when the pinmux subsystem patches
are accepted and the i2c driver is modified to use it. Till then, we
need to include these config options.

>
> Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>

Thanks Grant.

Regards,
Thomas.

[...]

Re: [PATCH v2 2/3] i2c: s3c2410: Add device tree support

[Grant Likely]

On Fri, Jul 22, 2011 at 03:48:37PM +0530, Thomas Abraham wrote:
> Add device tree probe support for Samsung's s3c2410 i2c driver.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  .../devicetree/bindings/i2c/samsung-i2c.txt        |   44 +++++++++++++++++
>  drivers/i2c/busses/i2c-s3c2410.c                   |   51 +++++++++++++++++++-
>  2 files changed, 94 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/samsung-i2c.txt b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> new file mode 100644
> index 0000000..4e1a2ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> @@ -0,0 +1,44 @@
> +* Samsung's I2C controller
> +
> +The Samsung's I2C controller is used to interface with I2C devices.
> +
> +Required properties:
> +  - compatible: value should be either of the following.
> +      (a) "samsung, s3c2410-i2c", for i2c compatible with s3c2410 i2c.
> +      (b) "samsung, s3c2440-i2c", for i2c compatible with s3c2440 i2c.
> +
> +  - reg: physical base address of the controller and length of memory mapped
> +    region.
> +
> +  - interrupts: interrupt number to the cpu.
> +
> +  - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
> +
> +  - gpios: The order of the gpios should be in the following order.
> +       <SDA, SCL>
> +
> +Optional properties:
> +  - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
> +    specified, default value is 0.
> +
> +  - samsung,i2c-max-bus-freq: Desired frequency in Hz of the bus. If not
> +    specified, the default value in Hz is 100000.
> +
> +Example:
> +
> +	i2c@13870000 {
> +		compatible = "samsung,s3c2440-i2c";
> +		reg = <0x13870000 0x100>;
> +		interrupts = <345>;
> +		samsung,i2c-sda-delay = <100>;
> +		samsung,i2c-max-bus-freq = <100000>;
> +		gpios = <&gpd1 2 0 /* SDA */
> +			 &gpd1 3 0 /* SCL */>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		wm8994@1a {
> +			compatible = "wlf,wm8994";
> +			reg = <0x1a>;
> +		};
> +	};
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index e132168..40264b0 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -35,6 +35,7 @@
>  #include <linux/cpufreq.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> +#include <linux/of_i2c.h>
>  
>  #include <asm/irq.h>
>  
> @@ -96,6 +97,10 @@ static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
>  	struct platform_device *pdev = to_platform_device(i2c->dev);
>  	enum s3c24xx_i2c_type type;
>  
> +	if (i2c->dev->of_node)
> +		return of_device_is_compatible(i2c->dev->of_node,
> +						"samsung,s3c2440-i2c");
> +
>  	type = platform_get_device_id(pdev)->driver_data;
>  	return type == TYPE_S3C2440;
>  }
> @@ -787,6 +792,34 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +
> +/* s3c24xx_i2c_parse_dt
> + *
> + * Parse the device tree node and retreive the platform data.
> +*/
> +
> +static void
> +s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
> +{
> +	struct s3c2410_platform_i2c *pdata = &i2c->pdata;
> +
> +	if (!np)
> +		return;
> +
> +	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
> +	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
> +	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
> +				(u32 *)&pdata->frequency);
> +}
> +#else
> +static void
> +s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
> +{
> +	return;
> +}
> +#endif
> +
>  /* s3c24xx_i2c_probe
>   *
>   * called by the bus driver when a suitable device is found
> @@ -812,6 +845,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  	}
>  
>  	memcpy(&i2c->pdata, pdata, sizeof(*pdata));
> +	s3c24xx_i2c_parse_dt(pdev->dev.of_node, i2c);
>  
>  	strlcpy(i2c->adap.name, "s3c2410-i2c", sizeof(i2c->adap.name));
>  	i2c->adap.owner   = THIS_MODULE;
> @@ -908,13 +942,16 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  	 */
>  
>  	i2c->adap.nr = pdata->bus_num;
> +	i2c->adap.dev.of_node = pdev->dev.of_node;
>  
> -	ret = i2c_add_numbered_adapter(&i2c->adap);
> +	ret = (pdev->dev.of_node) ? i2c_add_adapter(&i2c->adap) :
> +					i2c_add_numbered_adapter(&i2c->adap);

i2c_add_numbered_adapter will do the right thing now if i2c->adap.nr
is set to -1 when a bus number needs to be dynamically allocated.  You can simplify this.

>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to add bus to i2c core\n");
>  		goto err_cpufreq;
>  	}
>  
> +	of_i2c_register_devices(&i2c->adap);
>  	platform_set_drvdata(pdev, i2c);
>  
>  	dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
> @@ -1016,6 +1053,17 @@ static struct platform_device_id s3c24xx_driver_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id s3c24xx_i2c_match[] = {
> +	{ .compatible = "samsung,s3c2410-i2c" },
> +	{ .compatible = "samsung,s3c2440-i2c" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
> +#else
> +#define s3c24xx_i2c_match NULL
> +#endif
> +
>  static struct platform_driver s3c24xx_i2c_driver = {
>  	.probe		= s3c24xx_i2c_probe,
>  	.remove		= s3c24xx_i2c_remove,
> @@ -1024,6 +1072,7 @@ static struct platform_driver s3c24xx_i2c_driver = {
>  		.owner	= THIS_MODULE,
>  		.name	= "s3c-i2c",
>  		.pm	= S3C24XX_DEV_PM_OPS,
> +		.of_match_table	= s3c24xx_i2c_match,
>  	},

Looks pretty good.  After fixing the above comment, feel free to add
my:

Acked-by: Grant Likely <grant.likely@secretlab.ca>

>  };
>  
> -- 
> 1.6.6.rc2
> 

Re: [PATCH v2 2/3] i2c: s3c2410: Add device tree support

[Thomas Abraham]

Hi Grant,

On 31 July 2011 04:53, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Jul 22, 2011 at 03:48:37PM +0530, Thomas Abraham wrote:
>> Add device tree probe support for Samsung's s3c2410 i2c driver.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>>  .../devicetree/bindings/i2c/samsung-i2c.txt        |   44 +++++++++++++++++
>>  drivers/i2c/busses/i2c-s3c2410.c                   |   51 +++++++++++++++++++-
>>  2 files changed, 94 insertions(+), 1 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt
>>

[...]

>>
>> -     ret = i2c_add_numbered_adapter(&i2c->adap);
>> +     ret = (pdev->dev.of_node) ? i2c_add_adapter(&i2c->adap) :
>> +                                     i2c_add_numbered_adapter(&i2c->adap);
>
> i2c_add_numbered_adapter will do the right thing now if i2c->adap.nr
> is set to -1 when a bus number needs to be dynamically allocated.  You can simplify this.
>

Ok. I will change it to just use the i2c_add_numbered_adapter function
(just noticed your patch).

A question related to bus number: In case of dynamically allocated bus
number, how would the application know to which i2c bus a particular
slave device is connected? The application would be programmed to look
for a slave device on a particular bus. But with dynamic i2c bus
number, how should application find out the correct bus number?

>>       if (ret < 0) {
>>               dev_err(&pdev->dev, "failed to add bus to i2c core\n");
>>               goto err_cpufreq;
>>       }
>>
>> +     of_i2c_register_devices(&i2c->adap);
>>       platform_set_drvdata(pdev, i2c);
>>

[...]

>>       },
>
> Looks pretty good.  After fixing the above comment, feel free to add
> my:
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Thanks Grant. I will fix as per your comment.

Regards,
Thomas.

>
>>  };
>>
>> --
>> 1.6.6.rc2
>>
>

Re: [PATCH v2 1/3] i2c: s3c2410: Keep a copy of platform data and use it.

[Grant Likely]

On Fri, Jul 22, 2011 at 03:48:36PM +0530, Thomas Abraham wrote:
> The platform data is copied into driver's private data and the copy is
> used for all access to the platform data. This simpifies the addition
> of device tree support for the i2c-s3c2410 driver.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  drivers/i2c/busses/i2c-s3c2410.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index 6c00c10..e132168 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -78,6 +78,7 @@ struct s3c24xx_i2c {
>  	struct resource		*ioarea;
>  	struct i2c_adapter	adap;
>  
> +	struct s3c2410_platform_i2c	pdata;

After looking at a bunch of patches, there is another way do do this
I think.  Instead of copying pdata into the private data structure, it
can remain as a pointer if the DT code uses devm_kzmalloc to allocate
a s3c2410_platform_i2c structure when needed.

I leave it up to you (and Ben) though as to which you prefer.  I'm
okay either way, and you can add:

Acked-by: Grant Likely <grant.likely@secretlab.ca>


>  #ifdef CONFIG_CPU_FREQ
>  	struct notifier_block	freq_transition;
>  #endif
> @@ -626,7 +627,7 @@ static int s3c24xx_i2c_calcdivisor(unsigned long clkin, unsigned int wanted,
>  
>  static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
>  {
> -	struct s3c2410_platform_i2c *pdata = i2c->dev->platform_data;
> +	struct s3c2410_platform_i2c *pdata = &i2c->pdata;
>  	unsigned long clkin = clk_get_rate(i2c->clk);
>  	unsigned int divs, div1;
>  	unsigned long target_frequency;
> @@ -755,7 +756,7 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
>  
>  	/* get the plafrom data */
>  
> -	pdata = i2c->dev->platform_data;
> +	pdata = &i2c->pdata;
>  
>  	/* inititalise the gpio */
>  
> @@ -810,6 +811,8 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	memcpy(&i2c->pdata, pdata, sizeof(*pdata));
> +
>  	strlcpy(i2c->adap.name, "s3c2410-i2c", sizeof(i2c->adap.name));
>  	i2c->adap.owner   = THIS_MODULE;
>  	i2c->adap.algo    = &s3c24xx_i2c_algorithm;
> -- 
> 1.6.6.rc2
> 

Re: [PATCH v2 1/3] i2c: s3c2410: Keep a copy of platform data and use it.

[Thomas Abraham]

Hi Grant,

On 31 July 2011 04:51, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Jul 22, 2011 at 03:48:36PM +0530, Thomas Abraham wrote:
>> The platform data is copied into driver's private data and the copy is
>> used for all access to the platform data. This simpifies the addition
>> of device tree support for the i2c-s3c2410 driver.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>>  drivers/i2c/busses/i2c-s3c2410.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
>> index 6c00c10..e132168 100644
>> --- a/drivers/i2c/busses/i2c-s3c2410.c
>> +++ b/drivers/i2c/busses/i2c-s3c2410.c
>> @@ -78,6 +78,7 @@ struct s3c24xx_i2c {
>>       struct resource         *ioarea;
>>       struct i2c_adapter      adap;
>>
>> +     struct s3c2410_platform_i2c     pdata;
>
> After looking at a bunch of patches, there is another way do do this
> I think.  Instead of copying pdata into the private data structure, it
> can remain as a pointer if the DT code uses devm_kzmalloc to allocate
> a s3c2410_platform_i2c structure when needed.
>
> I leave it up to you (and Ben) though as to which you prefer.  I'm
> okay either way, and you can add:

I will submit another patch with the changes you have suggested. Ben
could then choose the approach that he feels is the best.

>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Thanks Grant.


Regards,
Thomas.

>
>
[...]

Re: [PATCH v2 0/3] Add device tree support for Samsung's I2C driver

[Ben Dooks]

On Fri, Jul 22, 2011 at 03:48:35PM +0530, Thomas Abraham wrote:
> This patchset adds device tree support for Samsung's I2C driver. The
> Exynos4 DT enabled machine is updated to support device tree based probe
> for I2C instance 0 and 1.

I'll look at taking the i2c specific changes for either this or the next
release depending on how I feel.
 
> Changes since v1:
> 
> - Addressed all comments from Grant Likely
>     - s3c24xx_i2c_is2440 function is simpler now.
>     - Consolidated all the scatterd dt support code into a single function.
>     - but the dependency on i2c pinmux handler function passed from the
>       platform data is retained. It would be removed eventually when the
>       new pinmux api is finalized and the i2c driver is modified to
>       support the pinmux settings using the pinmux api.
> 
> Thomas Abraham (3):
>   i2c: s3c2410: Keep a copy of platform data and use it.
>   i2c: s3c2410: Add device tree support
>   arm: dt: Add device tree support for i2c instance 0 and 1 on exynos4 dt machine
> 
>  .../devicetree/bindings/i2c/samsung-i2c.txt        |   44 +++++++++++++++
>  arch/arm/boot/dts/exynos4-smdkv310.dts             |   45 +++++++++++++++
>  arch/arm/mach-exynos4/Kconfig                      |    2 +
>  arch/arm/mach-exynos4/mach-exynos4-dt.c            |   23 ++++++++
>  drivers/i2c/busses/i2c-s3c2410.c                   |   58 +++++++++++++++++++-
>  5 files changed, 169 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.