lis3: lis3lv02d_i2c: Add device tree support
cover 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(+)
Signature · 1 line
1.7.9.5
PATCH 1/2 lis3: lis3lv02d_i2c: Add device tree support
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.
--- drivers/misc/lis3lv02d/lis3lv02d_i2c.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
drivers/misc/lis3lv02d/lis3lv02d_i2c.c +13 -0
@@ -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),
Signature · 1 line
1.7.9.5
PATCH 2/2 ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
In reply to AnilKumar ChAdd 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.
--- arch/arm/boot/dts/am335x-evm.dts | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
arch/arm/boot/dts/am335x-evm.dts +42 -0
@@ -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"
Signature · 1 line
1.7.9.5
Arnd Bergmann
maintainerPATCH 2/2 ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
Quoted 16 lines · AnilKumar Ch
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 <[email protected]>--- arch/arm/boot/dts/am335x-evm.dts | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)arch/arm/boot/dts/am335x-evm.dts +0
-0@@ -47,6 +47,15 @@ };
arch/arm/boot/dts/am335x-evm.dts line 57
Quoted 10 lines
arch/arm/boot/dts/am335x-evm.dts +8
-0}; + i2c2: i2c at 4802a000 { + status = "okay"; + clock-frequency = <400000>; + + lis331dlh: lis331dlh at 18 { + reg = <0x18>; + }; + };
Why do you put the "reg" property here
Quoted 6 lines
arch/arm/boot/dts/am335x-evm.dts +0
-0dcan1: d_can at 481d0000 { status = "okay"; pinctrl-names = "default";@@ -61,6 +70,39 @@ regulator-max-microvolt = <5000000>; regulator-boot-on;
arch/arm/boot/dts/am335x-evm.dts line 81
Quoted 10 lines
arch/arm/boot/dts/am335x-evm.dts +9
-0}; + + 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.
Also, I think you should remove the "-i2c" postfix from the name, that is already implied by the parent bus.
Quoted 13 lines
arch/arm/boot/dts/am335x-evm.dts +13
-0+ 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;
arch/arm/boot/dts/am335x-evm.dts line 105
Quoted 10 lines
arch/arm/boot/dts/am335x-evm.dts +10
-0+ 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
PATCH 2/2 ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
Hi Arnd,
Thanks for the review,
Quoted 19 lines · Arnd Bergmann
Quoted 18 lines · AnilKumar Ch
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 <[email protected]>--- arch/arm/boot/dts/am335x-evm.dts | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)arch/arm/boot/dts/am335x-evm.dts +0
-0@@ -47,6 +47,15 @@ }; };
Quoted 10 lines
arch/arm/boot/dts/am335x-evm.dts line 57
Quoted 8 lines
arch/arm/boot/dts/am335x-evm.dts +8
-0+ 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 12 lines
Quoted 12 lines
arch/arm/boot/dts/am335x-evm.dts +4
-0dcan1: 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";
Quoted 8 lines
arch/arm/boot/dts/am335x-evm.dts line 82
Quoted 5 lines
arch/arm/boot/dts/am335x-evm.dts +5
-0+ 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.
Quoted 3 lines
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 18 lines
Quoted 18 lines
arch/arm/boot/dts/am335x-evm.dts +17
-0+ 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;
Quoted 8 lines
arch/arm/boot/dts/am335x-evm.dts line 105
Quoted 6 lines
arch/arm/boot/dts/am335x-evm.dts +6
-0+ 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
Arnd Bergmann
maintainerPATCH 2/2 ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
Quoted 29 lines · Chimata
Quoted 5 lines · Arnd Bergmann
On Thursday 13 September 2012, AnilKumar Ch wrote:
Why do you put the "reg" property here
Here I specified reg property because lis331dlh I2C slave address is
0x18.Quoted 12 lines
Quoted 12 lines
arch/arm/boot/dts/am335x-evm.dts +4
-0dcan1: 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";Quoted 8 lines
arch/arm/boot/dts/am335x-evm.dts line 82
Quoted 5 lines
arch/arm/boot/dts/am335x-evm.dts +5
-0+ 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.
Quoted 2 lines
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 5 lines
Quoted 2 lines
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.
Quoted 5 lines
Document is already present,
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=2f2ff3cc8d930493f9a598b9192706c09403e12eSome 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
PATCH 2/2 ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
+Daniel
Quoted 30 lines · Arnd Bergmann
Quoted 29 lines · Chimata
Quoted 5 lines · Arnd Bergmann
On Thursday 13 September 2012, AnilKumar Ch wrote:
Why do you put the "reg" property here
Here I specified reg property because lis331dlh I2C slave address is
0x18.Quoted 20 lines
Quoted 7 lines
arch/arm/boot/dts/am335x-evm.dts +0
-0dcan1: d_can at 481d0000 { status = "okay"; pinctrl-names = "default";@@ -61,6 +70,39 @@ regulator-max-microvolt = <5000000>; regulator-boot-on;arch/arm/boot/dts/am335x-evm.dts line 81
Quoted 10 lines
arch/arm/boot/dts/am335x-evm.dts +9
-0}; + + 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.
Quoted 4 lines
Quoted 2 lines
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.
Basic lis3 parameter is in i2c2 node to tell the hierarchy of nodes and remaining parameters at the bottom. With this way we can understand easily what is start & end braces so that node hierarchy is cleaner. (This is my view)
Quoted 4 lines
Maybe a more logical way to do this would be use the existing i2c2 label and write all the additions as
Quoted 13 lines
i2c2: {
status = "okay";
clock-frequency = <400000>;
lis331dlh at 18 {
compatible = "st,lis3lv02d";
reg = <0x18>;
vdd-supply = <&lis3_reg>;
vdd-io-supply = <&lis3_reg>;
...
};If this is better/preferable way then I will change.
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
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".
Then, I will change to 'st,lis3lv02d' in lis3lv02d_i2c driver and same will added to .dts file.
Small question here, in my v2 version I have specified both the compatible names lis3lv02d and lis331dlh is it fine or only one is sufficient?
+static struct of_device_id lis3lv02d_i2c_dt_ids[] = {
+ { .compatible = "st,lis3lv02d" },
+ { .compatible = "st,lis331dlh" },
+ {}
+};
Quoted 5 lines
The normal convention is to use '-', not '_', so that part was ok.
I think naming the other one
lis3lv02d_spiwas a mistake, it should be named 'st,lis3lv02d' independent of the bus IMHO.
Yes, initially I expected the same. I think driver name is used as a compatible string.
Quoted 7 lines
Quoted 6 lines
Document is already present,
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=2f2ff3cc8d930493f9a598b9192706c09403e12eSome 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.
Can you just quickly review v2 series? So that I will send V3.
Thanks
AnilKumar
Arnd Bergmann
maintainerPATCH 2/2 ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
Review
Quoted 10 lines · Chimata
Small question here, in my v2 version I have specified both the compatible names lis3lv02d and lis331dlh is it fine or only one is sufficient?
(quoted diff) +5
-0+static struct of_device_id lis3lv02d_i2c_dt_ids[] = { + { .compatible = "st,lis3lv02d" }, + { .compatible = "st,lis331dlh" }, + {} +};
That's ok. In most cases, people will need the data field to point to a data structure with the differences between two chips, but not if they are identical from the software side.
It's also fine if you just list one entry here and put both values as "compatible" in the device tree, to signify that the device is backwards compatible with the older variant.
Arnd