diff mbox

[v6] ARM: davinci: i2c: add OF support

Message ID 1342514447-30810-1-git-send-email-hs@denx.de (mailing list archive)
State Awaiting Upstream
Headers show

Commit Message

Heiko Schocher July 17, 2012, 8:40 a.m. UTC
add of support for the davinci i2c driver.

Signed-off-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-i2c@vger.kernel.org
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>

---
- changes for v2:
- add comments from Sylwester Nawrocki <s.nawrocki@samsung.com>:
  - use "cell-index" instead "id"
  - OF_DEV_AUXDATA in the machine code, instead pre-define platform
    device name
- add comment from Grant Likely:
  - removed "id" resp. "cell-index" completely
  - fixed documentation
  - use of_match_ptr()
  - use devm_kzalloc() for allocating plattform data mem
  - fixed a whitespace issue
- no changes for v3
- changes for v4
  remove "pinmux-handle" property as discussed here:
  http://www.spinics.net/lists/arm-kernel/msg175701.html
  with Nori Sekhar

- changes for v5
  add comments from Grant Likely:
  - do not change value of dev->dev->platform_data, instead
    hold a copy in davinci_i2c_dev.

- changes for v6:
  add comments from Sekhar Nori:
  - removed unneccessary include
  - merge patch from Sekhar Nori:
    setup the newly introduced dev->pdata member correctly once in
    probe -> i2c_get_plattformdata(() not needed, remove a lot of
    checks for pdata in code
  - add Signed-off-by: Sekhar Nori <nsekhar@ti.com>
  - patch no longer in patchserie, as it has no dependencies.
---
 .../devicetree/bindings/arm/davinci/i2c.txt        |   31 ++++++++++++
 drivers/i2c/busses/i2c-davinci.c                   |   50 +++++++++++++++----
 2 files changed, 70 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/davinci/i2c.txt

Comments

Wolfram Sang July 20, 2012, 10:52 a.m. UTC | #1
Hi,

On Tue, Jul 17, 2012 at 10:40:47AM +0200, Heiko Schocher wrote:
> add of support for the davinci i2c driver.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-i2c@vger.kernel.org
> Cc: Ben Dooks <ben-linux@fluff.org>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> 
> ---
> - changes for v2:
> - add comments from Sylwester Nawrocki <s.nawrocki@samsung.com>:
>   - use "cell-index" instead "id"
>   - OF_DEV_AUXDATA in the machine code, instead pre-define platform
>     device name
> - add comment from Grant Likely:
>   - removed "id" resp. "cell-index" completely
>   - fixed documentation
>   - use of_match_ptr()
>   - use devm_kzalloc() for allocating plattform data mem
>   - fixed a whitespace issue
> - no changes for v3
> - changes for v4
>   remove "pinmux-handle" property as discussed here:
>   http://www.spinics.net/lists/arm-kernel/msg175701.html
>   with Nori Sekhar
> 
> - changes for v5
>   add comments from Grant Likely:
>   - do not change value of dev->dev->platform_data, instead
>     hold a copy in davinci_i2c_dev.
> 
> - changes for v6:
>   add comments from Sekhar Nori:
>   - removed unneccessary include
>   - merge patch from Sekhar Nori:
>     setup the newly introduced dev->pdata member correctly once in
>     probe -> i2c_get_plattformdata(() not needed, remove a lot of
>     checks for pdata in code
>   - add Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>   - patch no longer in patchserie, as it has no dependencies.
> ---
>  .../devicetree/bindings/arm/davinci/i2c.txt        |   31 ++++++++++++
>  drivers/i2c/busses/i2c-davinci.c                   |   50 +++++++++++++++----
>  2 files changed, 70 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/davinci/i2c.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/davinci/i2c.txt b/Documentation/devicetree/bindings/arm/davinci/i2c.txt
> new file mode 100644
> index 0000000..e98a025
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/davinci/i2c.txt

This should go bindings/i2c/...

> @@ -0,0 +1,31 @@
> +* Texas Instruments Davinci I2C
> +
> +This file provides information, what the device node for the
> +davinci i2c interface contain.
> +
> +Required properties:
> +- compatible: "ti,davinci-i2c";
> +- reg : Offset and length of the register set for the device
> +
> +Recommended properties :
> +- interrupts : <a> standard interrupt property.
> +- clock-frequency : desired I2C bus clock frequency in Hz.
> +
> +Optional properties:
> +- bus-delay: bus delay in usec

See my mail to Andrew regarding the timeout property in the mv64xxx
driver. I'd like to skip the binding discussion for now in order to make
it into 3.6. Is it okay for you to drop this binding and use a sane
default?

Rest looks good (from a visual review),

   Wolfram
Heiko Schocher July 30, 2012, 7:02 a.m. UTC | #2
Hello Wolfram,

On 20.07.2012 12:52, Wolfram Sang wrote:
> Hi,
>
> On Tue, Jul 17, 2012 at 10:40:47AM +0200, Heiko Schocher wrote:
>> add of support for the davinci i2c driver.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> Signed-off-by: Sekhar Nori<nsekhar@ti.com>
>> Cc: davinci-linux-open-source@linux.davincidsp.com
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: devicetree-discuss@lists.ozlabs.org
>> Cc: linux-i2c@vger.kernel.org
>> Cc: Ben Dooks<ben-linux@fluff.org>
>> Cc: Wolfram Sang<w.sang@pengutronix.de>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: Sekhar Nori<nsekhar@ti.com>
>> Cc: Wolfgang Denk<wd@denx.de>
>> Cc: Sylwester Nawrocki<s.nawrocki@samsung.com>
>>
>> ---
>> - changes for v2:
>> - add comments from Sylwester Nawrocki<s.nawrocki@samsung.com>:
>>    - use "cell-index" instead "id"
>>    - OF_DEV_AUXDATA in the machine code, instead pre-define platform
>>      device name
>> - add comment from Grant Likely:
>>    - removed "id" resp. "cell-index" completely
>>    - fixed documentation
>>    - use of_match_ptr()
>>    - use devm_kzalloc() for allocating plattform data mem
>>    - fixed a whitespace issue
>> - no changes for v3
>> - changes for v4
>>    remove "pinmux-handle" property as discussed here:
>>    http://www.spinics.net/lists/arm-kernel/msg175701.html
>>    with Nori Sekhar
>>
>> - changes for v5
>>    add comments from Grant Likely:
>>    - do not change value of dev->dev->platform_data, instead
>>      hold a copy in davinci_i2c_dev.
>>
>> - changes for v6:
>>    add comments from Sekhar Nori:
>>    - removed unneccessary include
>>    - merge patch from Sekhar Nori:
>>      setup the newly introduced dev->pdata member correctly once in
>>      probe ->  i2c_get_plattformdata(() not needed, remove a lot of
>>      checks for pdata in code
>>    - add Signed-off-by: Sekhar Nori<nsekhar@ti.com>
>>    - patch no longer in patchserie, as it has no dependencies.
>> ---
>>   .../devicetree/bindings/arm/davinci/i2c.txt        |   31 ++++++++++++
>>   drivers/i2c/busses/i2c-davinci.c                   |   50 +++++++++++++++----
>>   2 files changed, 70 insertions(+), 11 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/arm/davinci/i2c.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/davinci/i2c.txt b/Documentation/devicetree/bindings/arm/davinci/i2c.txt
>> new file mode 100644
>> index 0000000..e98a025
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/davinci/i2c.txt
>
> This should go bindings/i2c/...

Ok, done.

>> @@ -0,0 +1,31 @@
>> +* Texas Instruments Davinci I2C
>> +
>> +This file provides information, what the device node for the
>> +davinci i2c interface contain.
>> +
>> +Required properties:
>> +- compatible: "ti,davinci-i2c";
>> +- reg : Offset and length of the register set for the device
>> +
>> +Recommended properties :
>> +- interrupts :<a>  standard interrupt property.
>> +- clock-frequency : desired I2C bus clock frequency in Hz.
>> +
>> +Optional properties:
>> +- bus-delay: bus delay in usec
>
> See my mail to Andrew regarding the timeout property in the mv64xxx
> driver. I'd like to skip the binding discussion for now in order to make
> it into 3.6. Is it okay for you to drop this binding and use a sane
> default?

Hmm.. current none of code uses 0 as default. This works for the
enbw_cmc board. So I use 0 as default, and remove this property.

>
> Rest looks good (from a visual review),

Thanks for the review.

bye,
Heiko
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/arm/davinci/i2c.txt b/Documentation/devicetree/bindings/arm/davinci/i2c.txt
new file mode 100644
index 0000000..e98a025
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/davinci/i2c.txt
@@ -0,0 +1,31 @@ 
+* Texas Instruments Davinci I2C
+
+This file provides information, what the device node for the
+davinci i2c interface contain.
+
+Required properties:
+- compatible: "ti,davinci-i2c";
+- reg : Offset and length of the register set for the device
+
+Recommended properties :
+- interrupts : <a> standard interrupt property.
+- clock-frequency : desired I2C bus clock frequency in Hz.
+
+Optional properties:
+- bus-delay: bus delay in usec
+
+Example (enbw_cmc board):
+	i2c@1c22000 {
+		compatible = "ti,davinci-i2c";
+		reg = <0x22000 0x1000>;
+		clock-frequency = <100000>;
+		interrupts = <15>;
+		interrupt-parent = <&intc>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		dtt@48 {
+				compatible = "national,lm75";
+				reg = <0x48>;
+			};
+	};
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index a76d85f..8d4efce 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -38,6 +38,8 @@ 
 #include <linux/slab.h>
 #include <linux/cpufreq.h>
 #include <linux/gpio.h>
+#include <linux/of_i2c.h>
+#include <linux/of_device.h>
 
 #include <mach/hardware.h>
 #include <mach/i2c.h>
@@ -114,6 +116,7 @@  struct davinci_i2c_dev {
 	struct completion	xfr_complete;
 	struct notifier_block	freq_transition;
 #endif
+	struct davinci_i2c_platform_data *pdata;
 };
 
 /* default platform data to use if not supplied in the platform_device */
@@ -155,7 +158,7 @@  static void generic_i2c_clock_pulse(unsigned int scl_pin)
 static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 {
 	u32 flag = 0;
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
+	struct davinci_i2c_platform_data *pdata = dev->pdata;
 
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
 	/* Send NACK to the slave */
@@ -163,8 +166,7 @@  static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	flag |=  DAVINCI_I2C_MDR_NACK;
 	/* write the data into mode register */
 	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
-	if (pdata)
-		generic_i2c_clock_pulse(pdata->scl_pin);
+	generic_i2c_clock_pulse(pdata->scl_pin);
 	/* Send STOP */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
 	flag |= DAVINCI_I2C_MDR_STP;
@@ -187,7 +189,7 @@  static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
 
 static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
 {
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
+	struct davinci_i2c_platform_data *pdata = dev->pdata;
 	u16 psc;
 	u32 clk;
 	u32 d;
@@ -235,10 +237,7 @@  static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
  */
 static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 {
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
-
-	if (!pdata)
-		pdata = &davinci_i2c_platform_data_default;
+	struct davinci_i2c_platform_data *pdata = dev->pdata;
 
 	/* put I2C into reset */
 	davinci_i2c_reset_ctrl(dev, 0);
@@ -308,13 +307,11 @@  static int
 i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 {
 	struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
+	struct davinci_i2c_platform_data *pdata = dev->pdata;
 	u32 flag;
 	u16 w;
 	int r;
 
-	if (!pdata)
-		pdata = &davinci_i2c_platform_data_default;
 	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
 	if (pdata->bus_delay)
 		udelay(pdata->bus_delay);
@@ -635,6 +632,12 @@  static struct i2c_algorithm i2c_davinci_algo = {
 	.functionality	= i2c_davinci_func,
 };
 
+static const struct of_device_id davinci_i2c_of_match[] = {
+	{.compatible = "ti,davinci-i2c", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, davinci_i2c_of_match);
+
 static int davinci_i2c_probe(struct platform_device *pdev)
 {
 	struct davinci_i2c_dev *dev;
@@ -674,8 +677,30 @@  static int davinci_i2c_probe(struct platform_device *pdev)
 #endif
 	dev->dev = get_device(&pdev->dev);
 	dev->irq = irq->start;
+	dev->pdata = dev->dev->platform_data;
 	platform_set_drvdata(pdev, dev);
 
+	if (!dev->pdata && pdev->dev.of_node) {
+		u32 prop;
+
+		dev->pdata = devm_kzalloc(&pdev->dev,
+			sizeof(struct davinci_i2c_platform_data), GFP_KERNEL);
+		if (!dev->pdata) {
+			r = -ENOMEM;
+			goto err_free_mem;
+		}
+		memcpy(dev->pdata, &davinci_i2c_platform_data_default,
+			sizeof(struct davinci_i2c_platform_data));
+		if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",
+			&prop))
+			dev->pdata->bus_freq = prop / 1000;
+		if (!of_property_read_u32(pdev->dev.of_node, "bus-delay",
+			&prop))
+			dev->pdata->bus_delay = prop;
+	} else if (!dev->pdata) {
+		dev->pdata = &davinci_i2c_platform_data_default;
+	}
+
 	dev->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		r = -ENODEV;
@@ -711,6 +736,7 @@  static int davinci_i2c_probe(struct platform_device *pdev)
 	adap->algo = &i2c_davinci_algo;
 	adap->dev.parent = &pdev->dev;
 	adap->timeout = DAVINCI_I2C_TIMEOUT;
+	adap->dev.of_node = pdev->dev.of_node;
 
 	adap->nr = pdev->id;
 	r = i2c_add_numbered_adapter(adap);
@@ -718,6 +744,7 @@  static int davinci_i2c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failure adding adapter\n");
 		goto err_free_irq;
 	}
+	of_i2c_register_devices(adap);
 
 	return 0;
 
@@ -809,6 +836,7 @@  static struct platform_driver davinci_i2c_driver = {
 		.name	= "i2c_davinci",
 		.owner	= THIS_MODULE,
 		.pm	= davinci_i2c_pm_ops,
+		.of_match_table = of_match_ptr(davinci_i2c_of_match),
 	},
 };