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

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

[Thomas Abraham]

This patchset adds device tree support for Samsung's I2C driver.

Changes since v2:
- Addressed all comments from Grant Likely
    - Used dev_kzalloc to allocate memory for platform data.
    - Removed redundant check while adding i2c adapter.
- Added support to retrieve gpio pin information from dt and request
  gpio pins (retained Grant's Ack even though this is a new addition
  in this version of the patch).

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
      to make the dt support code non-invasive.
    - 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 (2):
  i2c: s3c2410: Keep a copy of platform data and use it
  i2c: s3c2410: Add device tree support

 .../devicetree/bindings/i2c/samsung-i2c.txt        |   39 ++++++
 drivers/i2c/busses/i2c-s3c2410.c                   |  128 ++++++++++++++++++--
 2 files changed, 159 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/samsung-i2c.txt

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

Cc: Ben Dooks <ben-linux@fluff.org>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/i2c/busses/i2c-s3c2410.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index f84a63c..266dd83 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
@@ -625,7 +626,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;
@@ -754,7 +755,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 */
 
@@ -793,7 +794,7 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
 static int s3c24xx_i2c_probe(struct platform_device *pdev)
 {
 	struct s3c24xx_i2c *i2c;
-	struct s3c2410_platform_i2c *pdata;
+	struct s3c2410_platform_i2c *pdata = NULL;
 	struct resource *res;
 	int ret;
 
@@ -809,6 +810,15 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	i2c->pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!i2c->pdata) {
+		ret = -ENOMEM;
+		goto err_noclk;
+	}
+
+	if (pdata)
+		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;
@@ -903,7 +913,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	 * being bus 0.
 	 */
 
-	i2c->adap.nr = pdata->bus_num;
+	i2c->adap.nr = i2c->pdata->bus_num;
 
 	ret = i2c_add_numbered_adapter(&i2c->adap);
 	if (ret < 0) {
-- 
1.6.6.rc2

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

[Thomas Abraham]

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

Cc: Ben Dooks <ben-linux@fluff.org>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
 .../devicetree/bindings/i2c/samsung-i2c.txt        |   39 +++++++
 drivers/i2c/busses/i2c-s3c2410.c                   |  110 +++++++++++++++++++-
 2 files changed, 145 insertions(+), 4 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..d04186d
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/samsung-i2c.txt
@@ -0,0 +1,39 @@
+* 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 the following: <SDA, SCL>.
+    The gpio specifier depends on the gpio controller.
+
+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 266dd83..aaeabf9 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -35,6 +35,8 @@
 #include <linux/cpufreq.h>
 #include <linux/slab.h>
 #include <linux/io.h>
+#include <linux/of_i2c.h>
+#include <linux/of_gpio.h>
 
 #include <asm/irq.h>
 
@@ -79,6 +81,7 @@ struct s3c24xx_i2c {
 	struct i2c_adapter	adap;
 
 	struct s3c2410_platform_i2c	*pdata;
+	int			gpios[2];
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 #endif
@@ -96,6 +99,12 @@ static inline int s3c24xx_i2c_is2440(struct s3c24xx_i2c *i2c)
 	struct platform_device *pdev = to_platform_device(i2c->dev);
 	enum s3c24xx_i2c_type type;
 
+#ifdef CONFIG_OF
+	if (i2c->dev->of_node)
+		return of_device_is_compatible(i2c->dev->of_node,
+				"samsung,s3c2440-i2c");
+#endif
+
 	type = platform_get_device_id(pdev)->driver_data;
 	return type == TYPE_S3C2440;
 }
@@ -742,6 +751,49 @@ static inline void s3c24xx_i2c_deregister_cpufreq(struct s3c24xx_i2c *i2c)
 }
 #endif
 
+#ifdef CONFIG_OF
+static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
+{
+	int idx, gpio, ret;
+
+	for (idx = 0; idx < 2; idx++) {
+		gpio = of_get_gpio(i2c->dev->of_node, idx);
+		if (!gpio_is_valid(gpio)) {
+			dev_err(i2c->dev, "invalid gpio[%d]: %d\n", idx, gpio);
+			goto free_gpio;
+		}
+
+		ret = gpio_request(gpio, "i2c-bus");
+		if (ret) {
+			dev_err(i2c->dev, "gpio [%d] request failed\n", gpio);
+			goto free_gpio;
+		}
+	}
+	return 0;
+
+free_gpio:
+	while (--idx >= 0)
+		gpio_free(i2c->gpios[idx]);
+	return -EINVAL;
+}
+
+static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
+{
+	unsigned int idx;
+	for (idx = 0; idx < 2; idx++)
+		gpio_free(i2c->gpios[idx]);
+}
+#else
+static int s3c24xx_i2c_parse_dt_gpio(struct s3c24xx_i2c *i2c)
+{
+	return -EINVAL;
+}
+
+static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c)
+{
+}
+#endif
+
 /* s3c24xx_i2c_init
  *
  * initialise the controller, set the IO lines and frequency
@@ -761,6 +813,9 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c)
 
 	if (pdata->cfg_gpio)
 		pdata->cfg_gpio(to_platform_device(i2c->dev));
+	else
+		if (s3c24xx_i2c_parse_dt_gpio(i2c))
+			return -EINVAL;
 
 	/* write slave address */
 
@@ -786,6 +841,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;
+
+	pdata->bus_num = -1; /* i2c bus number is dynamically assigned */
+	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
@@ -798,10 +881,12 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -EINVAL;
+	if (!pdev->dev.of_node) {
+		pdata = pdev->dev.platform_data;
+		if (!pdata) {
+			dev_err(&pdev->dev, "no platform data\n");
+			return -EINVAL;
+		}
 	}
 
 	i2c = kzalloc(sizeof(struct s3c24xx_i2c), GFP_KERNEL);
@@ -818,6 +903,8 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 
 	if (pdata)
 		memcpy(i2c->pdata, pdata, sizeof(*pdata));
+	else
+		s3c24xx_i2c_parse_dt(pdev->dev.of_node, i2c);
 
 	strlcpy(i2c->adap.name, "s3c2410-i2c", sizeof(i2c->adap.name));
 	i2c->adap.owner   = THIS_MODULE;
@@ -914,6 +1001,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 	 */
 
 	i2c->adap.nr = i2c->pdata->bus_num;
+	i2c->adap.dev.of_node = pdev->dev.of_node;
 
 	ret = i2c_add_numbered_adapter(&i2c->adap);
 	if (ret < 0) {
@@ -921,6 +1009,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));
@@ -969,6 +1058,7 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
 	iounmap(i2c->regs);
 
 	release_resource(i2c->ioarea);
+	s3c24xx_i2c_dt_gpio_free(i2c);
 	kfree(i2c->ioarea);
 	kfree(i2c);
 
@@ -1022,6 +1112,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,
@@ -1030,6 +1131,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

Re: [PATCH v3 1/2] i2c: s3c2410: Keep a copy of platform data and use it

[Stephen Boyd]

On 9/12/2011 9:16 PM, Thomas Abraham wrote:
> @@ -809,6 +810,15 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	i2c->pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!i2c->pdata) {
> +		ret = -ENOMEM;
> +		goto err_noclk;
> +	}
> +
> +	if (pdata)
> +		memcpy(i2c->pdata, pdata, sizeof(*pdata));
> +

Is there a devm_kmemdup()? If not, maybe it would be a good idea to add it.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

[Ben Dooks]

On Tue, Sep 13, 2011 at 09:46:03AM +0530, Thomas Abraham wrote:
> This patchset adds device tree support for Samsung's I2C driver.

I've applied these after a brief review. I'll give them a better
review before the weekend.

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

[Thomas Abraham]

Hi Ben,

On 15 September 2011 02:43, Ben Dooks <ben-i2c-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> On Tue, Sep 13, 2011 at 09:46:03AM +0530, Thomas Abraham wrote:
>> This patchset adds device tree support for Samsung's I2C driver.
>
> I've applied these after a brief review. I'll give them a better
> review before the weekend.

Thanks for your review.

Regards,
Thomas.