OMAP: I2C: Add device tree support
Subsystems (8)
cover OMAP: I2C: Add device tree support
As a part of arm consolidation activity, the OMAP board files needs to be converted to use device tree.
To begin with, two board files are selected to use device tree for I2C device registration. The I2C and it's child devices existing on beagle and panda boards are modified in order to use device tree nodes created in device tree blob.
This patch series demonstrates i2c nodes creation and it's usage in i2c and it's client drivers. Since, current implementation of OMAP I2C driver uses SoC specific HWMOD framework for clock initilization, enable/disable and other PM functionalities, the patch series cannot work independently since device registration through device tree bypasses HWMOD framework.
There should be method to control peripheral level clock and to use existing hwmod framework api's and features through device tree. This might require alignment and suggestions from hwmod maintainers and contributors so that it can be adapted for other drivers also.
TODO:
1. The board file changes will move to new file board_dt.c once board file is completly migrated to DT.
2. Look up table can be provided if specific device names are required and also platform_data can be passed through lookup table while populating device nodes.
Applies on top of:git://git.secretlab.ca/git/linux-2.6
Branch:devicetree/test
Compilation:
Compiles for omap2plus_defconfig
Testing:
Boot testing is done on beagle and panda and in i2c driver, the device tree node data is verified manually in i2c driver and i2c driver will not work after the probe due to non availability of clock for I2C controllers(Testing is exempted for I2C1 since it interfaces with power chip).
G, Manjunath Kondaiah (5):
OMAP3:I2C: Add device tree nodes for beagle board OMAP4:I2C: Add device tree nodes for panda board
OMAP3: Beagle: Update beagle board file to use DT
OMAP4: Panda: Update panda board file to use DT
OMAP: I2C: Convert I2C driver to use device tree
arch/arm/boot/dts/omap3-beagle-nunchuck.dts | 5 -- arch/arm/boot/dts/omap3-beagle.dts | 42 ++++++++++++++++++++ arch/arm/boot/dts/omap4-panda.dts | 57 +++++++++++++++++++++++++++ arch/arm/mach-omap2/board-omap3beagle.c | 24 +++++++++-- arch/arm/mach-omap2/board-omap4panda.c | 22 ++++++++++ drivers/i2c/busses/i2c-omap.c | 30 ++++++++++++++- 6 files changed, 170 insertions(+), 10 deletions(-)
Signature · 1 line
1.7.4.1
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
--- arch/arm/boot/dts/omap3-beagle-nunchuck.dts | 5 --- arch/arm/boot/dts/omap3-beagle.dts | 42 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-)
arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0 -5
@@ -2,11 +2,6 @@ / { i2c@48072000 { - compatible = "ti,omap3-i2c"; - reg = <0x48072000 0x80>; - #address-cells = <1>; - #size-cells = <0>; - eeprom@50 { compatible = "at,at24c01"; reg = < 0x50 >;
arch/arm/boot/dts/omap3-beagle.dts +42 -0
46 lines not on this page · raw message
Signature · 1 line
1.7.4.1
Tony Lindgren
maintainerRFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Hi,
Few comments on the .dts data layout below.
Quoted 4 lines · G, Manjunath Kondaiah
arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0
-0@@ -2,11 +2,6 @@
arch/arm/boot/dts/omap3-beagle-nunchuck.dts line 7
Quoted 10 lines
arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0
-5/ { i2c at 48072000 { - compatible = "ti,omap3-i2c"; - reg = <0x48072000 0x80>; - #address-cells = <1>; - #size-cells = <0>; - eeprom at 50 { compatible = "at,at24c01"; reg = < 0x50 >;
The board .dts file should include the omap3 SoC .dts file.
The omap3 SoC .dts file should have the devices mapped to L3 and L4 busses, and the then i2c at 1 would just contain the bus offset.
Then the i2c at 1 entry would be repeated in the board specific .dts and tell that the i2c at 1 is enabled.
Quoted 4 lines
arch/arm/boot/dts/omap3-beagle.dts +0
-0@@ -4,4 +4,46 @@ / {
arch/arm/boot/dts/omap3-beagle.dts line 14
Quoted 10 lines
arch/arm/boot/dts/omap3-beagle.dts +8
-0model = "TI OMAP3 BeagleBoard"; compatible = "ti,omap3-beagle"; + interrupt-parent = <&gic>; + + gic: interrupt-controller at 48241000 { + compatible = "ti,omap-gic", "arm,gic"; + interrupt-controller; + #interrupt-cells = <1>; + reg = <0x48200000 0x1000>; + };
There's no GIC on omap3, that's only on Cortex A9 systems.
Regards,
Tony
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Quoted 22 lines · Tony Lindgren
Hi,
Few comments on the .dts data layout below.
Quoted 4 lines · G, Manjunath Kondaiah
arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0
-0@@ -2,11 +2,6 @@arch/arm/boot/dts/omap3-beagle-nunchuck.dts line 7
Quoted 10 lines
arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0
-5/ { i2c at 48072000 { - compatible = "ti,omap3-i2c"; - reg = <0x48072000 0x80>; - #address-cells = <1>; - #size-cells = <0>; - eeprom at 50 { compatible = "at,at24c01"; reg = < 0x50 >;The board .dts file should include the omap3 SoC .dts file.
Quoted 5 lines
The omap3 SoC .dts file should have the devices mapped to L3 and L4 busses, and the then i2c at 1 would just contain the bus offset.
Then the i2c at 1 entry would be repeated in the board specific .dts and tell that the i2c at 1 is enabled.
yup.
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
In reply to G, Manjunath Kondaiaharch/arm/boot/dts/omap3-beagle-nunchuck.dts
Quoted 15 lines · Manjunath Kondaiah
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
Signed-off-by G, Manjunath Kondaiah <[email protected]>--- arch/arm/boot/dts/omap3-beagle-nunchuck.dts | 5 --- arch/arm/boot/dts/omap3-beagle.dts | 42 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-)arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0
-0
This hunk is of course only for my tree since I'm the only one who actually has this modified beagleboard. :-)
Quoted 23 lines
arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0
-5@@ -2,11 +2,6 @@ / { i2c at 48072000 { - compatible = "ti,omap3-i2c"; - reg = <0x48072000 0x80>; - #address-cells = <1>; - #size-cells = <0>; - eeprom at 50 { compatible = "at,at24c01"; reg = < 0x50 >;arch/arm/boot/dts/omap3-beagle.dts +3
-0@@ -4,4 +4,46 @@ / { model = "TI OMAP3 BeagleBoard"; compatible = "ti,omap3-beagle"; + interrupt-parent = <&gic>; + + gic: interrupt-controller at 48241000 {
arch/arm/boot/dts/omap3-beagle.dts line 19
Quoted 10 lines
arch/arm/boot/dts/omap3-beagle.dts +10
-0+ compatible = "ti,omap-gic", "arm,gic"; + interrupt-controller; + #interrupt-cells = <1>; + reg = <0x48200000 0x1000>; + }; + + i2c at 48070000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c";
ti,omap3-i2c
Use '-' not '_'. and the specific silicon implementation should be specified (omap3 vs. omap).
arch/arm/boot/dts/omap3-beagle.dts line 22
Quoted 3 lines
arch/arm/boot/dts/omap3-beagle.dts +3
-0+ reg = <0x48070000 0x100>; + interrupts = < 88 >; + interrupt-parent = <&gic>;
interrupt-parent isn't needed because it is inherited from the root node.
arch/arm/boot/dts/omap3-beagle.dts line 24
Quoted 2 lines
arch/arm/boot/dts/omap3-beagle.dts +2
-0+ clock-frequency = <2600>; + status = "disabled";
Drop 'status' when you move this node definition to arch/arm/boot/dts/omap3.dtsi. Board overlay files that include the omap3.dtsi should explicitly disable any devices that it does not use (which is opposite to what tegra currently does, but I'm going to change that).
arch/arm/boot/dts/omap3-beagle.dts line 35
Quoted 11 lines
arch/arm/boot/dts/omap3-beagle.dts +11
-0+ }; + + i2c at 48072000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48072000 0x100>; + interrupts = < 89 >; + interrupt-parent = <&gic>; + clock-frequency = <400>; + status = "ok";
Okay is spelled 'okay'. :-) The kernel does accept 'ok', but I discourage its usage... just because I'm a nitpick about stuff like that.
Actually, if the device is enabled, the status property can be dropped entirely because the default behaviour is to enable.
Quoted 17 lines
arch/arm/boot/dts/omap3-beagle.dts +13
-0+ }; + + i2c at 48060000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48060000 0x100>; + interrupts = < 93 >; + interrupt-parent = <&gic>; + clock-frequency = <100>; + status = "ok"; + }; + };Signature · 1 line
1.7.4.1
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Grant Likely wrote at Wednesday, July 06, 2011 12:56 PM:
Quoted 4 lines · Manjunath Kondaiah
Quoted 3 lines
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
...
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/omap3-beagle.dts +0
-0
...
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/omap3-beagle.dts +1
-0+ i2c at 48070000 {
...
Quoted 8 lines
arch/arm/boot/dts/omap3-beagle.dts line 24
Quoted 2 lines
arch/arm/boot/dts/omap3-beagle.dts +2
-0+ clock-frequency = <2600>; + status = "disabled";Drop 'status' when you move this node definition to
arch/arm/boot/dts/omap3.dtsi. Board overlay files that include theomap3.dtsishould explicitly disable any devices that it does not use (which is opposite to what tegra currently does, but I'm going to change that).
Purely out of idle curiosity: What's the benefit of one way over the other?
Signature · 1 line
nvpublic
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Quoted 13 lines · Stephen Warren
Grant Likely wrote at Wednesday, July 06, 2011 12:56 PM:
Quoted 4 lines · Manjunath Kondaiah
Quoted 3 lines
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
...
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/omap3-beagle.dts +0
-0...
Quoted 1 line
Quoted 1 line
+ ? i2c at 48070000 {
...
Quoted 3 lines
Quoted 2 lines
+ ? ? ? ? ? clock-frequency = <2600>;
+ ? ? ? ? ? status = "disabled";
Quoted 7 lines
Quoted 5 lines
Drop 'status' when you move this node definition to
arch/arm/boot/dts/omap3.dtsi. ?Board overlay files that include theomap3.dtsishould explicitly disable any devices that it does not use (which is opposite to what tegra currently does, but I'm going to change that).Purely out of idle curiosity: What's the benefit of one way over the other?
Mostly consistency. Most of the experience we have with the flattened device tree up to this point hasn't bothered with the 'status' property. It is only when AMP and hypervisors cam online that it became important to use a status property, and that only when the kernel needs to be told that the device does indeed exist, but it must not be touched. I'd like to continue that pattern for new DT users with the default assumption that a device is enabled unless the board .dts explicitly disables it.
g.
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Quoted 22 lines · Grant Likely
Quoted 13 lines · Stephen Warren
Grant Likely wrote at Wednesday, July 06, 2011 12:56 PM:
Quoted 4 lines · Manjunath Kondaiah
Quoted 3 lines
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
...
Quoted 1 line
Quoted 1 line
arch/arm/boot/dts/omap3-beagle.dts +0
-0...
Quoted 1 line
Quoted 1 line
+ ? i2c at 48070000 {
...
Quoted 3 lines
Quoted 2 lines
+ ? ? ? ? ? clock-frequency = <2600>;
+ ? ? ? ? ? status = "disabled";Quoted 7 lines
Quoted 5 lines
Drop 'status' when you move this node definition to
arch/arm/boot/dts/omap3.dtsi. ?Board overlay files that include theomap3.dtsishould explicitly disable any devices that it does not use (which is opposite to what tegra currently does, but I'm going to change that).Purely out of idle curiosity: What's the benefit of one way over the other?
Quoted 9 lines
Mostly consistency. Most of the experience we have with the flattened device tree up to this point hasn't bothered with the 'status' property. It is only when AMP and hypervisors cam online that it became important to use a status property, and that only when the kernel needs to be told that the device does indeed exist, but it must not be touched. I'd like to continue that pattern for new DT users with the default assumption that a device is enabled unless the board .dts explicitly disables it.
When going this way with my i.mx53 related dts files, I feel I like the opposite way.
i.mx53 has many number of peripherals inside, while individual board might only use small portion there. For example, i.mx53 has 5 uart controller instances inside, while quick start (aka loco) board only uses one. With the way you suggest here, we will have the following in imx53.dtsi.
uart0: uart at ... {
...
};uart1: uart at ... {
...
};uart2: uart at ... {
...
};uart3: uart at ... {
...
};uart4: uart at ... {
...
};And we will have to tell the 4 we do not use on quick start board in imx53-qs.dtsi
uart1: uart at ... {
...
status = "disabled";
};uart2: uart at ... {
...
status = "disabled";
};uart3: uart at ... {
...
status = "disabled";
};uart4: uart at ... {
...
status = "disabled";
};Besides the bothering that we have to list so many unused controllers in individual board dts file, it's also hard to tell which controllers are actually available on the board. People have to look at imx53.dts to get a full list and then exclude the ones in imx53-<board>.dts as "disabled".
And if we go the way opposite, adding "disabled" status for everyone in imx53.dts, we will only need to specify the peripherals that are actually available on board with "okay" status in imx53-<board>.dts. And it's much more clear for people to see what peripherals are available on individual board.
So I'm going the way than you suggested. Please let me know if you strongly dislikes it.
Signature · 2 lines
Regards, Shawn
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Quoted 8 lines · Shawn Guo
Quoted 8 lines
Mostly consistency. Most of the experience we have with the flattened device tree up to this point hasn't bothered with the 'status' property. It is only when AMP and hypervisors cam online that it became important to use a status property, and that only when the kernel needs to be told that the device does indeed exist, but it must not be touched. I'd like to continue that pattern for new DT users with the default assumption that a device is enabled unless the board .dts explicitly disables it.
[...]
Quoted 6 lines
Besides the bothering that we have to list so many unused controllers in individual board dts file, it's also hard to tell which controllers are actually available on the board. People have to look at
imx53.dtsto get a full list and then exclude the ones in imx53-<board>.dts as "disabled".
Quoted 8 lines
And if we go the way opposite, adding "disabled" status for everyone in
imx53.dts, we will only need to specify the peripherals that are actually available on board with "okay" status in imx53-<board>.dts. And it's much more clear for people to see what peripherals are available on individual board.So I'm going the way than you suggested. Please let me know if you strongly dislikes it.
Yes, I strongly dislike it. I understand the concern, but at this early stage with converting to device tree I think consistency between platforms is more important. We can talk about the issue at Linaro Connect in 2 weeks, but in the mean time please use the enabled-by-default/explicitly-disabled pattern.
g.
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Quoted 22 lines · Grant Likely
Quoted 8 lines · Shawn Guo
Quoted 8 lines
Mostly consistency. Most of the experience we have with the flattened device tree up to this point hasn't bothered with the 'status' property. It is only when AMP and hypervisors cam online that it became important to use a status property, and that only when the kernel needs to be told that the device does indeed exist, but it must not be touched. I'd like to continue that pattern for new DT users with the default assumption that a device is enabled unless the board .dts explicitly disables it.
[...]
Quoted 12 lines
Besides the bothering that we have to list so many unused controllers in individual board dts file, it's also hard to tell which controllers are actually available on the board. People have to look at
imx53.dtsto get a full list and then exclude the ones in imx53-<board>.dts as "disabled".And if we go the way opposite, adding "disabled" status for everyone in
imx53.dts, we will only need to specify the peripherals that are actually available on board with "okay" status in imx53-<board>.dts. And it's much more clear for people to see what peripherals are available on individual board.
Quoted 9 lines
Quoted 2 lines
So I'm going the way than you suggested. Please let me know if you strongly dislikes it.
Yes, I strongly dislike it. I understand the concern, but at this early stage with converting to device tree I think consistency between platforms is more important. We can talk about the issue at Linaro Connect in 2 weeks, but in the mean time please use the enabled-by-default/explicitly-disabled pattern.
Okay, hope you will not ask me to use the opposite pattern when you actually see the patch :)
Signature · 2 lines
Regards, Shawn
RFC PATCH 1/5 OMAP3:I2C: Add device tree nodes for beagle board
Quoted 26 lines · Shawn Guo
Quoted 25 lines · Grant Likely
Quoted 8 lines · Shawn Guo
Quoted 8 lines
Mostly consistency. ?Most of the experience we have with the flattened device tree up to this point hasn't bothered with the 'status' property. ?It is only when AMP and hypervisors cam online that it became important to use a status property, and that only when the kernel needs to be told that the device does indeed exist, but it must not be touched. ?I'd like to continue that pattern for new DT users with the default assumption that a device is enabled unless the board .dts explicitly disables it.
[...]
Quoted 14 lines
Besides the bothering that we have to list so many unused controllers in individual board dts file, it's also hard to tell which controllers are actually available on the board. ?People have to look at
imx53.dtsto get a full list and then exclude the ones in imx53-<board>.dts as "disabled".And if we go the way opposite, adding "disabled" status for everyone in
imx53.dts, we will only need to specify the peripherals that are actually available on board with "okay" status in imx53-<board>.dts. And it's much more clear for people to see what peripherals are available on individual board.So I'm going the way than you suggested. ?Please let me know if you strongly dislikes it.
Quoted 8 lines
Quoted 6 lines
Yes, I strongly dislike it. ?I understand the concern, but at this early stage with converting to device tree I think consistency between platforms is more important. ?We can talk about the issue at Linaro Connect in 2 weeks, but in the mean time please use the enabled-by-default/explicitly-disabled pattern.
Okay, hope you will not ask me to use the opposite pattern when you actually see the patch :)
:-)
g.
RFC PATCH 2/5 OMAP4:I2C: Add device tree nodes for panda board
In reply to G, Manjunath KondaiahAdd I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
--- arch/arm/boot/dts/omap4-panda.dts | 57 +++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-)
arch/arm/boot/dts/omap4-panda.dts +57 -0
@@ -8,4 +8,61 @@ / { model = "TI OMAP4 PandaBoard"; compatible = "ti,omap4-panda", "ti,omap4430"; + interrupt-parent = <&gic>; + + gic: interrupt-controller@48241000 { + compatible = "ti,omap-gic", "arm,gic"; + interrupt-controller; + #interrupt-cells = <1>; + reg = <0x48241000 0x1000>, + <0X48240100 0x0200>; + }; + + i2c@48070000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48070000 0x100>; + interrupts = < 88 >; + interrupt-parent = <&gic>; + clock-frequency = <400>; + status = "disabled"; + }; + + i2c@48072000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48072000 0x100>; + interrupts = < 89 >; + interrupt-parent = <&gic>; + clock-frequency = <400>; + status = "ok"; + }; + + i2c@48060000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48060000 0x100>; + interrupts = < 93 >; + interrupt-parent = <&gic>; + clock-frequency = <100>; + status = "ok"; + eeprom@50 { + compatible = "at,at24c01"; + reg = < 0x50 >; + }; + }; + + i2c@48350000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48350000 0x100>; + interrupts = < 94 >; + interrupt-parent = <&gic>; + clock-frequency = <400>; + status = "ok"; + }; };
Signature · 1 line
1.7.4.1
RFC PATCH 2/5 OMAP4:I2C: Add device tree nodes for panda board
Quoted 15 lines · Manjunath Kondaiah
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
Signed-off-by G, Manjunath Kondaiah <[email protected]>--- arch/arm/boot/dts/omap4-panda.dts | 57 +++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-)arch/arm/boot/dts/omap4-panda.dts +0
-0
arch/arm/boot/dts/omap4-panda.dts line 14
Quoted 7 lines
arch/arm/boot/dts/omap4-panda.dts +4
-0@@ -8,4 +8,61 @@ / { model = "TI OMAP4 PandaBoard"; compatible = "ti,omap4-panda", "ti,omap4430"; + interrupt-parent = <&gic>; + + gic: interrupt-controller at 48241000 { + compatible = "ti,omap-gic", "arm,gic";
ti,omap4-gic
arch/arm/boot/dts/omap4-panda.dts line 24
Quoted 10 lines
arch/arm/boot/dts/omap4-panda.dts +10
-0+ interrupt-controller; + #interrupt-cells = <1>; + reg = <0x48241000 0x1000>, + <0X48240100 0x0200>; + }; + + i2c at 48070000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c";
ti,omap4-i2c. Actually, it is quite possibly appropriate to do:
compatible = "ti,omap4-i2c", "ti,omap3-i2c";
arch/arm/boot/dts/omap4-panda.dts line 28
Quoted 4 lines
arch/arm/boot/dts/omap4-panda.dts +4
-0+ reg = <0x48070000 0x100>; + interrupts = < 88 >; + interrupt-parent = <&gic>; + clock-frequency = <400>;
Frequency should be a Hz value, not kHz or MHz.
Quoted 43 lines
arch/arm/boot/dts/omap4-panda.dts +39
-0+ status = "disabled"; + }; + + i2c at 48072000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48072000 0x100>; + interrupts = < 89 >; + interrupt-parent = <&gic>; + clock-frequency = <400>; + status = "ok"; + }; + + i2c at 48060000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48060000 0x100>; + interrupts = < 93 >; + interrupt-parent = <&gic>; + clock-frequency = <100>; + status = "ok"; + eeprom at 50 { + compatible = "at,at24c01"; + reg = < 0x50 >; + }; + }; + + i2c at 48350000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c"; + reg = <0x48350000 0x100>; + interrupts = < 94 >; + interrupt-parent = <&gic>; + clock-frequency = <400>; + status = "ok"; + }; };Signature · 1 line
1.7.4.1
RFC PATCH 2/5 OMAP4:I2C: Add device tree nodes for panda board
Quoted 39 lines · Grant Likely
Quoted 15 lines · Manjunath Kondaiah
Add I2C and it's child device nodes for beagle board. The I2C1 controller child devices are not populated and it should be handled along with OMAP clock changes.
Signed-off-by G, Manjunath Kondaiah <[email protected]>--- arch/arm/boot/dts/omap4-panda.dts | 57 +++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-)arch/arm/boot/dts/omap4-panda.dts +0
-0arch/arm/boot/dts/omap4-panda.dts line 14
Quoted 7 lines
arch/arm/boot/dts/omap4-panda.dts +4
-0@@ -8,4 +8,61 @@ / { model = "TI OMAP4 PandaBoard"; compatible = "ti,omap4-panda", "ti,omap4430"; + interrupt-parent = <&gic>; + + gic: interrupt-controller at 48241000 { + compatible = "ti,omap-gic", "arm,gic";ti,omap4-gic
arch/arm/boot/dts/omap4-panda.dts line 24
Quoted 10 lines
arch/arm/boot/dts/omap4-panda.dts +10
-0+ interrupt-controller; + #interrupt-cells = <1>; + reg = <0x48241000 0x1000>, + <0X48240100 0x0200>; + }; + + i2c at 48070000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap_i2c";ti,omap4-i2c. Actually, it is quite possibly appropriate to do:
Quoted 1 line
compatible = "ti,omap4-i2c", "ti,omap3-i2c";
ok. can also have "ti,omap2-i2c"
Quoted 7 lines
arch/arm/boot/dts/omap4-panda.dts line 28
Quoted 5 lines
arch/arm/boot/dts/omap4-panda.dts +4
-0+ reg = <0x48070000 0x100>; + interrupts = < 88 >; + interrupt-parent = <&gic>; + clock-frequency = <400>;Frequency should be a Hz value, not kHz or MHz.
current i2c-omap driver expects value to be in KHz. Will change it in driver and pass value in Hz here.
-Manjunath
RFC PATCH 3/5 OMAP3: Beagle: Update beagle board file to use DT
In reply to G, Manjunath KondaiahThe beagle board file is updated to use i2c nodes from device tree data structures.
--- arch/arm/mach-omap2/board-omap3beagle.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/board-omap3beagle.c +20 -4
@@ -20,6 +20,7 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/leds.h> #include <linux/gpio.h> #include <linux/input.h>
@@ -33,6 +34,7 @@#include <linux/regulator/machine.h> #include <linux/i2c/twl.h> +#include <linux/irq.h> #include <mach/hardware.h> #include <asm/mach-types.h>
@@ -421,8 +423,8 @@ static int __init omap3_beagle_i2c_init(void)omap3_pmic_init ("twl4030" , &beagle_twldata);/* Bus 3 is attached to the DVI port where devices like the pico DLP * projector don't work reliably with 400kHz */ +#if !defined(CONFIG_OF )omap_register_i2c_bus (3 ,100 , beagle_i2c_eeprom,ARRAY_SIZE (beagle_i2c_eeprom)); -#ifdef CONFIG_OF omap_register_i2c_bus (2 ,100 ,NULL ,0 );#endif /* CONFIG_OF */ return 0 ;
@@ -565,11 +567,24 @@ static void __init beagle_opp_init(void)return ; } +static struct of_device_id omap_dt_gic_match[]__initdata = { + { .compatible ="ti,omap-gic" , }, + {} +}; + +static struct of_device_id omap_dt_match_table[]__initdata = { + { .compatible ="ti,omap3-beagle" , }, + {} +}; +static void __init omap3_beagle_init (void ) { -#ifdef CONFIG_OF -of_platform_prepare (NULL ,NULL ); -#endif /* CONFIG_OF */ +struct device_node *node; + + node =of_find_matching_node_by_address (NULL , omap_dt_gic_match, +OMAP34XX_IC_BASE ); +if (node) +irq_domain_add_simple (node,0 );omap3_mux_init (board_mux,OMAP_PACKAGE_CBB );omap3_beagle_init_rev ();
@@ -597,6 +612,7 @@ static void __init omap3_beagle_init(void)beagle_display_init ();beagle_opp_init (); +of_platform_populate (NULL , omap_dt_match_table,NULL ,NULL ); }static const char *omap3_beagle_dt_match[]__initdata = {
Signature · 1 line
1.7.4.1
RFC PATCH 3/5 OMAP3: Beagle: Update beagle board file to use DT
Quoted 68 lines · Manjunath Kondaiah
The beagle board file is updated to use i2c nodes from device tree data structures.
Signed-off-by G, Manjunath Kondaiah <[email protected]>--- arch/arm/mach-omap2/board-omap3beagle.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)arch/arm/mach-omap2/board-omap3beagle.c +19
-4@@ -20,6 +20,7 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/leds.h> #include <linux/gpio.h> #include <linux/input.h> @@ -33,6 +34,7 @@#include <linux/regulator/machine.h> #include <linux/i2c/twl.h> +#include <linux/irq.h> #include <mach/hardware.h> #include <asm/mach-types.h> @@ -421,8 +423,8 @@ static int __init omap3_beagle_i2c_init(void)omap3_pmic_init ("twl4030" , &beagle_twldata);/* Bus 3 is attached to the DVI port where devices like the pico DLP * projector don't work reliably with 400kHz */ +#if !defined(CONFIG_OF )omap_register_i2c_bus (3 ,100 , beagle_i2c_eeprom,ARRAY_SIZE (beagle_i2c_eeprom)); -#ifdef CONFIG_OF omap_register_i2c_bus (2 ,100 ,NULL ,0 );#endif /* CONFIG_OF */ return 0 ;@@ -565,11 +567,24 @@ static void __init beagle_opp_init(void)return ; } +static struct of_device_id omap_dt_gic_match[]__initdata = { + { .compatible ="ti,omap-gic" , }, + {} +}; + +static struct of_device_id omap_dt_match_table[]__initdata = { + { .compatible ="ti,omap3-beagle" , }, + {} +}; +static void __init omap3_beagle_init (void ) { -#ifdef CONFIG_OF -of_platform_prepare (NULL ,NULL ); -#endif /* CONFIG_OF */ +struct device_node *node; + + node =of_find_matching_node_by_address (NULL , omap_dt_gic_match, +OMAP34XX_IC_BASE ); +if (node) +irq_domain_add_simple (node,0 );omap3_mux_init (board_mux,OMAP_PACKAGE_CBB );omap3_beagle_init_rev ();
arch/arm/mach-omap2/board-omap3beagle.c static void __init omap3_beagle_init(void) · line 615
Quoted 4 lines
arch/arm/mach-omap2/board-omap3beagle.c +1
-0@@ -597,6 +612,7 @@ static void __init omap3_beagle_init(void)beagle_display_init ();beagle_opp_init (); +of_platform_populate (NULL , omap_dt_match_table,NULL ,NULL );
This is probably a loosing approach. Until the DT stuff is stable on omap, you should avoid doing anything that might break existing board ports. Instead, I recommend creating an board-omap3-dt.c board file, and pull in only the static beagle device registrations that you need to get up and running, then you can pull them back out as the beagle DT board support matures.
g.
Quoted 6 lines
arch/arm/mach-omap2/board-omap3beagle.c +0
-0}static const char *omap3_beagle_dt_match[]__initdata = {Signature · 1 line
1.7.4.1
RFC PATCH 3/5 OMAP3: Beagle: Update beagle board file to use DT
Quoted 70 lines · Grant Likely
Quoted 69 lines · Manjunath Kondaiah
The beagle board file is updated to use i2c nodes from device tree data structures.
Signed-off-by G, Manjunath Kondaiah <[email protected]>--- arch/arm/mach-omap2/board-omap3beagle.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)arch/arm/mach-omap2/board-omap3beagle.c +19
-4@@ -20,6 +20,7 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/leds.h> #include <linux/gpio.h> #include <linux/input.h> @@ -33,6 +34,7 @@#include <linux/regulator/machine.h> #include <linux/i2c/twl.h> +#include <linux/irq.h> #include <mach/hardware.h> #include <asm/mach-types.h> @@ -421,8 +423,8 @@ static int __init omap3_beagle_i2c_init(void)omap3_pmic_init ("twl4030" , &beagle_twldata);/* Bus 3 is attached to the DVI port where devices like the pico DLP * projector don't work reliably with 400kHz */ +#if !defined(CONFIG_OF )omap_register_i2c_bus (3 ,100 , beagle_i2c_eeprom,ARRAY_SIZE (beagle_i2c_eeprom)); -#ifdef CONFIG_OF omap_register_i2c_bus (2 ,100 ,NULL ,0 );#endif /* CONFIG_OF */ return 0 ;@@ -565,11 +567,24 @@ static void __init beagle_opp_init(void)return ; } +static struct of_device_id omap_dt_gic_match[]__initdata = { + { .compatible ="ti,omap-gic" , }, + {} +}; + +static struct of_device_id omap_dt_match_table[]__initdata = { + { .compatible ="ti,omap3-beagle" , }, + {} +}; +static void __init omap3_beagle_init (void ) { -#ifdef CONFIG_OF -of_platform_prepare (NULL ,NULL ); -#endif /* CONFIG_OF */ +struct device_node *node; + + node =of_find_matching_node_by_address (NULL , omap_dt_gic_match, +OMAP34XX_IC_BASE ); +if (node) +irq_domain_add_simple (node,0 );omap3_mux_init (board_mux,OMAP_PACKAGE_CBB );omap3_beagle_init_rev ();@@ -597,6 +612,7 @@ static void __init omap3_beagle_init(void)
Quoted 10 lines
arch/arm/mach-omap2/board-omap3beagle.c static void __init omap3_beagle_init(void) · line 615
Quoted 3 lines
arch/arm/mach-omap2/board-omap3beagle.c +1
-0beagle_display_init ();beagle_opp_init (); +of_platform_populate (NULL , omap_dt_match_table,NULL ,NULL );This is probably a loosing approach. Until the DT stuff is stable on omap, you should avoid doing anything that might break existing board ports. Instead, I recommend creating an
board-omap3-dt.cboard file, and pull in only the static beagle device registrations that you need to get up and running, then you can pull them back out as the beagle DT board support matures.
yes. that is the plan. I have mentioned the same in PATCH 0/5
-Manjunath
RFC PATCH 4/5 OMAP4: Panda: Update panda board file to use DT
In reply to G, Manjunath KondaiahThe OMAP4 panda board file is updated to use i2c nodes from device tree data structures.
--- arch/arm/mach-omap2/board-omap4panda.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/board-omap4panda.c +22 -0
@@ -22,12 +22,15 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/leds.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/gpio.h> #include <linux/usb/otg.h> #include <linux/i2c/twl.h> #include <linux/regulator/machine.h> #include <linux/regulator/fixed.h> #include <linux/wl12xx.h> +#include <linux/irq.h> #include <mach/hardware.h> #include <mach/omap4-common.h>
@@ -410,6 +413,7 @@ static struct i2c_board_info __initdata panda_i2c_eeprom[] = {static int __init omap4_panda_i2c_init (void ) {omap4_pmic_init ("twl6030" , &omap4_panda_twldata); +#if !defined(CONFIG_OF )omap_register_i2c_bus (2 ,400 ,NULL ,0 );/* * Bus 3 is attached to the DVI port where devices like the pico DLP
@@ -418,6 +422,7 @@ static int __init omap4_panda_i2c_init(void)omap_register_i2c_bus (3 ,100 , panda_i2c_eeprom,ARRAY_SIZE (panda_i2c_eeprom));omap_register_i2c_bus (4 ,400 ,NULL ,0 ); +#endif return 0 ; }
@@ -681,9 +686,25 @@ void omap4_panda_display_init(void)omap_display_init (&omap4_panda_dss_data); } +static struct of_device_id omap_dt_gic_match[]__initdata = { + { .compatible ="ti,omap-gic" , }, + {} +}; + +static struct of_device_id omap_dt_match_table[]__initdata = { + { .compatible ="ti,omap4-panda" , }, + {} +}; +static void __init omap4_panda_init (void ) {int package =OMAP_PACKAGE_CBS ; +struct device_node *node; + + node =of_find_matching_node_by_address (NULL , omap_dt_gic_match, +OMAP44XX_GIC_DIST_BASE ); +if (node) +irq_domain_add_simple (node,0 );if (omap_rev () ==OMAP4430_REV_ES1_0 ) package =OMAP_PACKAGE_CBL ;
@@ -700,6 +721,7 @@ static void __init omap4_panda_init(void)omap4_ehci_init ();usb_musb_init (&musb_board_data);omap4_panda_display_init (); +of_platform_populate (NULL , omap_dt_match_table,NULL ,NULL ); }static void __init omap4_panda_map_io (void )
Signature · 1 line
1.7.4.1
RFC PATCH 4/5 OMAP4: Panda: Update panda board file to use DT
Quoted 38 lines · Manjunath Kondaiah
The OMAP4 panda board file is updated to use i2c nodes from device tree data structures.
Signed-off-by G, Manjunath Kondaiah <[email protected]>--- arch/arm/mach-omap2/board-omap4panda.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)arch/arm/mach-omap2/board-omap4panda.c +4
-0@@ -22,12 +22,15 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/leds.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/gpio.h> #include <linux/usb/otg.h> #include <linux/i2c/twl.h> #include <linux/regulator/machine.h> #include <linux/regulator/fixed.h> #include <linux/wl12xx.h> +#include <linux/irq.h> #include <mach/hardware.h> #include <mach/omap4-common.h> @@ -410,6 +413,7 @@ static struct i2c_board_info __initdata panda_i2c_eeprom[] = {static int __init omap4_panda_i2c_init (void ) {omap4_pmic_init ("twl6030" , &omap4_panda_twldata); +#if !defined(CONFIG_OF )omap_register_i2c_bus (2 ,400 ,NULL ,0 );/* * Bus 3 is attached to the DVI port where devices like the pico DLP
arch/arm/mach-omap2/board-omap4panda.c static int __init omap4_panda_i2c_init(void) · line 425
Quoted 4 lines
arch/arm/mach-omap2/board-omap4panda.c +1
-0@@ -418,6 +422,7 @@ static int __init omap4_panda_i2c_init(void)omap_register_i2c_bus (3 ,100 , panda_i2c_eeprom,ARRAY_SIZE (panda_i2c_eeprom));omap_register_i2c_bus (4 ,400 ,NULL ,0 ); +#endif
Yeah, don't do this. Anything we merge to mainline we want to make sure that a kernel image will still support both DT and non-DT boots. That won't work if you #ifdef out blocks of code.
Quoted 40 lines
arch/arm/mach-omap2/board-omap4panda.c +17
-0return 0 ; }@@ -681,9 +686,25 @@ void omap4_panda_display_init(void)omap_display_init (&omap4_panda_dss_data); } +static struct of_device_id omap_dt_gic_match[]__initdata = { + { .compatible ="ti,omap-gic" , }, + {} +}; + +static struct of_device_id omap_dt_match_table[]__initdata = { + { .compatible ="ti,omap4-panda" , }, + {} +}; +static void __init omap4_panda_init (void ) {int package =OMAP_PACKAGE_CBS ; +struct device_node *node; + + node =of_find_matching_node_by_address (NULL , omap_dt_gic_match, +OMAP44XX_GIC_DIST_BASE ); +if (node) +irq_domain_add_simple (node,0 );if (omap_rev () ==OMAP4430_REV_ES1_0 ) package =OMAP_PACKAGE_CBL ;@@ -700,6 +721,7 @@ static void __init omap4_panda_init(void)omap4_ehci_init ();usb_musb_init (&musb_board_data);omap4_panda_display_init (); +of_platform_populate (NULL , omap_dt_match_table,NULL ,NULL ); }static void __init omap4_panda_map_io (void )Signature · 1 line
1.7.4.1
RFC PATCH 5/5 OMAP: I2C: Convert I2C driver to use device tree
In reply to G, Manjunath KondaiahThe OMAP I2C driver is modified to use platform_device data from device tree data structures.
--- drivers/i2c/busses/i2c-omap.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)
drivers/i2c/busses/i2c-omap.c +29 -1
@@ -38,9 +38,13 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/of_i2c.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/slab.h> #include <linux/i2c-omap.h> #include <linux/pm_runtime.h> +#include <plat/i2c.h> /* I2C controller revisions */ #define OMAP_I2C_REV_2 0x20
@@ -972,6 +976,7 @@ static const struct i2c_algorithm omap_i2c_algo = { .functionality = omap_i2c_func, }; +static const struct of_device_id omap_i2c_of_match[];static int __devinit omap_i2c_probe (struct platform_device *pdev) {
@@ -979,10 +984,15 @@ omap_i2c_probe(struct platform_device *pdev)struct i2c_adapter *adap;struct resource *mem, *irq, *ioarea;struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data; +const struct of_device_id *match;irq_handler_t isr;int r;u32 speed =0 ; + match =of_match_device (omap_i2c_of_match, &pdev->dev); +if (!match) +dev_err (&pdev->dev,"DT: i2c device match not found.......\n" ); +/* NOTE: driver uses the static register mapping */ mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 );if (!mem) {
@@ -1007,10 +1017,19 @@ omap_i2c_probe(struct platform_device *pdev) r = -ENOMEM ; goto err_release_region; } - +/* I2C device without DT might use this */ if (pdata !=NULL ) { speed = pdata->clkrate; dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; + }else if (pdev->dev.of_node) { +const unsigned int *prop; + prop =of_get_property (pdev->dev.of_node, +"clock-frequency" ,NULL ); +if (prop) + speed =be32_to_cpup (prop); +else + speed =100 ; + dev->set_mpu_wkup_lat =NULL ; }else { speed =100 ;/* Default speed */ dev->set_mpu_wkup_lat =NULL ;
@@ -1045,6 +1064,7 @@ omap_i2c_probe(struct platform_device *pdev) dev->rev =omap_i2c_read_reg (dev,OMAP_I2C_REV_REG ) &0xff ; +if (dev->rev <=OMAP_I2C_REV_ON_3430 ) dev->errata |=I2C_OMAP3_1P153 ;
@@ -1073,6 +1093,7 @@ omap_i2c_probe(struct platform_device *pdev) (1000 * speed /8 ); } +/* reset ASAP, clearing any IRQs */ omap_i2c_init (dev);
@@ -1162,6 +1183,12 @@ static int omap_i2c_resume(struct device *dev)return 0 ; } +static const struct of_device_id omap_i2c_of_match[] = { + {.compatible ="ti,omap_i2c" , }, + {}, +} +MODULE_DEVICE_TABLE (of, omap_i2c_of_match); +static struct dev_pm_ops omap_i2c_pm_ops = { .suspend = omap_i2c_suspend, .resume = omap_i2c_resume,
@@ -1178,6 +1205,7 @@ static struct platform_driver omap_i2c_driver = { .name = "omap_i2c", .owner = THIS_MODULE, .pm = OMAP_I2C_PM_OPS, + .of_match_table = omap_i2c_of_match, }, };
Signature · 1 line
1.7.4.1
RFC PATCH 5/5 OMAP: I2C: Convert I2C driver to use device tree
Quoted 5 lines · Manjunath Kondaiah
The OMAP I2C driver is modified to use
platform_devicedata from device tree data structures.Signed-off-by G, Manjunath Kondaiah <[email protected]>
Mostly looks good, but a few things that need to be fixed. You can probably even get this change merged for v3.1
Quoted 33 lines
--- drivers/i2c/busses/i2c-omap.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)drivers/i2c/busses/i2c-omap.c +5
-0@@ -38,9 +38,13 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/of_i2c.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/slab.h> #include <linux/i2c-omap.h> #include <linux/pm_runtime.h> +#include <plat/i2c.h> /* I2C controller revisions */ #define OMAP_I2C_REV_2 0x20@@ -972,6 +976,7 @@ static const struct i2c_algorithm omap_i2c_algo = { .functionality = omap_i2c_func, }; +static const struct of_device_id omap_i2c_of_match[];static int __devinit omap_i2c_probe (struct platform_device *pdev) {@@ -979,10 +984,15 @@ omap_i2c_probe(struct platform_device *pdev)struct i2c_adapter *adap;struct resource *mem, *irq, *ioarea;
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 995
Quoted 10 lines
drivers/i2c/busses/i2c-omap.c +5
-0struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data; +const struct of_device_id *match;irq_handler_t isr;int r;u32 speed =0 ; + match =of_match_device (omap_i2c_of_match, &pdev->dev); +if (!match) +dev_err (&pdev->dev,"DT: i2c device match not found.......\n" ); +
This isn't an error. It just mean that the device wasn't registered from the device tree, and it needs to get its configuration from pdata.
In fact, you don't even need this block since the driver never uses the match entry returned by of_match_device()
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1010
Quoted 8 lines
drivers/i2c/busses/i2c-omap.c +0
-1/* NOTE: driver uses the static register mapping */ mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 );if (!mem) {@@ -1007,10 +1017,19 @@ omap_i2c_probe(struct platform_device *pdev) r = -ENOMEM ; goto err_release_region; } -
Don't drop the empty line.
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1020
Quoted 1 line
drivers/i2c/busses/i2c-omap.c +1
-0+/* I2C device without DT might use this */
No need for the comment.
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1031
Quoted 11 lines
drivers/i2c/busses/i2c-omap.c +8
-0if (pdata !=NULL ) { speed = pdata->clkrate; dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; + }else if (pdev->dev.of_node) { +const unsigned int *prop; + prop =of_get_property (pdev->dev.of_node, +"clock-frequency" ,NULL ); +if (prop) + speed =be32_to_cpup (prop); +else + speed =100 ;
There is a new function that makes this easier. Do this instead:
speed = 100; if (of_property_read_u32(pdev->dev.of_node, "clock-frequency", &prop) speed = prop / 1000000; /* speed in Hz, this might be wrong */
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1032
Quoted 1 line
drivers/i2c/busses/i2c-omap.c +1
-0+ dev->set_mpu_wkup_lat =NULL ;
dev is kzalloced, which means set_mpu_wkup_lat is already NULL.
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1067
Quoted 8 lines
drivers/i2c/busses/i2c-omap.c +1
-0}else { speed =100 ;/* Default speed */ dev->set_mpu_wkup_lat =NULL ;@@ -1045,6 +1064,7 @@ omap_i2c_probe(struct platform_device *pdev) dev->rev =omap_i2c_read_reg (dev,OMAP_I2C_REV_REG ) &0xff ; +
Drop the unrelated whitespace change.
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1096
Quoted 8 lines
drivers/i2c/busses/i2c-omap.c +1
-0if (dev->rev <=OMAP_I2C_REV_ON_3430 ) dev->errata |=I2C_OMAP3_1P153 ;@@ -1073,6 +1093,7 @@ omap_i2c_probe(struct platform_device *pdev) (1000 * speed /8 ); } +
Ditto.
drivers/i2c/busses/i2c-omap.c static int omap_i2c_resume(struct device *dev) · line 1191
Quoted 13 lines
drivers/i2c/busses/i2c-omap.c +6
-0/* reset ASAP, clearing any IRQs */ omap_i2c_init (dev);@@ -1162,6 +1183,12 @@ static int omap_i2c_resume(struct device *dev)return 0 ; } +static const struct of_device_id omap_i2c_of_match[] = { + {.compatible ="ti,omap_i2c" , }, + {}, +} +MODULE_DEVICE_TABLE (of, omap_i2c_of_match); +
This needs to be protected with #ifdef CONFIG_OF/#else/#endif. Don't break non-dt builds.
Quoted 14 lines
drivers/i2c/busses/i2c-omap.c +1
-0static struct dev_pm_ops omap_i2c_pm_ops = { .suspend = omap_i2c_suspend, .resume = omap_i2c_resume,@@ -1178,6 +1205,7 @@ static struct platform_driver omap_i2c_driver = { .name = "omap_i2c", .owner = THIS_MODULE, .pm = OMAP_I2C_PM_OPS, + .of_match_table = omap_i2c_of_match, }, };Signature · 1 line
1.7.4.1
RFC PATCH 5/5 OMAP: I2C: Convert I2C driver to use device tree
Quoted 9 lines · Grant Likely
Quoted 5 lines · Manjunath Kondaiah
The OMAP I2C driver is modified to use
platform_devicedata from device tree data structures.Signed-off-by G, Manjunath Kondaiah <[email protected]>Mostly looks good, but a few things that need to be fixed. You can probably even get this change merged for v3.1
Thanks. I will fix the review comments and we can have these changes with board-omapx-dt.c file so that dt and not dt builds can co exist.
For complete functionality with dt build, omap hwmod needs to be handled through DT framework. I am waiting for comments from omap hwmod maintainers.
Quoted 49 lines
Quoted 34 lines
--- drivers/i2c/busses/i2c-omap.c | 30 +++++++++++++++++++++++++++++- 1 files changed, 29 insertions(+), 1 deletions(-)drivers/i2c/busses/i2c-omap.c +5
-0@@ -38,9 +38,13 @@#include <linux/clk.h> #include <linux/io.h> #include <linux/of_i2c.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> #include <linux/slab.h> #include <linux/i2c-omap.h> #include <linux/pm_runtime.h> +#include <plat/i2c.h> /* I2C controller revisions */ #define OMAP_I2C_REV_2 0x20@@ -972,6 +976,7 @@ static const struct i2c_algorithm omap_i2c_algo = { .functionality = omap_i2c_func, }; +static const struct of_device_id omap_i2c_of_match[];static int __devinit omap_i2c_probe (struct platform_device *pdev) {@@ -979,10 +984,15 @@ omap_i2c_probe(struct platform_device *pdev)struct i2c_adapter *adap;struct resource *mem, *irq, *ioarea;drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 995
Quoted 10 lines
drivers/i2c/busses/i2c-omap.c +5
-0struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data; +const struct of_device_id *match;irq_handler_t isr;int r;u32 speed =0 ; + match =of_match_device (omap_i2c_of_match, &pdev->dev); +if (!match) +dev_err (&pdev->dev,"DT: i2c device match not found.......\n" ); +This isn't an error. It just mean that the device wasn't registered from the device tree, and it needs to get its configuration from pdata.
Quoted 2 lines
In fact, you don't even need this block since the driver never uses the match entry returned by
of_match_device()
ok. I will remove this.
Quoted 12 lines
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1010
Quoted 9 lines
drivers/i2c/busses/i2c-omap.c +0
-1/* NOTE: driver uses the static register mapping */ mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 );if (!mem) {@@ -1007,10 +1017,19 @@ omap_i2c_probe(struct platform_device *pdev) r = -ENOMEM ; goto err_release_region; } -Don't drop the empty line.
Quoted 3 lines
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1020
Quoted 1 line
drivers/i2c/busses/i2c-omap.c +1
-0+/* I2C device without DT might use this */ No need for the comment.
ok
Quoted 15 lines
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1031
Quoted 12 lines
drivers/i2c/busses/i2c-omap.c +8
-0if (pdata !=NULL ) { speed = pdata->clkrate; dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; + }else if (pdev->dev.of_node) { +const unsigned int *prop; + prop =of_get_property (pdev->dev.of_node, +"clock-frequency" ,NULL ); +if (prop) + speed =be32_to_cpup (prop); +else + speed =100 ;There is a new function that makes this easier. Do this instead:
Quoted 3 lines
speed = 100; if (of_property_read_u32(pdev->dev.of_node, "clock-frequency", &prop) speed = prop / 1000000; /* speed in Hz, this might be wrong */
These patches got merged after posting this series. I will take care of it in the next version.
BTW, I have posted seperate patch to use above API for code optimization.
Quoted 34 lines
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1032
Quoted 2 lines
drivers/i2c/busses/i2c-omap.c +1
-0+ dev->set_mpu_wkup_lat =NULL ;dev is kzalloced, which means
set_mpu_wkup_latis already NULL.drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1067
Quoted 8 lines
drivers/i2c/busses/i2c-omap.c +1
-0}else { speed =100 ;/* Default speed */ dev->set_mpu_wkup_lat =NULL ;@@ -1045,6 +1064,7 @@ omap_i2c_probe(struct platform_device *pdev) dev->rev =omap_i2c_read_reg (dev,OMAP_I2C_REV_REG ) &0xff ; +Drop the unrelated whitespace change.
drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1096
Quoted 8 lines
drivers/i2c/busses/i2c-omap.c +1
-0if (dev->rev <=OMAP_I2C_REV_ON_3430 ) dev->errata |=I2C_OMAP3_1P153 ;@@ -1073,6 +1093,7 @@ omap_i2c_probe(struct platform_device *pdev) (1000 * speed /8 ); } +Ditto.
Quoted 7 lines
drivers/i2c/busses/i2c-omap.c +0
-0/* reset ASAP, clearing any IRQs */ omap_i2c_init (dev);@@ -1162,6 +1183,12 @@ static int omap_i2c_resume(struct device *dev)return 0 ; }
Quoted 10 lines
drivers/i2c/busses/i2c-omap.c static int omap_i2c_resume(struct device *dev) · line 1191
Quoted 6 lines
drivers/i2c/busses/i2c-omap.c +6
-0+static const struct of_device_id omap_i2c_of_match[] = { + {.compatible ="ti,omap_i2c" , }, + {}, +} +MODULE_DEVICE_TABLE (of, omap_i2c_of_match); +This needs to be protected with #ifdef CONFIG_OF/#else/#endif. Don't break non-dt builds.
ok. Thanks.
-Manjunath
RFC PATCH 5/5 OMAP: I2C: Convert I2C driver to use device tree
Quoted 8 lines · Manjunath Kondaiah
Quoted 7 lines · Grant Likely
Quoted 5 lines · Manjunath Kondaiah
The OMAP I2C driver is modified to use
platform_devicedata from device tree data structures.Signed-off-by G, Manjunath Kondaiah <[email protected]>
Quoted 8 lines
Quoted 2 lines
Mostly looks good, but a few things that need to be fixed. You can probably even get this change merged for v3.1
Thanks. I will fix the review comments and we can have these changes with
board-omapx-dt.cfile so that dt and not dt builds can co exist.For complete functionality with dt build, omap hwmod needs to be handled through DT framework. I am waiting for comments from omap hwmod maintainers.
You should be able to sidestep this issue in the short term. As long as you can get the of_platform_populate() function to create omap_devices that look identical to the current statically allocated omap_devices. If you need to extend the auxdata structure to do so, then I'm okay with that. You could add special handling for devices compatible with "ti,omap3-device" or "ti,omap4-device".
I'm keen to see this done, and if you can put it together quickly, then I think I still have time to queue it up for v3.1 (especially since it would be a new block of code that should not break any existing boards).
g.