[PATCH 0/2] lis3: lis3lv02d_i2c: Add device tree support

First patch adds device tree support to lis3lv02d_i2c driver and
second patch adds platform data for lis331dlh driver to am335x
EVM. These patches were tested on AM335x-EVM.

AnilKumar Ch (2):
  lis3: lis3lv02d_i2c: Add device tree support
  ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm

 arch/arm/boot/dts/am335x-evm.dts       |   42 ++++++++++++++++++++++++++++++++
 drivers/misc/lis3lv02d/lis3lv02d_i2c.c |   13 ++++++++++
 2 files changed, 55 insertions(+)

-- 
1.7.9.5

Add device tree matching table support to lis3lv02d_i2c driver. If
the driver data is passed from device tree, then this driver picks
up platform data from device node through common/generic lis3lv02d.c
driver.

Signed-off-by: AnilKumar Ch <redacted>
---
 drivers/misc/lis3lv02d/lis3lv02d_i2c.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/misc/lis3lv02d/lis3lv02d_i2c.c b/drivers/misc/lis3lv02d/lis3lv02d_i2c.c
index 15255eb..e851ee8 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d_i2c.c
+++ b/drivers/misc/lis3lv02d/lis3lv02d_i2c.c

@@ -31,6 +31,9 @@
 #include <linux/i2c.h>
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+
 #include "lis3lv02d.h"
 
 #define DRV_NAME	"lis3lv02d_i2c"

@@ -102,12 +105,21 @@ static int lis3_i2c_init(struct lis3lv02d *lis3)
 static union axis_conversion lis3lv02d_axis_map =
 	{ .as_array = { LIS3_DEV_X, LIS3_DEV_Y, LIS3_DEV_Z } };
 
+static struct of_device_id lis3lv02d_i2c_dt_ids[] = {
+	{ .compatible = "st,lis3lv02d-i2c" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, lis3lv02d_i2c_dt_ids);
+
 static int __devinit lis3lv02d_i2c_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
 	int ret = 0;
 	struct lis3lv02d_platform_data *pdata = client->dev.platform_data;
 
+	if (of_match_device(lis3lv02d_i2c_dt_ids, &client->dev))
+		lis3_dev.of_node = client->dev.of_node;
+
 	if (pdata) {
 		if ((pdata->driver_features & LIS3_USE_BLOCK_READ) &&
 			(i2c_check_functionality(client->adapter,

@@ -255,6 +267,7 @@ static struct i2c_driver lis3lv02d_i2c_driver = {
 		.name   = DRV_NAME,
 		.owner  = THIS_MODULE,
 		.pm     = &lis3_pm_ops,
+		.of_match_table = of_match_ptr(lis3lv02d_i2c_dt_ids),
 	},
 	.probe	= lis3lv02d_i2c_probe,
 	.remove	= __devexit_p(lis3lv02d_i2c_remove),

-- 
1.7.9.5

Add lis331dlh device tree data to am335x-evm.dts. In AM335x EVM
lis331dlh accelerometer is connected to I2C2 bus. So this patch
change the status to "okay" to use I2C2 bus. Also added all the
required platform data to am335x-evm.

Signed-off-by: AnilKumar Ch <redacted>
---
 arch/arm/boot/dts/am335x-evm.dts |   42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 9fb59c5..9e5a878 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts

@@ -47,6 +47,15 @@
 			};
 		};
 
+		i2c2: i2c at 4802a000 {
+			status = "okay";
+			clock-frequency = <400000>;
+
+			lis331dlh: lis331dlh at 18 {
+				reg = <0x18>;
+			};
+		};
+
 		dcan1: d_can at 481d0000 {
 			status = "okay";
 			pinctrl-names = "default";

@@ -61,6 +70,39 @@
 		regulator-max-microvolt = <5000000>;
 		regulator-boot-on;
 	};
+
+	lis3_reg: fixedregulator at 1 {
+		compatible = "regulator-fixed";
+		regulator-name = "lis3_reg";
+		regulator-boot-on;
+	};
+};
+
+&lis331dlh {
+	compatible = "st,lis3lv02d-i2c";
+	Vdd-supply = <&lis3_reg>;
+	Vdd_IO-supply = <&lis3_reg>;
+
+	st,click-single-x;
+	st,click-single-y;
+	st,click-single-z;
+	st,click-thresh-x = <10>;
+	st,click-thresh-y = <10>;
+	st,click-thresh-z = <10>;
+	st,irq1-click;
+	st,irq2-click;
+	st,wakeup-x-lo;
+	st,wakeup-x-hi;
+	st,wakeup-y-lo;
+	st,wakeup-y-hi;
+	st,wakeup-z-lo;
+	st,wakeup-z-hi;
+	st,min-limit-x = <120>;
+	st,min-limit-y = <120>;
+	st,min-limit-z = <140>;
+	st,max-limit-x = <550>;
+	st,max-limit-y = <550>;
+	st,max-limit-z = <750>;
 };
 
 /include/ "tps65910.dtsi"

-- 
1.7.9.5

On Thursday 13 September 2012, AnilKumar Ch wrote:

Why do you put the "reg" property here

and all the rest here? At least I would expect the "compatible" property
to be in the same place above.

Also, I think you should remove the "-i2c" postfix from the name, that
is already implied by the parent bus.

+	Vdd-supply = <&lis3_reg>;
+	Vdd_IO-supply = <&lis3_reg>;
+
+	st,click-single-x;
+	st,click-single-y;
+	st,click-single-z;
+	st,click-thresh-x = <10>;
+	st,click-thresh-y = <10>;
+	st,click-thresh-z = <10>;
+	st,irq1-click;
+	st,irq2-click;
+	st,wakeup-x-lo;
+	st,wakeup-x-hi;
+	st,wakeup-y-lo;
+	st,wakeup-y-hi;
+	st,wakeup-z-lo;
+	st,wakeup-z-hi;
+	st,min-limit-x = <120>;
+	st,min-limit-y = <120>;
+	st,min-limit-z = <140>;
+	st,max-limit-x = <550>;
+	st,max-limit-y = <550>;
+	st,max-limit-z = <750>;

Is there a binding document that describes all these?

	Arnd

Hi Arnd,

Thanks for the review,

On Fri, Sep 14, 2012 at 13:56:06, Arnd Bergmann wrote:

On Thursday 13 September 2012, AnilKumar Ch wrote:

quoted

Add lis331dlh device tree data to am335x-evm.dts. In AM335x EVM
lis331dlh accelerometer is connected to I2C2 bus. So this patch
change the status to "okay" to use I2C2 bus. Also added all the
required platform data to am335x-evm.

Signed-off-by: AnilKumar Ch <redacted>
---
 arch/arm/boot/dts/am335x-evm.dts |   42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 9fb59c5..9e5a878 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts

@@ -47,6 +47,15 @@
 			};
 		};
 
+		i2c2: i2c at 4802a000 {
+			status = "okay";
+			clock-frequency = <400000>;
+
+			lis331dlh: lis331dlh at 18 {
+				reg = <0x18>;
+			};
+		};

Why do you put the "reg" property here

Here I specified reg property because lis331dlh I2C slave address is 0x18.

quoted

 		dcan1: d_can at 481d0000 {
 			status = "okay";
 			pinctrl-names = "default";

@@ -61,6 +70,39 @@
 		regulator-max-microvolt = <5000000>;
 		regulator-boot-on;
 	};
+
+	lis3_reg: fixedregulator at 1 {
+		compatible = "regulator-fixed";
+		regulator-name = "lis3_reg";
+		regulator-boot-on;
+	};
+};
+&lis331dlh {
+	compatible = "st,lis3lv02d-i2c";

and all the rest here? At least I would expect the "compatible" property
to be in the same place above.

This data is appended to above one, to make it readable I moved remaining
properties to here.

Also, I think you should remove the "-i2c" postfix from the name, that
is already implied by the parent bus.

I will remove, but in case of spi the compatible name is lis3lv02d_spi.
By mistake I have uses "-i2c" instead of "_i2c".

quoted

+	Vdd-supply = <&lis3_reg>;
+	Vdd_IO-supply = <&lis3_reg>;
+
+	st,click-single-x;
+	st,click-single-y;
+	st,click-single-z;
+	st,click-thresh-x = <10>;
+	st,click-thresh-y = <10>;
+	st,click-thresh-z = <10>;
+	st,irq1-click;
+	st,irq2-click;
+	st,wakeup-x-lo;
+	st,wakeup-x-hi;
+	st,wakeup-y-lo;
+	st,wakeup-y-hi;
+	st,wakeup-z-lo;
+	st,wakeup-z-hi;
+	st,min-limit-x = <120>;
+	st,min-limit-y = <120>;
+	st,min-limit-z = <140>;
+	st,max-limit-x = <550>;
+	st,max-limit-y = <550>;
+	st,max-limit-z = <750>;

Is there a binding document that describes all these?

Document is already present, 
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=2f2ff3cc8d930493f9a598b9192706c09403e12e

Some minor changes in docs, in my next version I will update document
as well. I will send V3 if there are no comments on v2.

Thanks
AnilKumar

On Friday 14 September 2012, AnilKumar, Chimata wrote:

quoted

On Fri, Sep 14, 2012 at 13:56:06, Arnd Bergmann wrote:
On Thursday 13 September 2012, AnilKumar Ch wrote:

quoted

Why do you put the "reg" property here

Here I specified reg property because lis331dlh I2C slave address is 0x18.

quoted

quoted

 		dcan1: d_can at 481d0000 {
 			status = "okay";
 			pinctrl-names = "default";

@@ -61,6 +70,39 @@
 		regulator-max-microvolt = <5000000>;
 		regulator-boot-on;
 	};
+
+	lis3_reg: fixedregulator at 1 {
+		compatible = "regulator-fixed";
+		regulator-name = "lis3_reg";
+		regulator-boot-on;
+	};
+};
+&lis331dlh {
+	compatible = "st,lis3lv02d-i2c";

and all the rest here? At least I would expect the "compatible" property
to be in the same place above.

This data is appended to above one, to make it readable I moved remaining
properties to here.

I don't follow how this is making things more readable.

Maybe a more logical way to do this would be use the existing i2c2 label
and write all the additions as

i2c2: {
	status = "okay";
	clock-frequency = <400000>;

	lis331dlh at 18 {
		compatible = "st,lis3lv02d";
		reg = <0x18>;

		vdd-supply = <&lis3_reg>;
		vdd-io-supply = <&lis3_reg>;
		...
	};

quoted

Also, I think you should remove the "-i2c" postfix from the name, that
is already implied by the parent bus.

I will remove, but in case of spi the compatible name is lis3lv02d_spi.
By mistake I have uses "-i2c" instead of "_i2c".

The normal convention is to use '-', not '_', so that part was ok.

I think naming the other one lis3lv02d_spi was a mistake, it should
be named 'st,lis3lv02d' independent of the bus IMHO.

Document is already present, 
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=2f2ff3cc8d930493f9a598b9192706c09403e12e

Some minor changes in docs, in my next version I will update document
as well. I will send V3 if there are no comments on v2.

Ok.

	Arnd