[PATCH 0/2] Add device tree support for Samsung's I2C driver.

[PATCH 0/2] 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 1.

Thomas Abraham (2):
  i2c: s3c24xx: Add device tree support
  arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

 .../devicetree/bindings/i2c/samsung-i2c.txt        |   43 ++++++++++
 arch/arm/boot/dts/exynos4-smdkv310.dts             |   19 ++++-
 arch/arm/mach-exynos4/Kconfig                      |    1 +
 arch/arm/mach-exynos4/mach-exynos4-dt.c            |    9 ++
 drivers/i2c/busses/i2c-s3c2410.c                   |   87 ++++++++++++++++----
 5 files changed, 143 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt

[PATCH 1/2] i2c: s3c24xx: Add device tree support

[Thomas Abraham]

Add device tree support for Samsung's s3c24xx i2c driver. It adds
support for parsing platform data from device tree and initialize
all the slave devices specified as child nodes.

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/i2c/samsung-i2c.txt        |   43 ++++++++++
 drivers/i2c/busses/i2c-s3c2410.c                   |   87 ++++++++++++++++----
 2 files changed, 115 insertions(+), 15 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..73191c3
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -0,0 +1,43 @@
+* 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-bus-number: Specifies the i2c bus number.
+
+  - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
+
+Optional properties:
+  - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
+    specified, default value is 0x10.
+
+  - 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-bus-number = <1>;
+		samsung,i2c-slave-addr = <16>;
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <100000>;
+		#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 6c00c10..5a84a0b 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -35,6 +35,9 @@
 #include <linux/cpufreq.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_i2c.h>
+#include <linux/of_device.h>
 
 #include <asm/irq.h>
 
@@ -83,6 +86,33 @@ struct s3c24xx_i2c {
 #endif
 };
 
+#ifdef CONFIG_OF
+static enum s3c24xx_i2c_type samsung_i2c_types[] = {
+	TYPE_S3C2410,
+	TYPE_S3C2440
+};
+
+static const struct of_device_id s3c24xx_i2c_match[] = {
+	{ .compatible = "samsung,s3c2410-i2c", .data = &samsung_i2c_types[0], },
+	{ .compatible = "samsung,s3c2440-i2c", .data = &samsung_i2c_types[1], },
+	{},
+};
+MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
+
+static const struct
+of_device_id *s3c24xx_i2c_get_of_device_id(struct platform_device *pdev)
+{
+	return of_match_device(s3c24xx_i2c_match, &pdev->dev);
+}
+#else
+#define s3c24xx_i2c_match NULL
+static const struct
+of_device_id *s3c24xx_i2c_get_of_device_id(struct platform_device *pdev)
+{
+	return NULL;
+}
+#endif
+
 /* default platform data removed, dev should always carry data. */
 
 /* s3c24xx_i2c_is2440()
@@ -93,9 +123,15 @@ struct s3c24xx_i2c {
 static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
 {
 	struct platform_device *pdev = to_platform_device(i2c->dev);
+	const struct of_device_id *i2c_of_id;
 	enum s3c24xx_i2c_type type;
 
-	type = platform_get_device_id(pdev)->driver_data;
+	i2c_of_id = s3c24xx_i2c_get_of_device_id(pdev);
+	if (platform_get_device_id(pdev))
+		type = platform_get_device_id(pdev)->driver_data;
+	else
+		type = *(enum s3c24xx_i2c_type *)i2c_of_id->data;
+
 	return type == TYPE_S3C2440;
 }
 
@@ -630,16 +666,21 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
 	unsigned long clkin = clk_get_rate(i2c->clk);
 	unsigned int divs, div1;
 	unsigned long target_frequency;
+	unsigned int max_freq = 0;
 	u32 iiccon;
 	int freq;
 
 	i2c->clkrate = clkin;
 	clkin /= 1000;		/* clkin now in KHz */
 
-	dev_dbg(i2c->dev, "pdata desired frequency %lu\n", pdata->frequency);
-
-	target_frequency = pdata->frequency ? pdata->frequency : 100000;
+	if (i2c->dev->of_node)
+		of_property_read_u32(i2c->dev->of_node,
+				"samsung,i2c-max-bus-freq", &max_freq);
+	else
+		max_freq = pdata->frequency;
+	dev_dbg(i2c->dev, "desired frequency %u\n", max_freq);
 
+	target_frequency = max_freq ? max_freq : 100000;
 	target_frequency /= 1000; /* Target frequency now in KHz */
 
 	freq = s3c24xx_i2c_calcdivisor(clkin, target_frequency, &div1, &divs);
@@ -663,19 +704,24 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
 	writel(iiccon, i2c->regs + S3C2410_IICCON);
 
 	if (s3c24xx_i2c_is2440(i2c)) {
-		unsigned long sda_delay;
+		unsigned int sda_delay = 0;
 
-		if (pdata->sda_delay) {
-			sda_delay = clkin * pdata->sda_delay;
+		if (i2c->dev->of_node)
+			of_property_read_u32(i2c->dev->of_node,
+				"samsung,i2c-sda-delay", &sda_delay);
+		else
+			sda_delay = pdata->sda_delay;
+
+		if (sda_delay) {
+			sda_delay *= clkin;
 			sda_delay = DIV_ROUND_UP(sda_delay, 1000000);
 			sda_delay = DIV_ROUND_UP(sda_delay, 5);
 			if (sda_delay > 3)
 				sda_delay = 3;
 			sda_delay |= S3C2410_IICLC_FILTER_ON;
-		} else
-			sda_delay = 0;
+		}
 
-		dev_dbg(i2c->dev, "IICLC=%08lx\n", sda_delay);
+		dev_dbg(i2c->dev, "IICLC=%08x\n", sda_delay);
 		writel(sda_delay, i2c->regs + S3C2440_IICLC);
 	}
 
@@ -751,7 +797,7 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
 {
 	unsigned long iicon = S3C2410_IICCON_IRQEN | S3C2410_IICCON_ACKEN;
 	struct s3c2410_platform_i2c *pdata;
-	unsigned int freq;
+	unsigned int freq, slave_addr = 0x10;
 
 	/* get the plafrom data */
 
@@ -763,10 +809,14 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
 		pdata->cfg_gpio(to_platform_device(i2c->dev));
 
 	/* write slave address */
+	if (i2c->dev->of_node)
+		of_property_read_u32(i2c->dev->of_node,
+				"samsung,i2c-slave-addr", &slave_addr);
+	else
+		slave_addr = pdata->slave_addr;
+	writeb(slave_addr, i2c->regs + S3C2410_IICADD);
 
-	writeb(pdata->slave_addr, i2c->regs + S3C2410_IICADD);
-
-	dev_info(i2c->dev, "slave address 0x%02x\n", pdata->slave_addr);
+	dev_info(i2c->dev, "slave address 0x%02x\n", slave_addr);
 
 	writel(iicon, i2c->regs + S3C2410_IICCON);
 
@@ -904,7 +954,12 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	 * being bus 0.
 	 */
 
-	i2c->adap.nr = pdata->bus_num;
+	if (i2c->dev->of_node)
+		of_property_read_u32(i2c->dev->of_node,
+				"samsung,i2c-bus-number", &i2c->adap.nr);
+	else
+		i2c->adap.nr = pdata->bus_num;
+	i2c->adap.dev.of_node = pdev->dev.of_node;
 
 	ret = i2c_add_numbered_adapter(&i2c->adap);
 	if (ret < 0) {
@@ -912,6 +967,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		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));
@@ -1021,6 +1077,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.7.1

[PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[Thomas Abraham]

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

Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
 arch/arm/mach-exynos4/Kconfig           |    1 +
 arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
 3 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
index d65c18c..29c40ed 100644
--- a/arch/arm/boot/dts/exynos4-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
@@ -23,7 +23,7 @@
 	};
 
 	chosen {
-		bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
+		bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
 	};
 
 	soc {
@@ -64,5 +64,22 @@
 			samsung,sdhci-cd-type = <0>;
 			samsung,sdhci-clkdiv-external;
 		};
+
+		i2c@13870000 {
+			compatible = "samsung,s3c2440-i2c";
+			reg = <0x13870000 0x100>;
+			interrupts = <345>;
+			samsung,i2c-bus-number = <1>;
+			samsung,i2c-slave-addr = <16>;
+			samsung,i2c-sda-delay = <100>;
+			samsung,i2c-max-bus-freq = <100000>;
+			#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..c7fce3e 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -193,6 +193,7 @@ config MACH_EXYNOS4_DT
 	select S3C_DEV_HSMMC
 	select S3C_DEV_HSMMC2
 	select EXYNOS4_SETUP_SDHCI
+	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..ef6b4cb 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>
 
@@ -62,6 +65,10 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
 	},
 };
 
+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
@@ -75,6 +82,8 @@ 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(1),
+				"s3c2440-i2c.1", &exynos4_dt_i2c_data1),
 	{},
 };
 
-- 
1.7.1

Re: [PATCH 1/2] i2c: s3c24xx: Add device tree support

[Grant Likely]

On Mon, Jul 18, 2011 at 06:20:40AM +0530, Thomas Abraham wrote:
> Add device tree support for Samsung's s3c24xx i2c driver. It adds
> support for parsing platform data from device tree and initialize
> all the slave devices specified as child nodes.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../devicetree/bindings/i2c/samsung-i2c.txt        |   43 ++++++++++
>  drivers/i2c/busses/i2c-s3c2410.c                   |   87 ++++++++++++++++----
>  2 files changed, 115 insertions(+), 15 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..73191c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
> @@ -0,0 +1,43 @@
> +* 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-bus-number: Specifies the i2c bus number.

Drop this property.  i2c bus numbers are dynamically assigned when
using the DT.

> +
> +  - samsung,i2c-sda-delay: Delay (in ns) applied to data line (SDA) edges.
> +
> +Optional properties:
> +  - samsung,i2c-slave-addr: Slave address in multi-master enviroment. If not
> +    specified, default value is 0x10.
> +
> +  - 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-bus-number = <1>;
> +		samsung,i2c-slave-addr = <16>;
> +		samsung,i2c-sda-delay = <100>;
> +		samsung,i2c-max-bus-freq = <100000>;
> +		#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 6c00c10..5a84a0b 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -35,6 +35,9 @@
>  #include <linux/cpufreq.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_i2c.h>
> +#include <linux/of_device.h>
>  
>  #include <asm/irq.h>
>  
> @@ -83,6 +86,33 @@ struct s3c24xx_i2c {
>  #endif
>  };
>  
> +#ifdef CONFIG_OF
> +static enum s3c24xx_i2c_type samsung_i2c_types[] = {
> +	TYPE_S3C2410,
> +	TYPE_S3C2440
> +};
> +
> +static const struct of_device_id s3c24xx_i2c_match[] = {
> +	{ .compatible = "samsung,s3c2410-i2c", .data = &samsung_i2c_types[0], },
> +	{ .compatible = "samsung,s3c2440-i2c", .data = &samsung_i2c_types[1], },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);

The samsung_i2c_types[] array is unnecessary.  You can simply do:

static const struct of_device_id s3c24xx_i2c_match[] = {
	{ .compatible = "samsung,s3c2410-i2c", .data = (void*)TYPE_S3C2410, },
	{ .compatible = "samsung,s3c2440-i2c", .data = (void*)TYPE_S3C2440, },
	{},
};
MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);

Or, drop the .data field entirely and call of_device_is_compatible()
directly in s3c24xx_i2c_is2440().

> +
> +static const struct
> +of_device_id *s3c24xx_i2c_get_of_device_id(struct platform_device *pdev)
> +{
> +	return of_match_device(s3c24xx_i2c_match, &pdev->dev);
> +}
> +#else
> +#define s3c24xx_i2c_match NULL
> +static const struct
> +of_device_id *s3c24xx_i2c_get_of_device_id(struct platform_device *pdev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  /* default platform data removed, dev should always carry data. */
>  
>  /* s3c24xx_i2c_is2440()
> @@ -93,9 +123,15 @@ struct s3c24xx_i2c {
>  static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
>  {
>  	struct platform_device *pdev = to_platform_device(i2c->dev);
> +	const struct of_device_id *i2c_of_id;
>  	enum s3c24xx_i2c_type type;
>  
> -	type = platform_get_device_id(pdev)->driver_data;
> +	i2c_of_id = s3c24xx_i2c_get_of_device_id(pdev);
> +	if (platform_get_device_id(pdev))
> +		type = platform_get_device_id(pdev)->driver_data;
> +	else
> +		type = *(enum s3c24xx_i2c_type *)i2c_of_id->data;
> +
>  	return type == TYPE_S3C2440;

In fact, most of these line changes go away by putting the folowing
lines above the platform_get_device_id() line:

>  }
>  
> @@ -630,16 +666,21 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
>  	unsigned long clkin = clk_get_rate(i2c->clk);
>  	unsigned int divs, div1;
>  	unsigned long target_frequency;
> +	unsigned int max_freq = 0;
>  	u32 iiccon;
>  	int freq;
>  
>  	i2c->clkrate = clkin;
>  	clkin /= 1000;		/* clkin now in KHz */
>  
> -	dev_dbg(i2c->dev, "pdata desired frequency %lu\n", pdata->frequency);
> -
> -	target_frequency = pdata->frequency ? pdata->frequency : 100000;
> +	if (i2c->dev->of_node)
> +		of_property_read_u32(i2c->dev->of_node,
> +				"samsung,i2c-max-bus-freq", &max_freq);

No need to test for i2c->dev->of_node.  of_property_read_u32() will
fail gracefully and not modify max_freq if of_node is NULL.

> +	else
> +		max_freq = pdata->frequency;

When using the DT, the driver needs to make sure pdata is valid before
dereferencing it.

> +	dev_dbg(i2c->dev, "desired frequency %u\n", max_freq);
>  
> +	target_frequency = max_freq ? max_freq : 100000;
>  	target_frequency /= 1000; /* Target frequency now in KHz */
>  
>  	freq = s3c24xx_i2c_calcdivisor(clkin, target_frequency, &div1, &divs);
> @@ -663,19 +704,24 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got)
>  	writel(iiccon, i2c->regs + S3C2410_IICCON);
>  
>  	if (s3c24xx_i2c_is2440(i2c)) {
> -		unsigned long sda_delay;
> +		unsigned int sda_delay = 0;

Are you sure that a 32bit integer will be able to hold the sda_delay
value?  Was there a reason it was an unsigned long in the first place?

>  
> -		if (pdata->sda_delay) {
> -			sda_delay = clkin * pdata->sda_delay;
> +		if (i2c->dev->of_node)
> +			of_property_read_u32(i2c->dev->of_node,
> +				"samsung,i2c-sda-delay", &sda_delay);

sda_delay must be a u32, not an unsigned int.

> +		else
> +			sda_delay = pdata->sda_delay;
> +
> +		if (sda_delay) {
> +			sda_delay *= clkin;
>  			sda_delay = DIV_ROUND_UP(sda_delay, 1000000);
>  			sda_delay = DIV_ROUND_UP(sda_delay, 5);
>  			if (sda_delay > 3)
>  				sda_delay = 3;
>  			sda_delay |= S3C2410_IICLC_FILTER_ON;
> -		} else
> -			sda_delay = 0;
> +		}
>  
> -		dev_dbg(i2c->dev, "IICLC=%08lx\n", sda_delay);
> +		dev_dbg(i2c->dev, "IICLC=%08x\n", sda_delay);
>  		writel(sda_delay, i2c->regs + S3C2440_IICLC);
>  	}
>  
> @@ -751,7 +797,7 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
>  {
>  	unsigned long iicon = S3C2410_IICCON_IRQEN | S3C2410_IICCON_ACKEN;
>  	struct s3c2410_platform_i2c *pdata;
> -	unsigned int freq;
> +	unsigned int freq, slave_addr = 0x10;
>  
>  	/* get the plafrom data */
>  
> @@ -763,10 +809,14 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
>  		pdata->cfg_gpio(to_platform_device(i2c->dev));
>  
>  	/* write slave address */
> +	if (i2c->dev->of_node)
> +		of_property_read_u32(i2c->dev->of_node,
> +				"samsung,i2c-slave-addr", &slave_addr);
> +	else
> +		slave_addr = pdata->slave_addr;
> +	writeb(slave_addr, i2c->regs + S3C2410_IICADD);
>  
> -	writeb(pdata->slave_addr, i2c->regs + S3C2410_IICADD);
> -
> -	dev_info(i2c->dev, "slave address 0x%02x\n", pdata->slave_addr);
> +	dev_info(i2c->dev, "slave address 0x%02x\n", slave_addr);
>  
>  	writel(iicon, i2c->regs + S3C2410_IICCON);
>  
> @@ -904,7 +954,12 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  	 * being bus 0.
>  	 */
>  
> -	i2c->adap.nr = pdata->bus_num;
> +	if (i2c->dev->of_node)
> +		of_property_read_u32(i2c->dev->of_node,
> +				"samsung,i2c-bus-number", &i2c->adap.nr);
> +	else
> +		i2c->adap.nr = pdata->bus_num;

Overall, this patch ends up being quite invasive.  You should refactor
to a separate function just for decoding device tree data, and
using it to populate a platform_data structure.  Then the patch
doesn't need to touch most of the existing platform_data code, and in
fact makes use of it.

One warning though, be careful to *not* modify the existing
pdev->dev.platform_data pointer.  If the driver needs to access
platform_data after probe() returns, then it will need to keep a copy
of it in the driver private data structure.

g.

> +	i2c->adap.dev.of_node = pdev->dev.of_node;
>  
>  	ret = i2c_add_numbered_adapter(&i2c->adap);
>  	if (ret < 0) {
> @@ -912,6 +967,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  		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));
> @@ -1021,6 +1077,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.7.1
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[Grant Likely]

On Mon, Jul 18, 2011 at 06:20:41AM +0530, Thomas Abraham wrote:
> Add device node for i2c instance 1 and list all its connected slave
> devices.
> 
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
>  arch/arm/mach-exynos4/Kconfig           |    1 +
>  arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
>  3 files changed, 28 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
> index d65c18c..29c40ed 100644
> --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
> +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
> @@ -23,7 +23,7 @@
>  	};
>  
>  	chosen {
> -		bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
> +		bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
>  	};
>  
>  	soc {
> @@ -64,5 +64,22 @@
>  			samsung,sdhci-cd-type = <0>;
>  			samsung,sdhci-clkdiv-external;
>  		};
> +
> +		i2c@13870000 {
> +			compatible = "samsung,s3c2440-i2c";
> +			reg = <0x13870000 0x100>;
> +			interrupts = <345>;
> +			samsung,i2c-bus-number = <1>;
> +			samsung,i2c-slave-addr = <16>;
> +			samsung,i2c-sda-delay = <100>;
> +			samsung,i2c-max-bus-freq = <100000>;
> +			#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..c7fce3e 100644
> --- a/arch/arm/mach-exynos4/Kconfig
> +++ b/arch/arm/mach-exynos4/Kconfig
> @@ -193,6 +193,7 @@ config MACH_EXYNOS4_DT
>  	select S3C_DEV_HSMMC
>  	select S3C_DEV_HSMMC2
>  	select EXYNOS4_SETUP_SDHCI
> +	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..ef6b4cb 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>
>  
> @@ -62,6 +65,10 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
>  	},
>  };
>  
> +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
> @@ -75,6 +82,8 @@ 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(1),
> +				"s3c2440-i2c.1", &exynos4_dt_i2c_data1),

Should not need the platform_data here.  Add DT support to the GPIO
driver and decode the data from there (which should be easy).

g.

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[Ben Dooks]

On Sun, Jul 17, 2011 at 10:30:59PM -0600, Grant Likely wrote:
> On Mon, Jul 18, 2011 at 06:20:41AM +0530, Thomas Abraham wrote:
> > Add device node for i2c instance 1 and list all its connected slave
> > devices.
> > 
> > Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> > ---
> >  arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
> >  arch/arm/mach-exynos4/Kconfig           |    1 +
> >  arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
> >  3 files changed, 28 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
> > index d65c18c..29c40ed 100644
> > --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
> > +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
> > @@ -23,7 +23,7 @@
> >  	};
> >  
> >  	chosen {
> > -		bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
> > +		bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
> >  	};
> >  
> >  	soc {
> > @@ -64,5 +64,22 @@
> >  			samsung,sdhci-cd-type = <0>;
> >  			samsung,sdhci-clkdiv-external;
> >  		};
> > +
> > +		i2c@13870000 {
> > +			compatible = "samsung,s3c2440-i2c";
> > +			reg = <0x13870000 0x100>;
> > +			interrupts = <345>;
> > +			samsung,i2c-bus-number = <1>;
> > +			samsung,i2c-slave-addr = <16>;
> > +			samsung,i2c-sda-delay = <100>;
> > +			samsung,i2c-max-bus-freq = <100000>;
> > +			#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..c7fce3e 100644
> > --- a/arch/arm/mach-exynos4/Kconfig
> > +++ b/arch/arm/mach-exynos4/Kconfig
> > @@ -193,6 +193,7 @@ config MACH_EXYNOS4_DT
> >  	select S3C_DEV_HSMMC
> >  	select S3C_DEV_HSMMC2
> >  	select EXYNOS4_SETUP_SDHCI
> > +	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..ef6b4cb 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>
> >  
> > @@ -62,6 +65,10 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
> >  	},
> >  };
> >  
> > +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
> > @@ -75,6 +82,8 @@ 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(1),
> > +				"s3c2440-i2c.1", &exynos4_dt_i2c_data1),
> 
> Should not need the platform_data here.  Add DT support to the GPIO
> driver and decode the data from there (which should be easy).

Hmm, do we have any sane way of passing which configuration settings
should be applied to the gpio pins (given that there can be up to 13
non-IO settings for some of these pins).

Also, since this data is really part of the SoC, I don't really want
to see n different .dts files hanging around with the same information
in it.

Maybe it is time for a standardised callback to allow devices to connect
themselves to the pins they need, and have dt capability of over-riding
the information if it needs it.

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

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

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[G, Manjunath Kondaiah]

Abraham,
Few comments on i2c child devices handling:

On 18 July 2011 06:20, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> Add device node for i2c instance 1 and list all its connected slave
> devices.
>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> ---
>  arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
>  arch/arm/mach-exynos4/Kconfig           |    1 +
>  arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
>  3 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
> index d65c18c..29c40ed 100644
> --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
> +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
> @@ -23,7 +23,7 @@
>        };
>
>        chosen {
> -               bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
> +               bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
>        };
>
>        soc {
> @@ -64,5 +64,22 @@
>                        samsung,sdhci-cd-type = <0>;
>                        samsung,sdhci-clkdiv-external;
>                };
> +
> +               i2c@13870000 {
> +                       compatible = "samsung,s3c2440-i2c";
> +                       reg = <0x13870000 0x100>;
> +                       interrupts = <345>;
> +                       samsung,i2c-bus-number = <1>;
> +                       samsung,i2c-slave-addr = <16>;
> +                       samsung,i2c-sda-delay = <100>;
> +                       samsung,i2c-max-bus-freq = <100000>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       wm8994@1a {
> +                               compatible = "wlf,wm8994";
> +                               reg = <0x1a>;
> +                       };

The i2c child devices are board specific.
For ex: few boards such as smdkxxx based on exynos4 has wm8994 with
i2c1 controller
where as other boards based on exynos4 does not have any i2c1 devices.
Hence these child entries should go to board specific .dts file right?
There should be soc specific .dts file and board specific dts file
should include soc dts file.

-M

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[Grant Likely]

On Mon, Jul 18, 2011 at 3:45 AM, Ben Dooks <ben-i2c-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> On Sun, Jul 17, 2011 at 10:30:59PM -0600, Grant Likely wrote:
>> On Mon, Jul 18, 2011 at 06:20:41AM +0530, Thomas Abraham wrote:
>> > Add device node for i2c instance 1 and list all its connected slave
>> > devices.
>> >
>> > Signed-off-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>> >  arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
>> >  arch/arm/mach-exynos4/Kconfig           |    1 +
>> >  arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
>> >  3 files changed, 28 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
>> > index d65c18c..29c40ed 100644
>> > --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
>> > +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
>> > @@ -23,7 +23,7 @@
>> >     };
>> >
>> >     chosen {
>> > -           bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
>> > +           bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
>> >     };
>> >
>> >     soc {
>> > @@ -64,5 +64,22 @@
>> >                     samsung,sdhci-cd-type = <0>;
>> >                     samsung,sdhci-clkdiv-external;
>> >             };
>> > +
>> > +           i2c@13870000 {
>> > +                   compatible = "samsung,s3c2440-i2c";
>> > +                   reg = <0x13870000 0x100>;
>> > +                   interrupts = <345>;
>> > +                   samsung,i2c-bus-number = <1>;
>> > +                   samsung,i2c-slave-addr = <16>;
>> > +                   samsung,i2c-sda-delay = <100>;
>> > +                   samsung,i2c-max-bus-freq = <100000>;
>> > +                   #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..c7fce3e 100644
>> > --- a/arch/arm/mach-exynos4/Kconfig
>> > +++ b/arch/arm/mach-exynos4/Kconfig
>> > @@ -193,6 +193,7 @@ config MACH_EXYNOS4_DT
>> >     select S3C_DEV_HSMMC
>> >     select S3C_DEV_HSMMC2
>> >     select EXYNOS4_SETUP_SDHCI
>> > +   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..ef6b4cb 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>
>> >
>> > @@ -62,6 +65,10 @@ static struct s3c2410_uartcfg smdkv310_uartcfgs[] __initdata = {
>> >     },
>> >  };
>> >
>> > +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
>> > @@ -75,6 +82,8 @@ 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(1),
>> > +                           "s3c2440-i2c.1", &exynos4_dt_i2c_data1),
>>
>> Should not need the platform_data here.  Add DT support to the GPIO
>> driver and decode the data from there (which should be easy).
>
> Hmm, do we have any sane way of passing which configuration settings
> should be applied to the gpio pins (given that there can be up to 13
> non-IO settings for some of these pins).

I've been looking at having an IO routing configuration node that sets
up the SoC for a particular board.  This works for static
configuration, which seems to be 99% of the use cases, but it doesn't
help much for IO configuration that needs to be manipulated at
runtime.

> Also, since this data is really part of the SoC, I don't really want
> to see n different .dts files hanging around with the same information
> in it.

Yes, the SoC details should be in a .dtsi (dts include file) file that
is included by the board file.  Properties in the board .dts file will
overlay and override data in the SoC file.

> Maybe it is time for a standardised callback to allow devices to connect
> themselves to the pins they need, and have dt capability of over-riding
> the information if it needs it.

Linus W. has been working on the pinmux subsystem to handle doing
that.  Personally, I'm not excited about device drivers managing their
own pin configuration just because the interactions and requirements
between device can be very complex.  I'd rather have it handled in a
single place for the whole system, but I'm not the one concentrating
on that problems space.

g.

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[Grant Likely]

On Mon, Jul 18, 2011 at 5:53 AM, G, Manjunath Kondaiah
<manjugk@linaro.org> wrote:
> Abraham,
> Few comments on i2c child devices handling:
>
> On 18 July 2011 06:20, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>> Add device node for i2c instance 1 and list all its connected slave
>> devices.
>>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> ---
>>  arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
>>  arch/arm/mach-exynos4/Kconfig           |    1 +
>>  arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
>>  3 files changed, 28 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
>> index d65c18c..29c40ed 100644
>> --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
>> +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
>> @@ -23,7 +23,7 @@
>>        };
>>
>>        chosen {
>> -               bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
>> +               bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
>>        };
>>
>>        soc {
>> @@ -64,5 +64,22 @@
>>                        samsung,sdhci-cd-type = <0>;
>>                        samsung,sdhci-clkdiv-external;
>>                };
>> +
>> +               i2c@13870000 {
>> +                       compatible = "samsung,s3c2440-i2c";
>> +                       reg = <0x13870000 0x100>;
>> +                       interrupts = <345>;
>> +                       samsung,i2c-bus-number = <1>;
>> +                       samsung,i2c-slave-addr = <16>;
>> +                       samsung,i2c-sda-delay = <100>;
>> +                       samsung,i2c-max-bus-freq = <100000>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +
>> +                       wm8994@1a {
>> +                               compatible = "wlf,wm8994";
>> +                               reg = <0x1a>;
>> +                       };
>
> The i2c child devices are board specific.
> For ex: few boards such as smdkxxx based on exynos4 has wm8994 with
> i2c1 controller
> where as other boards based on exynos4 does not have any i2c1 devices.
> Hence these child entries should go to board specific .dts file right?

No, the i2c bus node should still appear in the SoC .dtsi file.  If a
board doesn't use a particular i2c bus, then the board.dts file can
add a status = "disabled"; property to the i2c bus node to disable it.

g.

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[G, Manjunath Kondaiah]

On Tue, Jul 19, 2011 at 2:00 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Jul 18, 2011 at 5:53 AM, G, Manjunath Kondaiah
> <manjugk@linaro.org> wrote:
>> Abraham,
>> Few comments on i2c child devices handling:
>>
>> On 18 July 2011 06:20, Thomas Abraham <thomas.abraham@linaro.org> wrote:
>>> Add device node for i2c instance 1 and list all its connected slave
>>> devices.
>>>
>>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>>> ---
>>>  arch/arm/boot/dts/exynos4-smdkv310.dts  |   19 ++++++++++++++++++-
>>>  arch/arm/mach-exynos4/Kconfig           |    1 +
>>>  arch/arm/mach-exynos4/mach-exynos4-dt.c |    9 +++++++++
>>>  3 files changed, 28 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos4-smdkv310.dts b/arch/arm/boot/dts/exynos4-smdkv310.dts
>>> index d65c18c..29c40ed 100644
>>> --- a/arch/arm/boot/dts/exynos4-smdkv310.dts
>>> +++ b/arch/arm/boot/dts/exynos4-smdkv310.dts
>>> @@ -23,7 +23,7 @@
>>>        };
>>>
>>>        chosen {
>>> -               bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200";
>>> +               bootargs = "root=/dev/mmcblk0p1 rootfstype=ext3 rootwait console=ttySAC1,115200 init=/linuxrc";
>>>        };
>>>
>>>        soc {
>>> @@ -64,5 +64,22 @@
>>>                        samsung,sdhci-cd-type = <0>;
>>>                        samsung,sdhci-clkdiv-external;
>>>                };
>>> +
>>> +               i2c@13870000 {
>>> +                       compatible = "samsung,s3c2440-i2c";
>>> +                       reg = <0x13870000 0x100>;
>>> +                       interrupts = <345>;
>>> +                       samsung,i2c-bus-number = <1>;
>>> +                       samsung,i2c-slave-addr = <16>;
>>> +                       samsung,i2c-sda-delay = <100>;
>>> +                       samsung,i2c-max-bus-freq = <100000>;
>>> +                       #address-cells = <1>;
>>> +                       #size-cells = <0>;
>>> +
>>> +                       wm8994@1a {
>>> +                               compatible = "wlf,wm8994";
>>> +                               reg = <0x1a>;
>>> +                       };
>>
>> The i2c child devices are board specific.
>> For ex: few boards such as smdkxxx based on exynos4 has wm8994 with
>> i2c1 controller
>> where as other boards based on exynos4 does not have any i2c1 devices.
>> Hence these child entries should go to board specific .dts file right?
>
> No, the i2c bus node should still appear in the SoC .dtsi file.  If a
> board doesn't use a particular i2c bus, then the board.dts file can
> add a status = "disabled"; property to the i2c bus node to disable it.

that's right. I am referring to I2C slave devices. For a given SoC,
two different
boards can have different i2c slave devices with different slave
address. In that case,
it is better to have i2c child/slave nodes in board .dts file.

-M

Re: [PATCH 2/2] arm: dt: Add device tree support for i2c instance 1 on exynos4 dt machine

[Tony Lindgren]

* G, Manjunath Kondaiah <manjugk@ti.com> [110719 09:59]:
> On Tue, Jul 19, 2011 at 2:00 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >
> > No, the i2c bus node should still appear in the SoC .dtsi file.  If a
> > board doesn't use a particular i2c bus, then the board.dts file can
> > add a status = "disabled"; property to the i2c bus node to disable it.
> 
> that's right. I am referring to I2C slave devices. For a given SoC,
> two different
> boards can have different i2c slave devices with different slave
> address. In that case,
> it is better to have i2c child/slave nodes in board .dts file.

This is OK for now, but from bus/hwmod point of view we need to reset all
the unused devices as otherwise PM will never work properly.

Disabling each device in the board .dts file will never be done correctly
for most boards.. And on each board, probably 2/3 of the devices are unused
because of the pin limitations :)

So the disabling of unused devices should be done with a late_initcall and
controlled by kernel cmdline. We were thinking hwmod.disable=[0|1].

The late_initcall to disable unused device tree devices could also be
added later on to device tree core code.

Regards,

Tony