* [PATCH 0/4] dt: omap3: add device tree support @ 2011-07-13 22:06 G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers G, Manjunath Kondaiah ` (3 more replies) 0 siblings, 4 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-13 22:06 UTC (permalink / raw) To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r This patch series is for migrating omap3 beagle board file and to use device tree data structures in i2c-omap driver. These are basic changes in order to adopt device tree for using generic board file with omap3 platforms. Existing non dt setup and build is not disturbed and device tree boot needs to be explicitly enabled through menuconfig option by selecting CONFIG_OMAP3_DT Build and boot tested for both dt and not dt options on omap3 beagle board. Initial review comments and alignment can be found at: http://comments.gmane.org/gmane.linux.drivers.devicetree/6188 Baseline: Applies cleanly on top of: git://git.secretlab.ca/git/linux-2.6 Branch: origin/devicetree/test commit 88c559f5479ec28a1acd38c081d89f90a059a6ef Author: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> drivercore: Add driver probe deferral mechanism G, Manjunath Kondaiah (4): DT: OMAP3: Add SoC file for handling i2c controllers DT: OMAP3: Beagle board: set clock freq for i2c devices DT: OMAP3: Add generic board file for dt support DT: OMAP: Convert I2C driver to use device tree arch/arm/boot/dts/omap3-beagle-nunchuck.dts | 11 +- arch/arm/boot/dts/omap3-beagle.dts | 29 ++- arch/arm/boot/dts/omap3-soc.dtsi | 65 +++ arch/arm/mach-omap2/Kconfig | 12 + arch/arm/mach-omap2/Makefile | 2 + arch/arm/mach-omap2/board-omap3-dt.c | 623 +++++++++++++++++++++++++++ arch/arm/mach-omap2/board-omap3beagle.c | 13 - drivers/i2c/busses/i2c-omap.c | 48 ++- 8 files changed, 777 insertions(+), 26 deletions(-) create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c -- 1.7.4.1 ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers 2011-07-13 22:06 [PATCH 0/4] dt: omap3: add device tree support G, Manjunath Kondaiah @ 2011-07-13 22:06 ` G, Manjunath Kondaiah 2011-07-13 22:57 ` Grant Likely 2011-07-13 22:06 ` [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices G, Manjunath Kondaiah ` (2 subsequent siblings) 3 siblings, 1 reply; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-13 22:06 UTC (permalink / raw) To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Add omap3 SoC file for handling omap3 SoC i2c controllers existing on l4-core bus. Out of three i2c controllers, first i2c controller is interfaced with PMIC on all the boards of OMAP3. The clock for i2c controllers are controlled through omap hwmod framework hence first i2c controller device registration through dt is disabled till hwmod dependencies are resolved. Signed-off-by: G, Manjunath Kondaiah <manjugk-l0cyMroinI0@public.gmane.org> --- arch/arm/boot/dts/omap3-soc.dtsi | 65 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 65 insertions(+), 0 deletions(-) create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi new file mode 100644 index 0000000..f186a32 --- /dev/null +++ b/arch/arm/boot/dts/omap3-soc.dtsi @@ -0,0 +1,65 @@ +/* + * Device Tree Source for OMAP3 SoC + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * + * This file is licensed under the terms of the GNU General Public License + * version 2. This program is licensed "as is" without any warranty of any + * kind, whether express or implied. + */ + +/dts-v1/; +/include/ "skeleton.dtsi" + +/ { + #address-cells = <1>; + #size-cells = <1>; + model = "ti,omap3"; + compatible = "ti,omap3"; + + intc: interrupt-controller@0x48200000 { + compatible = "ti,omap3-intc", "arm,intc"; + interrupt-controller; + #interrupt-cells = <1>; + reg = <0x48200000 0x1000>; + }; + + l4-core { + compatible = "ti,l4-core"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x48000000 0x1000000>; + + i2c@1 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap3-i2c"; + reg = <0x70000 0x100>; + interrupts = < 88 >; + status = "disabled"; + }; + + i2c@2 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap3-i2c"; + reg = <0x72000 0x100>; + interrupts = < 89 >; + }; + + i2c@3 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "ti,omap3-i2c"; + reg = <0x60000 0x100>; + interrupts = < 93 >; + }; + }; + + l4-per { + compatible = "ti,l4-per"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x49000000 0x100000>; + }; +}; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers 2011-07-13 22:06 ` [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers G, Manjunath Kondaiah @ 2011-07-13 22:57 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 22:57 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > Add omap3 SoC file for handling omap3 SoC i2c controllers existing > on l4-core bus. > > Out of three i2c controllers, first i2c controller is interfaced with > PMIC on all the boards of OMAP3. The clock for i2c controllers are > controlled through omap hwmod framework hence first i2c controller > device registration through dt is disabled till hwmod dependencies > are resolved. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > arch/arm/boot/dts/omap3-soc.dtsi | 65 ++++++++++++++++++++++++++++++++++++++ > 1 files changed, 65 insertions(+), 0 deletions(-) > create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi > > diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi > new file mode 100644 > index 0000000..f186a32 > --- /dev/null > +++ b/arch/arm/boot/dts/omap3-soc.dtsi > @@ -0,0 +1,65 @@ > +/* > + * Device Tree Source for OMAP3 SoC > + * > + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > + * > + * This file is licensed under the terms of the GNU General Public License > + * version 2. This program is licensed "as is" without any warranty of any > + * kind, whether express or implied. > + */ > + > +/dts-v1/; > +/include/ "skeleton.dtsi" > + > +/ { > + #address-cells = <1>; > + #size-cells = <1>; > + model = "ti,omap3"; You can drop the model property here since this doesn't define a board. > + compatible = "ti,omap3"; > + > + intc: interrupt-controller@0x48200000 { > + compatible = "ti,omap3-intc", "arm,intc"; Which arm intc controller? For any new 'compatible' value you define, the patch needs to include documentation for it in Documentation/devicetree/bindings. > + interrupt-controller; > + #interrupt-cells = <1>; > + reg = <0x48200000 0x1000>; > + }; > + > + l4-core { > + compatible = "ti,l4-core"; Probably should be "ti,omap3-l4-core"? > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x48000000 0x1000000>; > + > + i2c@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,omap3-i2c"; > + reg = <0x70000 0x100>; > + interrupts = < 88 >; > + status = "disabled"; Drop the 'status' properties. I know the current tegra code does this, but I'd prefer devices to be enabled by default and for boards to explicitly disable them instead of the other way around. > + }; > + > + i2c@2 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,omap3-i2c"; > + reg = <0x72000 0x100>; > + interrupts = < 89 >; > + }; > + > + i2c@3 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,omap3-i2c"; > + reg = <0x60000 0x100>; > + interrupts = < 93 >; > + }; > + }; > + > + l4-per { > + compatible = "ti,l4-per"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0x49000000 0x100000>; > + }; > +}; > -- > 1.7.4.1 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers @ 2011-07-13 22:57 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 22:57 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > Add omap3 SoC file for handling omap3 SoC i2c controllers existing > on l4-core bus. > > Out of three i2c controllers, first i2c controller is interfaced with > PMIC on all the boards of OMAP3. The clock for i2c controllers are > controlled through omap hwmod framework hence first i2c controller > device registration through dt is disabled till hwmod dependencies > are resolved. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > ?arch/arm/boot/dts/omap3-soc.dtsi | ? 65 ++++++++++++++++++++++++++++++++++++++ > ?1 files changed, 65 insertions(+), 0 deletions(-) > ?create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi > > diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi > new file mode 100644 > index 0000000..f186a32 > --- /dev/null > +++ b/arch/arm/boot/dts/omap3-soc.dtsi > @@ -0,0 +1,65 @@ > +/* > + * Device Tree Source for OMAP3 SoC > + * > + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > + * > + * This file is licensed under the terms of the GNU General Public License > + * version 2. ?This program is licensed "as is" without any warranty of any > + * kind, whether express or implied. > + */ > + > +/dts-v1/; > +/include/ "skeleton.dtsi" > + > +/ { > + ? ? ? #address-cells = <1>; > + ? ? ? #size-cells = <1>; > + ? ? ? model = "ti,omap3"; You can drop the model property here since this doesn't define a board. > + ? ? ? compatible = "ti,omap3"; > + > + ? ? ? intc: interrupt-controller at 0x48200000 { > + ? ? ? ? ? ? ? compatible = "ti,omap3-intc", "arm,intc"; Which arm intc controller? For any new 'compatible' value you define, the patch needs to include documentation for it in Documentation/devicetree/bindings. > + ? ? ? ? ? ? ? interrupt-controller; > + ? ? ? ? ? ? ? #interrupt-cells = <1>; > + ? ? ? ? ? ? ? reg = <0x48200000 0x1000>; > + ? ? ? }; > + > + ? ? ? l4-core { > + ? ? ? ? ? ? ? compatible = "ti,l4-core"; Probably should be "ti,omap3-l4-core"? > + ? ? ? ? ? ? ? #address-cells = <1>; > + ? ? ? ? ? ? ? #size-cells = <1>; > + ? ? ? ? ? ? ? ranges = <0 0x48000000 0x1000000>; > + > + ? ? ? ? ? ? ? i2c at 1 { > + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; > + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; > + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; > + ? ? ? ? ? ? ? ? ? ? ? reg = <0x70000 0x100>; > + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 88 >; > + ? ? ? ? ? ? ? ? ? ? ? status = "disabled"; Drop the 'status' properties. I know the current tegra code does this, but I'd prefer devices to be enabled by default and for boards to explicitly disable them instead of the other way around. > + ? ? ? ? ? ? ? }; > + > + ? ? ? ? ? ? ? i2c at 2 { > + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; > + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; > + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; > + ? ? ? ? ? ? ? ? ? ? ? reg = <0x72000 0x100>; > + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 89 >; > + ? ? ? ? ? ? ? }; > + > + ? ? ? ? ? ? ? i2c at 3 { > + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; > + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; > + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; > + ? ? ? ? ? ? ? ? ? ? ? reg = <0x60000 0x100>; > + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 93 >; > + ? ? ? ? ? ? ? }; > + ? ? ? }; > + > + ? ? ? l4-per { > + ? ? ? ? ? ? ? compatible = "ti,l4-per"; > + ? ? ? ? ? ? ? #address-cells = <1>; > + ? ? ? ? ? ? ? #size-cells = <1>; > + ? ? ? ? ? ? ? ranges = <0 0x49000000 0x100000>; > + ? ? ? }; > +}; > -- > 1.7.4.1 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers 2011-07-13 22:57 ` Grant Likely @ 2011-07-13 22:58 ` Grant Likely -1 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 22:58 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 7:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> >> Add omap3 SoC file for handling omap3 SoC i2c controllers existing >> on l4-core bus. >> >> Out of three i2c controllers, first i2c controller is interfaced with >> PMIC on all the boards of OMAP3. The clock for i2c controllers are >> controlled through omap hwmod framework hence first i2c controller >> device registration through dt is disabled till hwmod dependencies >> are resolved. >> >> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> >> --- >> arch/arm/boot/dts/omap3-soc.dtsi | 65 ++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 65 insertions(+), 0 deletions(-) >> create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi >> >> diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi >> new file mode 100644 >> index 0000000..f186a32 >> --- /dev/null >> +++ b/arch/arm/boot/dts/omap3-soc.dtsi >> @@ -0,0 +1,65 @@ >> +/* >> + * Device Tree Source for OMAP3 SoC >> + * >> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ >> + * >> + * This file is licensed under the terms of the GNU General Public License >> + * version 2. This program is licensed "as is" without any warranty of any >> + * kind, whether express or implied. >> + */ >> + >> +/dts-v1/; >> +/include/ "skeleton.dtsi" >> + >> +/ { >> + #address-cells = <1>; >> + #size-cells = <1>; >> + model = "ti,omap3"; > > You can drop the model property here since this doesn't define a board. > >> + compatible = "ti,omap3"; >> + >> + intc: interrupt-controller@0x48200000 { >> + compatible = "ti,omap3-intc", "arm,intc"; > > Which arm intc controller? For any new 'compatible' value you define, > the patch needs to include documentation for it in > Documentation/devicetree/bindings. > >> + interrupt-controller; >> + #interrupt-cells = <1>; >> + reg = <0x48200000 0x1000>; >> + }; >> + >> + l4-core { >> + compatible = "ti,l4-core"; > > Probably should be "ti,omap3-l4-core"? > >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges = <0 0x48000000 0x1000000>; >> + >> + i2c@1 { This name is wrong. The address portion of the name must match the address in the first entry of the 'reg' property. So, that would be: "i2c@70000". >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "ti,omap3-i2c"; >> + reg = <0x70000 0x100>; >> + interrupts = < 88 >; >> + status = "disabled"; > > Drop the 'status' properties. I know the current tegra code does > this, but I'd prefer devices to be enabled by default and for boards > to explicitly disable them instead of the other way around. > >> + }; >> + >> + i2c@2 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "ti,omap3-i2c"; >> + reg = <0x72000 0x100>; >> + interrupts = < 89 >; >> + }; >> + >> + i2c@3 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "ti,omap3-i2c"; >> + reg = <0x60000 0x100>; >> + interrupts = < 93 >; >> + }; >> + }; >> + >> + l4-per { >> + compatible = "ti,l4-per"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges = <0 0x49000000 0x100000>; >> + }; >> +}; >> -- >> 1.7.4.1 >> >> > > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers @ 2011-07-13 22:58 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 22:58 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 7:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> >> Add omap3 SoC file for handling omap3 SoC i2c controllers existing >> on l4-core bus. >> >> Out of three i2c controllers, first i2c controller is interfaced with >> PMIC on all the boards of OMAP3. The clock for i2c controllers are >> controlled through omap hwmod framework hence first i2c controller >> device registration through dt is disabled till hwmod dependencies >> are resolved. >> >> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> >> --- >> ?arch/arm/boot/dts/omap3-soc.dtsi | ? 65 ++++++++++++++++++++++++++++++++++++++ >> ?1 files changed, 65 insertions(+), 0 deletions(-) >> ?create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi >> >> diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi >> new file mode 100644 >> index 0000000..f186a32 >> --- /dev/null >> +++ b/arch/arm/boot/dts/omap3-soc.dtsi >> @@ -0,0 +1,65 @@ >> +/* >> + * Device Tree Source for OMAP3 SoC >> + * >> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ >> + * >> + * This file is licensed under the terms of the GNU General Public License >> + * version 2. ?This program is licensed "as is" without any warranty of any >> + * kind, whether express or implied. >> + */ >> + >> +/dts-v1/; >> +/include/ "skeleton.dtsi" >> + >> +/ { >> + ? ? ? #address-cells = <1>; >> + ? ? ? #size-cells = <1>; >> + ? ? ? model = "ti,omap3"; > > You can drop the model property here since this doesn't define a board. > >> + ? ? ? compatible = "ti,omap3"; >> + >> + ? ? ? intc: interrupt-controller at 0x48200000 { >> + ? ? ? ? ? ? ? compatible = "ti,omap3-intc", "arm,intc"; > > Which arm intc controller? ?For any new 'compatible' value you define, > the patch needs to include documentation for it in > Documentation/devicetree/bindings. > >> + ? ? ? ? ? ? ? interrupt-controller; >> + ? ? ? ? ? ? ? #interrupt-cells = <1>; >> + ? ? ? ? ? ? ? reg = <0x48200000 0x1000>; >> + ? ? ? }; >> + >> + ? ? ? l4-core { >> + ? ? ? ? ? ? ? compatible = "ti,l4-core"; > > Probably should be "ti,omap3-l4-core"? > >> + ? ? ? ? ? ? ? #address-cells = <1>; >> + ? ? ? ? ? ? ? #size-cells = <1>; >> + ? ? ? ? ? ? ? ranges = <0 0x48000000 0x1000000>; >> + >> + ? ? ? ? ? ? ? i2c at 1 { This name is wrong. The address portion of the name must match the address in the first entry of the 'reg' property. So, that would be: "i2c at 70000". >> + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; >> + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; >> + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; >> + ? ? ? ? ? ? ? ? ? ? ? reg = <0x70000 0x100>; >> + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 88 >; >> + ? ? ? ? ? ? ? ? ? ? ? status = "disabled"; > > Drop the 'status' properties. ?I know the current tegra code does > this, but I'd prefer devices to be enabled by default and for boards > to explicitly disable them instead of the other way around. > >> + ? ? ? ? ? ? ? }; >> + >> + ? ? ? ? ? ? ? i2c at 2 { >> + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; >> + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; >> + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; >> + ? ? ? ? ? ? ? ? ? ? ? reg = <0x72000 0x100>; >> + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 89 >; >> + ? ? ? ? ? ? ? }; >> + >> + ? ? ? ? ? ? ? i2c at 3 { >> + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; >> + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; >> + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; >> + ? ? ? ? ? ? ? ? ? ? ? reg = <0x60000 0x100>; >> + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 93 >; >> + ? ? ? ? ? ? ? }; >> + ? ? ? }; >> + >> + ? ? ? l4-per { >> + ? ? ? ? ? ? ? compatible = "ti,l4-per"; >> + ? ? ? ? ? ? ? #address-cells = <1>; >> + ? ? ? ? ? ? ? #size-cells = <1>; >> + ? ? ? ? ? ? ? ranges = <0 0x49000000 0x100000>; >> + ? ? ? }; >> +}; >> -- >> 1.7.4.1 >> >> > > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers 2011-07-13 22:58 ` Grant Likely @ 2011-07-14 3:51 ` G, Manjunath Kondaiah -1 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-14 3:51 UTC (permalink / raw) To: Grant Likely; +Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 07:58:35AM +0900, Grant Likely wrote: > On Thu, Jul 14, 2011 at 7:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > >> > >> Add omap3 SoC file for handling omap3 SoC i2c controllers existing > >> on l4-core bus. > >> > >> Out of three i2c controllers, first i2c controller is interfaced with > >> PMIC on all the boards of OMAP3. The clock for i2c controllers are > >> controlled through omap hwmod framework hence first i2c controller > >> device registration through dt is disabled till hwmod dependencies > >> are resolved. > >> > >> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > >> --- > >> arch/arm/boot/dts/omap3-soc.dtsi | 65 ++++++++++++++++++++++++++++++++++++++ > >> 1 files changed, 65 insertions(+), 0 deletions(-) > >> create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi > >> > >> diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi > >> new file mode 100644 > >> index 0000000..f186a32 > >> --- /dev/null > >> +++ b/arch/arm/boot/dts/omap3-soc.dtsi > >> @@ -0,0 +1,65 @@ > >> +/* > >> + * Device Tree Source for OMAP3 SoC > >> + * > >> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > >> + * > >> + * This file is licensed under the terms of the GNU General Public License > >> + * version 2. This program is licensed "as is" without any warranty of any > >> + * kind, whether express or implied. > >> + */ > >> + > >> +/dts-v1/; > >> +/include/ "skeleton.dtsi" > >> + > >> +/ { > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + model = "ti,omap3"; > > > > You can drop the model property here since this doesn't define a board. > > > >> + compatible = "ti,omap3"; > >> + > >> + intc: interrupt-controller@0x48200000 { > >> + compatible = "ti,omap3-intc", "arm,intc"; > > > > Which arm intc controller? For any new 'compatible' value you define, > > the patch needs to include documentation for it in > > Documentation/devicetree/bindings. > > > >> + interrupt-controller; > >> + #interrupt-cells = <1>; > >> + reg = <0x48200000 0x1000>; > >> + }; > >> + > >> + l4-core { > >> + compatible = "ti,l4-core"; > > > > Probably should be "ti,omap3-l4-core"? > > > >> + #address-cells = <1>; > >> + #size-cells = <1>; > >> + ranges = <0 0x48000000 0x1000000>; > >> + > >> + i2c@1 { > > This name is wrong. The address portion of the name must match the > address in the first entry of the 'reg' property. So, that would be: > "i2c@70000". > > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + compatible = "ti,omap3-i2c"; > >> + reg = <0x70000 0x100>; > >> + interrupts = < 88 >; > >> + status = "disabled"; > > > > Drop the 'status' properties. I know the current tegra code does > > this, but I'd prefer devices to be enabled by default and for boards > > to explicitly disable them instead of the other way around. ok. I will use the convention. -Manjunath -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers @ 2011-07-14 3:51 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-14 3:51 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 07:58:35AM +0900, Grant Likely wrote: > On Thu, Jul 14, 2011 at 7:57 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > >> > >> Add omap3 SoC file for handling omap3 SoC i2c controllers existing > >> on l4-core bus. > >> > >> Out of three i2c controllers, first i2c controller is interfaced with > >> PMIC on all the boards of OMAP3. The clock for i2c controllers are > >> controlled through omap hwmod framework hence first i2c controller > >> device registration through dt is disabled till hwmod dependencies > >> are resolved. > >> > >> Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > >> --- > >> ?arch/arm/boot/dts/omap3-soc.dtsi | ? 65 ++++++++++++++++++++++++++++++++++++++ > >> ?1 files changed, 65 insertions(+), 0 deletions(-) > >> ?create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi > >> > >> diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi > >> new file mode 100644 > >> index 0000000..f186a32 > >> --- /dev/null > >> +++ b/arch/arm/boot/dts/omap3-soc.dtsi > >> @@ -0,0 +1,65 @@ > >> +/* > >> + * Device Tree Source for OMAP3 SoC > >> + * > >> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > >> + * > >> + * This file is licensed under the terms of the GNU General Public License > >> + * version 2. ?This program is licensed "as is" without any warranty of any > >> + * kind, whether express or implied. > >> + */ > >> + > >> +/dts-v1/; > >> +/include/ "skeleton.dtsi" > >> + > >> +/ { > >> + ? ? ? #address-cells = <1>; > >> + ? ? ? #size-cells = <1>; > >> + ? ? ? model = "ti,omap3"; > > > > You can drop the model property here since this doesn't define a board. > > > >> + ? ? ? compatible = "ti,omap3"; > >> + > >> + ? ? ? intc: interrupt-controller at 0x48200000 { > >> + ? ? ? ? ? ? ? compatible = "ti,omap3-intc", "arm,intc"; > > > > Which arm intc controller? ?For any new 'compatible' value you define, > > the patch needs to include documentation for it in > > Documentation/devicetree/bindings. > > > >> + ? ? ? ? ? ? ? interrupt-controller; > >> + ? ? ? ? ? ? ? #interrupt-cells = <1>; > >> + ? ? ? ? ? ? ? reg = <0x48200000 0x1000>; > >> + ? ? ? }; > >> + > >> + ? ? ? l4-core { > >> + ? ? ? ? ? ? ? compatible = "ti,l4-core"; > > > > Probably should be "ti,omap3-l4-core"? > > > >> + ? ? ? ? ? ? ? #address-cells = <1>; > >> + ? ? ? ? ? ? ? #size-cells = <1>; > >> + ? ? ? ? ? ? ? ranges = <0 0x48000000 0x1000000>; > >> + > >> + ? ? ? ? ? ? ? i2c at 1 { > > This name is wrong. The address portion of the name must match the > address in the first entry of the 'reg' property. So, that would be: > "i2c at 70000". > > >> + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; > >> + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; > >> + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; > >> + ? ? ? ? ? ? ? ? ? ? ? reg = <0x70000 0x100>; > >> + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 88 >; > >> + ? ? ? ? ? ? ? ? ? ? ? status = "disabled"; > > > > Drop the 'status' properties. ?I know the current tegra code does > > this, but I'd prefer devices to be enabled by default and for boards > > to explicitly disable them instead of the other way around. ok. I will use the convention. -Manjunath ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers 2011-07-13 22:57 ` Grant Likely @ 2011-07-14 3:34 ` G, Manjunath Kondaiah -1 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-14 3:34 UTC (permalink / raw) To: Grant Likely; +Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 07:57:16AM +0900, Grant Likely wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > > > Add omap3 SoC file for handling omap3 SoC i2c controllers existing > > on l4-core bus. > > > > Out of three i2c controllers, first i2c controller is interfaced with > > PMIC on all the boards of OMAP3. The clock for i2c controllers are > > controlled through omap hwmod framework hence first i2c controller > > device registration through dt is disabled till hwmod dependencies > > are resolved. > > > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > > --- > > arch/arm/boot/dts/omap3-soc.dtsi | 65 ++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 65 insertions(+), 0 deletions(-) > > create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi > > > > diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi > > new file mode 100644 > > index 0000000..f186a32 > > --- /dev/null > > +++ b/arch/arm/boot/dts/omap3-soc.dtsi > > @@ -0,0 +1,65 @@ > > +/* > > + * Device Tree Source for OMAP3 SoC > > + * > > + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > > + * > > + * This file is licensed under the terms of the GNU General Public License > > + * version 2. This program is licensed "as is" without any warranty of any > > + * kind, whether express or implied. > > + */ > > + > > +/dts-v1/; > > +/include/ "skeleton.dtsi" > > + > > +/ { > > + #address-cells = <1>; > > + #size-cells = <1>; > > + model = "ti,omap3"; > > You can drop the model property here since this doesn't define a board. ok > > > + compatible = "ti,omap3"; > > + > > + intc: interrupt-controller@0x48200000 { > > + compatible = "ti,omap3-intc", "arm,intc"; > > Which arm intc controller? For any new 'compatible' value you define, > the patch needs to include documentation for it in > Documentation/devicetree/bindings. for time being, I can drop this and introduce later with documentation. > > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + reg = <0x48200000 0x1000>; > > + }; > > + > > + l4-core { > > + compatible = "ti,l4-core"; > > Probably should be "ti,omap3-l4-core"? ok. > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges = <0 0x48000000 0x1000000>; > > + > > + i2c@1 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + compatible = "ti,omap3-i2c"; > > + reg = <0x70000 0x100>; > > + interrupts = < 88 >; > > + status = "disabled"; > > Drop the 'status' properties. I know the current tegra code does > this, but I'd prefer devices to be enabled by default and for boards > to explicitly disable them instead of the other way around. wanted to disable i2c1 by default. i will remove. -Manjunath -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers @ 2011-07-14 3:34 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-14 3:34 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 07:57:16AM +0900, Grant Likely wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > > > Add omap3 SoC file for handling omap3 SoC i2c controllers existing > > on l4-core bus. > > > > Out of three i2c controllers, first i2c controller is interfaced with > > PMIC on all the boards of OMAP3. The clock for i2c controllers are > > controlled through omap hwmod framework hence first i2c controller > > device registration through dt is disabled till hwmod dependencies > > are resolved. > > > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > > --- > > ?arch/arm/boot/dts/omap3-soc.dtsi | ? 65 ++++++++++++++++++++++++++++++++++++++ > > ?1 files changed, 65 insertions(+), 0 deletions(-) > > ?create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi > > > > diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi > > new file mode 100644 > > index 0000000..f186a32 > > --- /dev/null > > +++ b/arch/arm/boot/dts/omap3-soc.dtsi > > @@ -0,0 +1,65 @@ > > +/* > > + * Device Tree Source for OMAP3 SoC > > + * > > + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ > > + * > > + * This file is licensed under the terms of the GNU General Public License > > + * version 2. ?This program is licensed "as is" without any warranty of any > > + * kind, whether express or implied. > > + */ > > + > > +/dts-v1/; > > +/include/ "skeleton.dtsi" > > + > > +/ { > > + ? ? ? #address-cells = <1>; > > + ? ? ? #size-cells = <1>; > > + ? ? ? model = "ti,omap3"; > > You can drop the model property here since this doesn't define a board. ok > > > + ? ? ? compatible = "ti,omap3"; > > + > > + ? ? ? intc: interrupt-controller at 0x48200000 { > > + ? ? ? ? ? ? ? compatible = "ti,omap3-intc", "arm,intc"; > > Which arm intc controller? For any new 'compatible' value you define, > the patch needs to include documentation for it in > Documentation/devicetree/bindings. for time being, I can drop this and introduce later with documentation. > > > + ? ? ? ? ? ? ? interrupt-controller; > > + ? ? ? ? ? ? ? #interrupt-cells = <1>; > > + ? ? ? ? ? ? ? reg = <0x48200000 0x1000>; > > + ? ? ? }; > > + > > + ? ? ? l4-core { > > + ? ? ? ? ? ? ? compatible = "ti,l4-core"; > > Probably should be "ti,omap3-l4-core"? ok. > > > + ? ? ? ? ? ? ? #address-cells = <1>; > > + ? ? ? ? ? ? ? #size-cells = <1>; > > + ? ? ? ? ? ? ? ranges = <0 0x48000000 0x1000000>; > > + > > + ? ? ? ? ? ? ? i2c at 1 { > > + ? ? ? ? ? ? ? ? ? ? ? #address-cells = <1>; > > + ? ? ? ? ? ? ? ? ? ? ? #size-cells = <0>; > > + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; > > + ? ? ? ? ? ? ? ? ? ? ? reg = <0x70000 0x100>; > > + ? ? ? ? ? ? ? ? ? ? ? interrupts = < 88 >; > > + ? ? ? ? ? ? ? ? ? ? ? status = "disabled"; > > Drop the 'status' properties. I know the current tegra code does > this, but I'd prefer devices to be enabled by default and for boards > to explicitly disable them instead of the other way around. wanted to disable i2c1 by default. i will remove. -Manjunath ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices 2011-07-13 22:06 [PATCH 0/4] dt: omap3: add device tree support G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers G, Manjunath Kondaiah @ 2011-07-13 22:06 ` G, Manjunath Kondaiah 2011-07-13 23:04 ` Grant Likely 2011-07-13 22:06 ` [PATCH 3/4] dt: omap3: add generic board file for dt support G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree G, Manjunath Kondaiah 3 siblings, 1 reply; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-13 22:06 UTC (permalink / raw) To: devicetree-discuss; +Cc: linux-arm-kernel, linux-omap, grant.likely, ben-linux Update omap3 beagle dts file with required clock frequencies for the i2c client devices existing on beagle board. Beagle custom board dts file is cleaned up so that it can coexist with omap3 soc dts file. Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> --- arch/arm/boot/dts/omap3-beagle-nunchuck.dts | 11 +--------- arch/arm/boot/dts/omap3-beagle.dts | 29 +++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/arch/arm/boot/dts/omap3-beagle-nunchuck.dts b/arch/arm/boot/dts/omap3-beagle-nunchuck.dts index 2607be5..324ff86 100644 --- a/arch/arm/boot/dts/omap3-beagle-nunchuck.dts +++ b/arch/arm/boot/dts/omap3-beagle-nunchuck.dts @@ -1,16 +1,7 @@ /include/ "omap3-beagle.dts" / { - i2c@48072000 { - compatible = "ti,omap3-i2c"; - reg = <0x48072000 0x80>; - #address-cells = <1>; - #size-cells = <0>; - - eeprom@50 { - compatible = "at,at24c01"; - reg = < 0x50 >; - }; + i2c@2 { joystick@52 { compatible = "sparkfun,wiichuck"; reg = < 0x52 >; diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index 4439466..b45fbc9 100644 --- a/arch/arm/boot/dts/omap3-beagle.dts +++ b/arch/arm/boot/dts/omap3-beagle.dts @@ -1,7 +1,32 @@ -/dts-v1/; -/include/ "skeleton.dtsi" +/include/ "omap3-soc.dtsi" / { model = "TI OMAP3 BeagleBoard"; compatible = "ti,omap3-beagle"; + + i2c@1 { + #address-cells = <1>; + #size-cells = <0>; + clock-frequency = <2600000>; + status = "disabled"; + }; + + i2c@2 { + #address-cells = <1>; + #size-cells = <0>; + clock-frequency = <400000>; + }; + + i2c@3 { + #address-cells = <1>; + #size-cells = <0>; + clock-frequency = <400000>; + + eeprom@50 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "at,at24c01"; + reg = < 0x50 >; + }; + }; }; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices 2011-07-13 22:06 ` [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices G, Manjunath Kondaiah @ 2011-07-13 23:04 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 23:04 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > Update omap3 beagle dts file with required clock frequencies for the i2c > client devices existing on beagle board. > > Beagle custom board dts file is cleaned up so that it can coexist with omap3 > soc dts file. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > arch/arm/boot/dts/omap3-beagle-nunchuck.dts | 11 +--------- > arch/arm/boot/dts/omap3-beagle.dts | 29 +++++++++++++++++++++++++- > 2 files changed, 28 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/boot/dts/omap3-beagle-nunchuck.dts b/arch/arm/boot/dts/omap3-beagle-nunchuck.dts > index 2607be5..324ff86 100644 > --- a/arch/arm/boot/dts/omap3-beagle-nunchuck.dts > +++ b/arch/arm/boot/dts/omap3-beagle-nunchuck.dts > @@ -1,16 +1,7 @@ > /include/ "omap3-beagle.dts" > > / { > - i2c@48072000 { > - compatible = "ti,omap3-i2c"; > - reg = <0x48072000 0x80>; > - #address-cells = <1>; > - #size-cells = <0>; > - > - eeprom@50 { > - compatible = "at,at24c01"; > - reg = < 0x50 >; > - }; > + i2c@2 { > joystick@52 { > compatible = "sparkfun,wiichuck"; > reg = < 0x52 >; > diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts > index 4439466..b45fbc9 100644 > --- a/arch/arm/boot/dts/omap3-beagle.dts > +++ b/arch/arm/boot/dts/omap3-beagle.dts > @@ -1,7 +1,32 @@ > -/dts-v1/; > -/include/ "skeleton.dtsi" > +/include/ "omap3-soc.dtsi" > > / { > model = "TI OMAP3 BeagleBoard"; > compatible = "ti,omap3-beagle"; compatible = "ti,omap3-beagle", "ti,omap3"; > + > + i2c@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + clock-frequency = <2600000>; > + status = "disabled"; > + }; Since this file include the omap3-soc dtsi, you don't need to duplicate the #address-cells and #size-cells properties. You only need to specify the ones that have changed. Also, since the include file has the i2c nodes as children of the l4 bus, you should you need to either duplicate the same bus hierarchy here, or use an independent label reference by adding a label to the node in omap3-soc.dtsi, and using the following syntax /outside/ of the /{ ... } root node: &i2c-1 { clock-frequency = <2600000>; }; g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices @ 2011-07-13 23:04 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 23:04 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > Update omap3 beagle dts file with required clock frequencies for the i2c > client devices existing on beagle board. > > Beagle custom board dts file is cleaned up so that it can coexist with omap3 > soc dts file. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > ?arch/arm/boot/dts/omap3-beagle-nunchuck.dts | ? 11 +--------- > ?arch/arm/boot/dts/omap3-beagle.dts ? ? ? ? ?| ? 29 +++++++++++++++++++++++++- > ?2 files changed, 28 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/boot/dts/omap3-beagle-nunchuck.dts b/arch/arm/boot/dts/omap3-beagle-nunchuck.dts > index 2607be5..324ff86 100644 > --- a/arch/arm/boot/dts/omap3-beagle-nunchuck.dts > +++ b/arch/arm/boot/dts/omap3-beagle-nunchuck.dts > @@ -1,16 +1,7 @@ > ?/include/ "omap3-beagle.dts" > > ?/ { > - ? ? ? i2c at 48072000 { > - ? ? ? ? ? ? ? compatible = "ti,omap3-i2c"; > - ? ? ? ? ? ? ? reg = <0x48072000 0x80>; > - ? ? ? ? ? ? ? #address-cells = <1>; > - ? ? ? ? ? ? ? #size-cells = <0>; > - > - ? ? ? ? ? ? ? eeprom at 50 { > - ? ? ? ? ? ? ? ? ? ? ? compatible = "at,at24c01"; > - ? ? ? ? ? ? ? ? ? ? ? reg = < 0x50 >; > - ? ? ? ? ? ? ? }; > + ? ? ? i2c at 2 { > ? ? ? ? ? ? ? ?joystick at 52 { > ? ? ? ? ? ? ? ? ? ? ? ?compatible = "sparkfun,wiichuck"; > ? ? ? ? ? ? ? ? ? ? ? ?reg = < 0x52 >; > diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts > index 4439466..b45fbc9 100644 > --- a/arch/arm/boot/dts/omap3-beagle.dts > +++ b/arch/arm/boot/dts/omap3-beagle.dts > @@ -1,7 +1,32 @@ > -/dts-v1/; > -/include/ "skeleton.dtsi" > +/include/ "omap3-soc.dtsi" > > ?/ { > ? ? ? ?model = "TI OMAP3 BeagleBoard"; > ? ? ? ?compatible = "ti,omap3-beagle"; compatible = "ti,omap3-beagle", "ti,omap3"; > + > + ? ? ? i2c at 1 { > + ? ? ? ? ? ? ? #address-cells = <1>; > + ? ? ? ? ? ? ? #size-cells = <0>; > + ? ? ? ? ? ? ? clock-frequency = <2600000>; > + ? ? ? ? ? ? ? status = "disabled"; > + ? ? ? }; Since this file include the omap3-soc dtsi, you don't need to duplicate the #address-cells and #size-cells properties. You only need to specify the ones that have changed. Also, since the include file has the i2c nodes as children of the l4 bus, you should you need to either duplicate the same bus hierarchy here, or use an independent label reference by adding a label to the node in omap3-soc.dtsi, and using the following syntax /outside/ of the /{ ... } root node: &i2c-1 { clock-frequency = <2600000>; }; g. ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-13 22:06 [PATCH 0/4] dt: omap3: add device tree support G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices G, Manjunath Kondaiah @ 2011-07-13 22:06 ` G, Manjunath Kondaiah 2011-07-13 23:15 ` Grant Likely 2011-07-13 22:06 ` [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree G, Manjunath Kondaiah 3 siblings, 1 reply; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-13 22:06 UTC (permalink / raw) To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r The generic board file is created and derived from beagle board file. The beagle board file is migrated to boot from flattened device tree and the cleanup of generic board file will happen in different stages. The changes here focus on i2c device registration through dt and upcoming patches in the series will adopt i2c driver to use dt registered i2c devices. Signed-off-by: G, Manjunath Kondaiah <manjugk-l0cyMroinI0@public.gmane.org> --- arch/arm/mach-omap2/Kconfig | 12 + arch/arm/mach-omap2/Makefile | 2 + arch/arm/mach-omap2/board-omap3-dt.c | 623 +++++++++++++++++++++++++++++++ arch/arm/mach-omap2/board-omap3beagle.c | 13 - 4 files changed, 637 insertions(+), 13 deletions(-) create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index 19d5891..174f6d1 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP default y select OMAP_PACKAGE_CBP +config MACH_OMAP3_DT + bool "Generic OMAP3 board(FDT support)" + depends on ARCH_OMAP3 + select OMAP_PACKAGE_CBB + select USE_OF + select PROC_DEVICETREE + + help + Support for generic TI OMAP3 boards using Flattened Device Tree. + Say Y here to enable OMAP3 device tree support + More information at Documentation/devicetree + config MACH_TI8168EVM bool "TI8168 Evaluation Module" depends on SOC_OMAPTI816X diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index b148077..86e66f7 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -178,6 +178,8 @@ obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ hsmmc.o obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o +obj-$(CONFIG_MACH_OMAP3_DT) += board-omap3-dt.o \ + hsmmc.o obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ hsmmc.o obj-$(CONFIG_MACH_DEVKIT8000) += board-devkit8000.o \ diff --git a/arch/arm/mach-omap2/board-omap3-dt.c b/arch/arm/mach-omap2/board-omap3-dt.c new file mode 100644 index 0000000..14ee798 --- /dev/null +++ b/arch/arm/mach-omap2/board-omap3-dt.c @@ -0,0 +1,623 @@ +/* + * TI OMAP3 device tree board support + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/platform_device.h> +#include <linux/delay.h> +#include <linux/err.h> +#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> +#include <linux/gpio_keys.h> +#include <linux/opp.h> + +#include <linux/mtd/mtd.h> +#include <linux/mtd/partitions.h> +#include <linux/mtd/nand.h> +#include <linux/mmc/host.h> + +#include <linux/regulator/machine.h> +#include <linux/i2c/twl.h> +#include <linux/irq.h> + +#include <mach/hardware.h> +#include <asm/mach-types.h> +#include <asm/mach/arch.h> +#include <asm/mach/map.h> +#include <asm/mach/flash.h> + +#include <plat/board.h> +#include <plat/common.h> +#include <video/omapdss.h> +#include <video/omap-panel-generic-dpi.h> +#include <plat/gpmc.h> +#include <plat/nand.h> +#include <plat/usb.h> +#include <plat/omap_device.h> + +#include "mux.h" +#include "hsmmc.h" +#include "timer-gp.h" +#include "pm.h" +#include "common-board-devices.h" + +/* + * OMAP3 Beagle revision + * Run time detection of Beagle revision is done by reading GPIO. + * GPIO ID - + * AXBX = GPIO173, GPIO172, GPIO171: 1 1 1 + * C1_3 = GPIO173, GPIO172, GPIO171: 1 1 0 + * C4 = GPIO173, GPIO172, GPIO171: 1 0 1 + * XM = GPIO173, GPIO172, GPIO171: 0 0 0 + */ +enum { + OMAP3BEAGLE_BOARD_UNKN = 0, + OMAP3BEAGLE_BOARD_AXBX, + OMAP3BEAGLE_BOARD_C1_3, + OMAP3BEAGLE_BOARD_C4, + OMAP3BEAGLE_BOARD_XM, +}; + +static u8 omap3_beagle_version; + +static u8 omap3_beagle_get_rev(void) +{ + return omap3_beagle_version; +} + +static struct gpio omap3_beagle_rev_gpios[] __initdata = { + { 171, GPIOF_IN, "rev_id_0" }, + { 172, GPIOF_IN, "rev_id_1" }, + { 173, GPIOF_IN, "rev_id_2" }, +}; + +static void __init omap3_beagle_init_rev(void) +{ + int ret; + u16 beagle_rev = 0; + + omap_mux_init_gpio(171, OMAP_PIN_INPUT_PULLUP); + omap_mux_init_gpio(172, OMAP_PIN_INPUT_PULLUP); + omap_mux_init_gpio(173, OMAP_PIN_INPUT_PULLUP); + + ret = gpio_request_array(omap3_beagle_rev_gpios, + ARRAY_SIZE(omap3_beagle_rev_gpios)); + if (ret < 0) { + printk(KERN_ERR "Unable to get revision detection GPIO pins\n"); + omap3_beagle_version = OMAP3BEAGLE_BOARD_UNKN; + return; + } + + beagle_rev = gpio_get_value(171) | (gpio_get_value(172) << 1) + | (gpio_get_value(173) << 2); + + gpio_free_array(omap3_beagle_rev_gpios, + ARRAY_SIZE(omap3_beagle_rev_gpios)); + + switch (beagle_rev) { + case 7: + printk(KERN_INFO "OMAP3 Beagle Rev: Ax/Bx\n"); + omap3_beagle_version = OMAP3BEAGLE_BOARD_AXBX; + break; + case 6: + printk(KERN_INFO "OMAP3 Beagle Rev: C1/C2/C3\n"); + omap3_beagle_version = OMAP3BEAGLE_BOARD_C1_3; + break; + case 5: + printk(KERN_INFO "OMAP3 Beagle Rev: C4\n"); + omap3_beagle_version = OMAP3BEAGLE_BOARD_C4; + break; + case 0: + printk(KERN_INFO "OMAP3 Beagle Rev: xM\n"); + omap3_beagle_version = OMAP3BEAGLE_BOARD_XM; + break; + default: + printk(KERN_INFO "OMAP3 Beagle Rev: unknown %hd\n", beagle_rev); + omap3_beagle_version = OMAP3BEAGLE_BOARD_UNKN; + } +} + +static struct mtd_partition omap3beagle_nand_partitions[] = { + /* All the partition sizes are listed in terms of NAND block size */ + { + .name = "X-Loader", + .offset = 0, + .size = 4 * NAND_BLOCK_SIZE, + .mask_flags = MTD_WRITEABLE, /* force read-only */ + }, + { + .name = "U-Boot", + .offset = MTDPART_OFS_APPEND, /* Offset = 0x80000 */ + .size = 15 * NAND_BLOCK_SIZE, + .mask_flags = MTD_WRITEABLE, /* force read-only */ + }, + { + .name = "U-Boot Env", + .offset = MTDPART_OFS_APPEND, /* Offset = 0x260000 */ + .size = 1 * NAND_BLOCK_SIZE, + }, + { + .name = "Kernel", + .offset = MTDPART_OFS_APPEND, /* Offset = 0x280000 */ + .size = 32 * NAND_BLOCK_SIZE, + }, + { + .name = "File System", + .offset = MTDPART_OFS_APPEND, /* Offset = 0x680000 */ + .size = MTDPART_SIZ_FULL, + }, +}; + +/* DSS */ + +static int beagle_enable_dvi(struct omap_dss_device *dssdev) +{ + if (gpio_is_valid(dssdev->reset_gpio)) + gpio_set_value(dssdev->reset_gpio, 1); + + return 0; +} + +static void beagle_disable_dvi(struct omap_dss_device *dssdev) +{ + if (gpio_is_valid(dssdev->reset_gpio)) + gpio_set_value(dssdev->reset_gpio, 0); +} + +static struct panel_generic_dpi_data dvi_panel = { + .name = "generic", + .platform_enable = beagle_enable_dvi, + .platform_disable = beagle_disable_dvi, +}; + +static struct omap_dss_device beagle_dvi_device = { + .type = OMAP_DISPLAY_TYPE_DPI, + .name = "dvi", + .driver_name = "generic_dpi_panel", + .data = &dvi_panel, + .phy.dpi.data_lines = 24, + .reset_gpio = -EINVAL, +}; + +static struct omap_dss_device beagle_tv_device = { + .name = "tv", + .driver_name = "venc", + .type = OMAP_DISPLAY_TYPE_VENC, + .phy.venc.type = OMAP_DSS_VENC_TYPE_SVIDEO, +}; + +static struct omap_dss_device *beagle_dss_devices[] = { + &beagle_dvi_device, + &beagle_tv_device, +}; + +static struct omap_dss_board_info beagle_dss_data = { + .num_devices = ARRAY_SIZE(beagle_dss_devices), + .devices = beagle_dss_devices, + .default_device = &beagle_dvi_device, +}; + +static struct regulator_consumer_supply beagle_vdac_supply = + REGULATOR_SUPPLY("vdda_dac", "omapdss_venc"); + +static struct regulator_consumer_supply beagle_vdvi_supplies[] = { + REGULATOR_SUPPLY("vdds_dsi", "omapdss"), + REGULATOR_SUPPLY("vdds_dsi", "omapdss_dsi1"), +}; + +static void __init beagle_display_init(void) +{ + int r; + + r = gpio_request_one(beagle_dvi_device.reset_gpio, GPIOF_OUT_INIT_LOW, + "DVI reset"); + if (r < 0) + printk(KERN_ERR "Unable to get DVI reset GPIO\n"); +} + +#include "sdram-micron-mt46h32m32lf-6.h" + +static struct omap2_hsmmc_info mmc[] = { + { + .mmc = 1, + .caps = MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA, + .gpio_wp = 29, + }, + {} /* Terminator */ +}; + +static struct regulator_consumer_supply beagle_vmmc1_supply = { + .supply = "vmmc", +}; + +static struct regulator_consumer_supply beagle_vsim_supply = { + .supply = "vmmc_aux", +}; + +static struct gpio_led gpio_leds[]; + +static int beagle_twl_gpio_setup(struct device *dev, + unsigned gpio, unsigned ngpio) +{ + int r, usb_pwr_level; + + if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) { + mmc[0].gpio_wp = -EINVAL; + } else if ((omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_C1_3) || + (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_C4)) { + omap_mux_init_gpio(23, OMAP_PIN_INPUT); + mmc[0].gpio_wp = 23; + } else { + omap_mux_init_gpio(29, OMAP_PIN_INPUT); + } + /* gpio + 0 is "mmc0_cd" (input/IRQ) */ + mmc[0].gpio_cd = gpio + 0; + omap2_hsmmc_init(mmc); + + /* link regulators to MMC adapters */ + beagle_vmmc1_supply.dev = mmc[0].dev; + beagle_vsim_supply.dev = mmc[0].dev; + + /* + * TWL4030_GPIO_MAX + 0 == ledA, EHCI nEN_USB_PWR (out, XM active + * high / others active low) + * DVI reset GPIO is different between beagle revisions + */ + if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) { + usb_pwr_level = GPIOF_OUT_INIT_HIGH; + beagle_dvi_device.reset_gpio = 129; + /* + * gpio + 1 on Xm controls the TFP410's enable line (active low) + * gpio + 2 control varies depending on the board rev as below: + * P7/P8 revisions(prototype): Camera EN + * A2+ revisions (production): LDO (DVI, serial, led blocks) + */ + r = gpio_request_one(gpio + 1, GPIOF_OUT_INIT_LOW, + "nDVI_PWR_EN"); + if (r) + pr_err("%s: unable to configure nDVI_PWR_EN\n", + __func__); + r = gpio_request_one(gpio + 2, GPIOF_OUT_INIT_HIGH, + "DVI_LDO_EN"); + if (r) + pr_err("%s: unable to configure DVI_LDO_EN\n", + __func__); + } else { + usb_pwr_level = GPIOF_OUT_INIT_LOW; + beagle_dvi_device.reset_gpio = 170; + /* + * REVISIT: need ehci-omap hooks for external VBUS + * power switch and overcurrent detect + */ + if (gpio_request_one(gpio + 1, GPIOF_IN, "EHCI_nOC")) + pr_err("%s: unable to configure EHCI_nOC\n", __func__); + } + + gpio_request_one(gpio + TWL4030_GPIO_MAX, usb_pwr_level, "nEN_USB_PWR"); + + /* TWL4030_GPIO_MAX + 1 == ledB, PMU_STAT (out, active low LED) */ + gpio_leds[2].gpio = gpio + TWL4030_GPIO_MAX + 1; + + return 0; +} + +static struct twl4030_gpio_platform_data beagle_gpio_data = { + .gpio_base = OMAP_MAX_GPIO_LINES, + .irq_base = TWL4030_GPIO_IRQ_BASE, + .irq_end = TWL4030_GPIO_IRQ_END, + .use_leds = true, + .pullups = BIT(1), + .pulldowns = BIT(2) | BIT(6) | BIT(7) | BIT(8) | BIT(13) + | BIT(15) | BIT(16) | BIT(17), + .setup = beagle_twl_gpio_setup, +}; + +/* VMMC1 for MMC1 pins CMD, CLK, DAT0..DAT3 (20 mA, plus card == max 220 mA) */ +static struct regulator_init_data beagle_vmmc1 = { + .constraints = { + .min_uV = 1850000, + .max_uV = 3150000, + .valid_modes_mask = REGULATOR_MODE_NORMAL + | REGULATOR_MODE_STANDBY, + .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE + | REGULATOR_CHANGE_MODE + | REGULATOR_CHANGE_STATUS, + }, + .num_consumer_supplies = 1, + .consumer_supplies = &beagle_vmmc1_supply, +}; + +/* VSIM for MMC1 pins DAT4..DAT7 (2 mA, plus card == max 50 mA) */ +static struct regulator_init_data beagle_vsim = { + .constraints = { + .min_uV = 1800000, + .max_uV = 3000000, + .valid_modes_mask = REGULATOR_MODE_NORMAL + | REGULATOR_MODE_STANDBY, + .valid_ops_mask = REGULATOR_CHANGE_VOLTAGE + | REGULATOR_CHANGE_MODE + | REGULATOR_CHANGE_STATUS, + }, + .num_consumer_supplies = 1, + .consumer_supplies = &beagle_vsim_supply, +}; + +/* VDAC for DSS driving S-Video (8 mA unloaded, max 65 mA) */ +static struct regulator_init_data beagle_vdac = { + .constraints = { + .min_uV = 1800000, + .max_uV = 1800000, + .valid_modes_mask = REGULATOR_MODE_NORMAL + | REGULATOR_MODE_STANDBY, + .valid_ops_mask = REGULATOR_CHANGE_MODE + | REGULATOR_CHANGE_STATUS, + }, + .num_consumer_supplies = 1, + .consumer_supplies = &beagle_vdac_supply, +}; + +/* VPLL2 for digital video outputs */ +static struct regulator_init_data beagle_vpll2 = { + .constraints = { + .name = "VDVI", + .min_uV = 1800000, + .max_uV = 1800000, + .valid_modes_mask = REGULATOR_MODE_NORMAL + | REGULATOR_MODE_STANDBY, + .valid_ops_mask = REGULATOR_CHANGE_MODE + | REGULATOR_CHANGE_STATUS, + }, + .num_consumer_supplies = ARRAY_SIZE(beagle_vdvi_supplies), + .consumer_supplies = beagle_vdvi_supplies, +}; + +static struct twl4030_usb_data beagle_usb_data = { + .usb_mode = T2_USB_MODE_ULPI, +}; + +static struct twl4030_codec_audio_data beagle_audio_data; + +static struct twl4030_codec_data beagle_codec_data = { + .audio_mclk = 26000000, + .audio = &beagle_audio_data, +}; + +static struct twl4030_platform_data beagle_twldata = { + .irq_base = TWL4030_IRQ_BASE, + .irq_end = TWL4030_IRQ_END, + + /* platform_data for children goes here */ + .usb = &beagle_usb_data, + .gpio = &beagle_gpio_data, + .codec = &beagle_codec_data, + .vmmc1 = &beagle_vmmc1, + .vsim = &beagle_vsim, + .vdac = &beagle_vdac, + .vpll2 = &beagle_vpll2, +}; + +struct of_dev_auxdata omap3_auxdata_lookup[] __initdata = { + OF_DEV_AUXDATA("ti,omap3-i2c", 0, "i2c.1", &beagle_twldata), + OF_DEV_AUXDATA("ti,omap3-i2c", 0, "i2c.2", NULL), + OF_DEV_AUXDATA("ti,omap3-i2c", 0, "i2c.3", NULL), + {} +}; + +static int __init omap3_beagle_i2c_init(void) +{ + omap3_pmic_init("twl4030", &beagle_twldata); + return 0; +} + +static struct gpio_led gpio_leds[] = { + { + .name = "beagleboard::usr0", + .default_trigger = "heartbeat", + .gpio = 150, + }, + { + .name = "beagleboard::usr1", + .default_trigger = "mmc0", + .gpio = 149, + }, + { + .name = "beagleboard::pmu_stat", + .gpio = -EINVAL, /* gets replaced */ + .active_low = true, + }, +}; + +static struct gpio_led_platform_data gpio_led_info = { + .leds = gpio_leds, + .num_leds = ARRAY_SIZE(gpio_leds), +}; + +static struct platform_device leds_gpio = { + .name = "leds-gpio", + .id = -1, + .dev = { + .platform_data = &gpio_led_info, + }, +}; + +static struct gpio_keys_button gpio_buttons[] = { + { + .code = BTN_EXTRA, + .gpio = 7, + .desc = "user", + .wakeup = 1, + }, +}; + +static struct gpio_keys_platform_data gpio_key_info = { + .buttons = gpio_buttons, + .nbuttons = ARRAY_SIZE(gpio_buttons), +}; + +static struct platform_device keys_gpio = { + .name = "gpio-keys", + .id = -1, + .dev = { + .platform_data = &gpio_key_info, + }, +}; + +static void __init omap3_init_early(void) +{ + omap2_init_common_infrastructure(); + omap2_init_common_devices(mt46h32m32lf6_sdrc_params, + mt46h32m32lf6_sdrc_params); +} + +static void __init omap3_init_irq(void) +{ + omap_init_irq(); +#ifdef CONFIG_OMAP_32K_TIMER + omap2_gp_clockevent_set_gptimer(12); +#endif +} + +static struct platform_device *omap3_beagle_devices[] __initdata = { + &leds_gpio, + &keys_gpio, +}; + +static const struct usbhs_omap_board_data usbhs_bdata __initconst = { + + .port_mode[0] = OMAP_EHCI_PORT_MODE_PHY, + .port_mode[1] = OMAP_EHCI_PORT_MODE_PHY, + .port_mode[2] = OMAP_USBHS_PORT_MODE_UNUSED, + + .phy_reset = true, + .reset_gpio_port[0] = -EINVAL, + .reset_gpio_port[1] = 147, + .reset_gpio_port[2] = -EINVAL +}; + +#ifdef CONFIG_OMAP_MUX +static struct omap_board_mux board_mux[] __initdata = { + { .reg_offset = OMAP_MUX_TERMINATOR }, +}; +#endif + +static void __init beagle_opp_init(void) +{ + int r = 0; + + /* Initialize the omap3 opp table */ + if (omap3_opp_init()) { + pr_err("%s: opp default init failed\n", __func__); + return; + } + + /* Custom OPP enabled for XM */ + if (omap3_beagle_get_rev() == OMAP3BEAGLE_BOARD_XM) { + struct omap_hwmod *mh = omap_hwmod_lookup("mpu"); + struct omap_hwmod *dh = omap_hwmod_lookup("iva"); + struct device *dev; + + if (!mh || !dh) { + pr_err("%s: Aiee.. no mpu/dsp devices? %p %p\n", + __func__, mh, dh); + return; + } + /* Enable MPU 1GHz and lower opps */ + dev = &mh->od->pdev.dev; + r = opp_enable(dev, 800000000); + /* TODO: MPU 1GHz needs SR and ABB */ + + /* Enable IVA 800MHz and lower opps */ + dev = &dh->od->pdev.dev; + r |= opp_enable(dev, 660000000); + /* TODO: DSP 800MHz needs SR and ABB */ + if (r) { + pr_err("%s: failed to enable higher opp %d\n", + __func__, r); + /* + * Cleanup - disable the higher freqs - we dont care + * about the results + */ + dev = &mh->od->pdev.dev; + opp_disable(dev, 800000000); + dev = &dh->od->pdev.dev; + opp_disable(dev, 660000000); + } + } + return; +} + +static struct of_device_id omap3_dt_intc_match[] __initdata = { + { .compatible = "ti,omap3-intc", }, + {} +}; + +static struct of_device_id omap_dt_match_table[] __initdata = { + { .compatible = "ti,l4-core", }, + {} +}; + +static void __init omap3_init(void) +{ + struct device_node *node; + + node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, + OMAP34XX_IC_BASE); + if (node) + irq_domain_add_simple(node, 0); + + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); + omap3_beagle_init_rev(); + omap3_beagle_i2c_init(); + platform_add_devices(omap3_beagle_devices, + ARRAY_SIZE(omap3_beagle_devices)); + omap_display_init(&beagle_dss_data); + omap_serial_init(); + + omap_mux_init_gpio(170, OMAP_PIN_INPUT); + /* REVISIT leave DVI powered down until it's needed ... */ + gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); + + usb_musb_init(NULL); + usbhs_init(&usbhs_bdata); + omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, + ARRAY_SIZE(omap3beagle_nand_partitions)); + + /* Ensure msecure is mux'd to be able to set the RTC. */ + omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); + + /* Ensure SDRC pins are mux'd for self-refresh */ + omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); + omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); + + beagle_display_init(); + beagle_opp_init(); + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); +} + +static const char *omap3_dt_match[] __initdata = { + "ti,omap3-beagle", + NULL +}; + +DT_MACHINE_START(OMAP3_DT, "TI OMAP3 (Flattened Device Tree)") + .boot_params = 0x80000100, + .reserve = omap_reserve, + .map_io = omap3_map_io, + .init_early = omap3_init_early, + .init_irq = omap3_init_irq, + .init_machine = omap3_init, + .timer = &omap_timer, + .dt_compat = omap3_dt_match, +MACHINE_END diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c index 213c4cd..7efb178 100644 --- a/arch/arm/mach-omap2/board-omap3beagle.c +++ b/arch/arm/mach-omap2/board-omap3beagle.c @@ -19,7 +19,6 @@ #include <linux/err.h> #include <linux/clk.h> #include <linux/io.h> -#include <linux/of_platform.h> #include <linux/leds.h> #include <linux/gpio.h> #include <linux/input.h> @@ -422,9 +421,7 @@ static int __init omap3_beagle_i2c_init(void) /* Bus 3 is attached to the DVI port where devices like the pico DLP * projector don't work reliably with 400kHz */ 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; } @@ -567,10 +564,6 @@ static void __init beagle_opp_init(void) static void __init omap3_beagle_init(void) { -#ifdef CONFIG_OF - of_platform_prepare(NULL, NULL); -#endif /* CONFIG_OF */ - omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); omap3_beagle_init_rev(); omap3_beagle_i2c_init(); @@ -599,11 +592,6 @@ static void __init omap3_beagle_init(void) beagle_opp_init(); } -static const char *omap3_beagle_dt_match[] __initdata = { - "ti,omap3-beagle", - NULL -}; - MACHINE_START(OMAP3_BEAGLE, "OMAP3 Beagle Board") /* Maintainer: Syed Mohammed Khasim - http://beagleboard.org */ .boot_params = 0x80000100, @@ -613,5 +601,4 @@ MACHINE_START(OMAP3_BEAGLE, "OMAP3 Beagle Board") .init_irq = omap3_beagle_init_irq, .init_machine = omap3_beagle_init, .timer = &omap_timer, - .dt_compat = omap3_beagle_dt_match, MACHINE_END -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-13 22:06 ` [PATCH 3/4] dt: omap3: add generic board file for dt support G, Manjunath Kondaiah @ 2011-07-13 23:15 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 23:15 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > The generic board file is created and derived from beagle board file. > The beagle board file is migrated to boot from flattened device tree and > the cleanup of generic board file will happen in different stages. > > The changes here focus on i2c device registration through dt and upcoming > patches in the series will adopt i2c driver to use dt registered i2c > devices. Since this is a new board file instead of a modification of an existing one, I recommend *not* trying to completely duplicate the behaviour of the beagle board. Start small with only the registration of the UART and i2c drivers from the device tree and build up functionality until it can be used for all the OMAP3 boards. Otherwise the patch just adds a lot of code that needs to be removed again. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > arch/arm/mach-omap2/Kconfig | 12 + > arch/arm/mach-omap2/Makefile | 2 + > arch/arm/mach-omap2/board-omap3-dt.c | 623 +++++++++++++++++++++++++++++++ > arch/arm/mach-omap2/board-omap3beagle.c | 13 - > 4 files changed, 637 insertions(+), 13 deletions(-) > create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > index 19d5891..174f6d1 100644 > --- a/arch/arm/mach-omap2/Kconfig > +++ b/arch/arm/mach-omap2/Kconfig > @@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP > default y > select OMAP_PACKAGE_CBP > > +config MACH_OMAP3_DT > + bool "Generic OMAP3 board(FDT support)" > + depends on ARCH_OMAP3 > + select OMAP_PACKAGE_CBB > + select USE_OF > + select PROC_DEVICETREE Don't select PROC_DEVICETREE > + > + help > + Support for generic TI OMAP3 boards using Flattened Device Tree. > + Say Y here to enable OMAP3 device tree support > + More information at Documentation/devicetree > + > config MACH_TI8168EVM > bool "TI8168 Evaluation Module" > depends on SOC_OMAPTI816X > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > index b148077..86e66f7 100644 > --- a/arch/arm/mach-omap2/Makefile > +++ b/arch/arm/mach-omap2/Makefile > @@ -178,6 +178,8 @@ obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o > obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ > hsmmc.o > obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o > +obj-$(CONFIG_MACH_OMAP3_DT) += board-omap3-dt.o \ > + hsmmc.o > obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ > hsmmc.o This is an odd construct. Tony, why does each board add hsmmc to the oby-y variable instead of it having its own kconfig symbol? > +static void __init omap3_init(void) > +{ > + struct device_node *node; > + > + node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, > + OMAP34XX_IC_BASE); > + if (node) > + irq_domain_add_simple(node, 0); > + > + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); > + omap3_beagle_init_rev(); > + omap3_beagle_i2c_init(); > + platform_add_devices(omap3_beagle_devices, > + ARRAY_SIZE(omap3_beagle_devices)); > + omap_display_init(&beagle_dss_data); > + omap_serial_init(); > + > + omap_mux_init_gpio(170, OMAP_PIN_INPUT); > + /* REVISIT leave DVI powered down until it's needed ... */ > + gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); > + > + usb_musb_init(NULL); > + usbhs_init(&usbhs_bdata); > + omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, > + ARRAY_SIZE(omap3beagle_nand_partitions)); > + > + /* Ensure msecure is mux'd to be able to set the RTC. */ > + omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); > + > + /* Ensure SDRC pins are mux'd for self-refresh */ > + omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); > + omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); > + > + beagle_display_init(); > + beagle_opp_init(); > + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); Hmmm, I don't think this will work for OMAP because it will not create the i2c bus as an omap_device. It will only be a plane platform_deevice. You'll need to have a hook in of_platform_populate() to create devices the way omap3 infrastructure expects. > +} > + > +static const char *omap3_dt_match[] __initdata = { > + "ti,omap3-beagle", > + NULL "ti,omap3" should be sufficient. > +}; > + > +DT_MACHINE_START(OMAP3_DT, "TI OMAP3 (Flattened Device Tree)") > + .boot_params = 0x80000100, Drop boot_params. > + .reserve = omap_reserve, > + .map_io = omap3_map_io, > + .init_early = omap3_init_early, > + .init_irq = omap3_init_irq, > + .init_machine = omap3_init, > + .timer = &omap_timer, > + .dt_compat = omap3_dt_match, > +MACHINE_END > diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c > index 213c4cd..7efb178 100644 > --- a/arch/arm/mach-omap2/board-omap3beagle.c > +++ b/arch/arm/mach-omap2/board-omap3beagle.c > @@ -19,7 +19,6 @@ > #include <linux/err.h> > #include <linux/clk.h> > #include <linux/io.h> > -#include <linux/of_platform.h> > #include <linux/leds.h> > #include <linux/gpio.h> > #include <linux/input.h> > @@ -422,9 +421,7 @@ static int __init omap3_beagle_i2c_init(void) > /* Bus 3 is attached to the DVI port where devices like the pico DLP > * projector don't work reliably with 400kHz */ > 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; > } > > @@ -567,10 +564,6 @@ static void __init beagle_opp_init(void) > > static void __init omap3_beagle_init(void) > { > -#ifdef CONFIG_OF > - of_platform_prepare(NULL, NULL); > -#endif /* CONFIG_OF */ > - > omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); > omap3_beagle_init_rev(); > omap3_beagle_i2c_init(); > @@ -599,11 +592,6 @@ static void __init omap3_beagle_init(void) > beagle_opp_init(); > } > > -static const char *omap3_beagle_dt_match[] __initdata = { > - "ti,omap3-beagle", > - NULL > -}; > - > MACHINE_START(OMAP3_BEAGLE, "OMAP3 Beagle Board") > /* Maintainer: Syed Mohammed Khasim - http://beagleboard.org */ > .boot_params = 0x80000100, > @@ -613,5 +601,4 @@ MACHINE_START(OMAP3_BEAGLE, "OMAP3 Beagle Board") > .init_irq = omap3_beagle_init_irq, > .init_machine = omap3_beagle_init, > .timer = &omap_timer, > - .dt_compat = omap3_beagle_dt_match, > MACHINE_END > -- > 1.7.4.1 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-13 23:15 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 23:15 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > The generic board file is created and derived from beagle board file. > The beagle board file is migrated to boot from flattened device tree and > the cleanup of generic board file will happen in different stages. > > The changes here focus on i2c device registration through dt and upcoming > patches in the series will adopt i2c driver to use dt registered i2c > devices. Since this is a new board file instead of a modification of an existing one, I recommend *not* trying to completely duplicate the behaviour of the beagle board. Start small with only the registration of the UART and i2c drivers from the device tree and build up functionality until it can be used for all the OMAP3 boards. Otherwise the patch just adds a lot of code that needs to be removed again. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > ?arch/arm/mach-omap2/Kconfig ? ? ? ? ? ? | ? 12 + > ?arch/arm/mach-omap2/Makefile ? ? ? ? ? ?| ? ?2 + > ?arch/arm/mach-omap2/board-omap3-dt.c ? ?| ?623 +++++++++++++++++++++++++++++++ > ?arch/arm/mach-omap2/board-omap3beagle.c | ? 13 - > ?4 files changed, 637 insertions(+), 13 deletions(-) > ?create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > index 19d5891..174f6d1 100644 > --- a/arch/arm/mach-omap2/Kconfig > +++ b/arch/arm/mach-omap2/Kconfig > @@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP > ? ? ? ?default y > ? ? ? ?select OMAP_PACKAGE_CBP > > +config MACH_OMAP3_DT > + ? ? ? bool "Generic OMAP3 board(FDT support)" > + ? ? ? depends on ARCH_OMAP3 > + ? ? ? select OMAP_PACKAGE_CBB > + ? ? ? select USE_OF > + ? ? ? select PROC_DEVICETREE Don't select PROC_DEVICETREE > + > + ? ? ? help > + ? ? ? ? Support for generic TI OMAP3 boards using Flattened Device Tree. > + ? ? ? ? Say Y here to enable OMAP3 device tree support > + ? ? ? ? More information at Documentation/devicetree > + > ?config MACH_TI8168EVM > ? ? ? ?bool "TI8168 Evaluation Module" > ? ? ? ?depends on SOC_OMAPTI816X > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > index b148077..86e66f7 100644 > --- a/arch/arm/mach-omap2/Makefile > +++ b/arch/arm/mach-omap2/Makefile > @@ -178,6 +178,8 @@ obj-$(CONFIG_MACH_OMAP_H4) ? ? ? ? ?+= board-h4.o > ?obj-$(CONFIG_MACH_OMAP_2430SDP) ? ? ? ? ? ? ? ?+= board-2430sdp.o \ > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hsmmc.o > ?obj-$(CONFIG_MACH_OMAP_APOLLON) ? ? ? ? ? ? ? ?+= board-apollon.o > +obj-$(CONFIG_MACH_OMAP3_DT) ? ? ? ? ? ?+= board-omap3-dt.o \ > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?hsmmc.o > ?obj-$(CONFIG_MACH_OMAP3_BEAGLE) ? ? ? ? ? ? ? ?+= board-omap3beagle.o \ > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hsmmc.o This is an odd construct. Tony, why does each board add hsmmc to the oby-y variable instead of it having its own kconfig symbol? > +static void __init omap3_init(void) > +{ > + ? ? ? struct device_node *node; > + > + ? ? ? node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OMAP34XX_IC_BASE); > + ? ? ? if (node) > + ? ? ? ? ? ? ? irq_domain_add_simple(node, 0); > + > + ? ? ? omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); > + ? ? ? omap3_beagle_init_rev(); > + ? ? ? omap3_beagle_i2c_init(); > + ? ? ? platform_add_devices(omap3_beagle_devices, > + ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(omap3_beagle_devices)); > + ? ? ? omap_display_init(&beagle_dss_data); > + ? ? ? omap_serial_init(); > + > + ? ? ? omap_mux_init_gpio(170, OMAP_PIN_INPUT); > + ? ? ? /* REVISIT leave DVI powered down until it's needed ... */ > + ? ? ? gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); > + > + ? ? ? usb_musb_init(NULL); > + ? ? ? usbhs_init(&usbhs_bdata); > + ? ? ? omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?ARRAY_SIZE(omap3beagle_nand_partitions)); > + > + ? ? ? /* Ensure msecure is mux'd to be able to set the RTC. */ > + ? ? ? omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); > + > + ? ? ? /* Ensure SDRC pins are mux'd for self-refresh */ > + ? ? ? omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); > + ? ? ? omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); > + > + ? ? ? beagle_display_init(); > + ? ? ? beagle_opp_init(); > + ? ? ? of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); Hmmm, I don't think this will work for OMAP because it will not create the i2c bus as an omap_device. It will only be a plane platform_deevice. You'll need to have a hook in of_platform_populate() to create devices the way omap3 infrastructure expects. > +} > + > +static const char *omap3_dt_match[] __initdata = { > + ? ? ? "ti,omap3-beagle", > + ? ? ? NULL "ti,omap3" should be sufficient. > +}; > + > +DT_MACHINE_START(OMAP3_DT, "TI OMAP3 (Flattened Device Tree)") > + ? ? ? .boot_params ? ?= 0x80000100, Drop boot_params. > + ? ? ? .reserve ? ? ? ?= omap_reserve, > + ? ? ? .map_io ? ? ? ? = omap3_map_io, > + ? ? ? .init_early ? ? = omap3_init_early, > + ? ? ? .init_irq ? ? ? = omap3_init_irq, > + ? ? ? .init_machine ? = omap3_init, > + ? ? ? .timer ? ? ? ? ?= &omap_timer, > + ? ? ? .dt_compat ? ? ?= omap3_dt_match, > +MACHINE_END > diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c > index 213c4cd..7efb178 100644 > --- a/arch/arm/mach-omap2/board-omap3beagle.c > +++ b/arch/arm/mach-omap2/board-omap3beagle.c > @@ -19,7 +19,6 @@ > ?#include <linux/err.h> > ?#include <linux/clk.h> > ?#include <linux/io.h> > -#include <linux/of_platform.h> > ?#include <linux/leds.h> > ?#include <linux/gpio.h> > ?#include <linux/input.h> > @@ -422,9 +421,7 @@ static int __init omap3_beagle_i2c_init(void) > ? ? ? ?/* Bus 3 is attached to the DVI port where devices like the pico DLP > ? ? ? ? * projector don't work reliably with 400kHz */ > ? ? ? ?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; > ?} > > @@ -567,10 +564,6 @@ static void __init beagle_opp_init(void) > > ?static void __init omap3_beagle_init(void) > ?{ > -#ifdef CONFIG_OF > - ? ? ? of_platform_prepare(NULL, NULL); > -#endif /* CONFIG_OF */ > - > ? ? ? ?omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); > ? ? ? ?omap3_beagle_init_rev(); > ? ? ? ?omap3_beagle_i2c_init(); > @@ -599,11 +592,6 @@ static void __init omap3_beagle_init(void) > ? ? ? ?beagle_opp_init(); > ?} > > -static const char *omap3_beagle_dt_match[] __initdata = { > - ? ? ? "ti,omap3-beagle", > - ? ? ? NULL > -}; > - > ?MACHINE_START(OMAP3_BEAGLE, "OMAP3 Beagle Board") > ? ? ? ?/* Maintainer: Syed Mohammed Khasim - http://beagleboard.org */ > ? ? ? ?.boot_params ? ?= 0x80000100, > @@ -613,5 +601,4 @@ MACHINE_START(OMAP3_BEAGLE, "OMAP3 Beagle Board") > ? ? ? ?.init_irq ? ? ? = omap3_beagle_init_irq, > ? ? ? ?.init_machine ? = omap3_beagle_init, > ? ? ? ?.timer ? ? ? ? ?= &omap_timer, > - ? ? ? .dt_compat ? ? ?= omap3_beagle_dt_match, > ?MACHINE_END > -- > 1.7.4.1 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH] omap2+: Use Kconfig symbol in Makefile instead of obj-y 2011-07-13 23:15 ` Grant Likely @ 2011-07-14 8:53 ` Tony Lindgren -1 siblings, 0 replies; 60+ messages in thread From: Tony Lindgren @ 2011-07-14 8:53 UTC (permalink / raw) To: Grant Likely Cc: G, Manjunath Kondaiah, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux As noted by Grant Likely <grant.likely@secretlab.ca>, omap2+ Makefile unnecessarily repeats entries for common device init code instead of using Kconfig symbol. Remove references to hsmmc.o and board-flash.o. Also omap_phy_internal.o references can be removed once it has some Kconfig symbol to use. Signed-off-by: Tony Lindgren <tony@atomide.com --- * Grant Likely <grant.likely@secretlab.ca> [110713 16:10]: > > This is an odd construct. Tony, why does each board add hsmmc to the > oby-y variable instead of it having its own kconfig symbol? Heh that's true. Something that got added for one board got cloned all over the Makefile. Here's a patch to simplify that. diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index ff1466f..9c49a86 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -175,78 +175,66 @@ endif # Specific board support obj-$(CONFIG_MACH_OMAP_GENERIC) += board-generic.o obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o -obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ - hsmmc.o +obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o -obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ - hsmmc.o -obj-$(CONFIG_MACH_DEVKIT8000) += board-devkit8000.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP_LDP) += board-ldp.o \ - board-flash.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3530_LV_SOM) += board-omap3logic.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3_TORPEDO) += board-omap3logic.o \ - hsmmc.o -obj-$(CONFIG_MACH_OVERO) += board-overo.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3EVM) += board-omap3evm.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3_PANDORA) += board-omap3pandora.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP_3430SDP) += board-3430sdp.o \ - hsmmc.o \ - board-flash.o +obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o +obj-$(CONFIG_MACH_DEVKIT8000) += board-devkit8000.o +obj-$(CONFIG_MACH_OMAP_LDP) += board-ldp.o +obj-$(CONFIG_MACH_OMAP3530_LV_SOM) += board-omap3logic.o +obj-$(CONFIG_MACH_OMAP3_TORPEDO) += board-omap3logic.o +obj-$(CONFIG_MACH_ENCORE) += board-omap3encore.o +obj-$(CONFIG_MACH_OVERO) += board-overo.o +obj-$(CONFIG_MACH_OMAP3EVM) += board-omap3evm.o +obj-$(CONFIG_MACH_OMAP3_PANDORA) += board-omap3pandora.o +obj-$(CONFIG_MACH_OMAP_3430SDP) += board-3430sdp.o obj-$(CONFIG_MACH_NOKIA_N8X0) += board-n8x0.o obj-$(CONFIG_MACH_NOKIA_RM680) += board-rm680.o \ - sdram-nokia.o \ - hsmmc.o + sdram-nokia.o obj-$(CONFIG_MACH_NOKIA_RX51) += board-rx51.o \ sdram-nokia.o \ board-rx51-peripherals.o \ - board-rx51-video.o \ - hsmmc.o + board-rx51-video.o obj-$(CONFIG_MACH_OMAP_ZOOM2) += board-zoom.o \ board-zoom-peripherals.o \ board-zoom-display.o \ - board-flash.o \ - hsmmc.o \ board-zoom-debugboard.o obj-$(CONFIG_MACH_OMAP_ZOOM3) += board-zoom.o \ board-zoom-peripherals.o \ board-zoom-display.o \ - board-flash.o \ - hsmmc.o \ board-zoom-debugboard.o obj-$(CONFIG_MACH_OMAP_3630SDP) += board-3630sdp.o \ board-zoom-peripherals.o \ - board-zoom-display.o \ - board-flash.o \ - hsmmc.o -obj-$(CONFIG_MACH_CM_T35) += board-cm-t35.o \ - hsmmc.o + board-zoom-display.o +obj-$(CONFIG_MACH_CM_T35) += board-cm-t35.o obj-$(CONFIG_MACH_CM_T3517) += board-cm-t3517.o -obj-$(CONFIG_MACH_IGEP0020) += board-igep0020.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3_TOUCHBOOK) += board-omap3touchbook.o \ - hsmmc.o +obj-$(CONFIG_MACH_IGEP0020) += board-igep0020.o +obj-$(CONFIG_MACH_OMAP3_TOUCHBOOK) += board-omap3touchbook.o obj-$(CONFIG_MACH_OMAP_4430SDP) += board-4430sdp.o \ - hsmmc.o \ omap_phy_internal.o obj-$(CONFIG_MACH_OMAP4_PANDA) += board-omap4panda.o \ - hsmmc.o \ + omap_phy_internal.o + +obj-$(CONFIG_MACH_PCM049) += board-omap4pcm049.o \ omap_phy_internal.o obj-$(CONFIG_MACH_OMAP3517EVM) += board-am3517evm.o \ - omap_phy_internal.o \ + omap_phy_internal.o obj-$(CONFIG_MACH_CRANEBOARD) += board-am3517crane.o -obj-$(CONFIG_MACH_SBC3530) += board-omap3stalker.o \ - hsmmc.o +obj-$(CONFIG_MACH_SBC3530) += board-omap3stalker.o obj-$(CONFIG_MACH_TI8168EVM) += board-ti8168evm.o + # Platform specific device init code + +omap-flash-$(CONFIG_MTD_NAND_OMAP2) := board-flash.o +omap-flash-$(CONFIG_MTD_ONENAND_OMAP2) := board-flash.o +obj-y += $(omap-flash-y) $(omap-flash-m) + +omap-hsmmc-$(CONFIG_MMC_OMAP_HS) := hsmmc.o +obj-y += $(omap-hsmmc-m) $(omap-hsmmc-y) + + usbfs-$(CONFIG_ARCH_OMAP_OTG) := usb-fs.o obj-y += $(usbfs-m) $(usbfs-y) obj-y += usb-musb.o ^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH] omap2+: Use Kconfig symbol in Makefile instead of obj-y @ 2011-07-14 8:53 ` Tony Lindgren 0 siblings, 0 replies; 60+ messages in thread From: Tony Lindgren @ 2011-07-14 8:53 UTC (permalink / raw) To: linux-arm-kernel As noted by Grant Likely <grant.likely@secretlab.ca>, omap2+ Makefile unnecessarily repeats entries for common device init code instead of using Kconfig symbol. Remove references to hsmmc.o and board-flash.o. Also omap_phy_internal.o references can be removed once it has some Kconfig symbol to use. Signed-off-by: Tony Lindgren <tony at atomide.com --- * Grant Likely <grant.likely@secretlab.ca> [110713 16:10]: > > This is an odd construct. Tony, why does each board add hsmmc to the > oby-y variable instead of it having its own kconfig symbol? Heh that's true. Something that got added for one board got cloned all over the Makefile. Here's a patch to simplify that. diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index ff1466f..9c49a86 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -175,78 +175,66 @@ endif # Specific board support obj-$(CONFIG_MACH_OMAP_GENERIC) += board-generic.o obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o -obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ - hsmmc.o +obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o -obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ - hsmmc.o -obj-$(CONFIG_MACH_DEVKIT8000) += board-devkit8000.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP_LDP) += board-ldp.o \ - board-flash.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3530_LV_SOM) += board-omap3logic.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3_TORPEDO) += board-omap3logic.o \ - hsmmc.o -obj-$(CONFIG_MACH_OVERO) += board-overo.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3EVM) += board-omap3evm.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3_PANDORA) += board-omap3pandora.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP_3430SDP) += board-3430sdp.o \ - hsmmc.o \ - board-flash.o +obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o +obj-$(CONFIG_MACH_DEVKIT8000) += board-devkit8000.o +obj-$(CONFIG_MACH_OMAP_LDP) += board-ldp.o +obj-$(CONFIG_MACH_OMAP3530_LV_SOM) += board-omap3logic.o +obj-$(CONFIG_MACH_OMAP3_TORPEDO) += board-omap3logic.o +obj-$(CONFIG_MACH_ENCORE) += board-omap3encore.o +obj-$(CONFIG_MACH_OVERO) += board-overo.o +obj-$(CONFIG_MACH_OMAP3EVM) += board-omap3evm.o +obj-$(CONFIG_MACH_OMAP3_PANDORA) += board-omap3pandora.o +obj-$(CONFIG_MACH_OMAP_3430SDP) += board-3430sdp.o obj-$(CONFIG_MACH_NOKIA_N8X0) += board-n8x0.o obj-$(CONFIG_MACH_NOKIA_RM680) += board-rm680.o \ - sdram-nokia.o \ - hsmmc.o + sdram-nokia.o obj-$(CONFIG_MACH_NOKIA_RX51) += board-rx51.o \ sdram-nokia.o \ board-rx51-peripherals.o \ - board-rx51-video.o \ - hsmmc.o + board-rx51-video.o obj-$(CONFIG_MACH_OMAP_ZOOM2) += board-zoom.o \ board-zoom-peripherals.o \ board-zoom-display.o \ - board-flash.o \ - hsmmc.o \ board-zoom-debugboard.o obj-$(CONFIG_MACH_OMAP_ZOOM3) += board-zoom.o \ board-zoom-peripherals.o \ board-zoom-display.o \ - board-flash.o \ - hsmmc.o \ board-zoom-debugboard.o obj-$(CONFIG_MACH_OMAP_3630SDP) += board-3630sdp.o \ board-zoom-peripherals.o \ - board-zoom-display.o \ - board-flash.o \ - hsmmc.o -obj-$(CONFIG_MACH_CM_T35) += board-cm-t35.o \ - hsmmc.o + board-zoom-display.o +obj-$(CONFIG_MACH_CM_T35) += board-cm-t35.o obj-$(CONFIG_MACH_CM_T3517) += board-cm-t3517.o -obj-$(CONFIG_MACH_IGEP0020) += board-igep0020.o \ - hsmmc.o -obj-$(CONFIG_MACH_OMAP3_TOUCHBOOK) += board-omap3touchbook.o \ - hsmmc.o +obj-$(CONFIG_MACH_IGEP0020) += board-igep0020.o +obj-$(CONFIG_MACH_OMAP3_TOUCHBOOK) += board-omap3touchbook.o obj-$(CONFIG_MACH_OMAP_4430SDP) += board-4430sdp.o \ - hsmmc.o \ omap_phy_internal.o obj-$(CONFIG_MACH_OMAP4_PANDA) += board-omap4panda.o \ - hsmmc.o \ + omap_phy_internal.o + +obj-$(CONFIG_MACH_PCM049) += board-omap4pcm049.o \ omap_phy_internal.o obj-$(CONFIG_MACH_OMAP3517EVM) += board-am3517evm.o \ - omap_phy_internal.o \ + omap_phy_internal.o obj-$(CONFIG_MACH_CRANEBOARD) += board-am3517crane.o -obj-$(CONFIG_MACH_SBC3530) += board-omap3stalker.o \ - hsmmc.o +obj-$(CONFIG_MACH_SBC3530) += board-omap3stalker.o obj-$(CONFIG_MACH_TI8168EVM) += board-ti8168evm.o + # Platform specific device init code + +omap-flash-$(CONFIG_MTD_NAND_OMAP2) := board-flash.o +omap-flash-$(CONFIG_MTD_ONENAND_OMAP2) := board-flash.o +obj-y += $(omap-flash-y) $(omap-flash-m) + +omap-hsmmc-$(CONFIG_MMC_OMAP_HS) := hsmmc.o +obj-y += $(omap-hsmmc-m) $(omap-hsmmc-y) + + usbfs-$(CONFIG_ARCH_OMAP_OTG) := usb-fs.o obj-y += $(usbfs-m) $(usbfs-y) obj-y += usb-musb.o ^ permalink raw reply related [flat|nested] 60+ messages in thread
[parent not found: <CACxGe6sELS=C==16TZ2pfrxJDDyqjS_qOwJUfwzwMO8hqo8Xag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support [not found] ` <CACxGe6sELS=C==16TZ2pfrxJDDyqjS_qOwJUfwzwMO8hqo8Xag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2011-07-16 19:54 ` G, Manjunath Kondaiah 2011-07-16 20:07 ` G, Manjunath Kondaiah 0 siblings, 1 reply; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-16 19:54 UTC (permalink / raw) To: Grant Likely Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, linux-omap-u79uwXL29TY76Z2rM5mHXA, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [-- Attachment #1.1: Type: text/plain, Size: 8920 bytes --] Hi Grant, On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk-l0cyMroinI0@public.gmane.org> > wrote: > > > > The generic board file is created and derived from beagle board file. > > The beagle board file is migrated to boot from flattened device tree and > > the cleanup of generic board file will happen in different stages. > > > > The changes here focus on i2c device registration through dt and upcoming > > patches in the series will adopt i2c driver to use dt registered i2c > > devices. > > Since this is a new board file instead of a modification of an > existing one, I recommend *not* trying to completely duplicate the > behaviour of the beagle board. Start small with only the registration > of the UART and i2c drivers from the device tree and build up > functionality until it can be used for all the OMAP3 boards. > Otherwise the patch just adds a lot of code that needs to be removed > again. > agreed. Started with minimal board file with only serial and i2c. > > > > > Signed-off-by: G, Manjunath Kondaiah <manjugk-l0cyMroinI0@public.gmane.org> > > --- > > arch/arm/mach-omap2/Kconfig | 12 + > > arch/arm/mach-omap2/Makefile | 2 + > > arch/arm/mach-omap2/board-omap3-dt.c | 623 > +++++++++++++++++++++++++++++++ > > arch/arm/mach-omap2/board-omap3beagle.c | 13 - > > 4 files changed, 637 insertions(+), 13 deletions(-) > > create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c > > > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > > index 19d5891..174f6d1 100644 > > --- a/arch/arm/mach-omap2/Kconfig > > +++ b/arch/arm/mach-omap2/Kconfig > > @@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP > > default y > > select OMAP_PACKAGE_CBP > > > > +config MACH_OMAP3_DT > > + bool "Generic OMAP3 board(FDT support)" > > + depends on ARCH_OMAP3 > > + select OMAP_PACKAGE_CBB > > + select USE_OF > > + select PROC_DEVICETREE > > Don't select PROC_DEVICETREE > ok > > > + > > + help > > + Support for generic TI OMAP3 boards using Flattened Device > Tree. > > + Say Y here to enable OMAP3 device tree support > > + More information at Documentation/devicetree > > + > > config MACH_TI8168EVM > > bool "TI8168 Evaluation Module" > > depends on SOC_OMAPTI816X > > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > > index b148077..86e66f7 100644 > > --- a/arch/arm/mach-omap2/Makefile > > +++ b/arch/arm/mach-omap2/Makefile > > @@ -178,6 +178,8 @@ obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o > > obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ > > hsmmc.o > > obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o > > +obj-$(CONFIG_MACH_OMAP3_DT) += board-omap3-dt.o \ > > + hsmmc.o > > obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ > > hsmmc.o > > This is an odd construct. Tony, why does each board add hsmmc to the > oby-y variable instead of it having its own kconfig symbol? > > > +static void __init omap3_init(void) > > +{ > > + struct device_node *node; > > + > > + node = of_find_matching_node_by_address(NULL, > omap3_dt_intc_match, > > + OMAP34XX_IC_BASE); > > + if (node) > > + irq_domain_add_simple(node, 0); > > + > > + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); > > + omap3_beagle_init_rev(); > > + omap3_beagle_i2c_init(); > > + platform_add_devices(omap3_beagle_devices, > > + ARRAY_SIZE(omap3_beagle_devices)); > > + omap_display_init(&beagle_dss_data); > > + omap_serial_init(); > > + > > + omap_mux_init_gpio(170, OMAP_PIN_INPUT); > > + /* REVISIT leave DVI powered down until it's needed ... */ > > + gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); > > + > > + usb_musb_init(NULL); > > + usbhs_init(&usbhs_bdata); > > + omap_nand_flash_init(NAND_BUSWIDTH_16, > omap3beagle_nand_partitions, > > + ARRAY_SIZE(omap3beagle_nand_partitions)); > > + > > + /* Ensure msecure is mux'd to be able to set the RTC. */ > > + omap_mux_init_signal("sys_drm_msecure", > OMAP_PIN_OFF_OUTPUT_HIGH); > > + > > + /* Ensure SDRC pins are mux'd for self-refresh */ > > + omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); > > + omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); > > + > > + beagle_display_init(); > > + beagle_opp_init(); > > + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); > > Hmmm, I don't think this will work for OMAP because it will not create > the i2c bus as an omap_device. It will only be a plane > platform_deevice. You'll need to have a hook in > of_platform_populate() to create devices the way omap3 infrastructure > expects. > > This dependency is mentioned in patch series. Since you pointed out this issue, I would like to propose following approach for hooking up omap hwmod· framework with dt. Though, the current approach focus only on i2c driver, it can be extended and generalized as it evolves with other board and driver cleanup activities. The following changes are not tested and also not compiled, it is only proposal I am thinking to implement. Let me know if you see any serious issues with the approach. diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c1773456..104ef31 100644--- a/drivers/of/platform.c+++ b/drivers/of/platform.c@@ -457,6 +457,8 @@ void of_platform_prepare(struct device_node *root, } } ++ #ifdef CONFIG_ARM_AMBA static struct amba_device *of_amba_device_create(struct device_node *node, const char *bus_id,@@ -537,13 +539,60 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l continue; if (res.start != lookup->phys_addr) continue;- pr_debug("%s: devname=%s\n", np->full_name, lookup->name);+ printk("%s: devname=%s\n", np->full_name, lookup->name); return lookup; } } return NULL; } +static struct omap_device *of_omap_i2c_device_create(struct device_node *node,+ const char *bus_id,+ void *platform_data,+ struct device *parent)+{+ struct platform_device *pdev;+ struct i2c_board_info *i2c_board_info;+ u8 id;++ printk("Creating omap i2c device %s\n", node->full_name);++ if (!of_device_is_available(node))+ return NULL;++ id = simple_strtol(bus_id, NULL, 0);+ pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);+ pdev->dev.of_node = of_node_get(node);+ if (!pdev->dev.of_node) {+ speed = 100;+ } else {+ u32 prop;+ if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",+ &prop))+ speed = prop/100;+ else+ speed = 100;+ }+ printk("%s : speed->%d\n",__func__, speed);++ for_each_child_of_node(bus, child) {+ u32 prop;++ printk(" create child: %s\n", child->full_name);+ i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL);+ if (!of_property_read_u32(pdev->dev.of_node, "reg",+ &prop))+ i2c_board_info->addr = prop;+ if (!of_property_read_u32(pdev->dev.of_node, "interrupts",+ &prop))+ i2c_board_info->irq = prop;+ i2c_board_info->platform_data = platform_data;+ strncpy(i2c_board_info->type, child->name,+ sizeof(i2c_board_info->type));+ }+ omap_register_i2c_bus(id, speed, i2c_board_info, 1);+}+ /** * of_platform_bus_create() - Create a device for a node and its children. * @bus: device node of the bus to instantiate@@ -593,6 +642,11 @@ static int of_platform_bus_create(struct device_node *bus, return 0; } + if (of_device_is_compatible(bus, "ti,omap3-i2c")) {+ of_omap_i2c_device_create(bus, bus_id, platform_data, parent);+ return 0;+ }+ dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent); if (!dev || !of_match_node(matches, bus)) return 0; -M [-- Attachment #1.2: Type: text/html, Size: 12985 bytes --] [-- Attachment #2: Type: text/plain, Size: 192 bytes --] _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-16 19:54 ` [PATCH 3/4] dt: omap3: add generic board file for dt support G, Manjunath Kondaiah @ 2011-07-16 20:07 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-16 20:07 UTC (permalink / raw) To: Grant Likely; +Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux ps: posting again since my mailer has triggered the mail in html format hence it does not reach some mailing lists. Hi Grant, > On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> >> On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> > >> > The generic board file is created and derived from beagle board file. >> > The beagle board file is migrated to boot from flattened device tree and >> > the cleanup of generic board file will happen in different stages. >> > >> > The changes here focus on i2c device registration through dt and upcoming >> > patches in the series will adopt i2c driver to use dt registered i2c >> > devices. >> >> Since this is a new board file instead of a modification of an >> existing one, I recommend *not* trying to completely duplicate the >> behaviour of the beagle board. Start small with only the registration >> of the UART and i2c drivers from the device tree and build up >> functionality until it can be used for all the OMAP3 boards. >> Otherwise the patch just adds a lot of code that needs to be removed >> again. > agreed. Started with minimal board file with only serial and i2c. >> >> > >> > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> >> > --- >> > arch/arm/mach-omap2/Kconfig | 12 + >> > arch/arm/mach-omap2/Makefile | 2 + >> > arch/arm/mach-omap2/board-omap3-dt.c | 623 +++++++++++++++++++++++++++++++ >> > arch/arm/mach-omap2/board-omap3beagle.c | 13 - >> > 4 files changed, 637 insertions(+), 13 deletions(-) >> > create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c >> > >> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig >> > index 19d5891..174f6d1 100644 >> > --- a/arch/arm/mach-omap2/Kconfig >> > +++ b/arch/arm/mach-omap2/Kconfig >> > @@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP >> > default y >> > select OMAP_PACKAGE_CBP >> > >> > +config MACH_OMAP3_DT >> > + bool "Generic OMAP3 board(FDT support)" >> > + depends on ARCH_OMAP3 >> > + select OMAP_PACKAGE_CBB >> > + select USE_OF >> > + select PROC_DEVICETREE >> >> Don't select PROC_DEVICETREE > ok >> >> > + >> > + help >> > + Support for generic TI OMAP3 boards using Flattened Device Tree. >> > + Say Y here to enable OMAP3 device tree support >> > + More information at Documentation/devicetree >> > + >> > config MACH_TI8168EVM >> > bool "TI8168 Evaluation Module" >> > depends on SOC_OMAPTI816X >> > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile >> > index b148077..86e66f7 100644 >> > --- a/arch/arm/mach-omap2/Makefile >> > +++ b/arch/arm/mach-omap2/Makefile >> > @@ -178,6 +178,8 @@ obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o >> > obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ >> > hsmmc.o >> > obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o >> > +obj-$(CONFIG_MACH_OMAP3_DT) += board-omap3-dt.o \ >> > + hsmmc.o >> > obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ >> > hsmmc.o >> >> This is an odd construct. Tony, why does each board add hsmmc to the >> oby-y variable instead of it having its own kconfig symbol? >> >> > +static void __init omap3_init(void) >> > +{ >> > + struct device_node *node; >> > + >> > + node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, >> > + OMAP34XX_IC_BASE); >> > + if (node) >> > + irq_domain_add_simple(node, 0); >> > + >> > + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); >> > + omap3_beagle_init_rev(); >> > + omap3_beagle_i2c_init(); >> > + platform_add_devices(omap3_beagle_devices, >> > + ARRAY_SIZE(omap3_beagle_devices)); >> > + omap_display_init(&beagle_dss_data); >> > + omap_serial_init(); >> > + >> > + omap_mux_init_gpio(170, OMAP_PIN_INPUT); >> > + /* REVISIT leave DVI powered down until it's needed ... */ >> > + gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); >> > + >> > + usb_musb_init(NULL); >> > + usbhs_init(&usbhs_bdata); >> > + omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, >> > + ARRAY_SIZE(omap3beagle_nand_partitions)); >> > + >> > + /* Ensure msecure is mux'd to be able to set the RTC. */ >> > + omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); >> > + >> > + /* Ensure SDRC pins are mux'd for self-refresh */ >> > + omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); >> > + omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); >> > + >> > + beagle_display_init(); >> > + beagle_opp_init(); >> > + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); >> >> Hmmm, I don't think this will work for OMAP because it will not create >> the i2c bus as an omap_device. It will only be a plane >> platform_deevice. You'll need to have a hook in >> of_platform_populate() to create devices the way omap3 infrastructure >> expects. >> > This dependency is mentioned in patch series. Since you pointed out this issue, I would like to propose following approach for hooking up omap hwmod· framework with dt. Though, the current approach focus only on i2c driver, it can be extended and generalized as it evolves with other board and driver cleanup activities. The following changes are not tested and also not compiled, it is only proposal I am thinking to implement. Let me know if you see any serious issues with the approach. diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c1773456..104ef31 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -457,6 +457,8 @@ void of_platform_prepare(struct device_node *root, } } + + #ifdef CONFIG_ARM_AMBA static struct amba_device *of_amba_device_create(struct device_node *node, const char *bus_id, @@ -537,13 +539,60 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l continue; if (res.start != lookup->phys_addr) continue; - pr_debug("%s: devname=%s\n", np->full_name, lookup->name); + printk("%s: devname=%s\n", np->full_name, lookup->name); return lookup; } } return NULL; } +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, + const char *bus_id, + void *platform_data, + struct device *parent) +{ + struct platform_device *pdev; + struct i2c_board_info *i2c_board_info; + u8 id; + + printk("Creating omap i2c device %s\n", node->full_name); + + if (!of_device_is_available(node)) + return NULL; + + id = simple_strtol(bus_id, NULL, 0); + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); + pdev->dev.of_node = of_node_get(node); + if (!pdev->dev.of_node) { + speed = 100; + } else { + u32 prop; + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", + &prop)) + speed = prop/100; + else + speed = 100; + } + printk("%s : speed->%d\n",__func__, speed); + + for_each_child_of_node(bus, child) { + u32 prop; + + printk(" create child: %s\n", child->full_name); + i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); + if (!of_property_read_u32(pdev->dev.of_node, "reg", + &prop)) + i2c_board_info->addr = prop; + if (!of_property_read_u32(pdev->dev.of_node, "interrupts", + &prop)) + i2c_board_info->irq = prop; + i2c_board_info->platform_data = platform_data; + strncpy(i2c_board_info->type, child->name, + sizeof(i2c_board_info->type)); + } + omap_register_i2c_bus(id, speed, i2c_board_info, 1); +} + /** * of_platform_bus_create() - Create a device for a node and its children. * @bus: device node of the bus to instantiate @@ -593,6 +642,11 @@ static int of_platform_bus_create(struct device_node *bus, return 0; } + if (of_device_is_compatible(bus, "ti,omap3-i2c")) { + of_omap_i2c_device_create(bus, bus_id, platform_data, parent); + return 0; + } + dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent); if (!dev || !of_match_node(matches, bus)) return 0; -M -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-16 20:07 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-16 20:07 UTC (permalink / raw) To: linux-arm-kernel ps: posting again since my mailer has triggered the mail in html format hence it does not reach some mailing lists. Hi Grant, > On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> >> On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> > >> > The generic board file is created and derived from beagle board file. >> > The beagle board file is migrated to boot from flattened device tree and >> > the cleanup of generic board file will happen in different stages. >> > >> > The changes here focus on i2c device registration through dt and upcoming >> > patches in the series will adopt i2c driver to use dt registered i2c >> > devices. >> >> Since this is a new board file instead of a modification of an >> existing one, I recommend *not* trying to completely duplicate the >> behaviour of the beagle board. Start small with only the registration >> of the UART and i2c drivers from the device tree and build up >> functionality until it can be used for all the OMAP3 boards. >> Otherwise the patch just adds a lot of code that needs to be removed >> again. > agreed. Started with minimal board file with only serial and i2c. >> >> > >> > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> >> > --- >> > arch/arm/mach-omap2/Kconfig | 12 + >> > arch/arm/mach-omap2/Makefile | 2 + >> > arch/arm/mach-omap2/board-omap3-dt.c | 623 +++++++++++++++++++++++++++++++ >> > arch/arm/mach-omap2/board-omap3beagle.c | 13 - >> > 4 files changed, 637 insertions(+), 13 deletions(-) >> > create mode 100644 arch/arm/mach-omap2/board-omap3-dt.c >> > >> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig >> > index 19d5891..174f6d1 100644 >> > --- a/arch/arm/mach-omap2/Kconfig >> > +++ b/arch/arm/mach-omap2/Kconfig >> > @@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP >> > default y >> > select OMAP_PACKAGE_CBP >> > >> > +config MACH_OMAP3_DT >> > + bool "Generic OMAP3 board(FDT support)" >> > + depends on ARCH_OMAP3 >> > + select OMAP_PACKAGE_CBB >> > + select USE_OF >> > + select PROC_DEVICETREE >> >> Don't select PROC_DEVICETREE > ok >> >> > + >> > + help >> > + Support for generic TI OMAP3 boards using Flattened Device Tree. >> > + Say Y here to enable OMAP3 device tree support >> > + More information at Documentation/devicetree >> > + >> > config MACH_TI8168EVM >> > bool "TI8168 Evaluation Module" >> > depends on SOC_OMAPTI816X >> > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile >> > index b148077..86e66f7 100644 >> > --- a/arch/arm/mach-omap2/Makefile >> > +++ b/arch/arm/mach-omap2/Makefile >> > @@ -178,6 +178,8 @@ obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o >> > obj-$(CONFIG_MACH_OMAP_2430SDP) += board-2430sdp.o \ >> > hsmmc.o >> > obj-$(CONFIG_MACH_OMAP_APOLLON) += board-apollon.o >> > +obj-$(CONFIG_MACH_OMAP3_DT) += board-omap3-dt.o \ >> > + hsmmc.o >> > obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ >> > hsmmc.o >> >> This is an odd construct. Tony, why does each board add hsmmc to the >> oby-y variable instead of it having its own kconfig symbol? >> >> > +static void __init omap3_init(void) >> > +{ >> > + struct device_node *node; >> > + >> > + node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, >> > + OMAP34XX_IC_BASE); >> > + if (node) >> > + irq_domain_add_simple(node, 0); >> > + >> > + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); >> > + omap3_beagle_init_rev(); >> > + omap3_beagle_i2c_init(); >> > + platform_add_devices(omap3_beagle_devices, >> > + ARRAY_SIZE(omap3_beagle_devices)); >> > + omap_display_init(&beagle_dss_data); >> > + omap_serial_init(); >> > + >> > + omap_mux_init_gpio(170, OMAP_PIN_INPUT); >> > + /* REVISIT leave DVI powered down until it's needed ... */ >> > + gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); >> > + >> > + usb_musb_init(NULL); >> > + usbhs_init(&usbhs_bdata); >> > + omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, >> > + ARRAY_SIZE(omap3beagle_nand_partitions)); >> > + >> > + /* Ensure msecure is mux'd to be able to set the RTC. */ >> > + omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); >> > + >> > + /* Ensure SDRC pins are mux'd for self-refresh */ >> > + omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); >> > + omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); >> > + >> > + beagle_display_init(); >> > + beagle_opp_init(); >> > + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); >> >> Hmmm, I don't think this will work for OMAP because it will not create >> the i2c bus as an omap_device. It will only be a plane >> platform_deevice. You'll need to have a hook in >> of_platform_populate() to create devices the way omap3 infrastructure >> expects. >> > This dependency is mentioned in patch series. Since you pointed out this issue, I would like to propose following approach for hooking up omap hwmod? framework with dt. Though, the current approach focus only on i2c driver, it can be extended and generalized as it evolves with other board and driver cleanup activities. The following changes are not tested and also not compiled, it is only proposal I am thinking to implement. Let me know if you see any serious issues with the approach. diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c1773456..104ef31 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -457,6 +457,8 @@ void of_platform_prepare(struct device_node *root, } } + + #ifdef CONFIG_ARM_AMBA static struct amba_device *of_amba_device_create(struct device_node *node, const char *bus_id, @@ -537,13 +539,60 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l continue; if (res.start != lookup->phys_addr) continue; - pr_debug("%s: devname=%s\n", np->full_name, lookup->name); + printk("%s: devname=%s\n", np->full_name, lookup->name); return lookup; } } return NULL; } +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, + const char *bus_id, + void *platform_data, + struct device *parent) +{ + struct platform_device *pdev; + struct i2c_board_info *i2c_board_info; + u8 id; + + printk("Creating omap i2c device %s\n", node->full_name); + + if (!of_device_is_available(node)) + return NULL; + + id = simple_strtol(bus_id, NULL, 0); + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); + pdev->dev.of_node = of_node_get(node); + if (!pdev->dev.of_node) { + speed = 100; + } else { + u32 prop; + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", + &prop)) + speed = prop/100; + else + speed = 100; + } + printk("%s : speed->%d\n",__func__, speed); + + for_each_child_of_node(bus, child) { + u32 prop; + + printk(" create child: %s\n", child->full_name); + i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); + if (!of_property_read_u32(pdev->dev.of_node, "reg", + &prop)) + i2c_board_info->addr = prop; + if (!of_property_read_u32(pdev->dev.of_node, "interrupts", + &prop)) + i2c_board_info->irq = prop; + i2c_board_info->platform_data = platform_data; + strncpy(i2c_board_info->type, child->name, + sizeof(i2c_board_info->type)); + } + omap_register_i2c_bus(id, speed, i2c_board_info, 1); +} + /** * of_platform_bus_create() - Create a device for a node and its children. * @bus: device node of the bus to instantiate @@ -593,6 +642,11 @@ static int of_platform_bus_create(struct device_node *bus, return 0; } + if (of_device_is_compatible(bus, "ti,omap3-i2c")) { + of_omap_i2c_device_create(bus, bus_id, platform_data, parent); + return 0; + } + dev = of_platform_device_create_pdata(bus, bus_id, platform_data, parent); if (!dev || !of_match_node(matches, bus)) return 0; -M ^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-16 20:07 ` G, Manjunath Kondaiah @ 2011-07-17 5:13 ` Grant Likely -1 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-17 5:13 UTC (permalink / raw) To: G, Manjunath Kondaiah, Kevin Hilman, Tony Lindgren Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux Hi Manjunath, Comments below. I left in a lot of context for the new folks that I've cc'd (Tony and Kevin). On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>> > +static void __init omap3_init(void) >>> > +{ >>> > + struct device_node *node; >>> > + >>> > + node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, >>> > + OMAP34XX_IC_BASE); >>> > + if (node) >>> > + irq_domain_add_simple(node, 0); >>> > + >>> > + omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); >>> > + omap3_beagle_init_rev(); >>> > + omap3_beagle_i2c_init(); >>> > + platform_add_devices(omap3_beagle_devices, >>> > + ARRAY_SIZE(omap3_beagle_devices)); >>> > + omap_display_init(&beagle_dss_data); >>> > + omap_serial_init(); >>> > + >>> > + omap_mux_init_gpio(170, OMAP_PIN_INPUT); >>> > + /* REVISIT leave DVI powered down until it's needed ... */ >>> > + gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); >>> > + >>> > + usb_musb_init(NULL); >>> > + usbhs_init(&usbhs_bdata); >>> > + omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, >>> > + ARRAY_SIZE(omap3beagle_nand_partitions)); >>> > + >>> > + /* Ensure msecure is mux'd to be able to set the RTC. */ >>> > + omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); >>> > + >>> > + /* Ensure SDRC pins are mux'd for self-refresh */ >>> > + omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); >>> > + omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); >>> > + >>> > + beagle_display_init(); >>> > + beagle_opp_init(); >>> > + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); >>> >>> Hmmm, I don't think this will work for OMAP because it will not create >>> the i2c bus as an omap_device. It will only be a plane >>> platform_deevice. You'll need to have a hook in >>> of_platform_populate() to create devices the way omap3 infrastructure >>> expects. >>> >> > This dependency is mentioned in patch series. Since you pointed out this > issue, I would like to propose following approach for hooking up omap hwmod· > framework with dt. Though, the current approach focus only on i2c driver, it > can be extended and generalized as it evolves with other board and > driver cleanup > activities. The following changes are not tested and also not compiled, it is > only proposal I am thinking to implement. > > Let me know if you see any serious issues with the approach. > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index c1773456..104ef31 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -457,6 +457,8 @@ void of_platform_prepare(struct device_node *root, > } > } > > + > + > #ifdef CONFIG_ARM_AMBA > static struct amba_device *of_amba_device_create(struct device_node *node, > const char *bus_id, > @@ -537,13 +539,60 @@ static const struct of_dev_auxdata > *of_dev_lookup(const struct of_dev_auxdata *l > continue; > if (res.start != lookup->phys_addr) > continue; > - pr_debug("%s: devname=%s\n", np->full_name, > lookup->name); > + printk("%s: devname=%s\n", np->full_name, lookup->name); > return lookup; > } > } > return NULL; > } > > +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, > + const char *bus_id, > + void *platform_data, > + struct device *parent) > +{ > + struct platform_device *pdev; > + struct i2c_board_info *i2c_board_info; > + u8 id; > + > + printk("Creating omap i2c device %s\n", node->full_name); > + > + if (!of_device_is_available(node)) > + return NULL; > + > + id = simple_strtol(bus_id, NULL, 0); > + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); > + pdev->dev.of_node = of_node_get(node); > + if (!pdev->dev.of_node) { > + speed = 100; > + } else { > + u32 prop; > + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > + &prop)) > + speed = prop/100; > + else > + speed = 100; > + } > + printk("%s : speed->%d\n",__func__, speed); > + > + for_each_child_of_node(bus, child) { > + u32 prop; > + > + printk(" create child: %s\n", child->full_name); > + i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); > + if (!of_property_read_u32(pdev->dev.of_node, "reg", > + &prop)) > + i2c_board_info->addr = prop; > + if (!of_property_read_u32(pdev->dev.of_node, "interrupts", > + &prop)) > + i2c_board_info->irq = prop; > + i2c_board_info->platform_data = platform_data; > + strncpy(i2c_board_info->type, child->name, > + sizeof(i2c_board_info->type)); > + } > + omap_register_i2c_bus(id, speed, i2c_board_info, 1); While this does in a sense work, and creates an omap_device for the i2c bus that does get probed, it ends up encoding an awful lot of device specific details into the generic devicetree support code. The i2c bus driver itself must be responsible for decoding the speed and child nodes, and in fact it can easily be modified to do so (I've already demonstrated how to do so). The real problem is making sure the platform_device is created in a way that attaches the correct hwmod data. For this context, the current omap_register_i2c_bus() isn't a useful method for doing so. So what is to be done? omap_register_i2c_bus() does three things; 1) register an i2c board info for the bus with i2c_register_board_info(), 2) fill platform_data for the device, and 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. Of these three, 1 & 2 must not be done when using the DT. Only omap_i2c_add_bus() does something useful, but that is still specific to the i2c device. omap_i2c_add_bus() splits to omap{1,2}_add_bus(). omap1_i2c_add_bus() sets up pinmux and calls platform_device register. pinmux setup needs to be factored out anyway for generic DT platform device registration, which just leaves platform_device creation which is already handled by of_platform_populate(). omap2_i2c_add_bus() does the same thing, except it also looks up the hwmod data (*oh) and uses it to call omap_device_build(). omap_device_build() or something equivalent needs to be called for every omap device in the system, which is to create platform_devices with hwmod attached. Now we're starting to hit generic code. :-) The way I see it, you've got two options: 1) modify the of_platform_bus_create() to call some kind of of_platform_bus_create_omap() for devices that match "ti,omap3-device" or something. 2) Leave of_platform_bus_create(), and instead us a notifier to attach hwmod data to normal platform devices. omap_device_build() is actually pretty simple. It allocated a device, it attaches platform_data and hwmod pointers to the device and registers it. omap_device_register() is just a wrapper around platform_device_register(). My preference is definitely #2, but there is a wrinkle in this approach. Unfortunately omap_devices are not simply plain platform_devices with extra data attached, an omap_device actually embeds the platform_device inside it, which cannot be attached after the fact. I think I had talked with Kevin (cc'd) about eliminating the embedding, but I cannot remember clearly on this point. As long as platform_device remains embedded inside struct omap_device, #2 won't work. In either case, looking up the correct hwmod data should be easy for any device provided the omap code maintains a lookup table of compatible strings and base addresses of devices (much like auxdata). In fact, I'd be okay with extending auxdata to include OMAP fields if that would be easiest since the whole point of auxdata is to ease the transition to DT support. When a matching device is created, the hwmod pointer can easily be attached. This should work transparently for all omap devices, not just the i2c bus. > +} > + > /** > * of_platform_bus_create() - Create a device for a node and its children. > * @bus: device node of the bus to instantiate > @@ -593,6 +642,11 @@ static int of_platform_bus_create(struct device_node *bus, > return 0; > } > > + if (of_device_is_compatible(bus, "ti,omap3-i2c")) { > + of_omap_i2c_device_create(bus, bus_id, platform_data, parent); > + return 0; > + } > + > dev = of_platform_device_create_pdata(bus, bus_id, > platform_data, parent); > if (!dev || !of_match_node(matches, bus)) > return 0; > > -M > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-17 5:13 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-17 5:13 UTC (permalink / raw) To: linux-arm-kernel Hi Manjunath, Comments below. I left in a lot of context for the new folks that I've cc'd (Tony and Kevin). On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>> > +static void __init omap3_init(void) >>> > +{ >>> > + ? ? ? struct device_node *node; >>> > + >>> > + ? ? ? node = of_find_matching_node_by_address(NULL, omap3_dt_intc_match, >>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OMAP34XX_IC_BASE); >>> > + ? ? ? if (node) >>> > + ? ? ? ? ? ? ? irq_domain_add_simple(node, 0); >>> > + >>> > + ? ? ? omap3_mux_init(board_mux, OMAP_PACKAGE_CBB); >>> > + ? ? ? omap3_beagle_init_rev(); >>> > + ? ? ? omap3_beagle_i2c_init(); >>> > + ? ? ? platform_add_devices(omap3_beagle_devices, >>> > + ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(omap3_beagle_devices)); >>> > + ? ? ? omap_display_init(&beagle_dss_data); >>> > + ? ? ? omap_serial_init(); >>> > + >>> > + ? ? ? omap_mux_init_gpio(170, OMAP_PIN_INPUT); >>> > + ? ? ? /* REVISIT leave DVI powered down until it's needed ... */ >>> > + ? ? ? gpio_request_one(170, GPIOF_OUT_INIT_HIGH, "DVI_nPD"); >>> > + >>> > + ? ? ? usb_musb_init(NULL); >>> > + ? ? ? usbhs_init(&usbhs_bdata); >>> > + ? ? ? omap_nand_flash_init(NAND_BUSWIDTH_16, omap3beagle_nand_partitions, >>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?ARRAY_SIZE(omap3beagle_nand_partitions)); >>> > + >>> > + ? ? ? /* Ensure msecure is mux'd to be able to set the RTC. */ >>> > + ? ? ? omap_mux_init_signal("sys_drm_msecure", OMAP_PIN_OFF_OUTPUT_HIGH); >>> > + >>> > + ? ? ? /* Ensure SDRC pins are mux'd for self-refresh */ >>> > + ? ? ? omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT); >>> > + ? ? ? omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT); >>> > + >>> > + ? ? ? beagle_display_init(); >>> > + ? ? ? beagle_opp_init(); >>> > + ? ? ? of_platform_populate(NULL, omap_dt_match_table, NULL, NULL); >>> >>> Hmmm, I don't think this will work for OMAP because it will not create >>> the i2c bus as an omap_device. ?It will only be a plane >>> platform_deevice. ?You'll need to have a hook in >>> of_platform_populate() to create devices the way omap3 infrastructure >>> expects. >>> >> > This dependency is mentioned in patch series. Since you pointed out this > issue, I would like to propose following approach for hooking up omap hwmod? > framework with dt. Though, the current approach focus only on i2c driver, it > can be extended and generalized as it evolves with other board and > driver cleanup > activities. The following changes are not tested and also not compiled, ?it is > only proposal I am thinking to implement. > > Let me know if you see any serious issues with the approach. > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index c1773456..104ef31 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -457,6 +457,8 @@ void of_platform_prepare(struct device_node *root, > ? ? ? ?} > ?} > > + > + > ?#ifdef CONFIG_ARM_AMBA > ?static struct amba_device *of_amba_device_create(struct device_node *node, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *bus_id, > @@ -537,13 +539,60 @@ static const struct of_dev_auxdata > *of_dev_lookup(const struct of_dev_auxdata *l > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue; > ? ? ? ? ? ? ? ? ? ? ? ?if (res.start != lookup->phys_addr) > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue; > - ? ? ? ? ? ? ? ? ? ? ? pr_debug("%s: devname=%s\n", np->full_name, > lookup->name); > + ? ? ? ? ? ? ? ? ? ? ? printk("%s: devname=%s\n", np->full_name, lookup->name); > ? ? ? ? ? ? ? ? ? ? ? ?return lookup; > ? ? ? ? ? ? ? ?} > ? ? ? ?} > ? ? ? ?return NULL; > ?} > > +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *bus_id, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *platform_data, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device *parent) > +{ > + ? ? ? struct platform_device *pdev; > + ? ? ? struct i2c_board_info *i2c_board_info; > + ? ? ? u8 id; > + > + ? ? ? printk("Creating omap i2c device %s\n", node->full_name); > + > + ? ? ? if (!of_device_is_available(node)) > + ? ? ? ? ? ? ? return NULL; > + > + ? ? ? id = simple_strtol(bus_id, NULL, 0); > + ? ? ? pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); > + ? ? ? pdev->dev.of_node = of_node_get(node); > + ? ? ? if (!pdev->dev.of_node) { > + ? ? ? ? ? ? ? speed = 100; > + ? ? ? } else { > + ? ? ? ? ? ? ? u32 prop; > + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > + ? ? ? ? ? ? ? ? ? ? ? speed = prop/100; > + ? ? ? ? ? ? ? else > + ? ? ? ? ? ? ? ? ? ? ? speed = 100; > + ? ? ? } > + ? ? ? printk("%s : speed->%d\n",__func__, speed); > + > + ? ? ? for_each_child_of_node(bus, child) { > + ? ? ? ? ? ? ? u32 prop; > + > + ? ? ? ? ? ? ? printk(" ? create child: %s\n", child->full_name); > + ? ? ? ? ? ? ? i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); > + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "reg", > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > + ? ? ? ? ? ? ? i2c_board_info->addr = prop; > + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "interrupts", > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > + ? ? ? ? ? ? ? i2c_board_info->irq = prop; > + ? ? ? ? ? ? ? i2c_board_info->platform_data = platform_data; > + ? ? ? ? ? ? ? strncpy(i2c_board_info->type, child->name, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(i2c_board_info->type)); > + ? ? ? } > + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1); While this does in a sense work, and creates an omap_device for the i2c bus that does get probed, it ends up encoding an awful lot of device specific details into the generic devicetree support code. The i2c bus driver itself must be responsible for decoding the speed and child nodes, and in fact it can easily be modified to do so (I've already demonstrated how to do so). The real problem is making sure the platform_device is created in a way that attaches the correct hwmod data. For this context, the current omap_register_i2c_bus() isn't a useful method for doing so. So what is to be done? omap_register_i2c_bus() does three things; 1) register an i2c board info for the bus with i2c_register_board_info(), 2) fill platform_data for the device, and 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. Of these three, 1 & 2 must not be done when using the DT. Only omap_i2c_add_bus() does something useful, but that is still specific to the i2c device. omap_i2c_add_bus() splits to omap{1,2}_add_bus(). omap1_i2c_add_bus() sets up pinmux and calls platform_device register. pinmux setup needs to be factored out anyway for generic DT platform device registration, which just leaves platform_device creation which is already handled by of_platform_populate(). omap2_i2c_add_bus() does the same thing, except it also looks up the hwmod data (*oh) and uses it to call omap_device_build(). omap_device_build() or something equivalent needs to be called for every omap device in the system, which is to create platform_devices with hwmod attached. Now we're starting to hit generic code. :-) The way I see it, you've got two options: 1) modify the of_platform_bus_create() to call some kind of of_platform_bus_create_omap() for devices that match "ti,omap3-device" or something. 2) Leave of_platform_bus_create(), and instead us a notifier to attach hwmod data to normal platform devices. omap_device_build() is actually pretty simple. It allocated a device, it attaches platform_data and hwmod pointers to the device and registers it. omap_device_register() is just a wrapper around platform_device_register(). My preference is definitely #2, but there is a wrinkle in this approach. Unfortunately omap_devices are not simply plain platform_devices with extra data attached, an omap_device actually embeds the platform_device inside it, which cannot be attached after the fact. I think I had talked with Kevin (cc'd) about eliminating the embedding, but I cannot remember clearly on this point. As long as platform_device remains embedded inside struct omap_device, #2 won't work. In either case, looking up the correct hwmod data should be easy for any device provided the omap code maintains a lookup table of compatible strings and base addresses of devices (much like auxdata). In fact, I'd be okay with extending auxdata to include OMAP fields if that would be easiest since the whole point of auxdata is to ease the transition to DT support. When a matching device is created, the hwmod pointer can easily be attached. This should work transparently for all omap devices, not just the i2c bus. > +} > + > ?/** > ?* of_platform_bus_create() - Create a device for a node and its children. > ?* @bus: device node of the bus to instantiate > @@ -593,6 +642,11 @@ static int of_platform_bus_create(struct device_node *bus, > ? ? ? ? ? ? ? ?return 0; > ? ? ? ?} > > + ? ? ? if (of_device_is_compatible(bus, "ti,omap3-i2c")) { > + ? ? ? ? ? ? ? of_omap_i2c_device_create(bus, bus_id, platform_data, parent); > + ? ? ? ? ? ? ? return 0; > + ? ? ? } > + > ? ? ? ?dev = of_platform_device_create_pdata(bus, bus_id, > platform_data, parent); > ? ? ? ?if (!dev || !of_match_node(matches, bus)) > ? ? ? ? ? ? ? ?return 0; > > -M > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-17 5:13 ` Grant Likely @ 2011-07-18 9:07 ` Tony Lindgren -1 siblings, 0 replies; 60+ messages in thread From: Tony Lindgren @ 2011-07-18 9:07 UTC (permalink / raw) To: Grant Likely Cc: G, Manjunath Kondaiah, Kevin Hilman, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux * Grant Likely <grant.likely@secretlab.ca> [110716 22:08]: > > The way I see it, you've got two options: > > 1) modify the of_platform_bus_create() to call some kind of > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > or something. > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > hwmod data to normal platform devices. omap_device_build() is > actually pretty simple. It allocated a device, it attaches > platform_data and hwmod pointers to the device and registers it. > omap_device_register() is just a wrapper around > platform_device_register(). > > My preference is definitely #2, but there is a wrinkle in this > approach. Unfortunately omap_devices are not simply plain > platform_devices with extra data attached, an omap_device actually > embeds the platform_device inside it, which cannot be attached after > the fact. I think I had talked with Kevin (cc'd) about eliminating > the embedding, but I cannot remember clearly on this point. As long > as platform_device remains embedded inside struct omap_device, #2 > won't work. > > In either case, looking up the correct hwmod data should be easy for > any device provided the omap code maintains a lookup table of > compatible strings and base addresses of devices (much like auxdata). > In fact, I'd be okay with extending auxdata to include OMAP fields if > that would be easiest since the whole point of auxdata is to ease the > transition to DT support. When a matching device is created, the > hwmod pointer can easily be attached. This should work transparently > for all omap devices, not just the i2c bus. Well we should be able to automgagically build the omap_device for each device tree entry. And then the device driver probe and runtime PM should be able to take care of the rest for the driver. And then there's no more driver specific platform init code needed ;) How about if we just have the hwmod code call omap_device_build for each device tree entry? Regards, Tony ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-18 9:07 ` Tony Lindgren 0 siblings, 0 replies; 60+ messages in thread From: Tony Lindgren @ 2011-07-18 9:07 UTC (permalink / raw) To: linux-arm-kernel * Grant Likely <grant.likely@secretlab.ca> [110716 22:08]: > > The way I see it, you've got two options: > > 1) modify the of_platform_bus_create() to call some kind of > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > or something. > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > hwmod data to normal platform devices. omap_device_build() is > actually pretty simple. It allocated a device, it attaches > platform_data and hwmod pointers to the device and registers it. > omap_device_register() is just a wrapper around > platform_device_register(). > > My preference is definitely #2, but there is a wrinkle in this > approach. Unfortunately omap_devices are not simply plain > platform_devices with extra data attached, an omap_device actually > embeds the platform_device inside it, which cannot be attached after > the fact. I think I had talked with Kevin (cc'd) about eliminating > the embedding, but I cannot remember clearly on this point. As long > as platform_device remains embedded inside struct omap_device, #2 > won't work. > > In either case, looking up the correct hwmod data should be easy for > any device provided the omap code maintains a lookup table of > compatible strings and base addresses of devices (much like auxdata). > In fact, I'd be okay with extending auxdata to include OMAP fields if > that would be easiest since the whole point of auxdata is to ease the > transition to DT support. When a matching device is created, the > hwmod pointer can easily be attached. This should work transparently > for all omap devices, not just the i2c bus. Well we should be able to automgagically build the omap_device for each device tree entry. And then the device driver probe and runtime PM should be able to take care of the rest for the driver. And then there's no more driver specific platform init code needed ;) How about if we just have the hwmod code call omap_device_build for each device tree entry? Regards, Tony ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-18 9:07 ` Tony Lindgren @ 2011-07-19 21:34 ` Grant Likely -1 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-19 21:34 UTC (permalink / raw) To: Tony Lindgren Cc: G, Manjunath Kondaiah, Kevin Hilman, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: > * Grant Likely <grant.likely@secretlab.ca> [110716 22:08]: > > > > The way I see it, you've got two options: > > > > 1) modify the of_platform_bus_create() to call some kind of > > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > > or something. > > > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > > hwmod data to normal platform devices. omap_device_build() is > > actually pretty simple. It allocated a device, it attaches > > platform_data and hwmod pointers to the device and registers it. > > omap_device_register() is just a wrapper around > > platform_device_register(). > > > > My preference is definitely #2, but there is a wrinkle in this > > approach. Unfortunately omap_devices are not simply plain > > platform_devices with extra data attached, an omap_device actually > > embeds the platform_device inside it, which cannot be attached after > > the fact. I think I had talked with Kevin (cc'd) about eliminating > > the embedding, but I cannot remember clearly on this point. As long > > as platform_device remains embedded inside struct omap_device, #2 > > won't work. > > > > In either case, looking up the correct hwmod data should be easy for > > any device provided the omap code maintains a lookup table of > > compatible strings and base addresses of devices (much like auxdata). > > In fact, I'd be okay with extending auxdata to include OMAP fields if > > that would be easiest since the whole point of auxdata is to ease the > > transition to DT support. When a matching device is created, the > > hwmod pointer can easily be attached. This should work transparently > > for all omap devices, not just the i2c bus. > > Well we should be able to automgagically build the omap_device for > each device tree entry. > > And then the device driver probe and runtime PM should be able to take > care of the rest for the driver. And then there's no more driver > specific platform init code needed ;) Right! That's the solution I'd like to see. > How about if we just have the hwmod code call omap_device_build for > each device tree entry? I think that is pretty much equivalent to suggestion #1 above, only I'm suggesting to take advantage of the infrastructure already available in driver/of/platform.c in the form of of_platform_populate(). The "of_platform_bus_create_omap()" function suggested above I assumed would directly call omap_device_build(). There are already hooks in the _populate call path to handle the creation of amba_devices. I have no problem doing the same thing for omap devices. g. ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-19 21:34 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-19 21:34 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: > * Grant Likely <grant.likely@secretlab.ca> [110716 22:08]: > > > > The way I see it, you've got two options: > > > > 1) modify the of_platform_bus_create() to call some kind of > > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > > or something. > > > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > > hwmod data to normal platform devices. omap_device_build() is > > actually pretty simple. It allocated a device, it attaches > > platform_data and hwmod pointers to the device and registers it. > > omap_device_register() is just a wrapper around > > platform_device_register(). > > > > My preference is definitely #2, but there is a wrinkle in this > > approach. Unfortunately omap_devices are not simply plain > > platform_devices with extra data attached, an omap_device actually > > embeds the platform_device inside it, which cannot be attached after > > the fact. I think I had talked with Kevin (cc'd) about eliminating > > the embedding, but I cannot remember clearly on this point. As long > > as platform_device remains embedded inside struct omap_device, #2 > > won't work. > > > > In either case, looking up the correct hwmod data should be easy for > > any device provided the omap code maintains a lookup table of > > compatible strings and base addresses of devices (much like auxdata). > > In fact, I'd be okay with extending auxdata to include OMAP fields if > > that would be easiest since the whole point of auxdata is to ease the > > transition to DT support. When a matching device is created, the > > hwmod pointer can easily be attached. This should work transparently > > for all omap devices, not just the i2c bus. > > Well we should be able to automgagically build the omap_device for > each device tree entry. > > And then the device driver probe and runtime PM should be able to take > care of the rest for the driver. And then there's no more driver > specific platform init code needed ;) Right! That's the solution I'd like to see. > How about if we just have the hwmod code call omap_device_build for > each device tree entry? I think that is pretty much equivalent to suggestion #1 above, only I'm suggesting to take advantage of the infrastructure already available in driver/of/platform.c in the form of of_platform_populate(). The "of_platform_bus_create_omap()" function suggested above I assumed would directly call omap_device_build(). There are already hooks in the _populate call path to handle the creation of amba_devices. I have no problem doing the same thing for omap devices. g. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-19 21:34 ` Grant Likely @ 2011-07-21 7:18 ` Tony Lindgren -1 siblings, 0 replies; 60+ messages in thread From: Tony Lindgren @ 2011-07-21 7:18 UTC (permalink / raw) To: Grant Likely Cc: Kevin Hilman, G, Manjunath Kondaiah, devicetree-discuss, ben-linux, linux-omap, linux-arm-kernel * Grant Likely <grant.likely@secretlab.ca> [110719 14:29]: > On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: > > * Grant Likely <grant.likely@secretlab.ca> [110716 22:08]: > > > > > > The way I see it, you've got two options: > > > > > > 1) modify the of_platform_bus_create() to call some kind of > > > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > > > or something. > > > > > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > > > hwmod data to normal platform devices. omap_device_build() is > > > actually pretty simple. It allocated a device, it attaches > > > platform_data and hwmod pointers to the device and registers it. > > > omap_device_register() is just a wrapper around > > > platform_device_register(). > > > > > > My preference is definitely #2, but there is a wrinkle in this > > > approach. Unfortunately omap_devices are not simply plain > > > platform_devices with extra data attached, an omap_device actually > > > embeds the platform_device inside it, which cannot be attached after > > > the fact. I think I had talked with Kevin (cc'd) about eliminating > > > the embedding, but I cannot remember clearly on this point. As long > > > as platform_device remains embedded inside struct omap_device, #2 > > > won't work. > > > > > > In either case, looking up the correct hwmod data should be easy for > > > any device provided the omap code maintains a lookup table of > > > compatible strings and base addresses of devices (much like auxdata). > > > In fact, I'd be okay with extending auxdata to include OMAP fields if > > > that would be easiest since the whole point of auxdata is to ease the > > > transition to DT support. When a matching device is created, the > > > hwmod pointer can easily be attached. This should work transparently > > > for all omap devices, not just the i2c bus. > > > > Well we should be able to automgagically build the omap_device for > > each device tree entry. > > > > And then the device driver probe and runtime PM should be able to take > > care of the rest for the driver. And then there's no more driver > > specific platform init code needed ;) > > Right! That's the solution I'd like to see. > > > How about if we just have the hwmod code call omap_device_build for > > each device tree entry? > > I think that is pretty much equivalent to suggestion #1 above, only > I'm suggesting to take advantage of the infrastructure already > available in driver/of/platform.c in the form of > of_platform_populate(). The "of_platform_bus_create_omap()" function > suggested above I assumed would directly call omap_device_build(). OK > There are already hooks in the _populate call path to handle the > creation of amba_devices. I have no problem doing the same thing for > omap devices. OK Regards, Tony ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 7:18 ` Tony Lindgren 0 siblings, 0 replies; 60+ messages in thread From: Tony Lindgren @ 2011-07-21 7:18 UTC (permalink / raw) To: linux-arm-kernel * Grant Likely <grant.likely@secretlab.ca> [110719 14:29]: > On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: > > * Grant Likely <grant.likely@secretlab.ca> [110716 22:08]: > > > > > > The way I see it, you've got two options: > > > > > > 1) modify the of_platform_bus_create() to call some kind of > > > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > > > or something. > > > > > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > > > hwmod data to normal platform devices. omap_device_build() is > > > actually pretty simple. It allocated a device, it attaches > > > platform_data and hwmod pointers to the device and registers it. > > > omap_device_register() is just a wrapper around > > > platform_device_register(). > > > > > > My preference is definitely #2, but there is a wrinkle in this > > > approach. Unfortunately omap_devices are not simply plain > > > platform_devices with extra data attached, an omap_device actually > > > embeds the platform_device inside it, which cannot be attached after > > > the fact. I think I had talked with Kevin (cc'd) about eliminating > > > the embedding, but I cannot remember clearly on this point. As long > > > as platform_device remains embedded inside struct omap_device, #2 > > > won't work. > > > > > > In either case, looking up the correct hwmod data should be easy for > > > any device provided the omap code maintains a lookup table of > > > compatible strings and base addresses of devices (much like auxdata). > > > In fact, I'd be okay with extending auxdata to include OMAP fields if > > > that would be easiest since the whole point of auxdata is to ease the > > > transition to DT support. When a matching device is created, the > > > hwmod pointer can easily be attached. This should work transparently > > > for all omap devices, not just the i2c bus. > > > > Well we should be able to automgagically build the omap_device for > > each device tree entry. > > > > And then the device driver probe and runtime PM should be able to take > > care of the rest for the driver. And then there's no more driver > > specific platform init code needed ;) > > Right! That's the solution I'd like to see. > > > How about if we just have the hwmod code call omap_device_build for > > each device tree entry? > > I think that is pretty much equivalent to suggestion #1 above, only > I'm suggesting to take advantage of the infrastructure already > available in driver/of/platform.c in the form of > of_platform_populate(). The "of_platform_bus_create_omap()" function > suggested above I assumed would directly call omap_device_build(). OK > There are already hooks in the _populate call path to handle the > creation of amba_devices. I have no problem doing the same thing for > omap devices. OK Regards, Tony ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-19 21:34 ` Grant Likely @ 2011-07-21 8:55 ` Rajendra Nayak -1 siblings, 0 replies; 60+ messages in thread From: Rajendra Nayak @ 2011-07-21 8:55 UTC (permalink / raw) To: Grant Likely Cc: Kevin Hilman, Paul Walmsley, Cousson, Benoit, Tony Lindgren, G, Manjunath Kondaiah, devicetree-discuss, ben-linux, linux-omap, linux-arm-kernel On 7/20/2011 3:04 AM, Grant Likely wrote: > On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: >> * Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: >>> >>> The way I see it, you've got two options: >>> >>> 1) modify the of_platform_bus_create() to call some kind of >>> of_platform_bus_create_omap() for devices that match "ti,omap3-device" >>> or something. >>> >>> 2) Leave of_platform_bus_create(), and instead us a notifier to attach >>> hwmod data to normal platform devices. omap_device_build() is >>> actually pretty simple. It allocated a device, it attaches >>> platform_data and hwmod pointers to the device and registers it. >>> omap_device_register() is just a wrapper around >>> platform_device_register(). >>> >>> My preference is definitely #2, but there is a wrinkle in this >>> approach. Unfortunately omap_devices are not simply plain >>> platform_devices with extra data attached, an omap_device actually >>> embeds the platform_device inside it, which cannot be attached after >>> the fact. I think I had talked with Kevin (cc'd) about eliminating >>> the embedding, but I cannot remember clearly on this point. As long >>> as platform_device remains embedded inside struct omap_device, #2 >>> won't work. >>> >>> In either case, looking up the correct hwmod data should be easy for >>> any device provided the omap code maintains a lookup table of >>> compatible strings and base addresses of devices (much like auxdata). >>> In fact, I'd be okay with extending auxdata to include OMAP fields if >>> that would be easiest since the whole point of auxdata is to ease the >>> transition to DT support. When a matching device is created, the >>> hwmod pointer can easily be attached. This should work transparently >>> for all omap devices, not just the i2c bus. >> >> Well we should be able to automgagically build the omap_device for >> each device tree entry. >> >> And then the device driver probe and runtime PM should be able to take >> care of the rest for the driver. And then there's no more driver >> specific platform init code needed ;) > > Right! That's the solution I'd like to see. > >> How about if we just have the hwmod code call omap_device_build for >> each device tree entry? > > I think that is pretty much equivalent to suggestion #1 above, only > I'm suggesting to take advantage of the infrastructure already > available in driver/of/platform.c in the form of > of_platform_populate(). The "of_platform_bus_create_omap()" function > suggested above I assumed would directly call omap_device_build(). In fact a lot of what omap_device_build() does today might not even be needed anymore. A lot of what it does is populate the platform_device structure by looking up the hwmod structs. Most of that information would now come from DT and hence all of that can be taken off from the hwmod structs. What will still be needed in hwmod is other data needed to runtime enable/idle the devices. That data however still needs to be linked with the platform_device's that DT would create which is what I guess could be done in something like a of_platform_bus_create_omap(). Paul/Benoit, do you guys agree we can get rid of some of the data from hwmod, whatever would now get passed in from DT, and keep only the PRCM/OCP related stuff for runtime handling? > > There are already hooks in the _populate call path to handle the > creation of amba_devices. I have no problem doing the same thing for > omap devices. > > g. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 8:55 ` Rajendra Nayak 0 siblings, 0 replies; 60+ messages in thread From: Rajendra Nayak @ 2011-07-21 8:55 UTC (permalink / raw) To: linux-arm-kernel On 7/20/2011 3:04 AM, Grant Likely wrote: > On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: >> * Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: >>> >>> The way I see it, you've got two options: >>> >>> 1) modify the of_platform_bus_create() to call some kind of >>> of_platform_bus_create_omap() for devices that match "ti,omap3-device" >>> or something. >>> >>> 2) Leave of_platform_bus_create(), and instead us a notifier to attach >>> hwmod data to normal platform devices. omap_device_build() is >>> actually pretty simple. It allocated a device, it attaches >>> platform_data and hwmod pointers to the device and registers it. >>> omap_device_register() is just a wrapper around >>> platform_device_register(). >>> >>> My preference is definitely #2, but there is a wrinkle in this >>> approach. Unfortunately omap_devices are not simply plain >>> platform_devices with extra data attached, an omap_device actually >>> embeds the platform_device inside it, which cannot be attached after >>> the fact. I think I had talked with Kevin (cc'd) about eliminating >>> the embedding, but I cannot remember clearly on this point. As long >>> as platform_device remains embedded inside struct omap_device, #2 >>> won't work. >>> >>> In either case, looking up the correct hwmod data should be easy for >>> any device provided the omap code maintains a lookup table of >>> compatible strings and base addresses of devices (much like auxdata). >>> In fact, I'd be okay with extending auxdata to include OMAP fields if >>> that would be easiest since the whole point of auxdata is to ease the >>> transition to DT support. When a matching device is created, the >>> hwmod pointer can easily be attached. This should work transparently >>> for all omap devices, not just the i2c bus. >> >> Well we should be able to automgagically build the omap_device for >> each device tree entry. >> >> And then the device driver probe and runtime PM should be able to take >> care of the rest for the driver. And then there's no more driver >> specific platform init code needed ;) > > Right! That's the solution I'd like to see. > >> How about if we just have the hwmod code call omap_device_build for >> each device tree entry? > > I think that is pretty much equivalent to suggestion #1 above, only > I'm suggesting to take advantage of the infrastructure already > available in driver/of/platform.c in the form of > of_platform_populate(). The "of_platform_bus_create_omap()" function > suggested above I assumed would directly call omap_device_build(). In fact a lot of what omap_device_build() does today might not even be needed anymore. A lot of what it does is populate the platform_device structure by looking up the hwmod structs. Most of that information would now come from DT and hence all of that can be taken off from the hwmod structs. What will still be needed in hwmod is other data needed to runtime enable/idle the devices. That data however still needs to be linked with the platform_device's that DT would create which is what I guess could be done in something like a of_platform_bus_create_omap(). Paul/Benoit, do you guys agree we can get rid of some of the data from hwmod, whatever would now get passed in from DT, and keep only the PRCM/OCP related stuff for runtime handling? > > There are already hooks in the _populate call path to handle the > creation of amba_devices. I have no problem doing the same thing for > omap devices. > > g. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 60+ messages in thread
[parent not found: <4E27E967.7090501-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-21 8:55 ` Rajendra Nayak @ 2011-07-21 9:09 ` Felipe Balbi -1 siblings, 0 replies; 60+ messages in thread From: Felipe Balbi @ 2011-07-21 9:09 UTC (permalink / raw) To: Rajendra Nayak Cc: Kevin Hilman, Paul Walmsley, Cousson, Benoit, Tony Lindgren, G, Manjunath Kondaiah, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [-- Attachment #1.1: Type: text/plain, Size: 3983 bytes --] Hi, On Thu, Jul 21, 2011 at 02:25:03PM +0530, Rajendra Nayak wrote: > On 7/20/2011 3:04 AM, Grant Likely wrote: > >On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: > >>* Grant Likely<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> [110716 22:08]: > >>> > >>>The way I see it, you've got two options: > >>> > >>>1) modify the of_platform_bus_create() to call some kind of > >>>of_platform_bus_create_omap() for devices that match "ti,omap3-device" > >>>or something. > >>> > >>>2) Leave of_platform_bus_create(), and instead us a notifier to attach > >>>hwmod data to normal platform devices. omap_device_build() is > >>>actually pretty simple. It allocated a device, it attaches > >>>platform_data and hwmod pointers to the device and registers it. > >>>omap_device_register() is just a wrapper around > >>>platform_device_register(). > >>> > >>>My preference is definitely #2, but there is a wrinkle in this > >>>approach. Unfortunately omap_devices are not simply plain > >>>platform_devices with extra data attached, an omap_device actually > >>>embeds the platform_device inside it, which cannot be attached after > >>>the fact. I think I had talked with Kevin (cc'd) about eliminating > >>>the embedding, but I cannot remember clearly on this point. As long > >>>as platform_device remains embedded inside struct omap_device, #2 > >>>won't work. > >>> > >>>In either case, looking up the correct hwmod data should be easy for > >>>any device provided the omap code maintains a lookup table of > >>>compatible strings and base addresses of devices (much like auxdata). > >>>In fact, I'd be okay with extending auxdata to include OMAP fields if > >>>that would be easiest since the whole point of auxdata is to ease the > >>>transition to DT support. When a matching device is created, the > >>>hwmod pointer can easily be attached. This should work transparently > >>>for all omap devices, not just the i2c bus. > >> > >>Well we should be able to automgagically build the omap_device for > >>each device tree entry. > >> > >>And then the device driver probe and runtime PM should be able to take > >>care of the rest for the driver. And then there's no more driver > >>specific platform init code needed ;) > > > >Right! That's the solution I'd like to see. > > > >>How about if we just have the hwmod code call omap_device_build for > >>each device tree entry? > > > >I think that is pretty much equivalent to suggestion #1 above, only > >I'm suggesting to take advantage of the infrastructure already > >available in driver/of/platform.c in the form of > >of_platform_populate(). The "of_platform_bus_create_omap()" function > >suggested above I assumed would directly call omap_device_build(). > > In fact a lot of what omap_device_build() does today might not even be > needed anymore. A lot of what it does is populate the platform_device > structure by looking up the hwmod structs. > Most of that information would now come from DT and hence all of that > can be taken off from the hwmod structs. What will still be needed in > hwmod is other data needed to runtime enable/idle the devices. That > data however still needs to be linked with the platform_device's that > DT would create which is what I guess could be done in something > like a of_platform_bus_create_omap(). > > Paul/Benoit, do you guys agree we can get rid of some of the data > from hwmod, whatever would now get passed in from DT, and keep > only the PRCM/OCP related stuff for runtime handling? IMHO, all omap_hwmod_*_data.c files become pretty much useless if we move completely to DT. All hwmod data is passing today, can be passed via DT and in a similar Hierarchical manner. Now WRT omap_device_build() and PM, I think that's still necessary because it simplifies a lot PM handling. But the data files themselves can "easily" be purged from kernel and converted into DT. -- balbi [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] [-- Attachment #2: Type: text/plain, Size: 192 bytes --] _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 9:09 ` Felipe Balbi 0 siblings, 0 replies; 60+ messages in thread From: Felipe Balbi @ 2011-07-21 9:09 UTC (permalink / raw) To: linux-arm-kernel Hi, On Thu, Jul 21, 2011 at 02:25:03PM +0530, Rajendra Nayak wrote: > On 7/20/2011 3:04 AM, Grant Likely wrote: > >On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: > >>* Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: > >>> > >>>The way I see it, you've got two options: > >>> > >>>1) modify the of_platform_bus_create() to call some kind of > >>>of_platform_bus_create_omap() for devices that match "ti,omap3-device" > >>>or something. > >>> > >>>2) Leave of_platform_bus_create(), and instead us a notifier to attach > >>>hwmod data to normal platform devices. omap_device_build() is > >>>actually pretty simple. It allocated a device, it attaches > >>>platform_data and hwmod pointers to the device and registers it. > >>>omap_device_register() is just a wrapper around > >>>platform_device_register(). > >>> > >>>My preference is definitely #2, but there is a wrinkle in this > >>>approach. Unfortunately omap_devices are not simply plain > >>>platform_devices with extra data attached, an omap_device actually > >>>embeds the platform_device inside it, which cannot be attached after > >>>the fact. I think I had talked with Kevin (cc'd) about eliminating > >>>the embedding, but I cannot remember clearly on this point. As long > >>>as platform_device remains embedded inside struct omap_device, #2 > >>>won't work. > >>> > >>>In either case, looking up the correct hwmod data should be easy for > >>>any device provided the omap code maintains a lookup table of > >>>compatible strings and base addresses of devices (much like auxdata). > >>>In fact, I'd be okay with extending auxdata to include OMAP fields if > >>>that would be easiest since the whole point of auxdata is to ease the > >>>transition to DT support. When a matching device is created, the > >>>hwmod pointer can easily be attached. This should work transparently > >>>for all omap devices, not just the i2c bus. > >> > >>Well we should be able to automgagically build the omap_device for > >>each device tree entry. > >> > >>And then the device driver probe and runtime PM should be able to take > >>care of the rest for the driver. And then there's no more driver > >>specific platform init code needed ;) > > > >Right! That's the solution I'd like to see. > > > >>How about if we just have the hwmod code call omap_device_build for > >>each device tree entry? > > > >I think that is pretty much equivalent to suggestion #1 above, only > >I'm suggesting to take advantage of the infrastructure already > >available in driver/of/platform.c in the form of > >of_platform_populate(). The "of_platform_bus_create_omap()" function > >suggested above I assumed would directly call omap_device_build(). > > In fact a lot of what omap_device_build() does today might not even be > needed anymore. A lot of what it does is populate the platform_device > structure by looking up the hwmod structs. > Most of that information would now come from DT and hence all of that > can be taken off from the hwmod structs. What will still be needed in > hwmod is other data needed to runtime enable/idle the devices. That > data however still needs to be linked with the platform_device's that > DT would create which is what I guess could be done in something > like a of_platform_bus_create_omap(). > > Paul/Benoit, do you guys agree we can get rid of some of the data > from hwmod, whatever would now get passed in from DT, and keep > only the PRCM/OCP related stuff for runtime handling? IMHO, all omap_hwmod_*_data.c files become pretty much useless if we move completely to DT. All hwmod data is passing today, can be passed via DT and in a similar Hierarchical manner. Now WRT omap_device_build() and PM, I think that's still necessary because it simplifies a lot PM handling. But the data files themselves can "easily" be purged from kernel and converted into DT. -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110721/ed02f2b1/attachment.sig> ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-21 9:09 ` Felipe Balbi @ 2011-07-21 9:33 ` Rajendra Nayak -1 siblings, 0 replies; 60+ messages in thread From: Rajendra Nayak @ 2011-07-21 9:33 UTC (permalink / raw) To: balbi Cc: Kevin Hilman, Paul Walmsley, Cousson, Benoit, Tony Lindgren, G, Manjunath Kondaiah, devicetree-discuss, Grant Likely, ben-linux, linux-omap, linux-arm-kernel On 7/21/2011 2:39 PM, Felipe Balbi wrote: > Hi, > > On Thu, Jul 21, 2011 at 02:25:03PM +0530, Rajendra Nayak wrote: >> On 7/20/2011 3:04 AM, Grant Likely wrote: >>> On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: >>>> * Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: >>>>> >>>>> The way I see it, you've got two options: >>>>> >>>>> 1) modify the of_platform_bus_create() to call some kind of >>>>> of_platform_bus_create_omap() for devices that match "ti,omap3-device" >>>>> or something. >>>>> >>>>> 2) Leave of_platform_bus_create(), and instead us a notifier to attach >>>>> hwmod data to normal platform devices. omap_device_build() is >>>>> actually pretty simple. It allocated a device, it attaches >>>>> platform_data and hwmod pointers to the device and registers it. >>>>> omap_device_register() is just a wrapper around >>>>> platform_device_register(). >>>>> >>>>> My preference is definitely #2, but there is a wrinkle in this >>>>> approach. Unfortunately omap_devices are not simply plain >>>>> platform_devices with extra data attached, an omap_device actually >>>>> embeds the platform_device inside it, which cannot be attached after >>>>> the fact. I think I had talked with Kevin (cc'd) about eliminating >>>>> the embedding, but I cannot remember clearly on this point. As long >>>>> as platform_device remains embedded inside struct omap_device, #2 >>>>> won't work. >>>>> >>>>> In either case, looking up the correct hwmod data should be easy for >>>>> any device provided the omap code maintains a lookup table of >>>>> compatible strings and base addresses of devices (much like auxdata). >>>>> In fact, I'd be okay with extending auxdata to include OMAP fields if >>>>> that would be easiest since the whole point of auxdata is to ease the >>>>> transition to DT support. When a matching device is created, the >>>>> hwmod pointer can easily be attached. This should work transparently >>>>> for all omap devices, not just the i2c bus. >>>> >>>> Well we should be able to automgagically build the omap_device for >>>> each device tree entry. >>>> >>>> And then the device driver probe and runtime PM should be able to take >>>> care of the rest for the driver. And then there's no more driver >>>> specific platform init code needed ;) >>> >>> Right! That's the solution I'd like to see. >>> >>>> How about if we just have the hwmod code call omap_device_build for >>>> each device tree entry? >>> >>> I think that is pretty much equivalent to suggestion #1 above, only >>> I'm suggesting to take advantage of the infrastructure already >>> available in driver/of/platform.c in the form of >>> of_platform_populate(). The "of_platform_bus_create_omap()" function >>> suggested above I assumed would directly call omap_device_build(). >> >> In fact a lot of what omap_device_build() does today might not even be >> needed anymore. A lot of what it does is populate the platform_device >> structure by looking up the hwmod structs. >> Most of that information would now come from DT and hence all of that >> can be taken off from the hwmod structs. What will still be needed in >> hwmod is other data needed to runtime enable/idle the devices. That >> data however still needs to be linked with the platform_device's that >> DT would create which is what I guess could be done in something >> like a of_platform_bus_create_omap(). >> >> Paul/Benoit, do you guys agree we can get rid of some of the data >> from hwmod, whatever would now get passed in from DT, and keep >> only the PRCM/OCP related stuff for runtime handling? > > IMHO, all omap_hwmod_*_data.c files become pretty much useless if we > move completely to DT. All hwmod data is passing today, can be passed > via DT and in a similar Hierarchical manner. Would the data representation be equally readable? That's one of the problems I faced when I started looking at it initially trying to move a lot of these structures. > > Now WRT omap_device_build() and PM, I think that's still necessary > because it simplifies a lot PM handling. But the data files themselves > can "easily" be purged from kernel and converted into DT. > ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 9:33 ` Rajendra Nayak 0 siblings, 0 replies; 60+ messages in thread From: Rajendra Nayak @ 2011-07-21 9:33 UTC (permalink / raw) To: linux-arm-kernel On 7/21/2011 2:39 PM, Felipe Balbi wrote: > Hi, > > On Thu, Jul 21, 2011 at 02:25:03PM +0530, Rajendra Nayak wrote: >> On 7/20/2011 3:04 AM, Grant Likely wrote: >>> On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: >>>> * Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: >>>>> >>>>> The way I see it, you've got two options: >>>>> >>>>> 1) modify the of_platform_bus_create() to call some kind of >>>>> of_platform_bus_create_omap() for devices that match "ti,omap3-device" >>>>> or something. >>>>> >>>>> 2) Leave of_platform_bus_create(), and instead us a notifier to attach >>>>> hwmod data to normal platform devices. omap_device_build() is >>>>> actually pretty simple. It allocated a device, it attaches >>>>> platform_data and hwmod pointers to the device and registers it. >>>>> omap_device_register() is just a wrapper around >>>>> platform_device_register(). >>>>> >>>>> My preference is definitely #2, but there is a wrinkle in this >>>>> approach. Unfortunately omap_devices are not simply plain >>>>> platform_devices with extra data attached, an omap_device actually >>>>> embeds the platform_device inside it, which cannot be attached after >>>>> the fact. I think I had talked with Kevin (cc'd) about eliminating >>>>> the embedding, but I cannot remember clearly on this point. As long >>>>> as platform_device remains embedded inside struct omap_device, #2 >>>>> won't work. >>>>> >>>>> In either case, looking up the correct hwmod data should be easy for >>>>> any device provided the omap code maintains a lookup table of >>>>> compatible strings and base addresses of devices (much like auxdata). >>>>> In fact, I'd be okay with extending auxdata to include OMAP fields if >>>>> that would be easiest since the whole point of auxdata is to ease the >>>>> transition to DT support. When a matching device is created, the >>>>> hwmod pointer can easily be attached. This should work transparently >>>>> for all omap devices, not just the i2c bus. >>>> >>>> Well we should be able to automgagically build the omap_device for >>>> each device tree entry. >>>> >>>> And then the device driver probe and runtime PM should be able to take >>>> care of the rest for the driver. And then there's no more driver >>>> specific platform init code needed ;) >>> >>> Right! That's the solution I'd like to see. >>> >>>> How about if we just have the hwmod code call omap_device_build for >>>> each device tree entry? >>> >>> I think that is pretty much equivalent to suggestion #1 above, only >>> I'm suggesting to take advantage of the infrastructure already >>> available in driver/of/platform.c in the form of >>> of_platform_populate(). The "of_platform_bus_create_omap()" function >>> suggested above I assumed would directly call omap_device_build(). >> >> In fact a lot of what omap_device_build() does today might not even be >> needed anymore. A lot of what it does is populate the platform_device >> structure by looking up the hwmod structs. >> Most of that information would now come from DT and hence all of that >> can be taken off from the hwmod structs. What will still be needed in >> hwmod is other data needed to runtime enable/idle the devices. That >> data however still needs to be linked with the platform_device's that >> DT would create which is what I guess could be done in something >> like a of_platform_bus_create_omap(). >> >> Paul/Benoit, do you guys agree we can get rid of some of the data >> from hwmod, whatever would now get passed in from DT, and keep >> only the PRCM/OCP related stuff for runtime handling? > > IMHO, all omap_hwmod_*_data.c files become pretty much useless if we > move completely to DT. All hwmod data is passing today, can be passed > via DT and in a similar Hierarchical manner. Would the data representation be equally readable? That's one of the problems I faced when I started looking at it initially trying to move a lot of these structures. > > Now WRT omap_device_build() and PM, I think that's still necessary > because it simplifies a lot PM handling. But the data files themselves > can "easily" be purged from kernel and converted into DT. > ^ permalink raw reply [flat|nested] 60+ messages in thread
[parent not found: <4E27F264.4040409-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-21 9:33 ` Rajendra Nayak @ 2011-07-21 9:39 ` Felipe Balbi -1 siblings, 0 replies; 60+ messages in thread From: Felipe Balbi @ 2011-07-21 9:39 UTC (permalink / raw) To: Rajendra Nayak Cc: Kevin Hilman, Paul Walmsley, Cousson, Benoit, Tony Lindgren, G, Manjunath Kondaiah, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, balbi-l0cyMroinI0, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [-- Attachment #1.1: Type: text/plain, Size: 516 bytes --] Hi, On Thu, Jul 21, 2011 at 03:03:24PM +0530, Rajendra Nayak wrote: > >IMHO, all omap_hwmod_*_data.c files become pretty much useless if we > >move completely to DT. All hwmod data is passing today, can be passed > >via DT and in a similar Hierarchical manner. > > Would the data representation be equally readable? > That's one of the problems I faced when I started > looking at it initially trying to move a lot of these > structures. not to my eyes, heh. I'm too used to C, though. -- balbi [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] [-- Attachment #2: Type: text/plain, Size: 192 bytes --] _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 9:39 ` Felipe Balbi 0 siblings, 0 replies; 60+ messages in thread From: Felipe Balbi @ 2011-07-21 9:39 UTC (permalink / raw) To: linux-arm-kernel Hi, On Thu, Jul 21, 2011 at 03:03:24PM +0530, Rajendra Nayak wrote: > >IMHO, all omap_hwmod_*_data.c files become pretty much useless if we > >move completely to DT. All hwmod data is passing today, can be passed > >via DT and in a similar Hierarchical manner. > > Would the data representation be equally readable? > That's one of the problems I faced when I started > looking at it initially trying to move a lot of these > structures. not to my eyes, heh. I'm too used to C, though. -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 490 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110721/91fe2145/attachment.sig> ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-21 8:55 ` Rajendra Nayak @ 2011-07-28 18:20 ` Cousson, Benoit -1 siblings, 0 replies; 60+ messages in thread From: Cousson, Benoit @ 2011-07-28 18:20 UTC (permalink / raw) To: Nayak, Rajendra Cc: Grant Likely, Tony Lindgren, Hilman, Kevin, G, Manjunath Kondaiah, devicetree-discuss, ben-linux, linux-omap, linux-arm-kernel, Paul Walmsley Hi Rajendra, On 7/21/2011 10:55 AM, Nayak, Rajendra wrote: > On 7/20/2011 3:04 AM, Grant Likely wrote: >> On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: >>> * Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: >>>> >>>> The way I see it, you've got two options: >>>> >>>> 1) modify the of_platform_bus_create() to call some kind of >>>> of_platform_bus_create_omap() for devices that match "ti,omap3-device" >>>> or something. >>>> >>>> 2) Leave of_platform_bus_create(), and instead us a notifier to attach >>>> hwmod data to normal platform devices. omap_device_build() is >>>> actually pretty simple. It allocated a device, it attaches >>>> platform_data and hwmod pointers to the device and registers it. >>>> omap_device_register() is just a wrapper around >>>> platform_device_register(). >>>> >>>> My preference is definitely #2, but there is a wrinkle in this >>>> approach. Unfortunately omap_devices are not simply plain >>>> platform_devices with extra data attached, an omap_device actually >>>> embeds the platform_device inside it, which cannot be attached after >>>> the fact. I think I had talked with Kevin (cc'd) about eliminating >>>> the embedding, but I cannot remember clearly on this point. As long >>>> as platform_device remains embedded inside struct omap_device, #2 >>>> won't work. >>>> >>>> In either case, looking up the correct hwmod data should be easy for >>>> any device provided the omap code maintains a lookup table of >>>> compatible strings and base addresses of devices (much like auxdata). >>>> In fact, I'd be okay with extending auxdata to include OMAP fields if >>>> that would be easiest since the whole point of auxdata is to ease the >>>> transition to DT support. When a matching device is created, the >>>> hwmod pointer can easily be attached. This should work transparently >>>> for all omap devices, not just the i2c bus. >>> >>> Well we should be able to automgagically build the omap_device for >>> each device tree entry. >>> >>> And then the device driver probe and runtime PM should be able to take >>> care of the rest for the driver. And then there's no more driver >>> specific platform init code needed ;) >> >> Right! That's the solution I'd like to see. >> >>> How about if we just have the hwmod code call omap_device_build for >>> each device tree entry? >> >> I think that is pretty much equivalent to suggestion #1 above, only >> I'm suggesting to take advantage of the infrastructure already >> available in driver/of/platform.c in the form of >> of_platform_populate(). The "of_platform_bus_create_omap()" function >> suggested above I assumed would directly call omap_device_build(). > > In fact a lot of what omap_device_build() does today might not even be > needed anymore. A lot of what it does is populate the platform_device > structure by looking up the hwmod structs. > Most of that information would now come from DT and hence all of that > can be taken off from the hwmod structs. What will still be needed in > hwmod is other data needed to runtime enable/idle the devices. That > data however still needs to be linked with the platform_device's that > DT would create which is what I guess could be done in something > like a of_platform_bus_create_omap(). > > Paul/Benoit, do you guys agree we can get rid of some of the data > from hwmod, whatever would now get passed in from DT, and keep > only the PRCM/OCP related stuff for runtime handling? We need to discuss that further, but last time we discussed with Paul and Tony, we were considering starting with a DT that will just contain the list of devices and a reference to one of several hwmod name. That will allow us to get rid of all the crappy devices init code we have here and there in OMAP today. That phase can already keep us busy for some time :-) The DT format has some limitation today to describe a device that is connected to several buses and thus have several addresses. The DT format is providing only the CPU view of the system, meaning that we cannot take advantage of it to give the memory map of the DSP, or the CortexM3 inside OMAP4 for example. OK, I know, hwmod is not providing that information either today... but that was the plan:-) Because of that we will miss a lot of data we are retrieving today from hwmod. So for sure, we can define any kind of data in DT, but it will be much better to support that kind of details in the format directly instead of adding some custom TI nodes. For my point of view, the DT format should evolve toward a full hierarchical HW representation from the board level instead of a CPU centric one. But that just my .2 cents. Meanwhile, maybe we can start moving basic data from hwmod to DT. At first we should probably just do the device -> hwmod binding using DT. There is so much stuff to do, that the hardest part is to figure out where to start:-) Regards, Benoit ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-28 18:20 ` Cousson, Benoit 0 siblings, 0 replies; 60+ messages in thread From: Cousson, Benoit @ 2011-07-28 18:20 UTC (permalink / raw) To: linux-arm-kernel Hi Rajendra, On 7/21/2011 10:55 AM, Nayak, Rajendra wrote: > On 7/20/2011 3:04 AM, Grant Likely wrote: >> On Mon, Jul 18, 2011 at 02:07:10AM -0700, Tony Lindgren wrote: >>> * Grant Likely<grant.likely@secretlab.ca> [110716 22:08]: >>>> >>>> The way I see it, you've got two options: >>>> >>>> 1) modify the of_platform_bus_create() to call some kind of >>>> of_platform_bus_create_omap() for devices that match "ti,omap3-device" >>>> or something. >>>> >>>> 2) Leave of_platform_bus_create(), and instead us a notifier to attach >>>> hwmod data to normal platform devices. omap_device_build() is >>>> actually pretty simple. It allocated a device, it attaches >>>> platform_data and hwmod pointers to the device and registers it. >>>> omap_device_register() is just a wrapper around >>>> platform_device_register(). >>>> >>>> My preference is definitely #2, but there is a wrinkle in this >>>> approach. Unfortunately omap_devices are not simply plain >>>> platform_devices with extra data attached, an omap_device actually >>>> embeds the platform_device inside it, which cannot be attached after >>>> the fact. I think I had talked with Kevin (cc'd) about eliminating >>>> the embedding, but I cannot remember clearly on this point. As long >>>> as platform_device remains embedded inside struct omap_device, #2 >>>> won't work. >>>> >>>> In either case, looking up the correct hwmod data should be easy for >>>> any device provided the omap code maintains a lookup table of >>>> compatible strings and base addresses of devices (much like auxdata). >>>> In fact, I'd be okay with extending auxdata to include OMAP fields if >>>> that would be easiest since the whole point of auxdata is to ease the >>>> transition to DT support. When a matching device is created, the >>>> hwmod pointer can easily be attached. This should work transparently >>>> for all omap devices, not just the i2c bus. >>> >>> Well we should be able to automgagically build the omap_device for >>> each device tree entry. >>> >>> And then the device driver probe and runtime PM should be able to take >>> care of the rest for the driver. And then there's no more driver >>> specific platform init code needed ;) >> >> Right! That's the solution I'd like to see. >> >>> How about if we just have the hwmod code call omap_device_build for >>> each device tree entry? >> >> I think that is pretty much equivalent to suggestion #1 above, only >> I'm suggesting to take advantage of the infrastructure already >> available in driver/of/platform.c in the form of >> of_platform_populate(). The "of_platform_bus_create_omap()" function >> suggested above I assumed would directly call omap_device_build(). > > In fact a lot of what omap_device_build() does today might not even be > needed anymore. A lot of what it does is populate the platform_device > structure by looking up the hwmod structs. > Most of that information would now come from DT and hence all of that > can be taken off from the hwmod structs. What will still be needed in > hwmod is other data needed to runtime enable/idle the devices. That > data however still needs to be linked with the platform_device's that > DT would create which is what I guess could be done in something > like a of_platform_bus_create_omap(). > > Paul/Benoit, do you guys agree we can get rid of some of the data > from hwmod, whatever would now get passed in from DT, and keep > only the PRCM/OCP related stuff for runtime handling? We need to discuss that further, but last time we discussed with Paul and Tony, we were considering starting with a DT that will just contain the list of devices and a reference to one of several hwmod name. That will allow us to get rid of all the crappy devices init code we have here and there in OMAP today. That phase can already keep us busy for some time :-) The DT format has some limitation today to describe a device that is connected to several buses and thus have several addresses. The DT format is providing only the CPU view of the system, meaning that we cannot take advantage of it to give the memory map of the DSP, or the CortexM3 inside OMAP4 for example. OK, I know, hwmod is not providing that information either today... but that was the plan:-) Because of that we will miss a lot of data we are retrieving today from hwmod. So for sure, we can define any kind of data in DT, but it will be much better to support that kind of details in the format directly instead of adding some custom TI nodes. For my point of view, the DT format should evolve toward a full hierarchical HW representation from the board level instead of a CPU centric one. But that just my .2 cents. Meanwhile, maybe we can start moving basic data from hwmod to DT. At first we should probably just do the device -> hwmod binding using DT. There is so much stuff to do, that the hardest part is to figure out where to start:-) Regards, Benoit ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-17 5:13 ` Grant Likely @ 2011-07-18 10:15 ` G, Manjunath Kondaiah -1 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-18 10:15 UTC (permalink / raw) To: Grant Likely Cc: Kevin Hilman, Tony Lindgren, devicetree-discuss, linux-omap, ben-linux, linux-arm-kernel Hi Grant, On 17 July 2011 10:43, Grant Likely <grant.likely@secretlab.ca> wrote: > Hi Manjunath, > > Comments below. I left in a lot of context for the new folks that > I've cc'd (Tony and Kevin). > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>>> > +static void __init omap3_init(void) >>>> > +{ >>>> > + struct device_node *node; >>>> > + [...] >> +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, >> + const char *bus_id, >> + void *platform_data, >> + struct device *parent) >> +{ >> + struct platform_device *pdev; >> + struct i2c_board_info *i2c_board_info; >> + u8 id; >> + >> + printk("Creating omap i2c device %s\n", node->full_name); >> + >> + if (!of_device_is_available(node)) >> + return NULL; >> + >> + id = simple_strtol(bus_id, NULL, 0); >> + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); >> + pdev->dev.of_node = of_node_get(node); >> + if (!pdev->dev.of_node) { >> + speed = 100; >> + } else { >> + u32 prop; >> + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", >> + &prop)) >> + speed = prop/100; >> + else >> + speed = 100; >> + } >> + printk("%s : speed->%d\n",__func__, speed); >> + >> + for_each_child_of_node(bus, child) { >> + u32 prop; >> + >> + printk(" create child: %s\n", child->full_name); >> + i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); >> + if (!of_property_read_u32(pdev->dev.of_node, "reg", >> + &prop)) >> + i2c_board_info->addr = prop; >> + if (!of_property_read_u32(pdev->dev.of_node, "interrupts", >> + &prop)) >> + i2c_board_info->irq = prop; >> + i2c_board_info->platform_data = platform_data; >> + strncpy(i2c_board_info->type, child->name, >> + sizeof(i2c_board_info->type)); >> + } >> + omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > While this does in a sense work, and creates an omap_device for the > i2c bus that does get probed, it ends up encoding an awful lot of > device specific details into the generic devicetree support code. The > i2c bus driver itself must be responsible for decoding the speed and > child nodes, and in fact it can easily be modified to do so (I've Decoding speed in i2c driver seems to be fine. But the i2c child nodes are board specific. We might bring board specific handling code into i2c driver with this approach. BTW, I have observed that, if we create i2c device node in omapx-soc.dtsi file and the define i2c bus clock-frequency in beagle.dts, the clock-frequency is not available in driver probe. Is this expected behavior? -M -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-18 10:15 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-18 10:15 UTC (permalink / raw) To: linux-arm-kernel Hi Grant, On 17 July 2011 10:43, Grant Likely <grant.likely@secretlab.ca> wrote: > Hi Manjunath, > > Comments below. ?I left in a lot of context for the new folks that > I've cc'd (Tony and Kevin). > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>>> > +static void __init omap3_init(void) >>>> > +{ >>>> > + ? ? ? struct device_node *node; >>>> > + [...] >> +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *bus_id, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *platform_data, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device *parent) >> +{ >> + ? ? ? struct platform_device *pdev; >> + ? ? ? struct i2c_board_info *i2c_board_info; >> + ? ? ? u8 id; >> + >> + ? ? ? printk("Creating omap i2c device %s\n", node->full_name); >> + >> + ? ? ? if (!of_device_is_available(node)) >> + ? ? ? ? ? ? ? return NULL; >> + >> + ? ? ? id = simple_strtol(bus_id, NULL, 0); >> + ? ? ? pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); >> + ? ? ? pdev->dev.of_node = of_node_get(node); >> + ? ? ? if (!pdev->dev.of_node) { >> + ? ? ? ? ? ? ? speed = 100; >> + ? ? ? } else { >> + ? ? ? ? ? ? ? u32 prop; >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) >> + ? ? ? ? ? ? ? ? ? ? ? speed = prop/100; >> + ? ? ? ? ? ? ? else >> + ? ? ? ? ? ? ? ? ? ? ? speed = 100; >> + ? ? ? } >> + ? ? ? printk("%s : speed->%d\n",__func__, speed); >> + >> + ? ? ? for_each_child_of_node(bus, child) { >> + ? ? ? ? ? ? ? u32 prop; >> + >> + ? ? ? ? ? ? ? printk(" ? create child: %s\n", child->full_name); >> + ? ? ? ? ? ? ? i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "reg", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) >> + ? ? ? ? ? ? ? i2c_board_info->addr = prop; >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "interrupts", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) >> + ? ? ? ? ? ? ? i2c_board_info->irq = prop; >> + ? ? ? ? ? ? ? i2c_board_info->platform_data = platform_data; >> + ? ? ? ? ? ? ? strncpy(i2c_board_info->type, child->name, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(i2c_board_info->type)); >> + ? ? ? } >> + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > While this does in a sense work, and creates an omap_device for the > i2c bus that does get probed, it ends up encoding an awful lot of > device specific details into the generic devicetree support code. ?The > i2c bus driver itself must be responsible for decoding the speed and > child nodes, and in fact it can easily be modified to do so (I've Decoding speed in i2c driver seems to be fine. But the i2c child nodes are board specific. We might bring board specific handling code into i2c driver with this approach. BTW, I have observed that, if we create i2c device node in omapx-soc.dtsi file and the define i2c bus clock-frequency in beagle.dts, the clock-frequency is not available in driver probe. Is this expected behavior? -M ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-18 10:15 ` G, Manjunath Kondaiah @ 2011-07-19 21:36 ` Grant Likely -1 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-19 21:36 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: Kevin Hilman, Tony Lindgren, devicetree-discuss, linux-omap, ben-linux, linux-arm-kernel On Mon, Jul 18, 2011 at 03:45:57PM +0530, G, Manjunath Kondaiah wrote: > Hi Grant, > > On 17 July 2011 10:43, Grant Likely <grant.likely@secretlab.ca> wrote: > > Hi Manjunath, > > > > Comments below. I left in a lot of context for the new folks that > > I've cc'd (Tony and Kevin). > > > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > >>>> > +static void __init omap3_init(void) > >>>> > +{ > >>>> > + struct device_node *node; > >>>> > + > [...] > >> +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, > >> + const char *bus_id, > >> + void *platform_data, > >> + struct device *parent) > >> +{ > >> + struct platform_device *pdev; > >> + struct i2c_board_info *i2c_board_info; > >> + u8 id; > >> + > >> + printk("Creating omap i2c device %s\n", node->full_name); > >> + > >> + if (!of_device_is_available(node)) > >> + return NULL; > >> + > >> + id = simple_strtol(bus_id, NULL, 0); > >> + pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); > >> + pdev->dev.of_node = of_node_get(node); > >> + if (!pdev->dev.of_node) { > >> + speed = 100; > >> + } else { > >> + u32 prop; > >> + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > >> + &prop)) > >> + speed = prop/100; > >> + else > >> + speed = 100; > >> + } > >> + printk("%s : speed->%d\n",__func__, speed); > >> + > >> + for_each_child_of_node(bus, child) { > >> + u32 prop; > >> + > >> + printk(" create child: %s\n", child->full_name); > >> + i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); > >> + if (!of_property_read_u32(pdev->dev.of_node, "reg", > >> + &prop)) > >> + i2c_board_info->addr = prop; > >> + if (!of_property_read_u32(pdev->dev.of_node, "interrupts", > >> + &prop)) > >> + i2c_board_info->irq = prop; > >> + i2c_board_info->platform_data = platform_data; > >> + strncpy(i2c_board_info->type, child->name, > >> + sizeof(i2c_board_info->type)); > >> + } > >> + omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > > > While this does in a sense work, and creates an omap_device for the > > i2c bus that does get probed, it ends up encoding an awful lot of > > device specific details into the generic devicetree support code. The > > i2c bus driver itself must be responsible for decoding the speed and > > child nodes, and in fact it can easily be modified to do so (I've > > Decoding speed in i2c driver seems to be fine. But the i2c child nodes are > board specific. We might bring board specific handling code into i2c driver > with this approach. It shouldn't. They're just i2c devices, and the child nodes will always follow the i2c device binding. > BTW, I have observed that, if we create i2c device node in omapx-soc.dtsi > file and the define i2c bus clock-frequency in beagle.dts, the clock-frequency > is not available in driver probe. Is this expected behavior? No, it sounds like something isn't getting set up correctly. Send me your current beagle.dts and omap3-soc.dtsi files. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-19 21:36 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-19 21:36 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jul 18, 2011 at 03:45:57PM +0530, G, Manjunath Kondaiah wrote: > Hi Grant, > > On 17 July 2011 10:43, Grant Likely <grant.likely@secretlab.ca> wrote: > > Hi Manjunath, > > > > Comments below. ?I left in a lot of context for the new folks that > > I've cc'd (Tony and Kevin). > > > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > >>>> > +static void __init omap3_init(void) > >>>> > +{ > >>>> > + ? ? ? struct device_node *node; > >>>> > + > [...] > >> +static struct omap_device *of_omap_i2c_device_create(struct device_node *node, > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *bus_id, > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *platform_data, > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device *parent) > >> +{ > >> + ? ? ? struct platform_device *pdev; > >> + ? ? ? struct i2c_board_info *i2c_board_info; > >> + ? ? ? u8 id; > >> + > >> + ? ? ? printk("Creating omap i2c device %s\n", node->full_name); > >> + > >> + ? ? ? if (!of_device_is_available(node)) > >> + ? ? ? ? ? ? ? return NULL; > >> + > >> + ? ? ? id = simple_strtol(bus_id, NULL, 0); > >> + ? ? ? pdev = kzalloc(sizeof(*pdev), GFP_KERNEL); > >> + ? ? ? pdev->dev.of_node = of_node_get(node); > >> + ? ? ? if (!pdev->dev.of_node) { > >> + ? ? ? ? ? ? ? speed = 100; > >> + ? ? ? } else { > >> + ? ? ? ? ? ? ? u32 prop; > >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > >> + ? ? ? ? ? ? ? ? ? ? ? speed = prop/100; > >> + ? ? ? ? ? ? ? else > >> + ? ? ? ? ? ? ? ? ? ? ? speed = 100; > >> + ? ? ? } > >> + ? ? ? printk("%s : speed->%d\n",__func__, speed); > >> + > >> + ? ? ? for_each_child_of_node(bus, child) { > >> + ? ? ? ? ? ? ? u32 prop; > >> + > >> + ? ? ? ? ? ? ? printk(" ? create child: %s\n", child->full_name); > >> + ? ? ? ? ? ? ? i2c_board_info = kzalloc(sizeof(*i2c_board_info), GFP_KERNEL); > >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "reg", > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > >> + ? ? ? ? ? ? ? i2c_board_info->addr = prop; > >> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "interrupts", > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > >> + ? ? ? ? ? ? ? i2c_board_info->irq = prop; > >> + ? ? ? ? ? ? ? i2c_board_info->platform_data = platform_data; > >> + ? ? ? ? ? ? ? strncpy(i2c_board_info->type, child->name, > >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof(i2c_board_info->type)); > >> + ? ? ? } > >> + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > > > While this does in a sense work, and creates an omap_device for the > > i2c bus that does get probed, it ends up encoding an awful lot of > > device specific details into the generic devicetree support code. ?The > > i2c bus driver itself must be responsible for decoding the speed and > > child nodes, and in fact it can easily be modified to do so (I've > > Decoding speed in i2c driver seems to be fine. But the i2c child nodes are > board specific. We might bring board specific handling code into i2c driver > with this approach. It shouldn't. They're just i2c devices, and the child nodes will always follow the i2c device binding. > BTW, I have observed that, if we create i2c device node in omapx-soc.dtsi > file and the define i2c bus clock-frequency in beagle.dts, the clock-frequency > is not available in driver probe. Is this expected behavior? No, it sounds like something isn't getting set up correctly. Send me your current beagle.dts and omap3-soc.dtsi files. g. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-17 5:13 ` Grant Likely @ 2011-07-19 5:58 ` G, Manjunath Kondaiah -1 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-19 5:58 UTC (permalink / raw) To: Grant Likely Cc: Kevin Hilman, Tony Lindgren, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux Grant/Kevin, On Sun, Jul 17, 2011 at 10:43 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > Hi Manjunath, > > Comments below. I left in a lot of context for the new folks that > I've cc'd (Tony and Kevin). > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>>> > +static void __init omap3_init(void) >>>> > +{ [...] >> + omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > While this does in a sense work, and creates an omap_device for the > i2c bus that does get probed, it ends up encoding an awful lot of > device specific details into the generic devicetree support code. The > i2c bus driver itself must be responsible for decoding the speed and > child nodes, and in fact it can easily be modified to do so (I've > already demonstrated how to do so). The real problem is making sure > the platform_device is created in a way that attaches the correct > hwmod data. For this context, the current omap_register_i2c_bus() > isn't a useful method for doing so. > > So what is to be done? omap_register_i2c_bus() does three things; > 1) register an i2c board info for the bus with i2c_register_board_info(), > 2) fill platform_data for the device, and > 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. > > Of these three, 1 & 2 must not be done when using the DT. Only > omap_i2c_add_bus() does something useful, but that is still specific > to the i2c device. > > omap_i2c_add_bus() splits to omap{1,2}_add_bus(). > > omap1_i2c_add_bus() sets up pinmux and calls platform_device register. > pinmux setup needs to be factored out anyway for generic DT platform > device registration, which just leaves platform_device creation which > is already handled by of_platform_populate(). > > omap2_i2c_add_bus() does the same thing, except it also looks up the > hwmod data (*oh) and uses it to call omap_device_build(). > omap_device_build() or something equivalent needs to be called for > every omap device in the system, which is to create platform_devices > with hwmod attached. Now we're starting to hit generic code. :-) > > The way I see it, you've got two options: > > 1) modify the of_platform_bus_create() to call some kind of > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > or something. > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > hwmod data to normal platform devices. omap_device_build() is > actually pretty simple. It allocated a device, it attaches > platform_data and hwmod pointers to the device and registers it. > omap_device_register() is just a wrapper around > platform_device_register(). > > My preference is definitely #2, but there is a wrinkle in this > approach. Unfortunately omap_devices are not simply plain > platform_devices with extra data attached, an omap_device actually > embeds the platform_device inside it, which cannot be attached after > the fact. I think I had talked with Kevin (cc'd) about eliminating > the embedding, but I cannot remember clearly on this point. As long > as platform_device remains embedded inside struct omap_device, #2 > won't work. Can you please elaborate more on this issue? -Manjunath -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-19 5:58 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-19 5:58 UTC (permalink / raw) To: linux-arm-kernel Grant/Kevin, On Sun, Jul 17, 2011 at 10:43 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > Hi Manjunath, > > Comments below. ?I left in a lot of context for the new folks that > I've cc'd (Tony and Kevin). > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >>>> > +static void __init omap3_init(void) >>>> > +{ [...] >> + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > While this does in a sense work, and creates an omap_device for the > i2c bus that does get probed, it ends up encoding an awful lot of > device specific details into the generic devicetree support code. ?The > i2c bus driver itself must be responsible for decoding the speed and > child nodes, and in fact it can easily be modified to do so (I've > already demonstrated how to do so). ?The real problem is making sure > the platform_device is created in a way that attaches the correct > hwmod data. For this context, the current omap_register_i2c_bus() > isn't a useful method for doing so. > > So what is to be done? ?omap_register_i2c_bus() does three things; > 1) register an i2c board info for the bus with i2c_register_board_info(), > 2) fill platform_data for the device, and > 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. > > Of these three, 1 & 2 must not be done when using the DT. Only > omap_i2c_add_bus() does something useful, but that is still specific > to the i2c device. > > omap_i2c_add_bus() splits to omap{1,2}_add_bus(). > > omap1_i2c_add_bus() sets up pinmux and calls platform_device register. > ?pinmux setup needs to be factored out anyway for generic DT platform > device registration, which just leaves platform_device creation which > is already handled by of_platform_populate(). > > omap2_i2c_add_bus() does the same thing, except it also looks up the > hwmod data (*oh) and uses it to call omap_device_build(). > omap_device_build() or something equivalent needs to be called for > every omap device in the system, which is to create platform_devices > with hwmod attached. ?Now we're starting to hit generic code. ?:-) > > The way I see it, you've got two options: > > 1) modify the of_platform_bus_create() to call some kind of > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > or something. > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > hwmod data to normal platform devices. ?omap_device_build() is > actually pretty simple. ?It allocated a device, it attaches > platform_data and hwmod pointers to the device and registers it. > omap_device_register() is just a wrapper around > platform_device_register(). > > My preference is definitely #2, but there is a wrinkle in this > approach. ?Unfortunately omap_devices are not simply plain > platform_devices with extra data attached, an omap_device actually > embeds the platform_device inside it, which cannot be attached after > the fact. ?I think I had talked with Kevin (cc'd) about eliminating > the embedding, but I cannot remember clearly on this point. ?As long > as platform_device remains embedded inside struct omap_device, #2 > won't work. Can you please elaborate more on this issue? -Manjunath ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-19 5:58 ` G, Manjunath Kondaiah @ 2011-07-19 21:37 ` Grant Likely -1 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-19 21:37 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: Kevin Hilman, Tony Lindgren, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Tue, Jul 19, 2011 at 11:28:53AM +0530, G, Manjunath Kondaiah wrote: > Grant/Kevin, > > On Sun, Jul 17, 2011 at 10:43 AM, Grant Likely > <grant.likely@secretlab.ca> wrote: > > Hi Manjunath, > > > > Comments below. I left in a lot of context for the new folks that > > I've cc'd (Tony and Kevin). > > > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > >>>> > +static void __init omap3_init(void) > >>>> > +{ > [...] > >> + omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > > > While this does in a sense work, and creates an omap_device for the > > i2c bus that does get probed, it ends up encoding an awful lot of > > device specific details into the generic devicetree support code. The > > i2c bus driver itself must be responsible for decoding the speed and > > child nodes, and in fact it can easily be modified to do so (I've > > already demonstrated how to do so). The real problem is making sure > > the platform_device is created in a way that attaches the correct > > hwmod data. For this context, the current omap_register_i2c_bus() > > isn't a useful method for doing so. > > > > So what is to be done? omap_register_i2c_bus() does three things; > > 1) register an i2c board info for the bus with i2c_register_board_info(), > > 2) fill platform_data for the device, and > > 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. > > > > Of these three, 1 & 2 must not be done when using the DT. Only > > omap_i2c_add_bus() does something useful, but that is still specific > > to the i2c device. > > > > omap_i2c_add_bus() splits to omap{1,2}_add_bus(). > > > > omap1_i2c_add_bus() sets up pinmux and calls platform_device register. > > pinmux setup needs to be factored out anyway for generic DT platform > > device registration, which just leaves platform_device creation which > > is already handled by of_platform_populate(). > > > > omap2_i2c_add_bus() does the same thing, except it also looks up the > > hwmod data (*oh) and uses it to call omap_device_build(). > > omap_device_build() or something equivalent needs to be called for > > every omap device in the system, which is to create platform_devices > > with hwmod attached. Now we're starting to hit generic code. :-) > > > > The way I see it, you've got two options: > > > > 1) modify the of_platform_bus_create() to call some kind of > > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > > or something. > > > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > > hwmod data to normal platform devices. omap_device_build() is > > actually pretty simple. It allocated a device, it attaches > > platform_data and hwmod pointers to the device and registers it. > > omap_device_register() is just a wrapper around > > platform_device_register(). > > > > My preference is definitely #2, but there is a wrinkle in this > > approach. Unfortunately omap_devices are not simply plain > > platform_devices with extra data attached, an omap_device actually > > embeds the platform_device inside it, which cannot be attached after > > the fact. I think I had talked with Kevin (cc'd) about eliminating > > the embedding, but I cannot remember clearly on this point. As long > > as platform_device remains embedded inside struct omap_device, #2 > > won't work. > > Can you please elaborate more on this issue? Look at the of_platform_populate() call path (in devicetree/next) and see how it handles amba devices. Do the same thing for omap_devices. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-19 21:37 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-19 21:37 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jul 19, 2011 at 11:28:53AM +0530, G, Manjunath Kondaiah wrote: > Grant/Kevin, > > On Sun, Jul 17, 2011 at 10:43 AM, Grant Likely > <grant.likely@secretlab.ca> wrote: > > Hi Manjunath, > > > > Comments below. ?I left in a lot of context for the new folks that > > I've cc'd (Tony and Kevin). > > > > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > >>>> > +static void __init omap3_init(void) > >>>> > +{ > [...] > >> + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1); > > > > While this does in a sense work, and creates an omap_device for the > > i2c bus that does get probed, it ends up encoding an awful lot of > > device specific details into the generic devicetree support code. ?The > > i2c bus driver itself must be responsible for decoding the speed and > > child nodes, and in fact it can easily be modified to do so (I've > > already demonstrated how to do so). ?The real problem is making sure > > the platform_device is created in a way that attaches the correct > > hwmod data. For this context, the current omap_register_i2c_bus() > > isn't a useful method for doing so. > > > > So what is to be done? ?omap_register_i2c_bus() does three things; > > 1) register an i2c board info for the bus with i2c_register_board_info(), > > 2) fill platform_data for the device, and > > 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. > > > > Of these three, 1 & 2 must not be done when using the DT. Only > > omap_i2c_add_bus() does something useful, but that is still specific > > to the i2c device. > > > > omap_i2c_add_bus() splits to omap{1,2}_add_bus(). > > > > omap1_i2c_add_bus() sets up pinmux and calls platform_device register. > > ?pinmux setup needs to be factored out anyway for generic DT platform > > device registration, which just leaves platform_device creation which > > is already handled by of_platform_populate(). > > > > omap2_i2c_add_bus() does the same thing, except it also looks up the > > hwmod data (*oh) and uses it to call omap_device_build(). > > omap_device_build() or something equivalent needs to be called for > > every omap device in the system, which is to create platform_devices > > with hwmod attached. ?Now we're starting to hit generic code. ?:-) > > > > The way I see it, you've got two options: > > > > 1) modify the of_platform_bus_create() to call some kind of > > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > > or something. > > > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > > hwmod data to normal platform devices. ?omap_device_build() is > > actually pretty simple. ?It allocated a device, it attaches > > platform_data and hwmod pointers to the device and registers it. > > omap_device_register() is just a wrapper around > > platform_device_register(). > > > > My preference is definitely #2, but there is a wrinkle in this > > approach. ?Unfortunately omap_devices are not simply plain > > platform_devices with extra data attached, an omap_device actually > > embeds the platform_device inside it, which cannot be attached after > > the fact. ?I think I had talked with Kevin (cc'd) about eliminating > > the embedding, but I cannot remember clearly on this point. ?As long > > as platform_device remains embedded inside struct omap_device, #2 > > won't work. > > Can you please elaborate more on this issue? Look at the of_platform_populate() call path (in devicetree/next) and see how it handles amba devices. Do the same thing for omap_devices. g. ^ permalink raw reply [flat|nested] 60+ messages in thread
[parent not found: <20110719213737.GQ6848-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>]
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-19 21:37 ` Grant Likely @ 2011-07-21 10:24 ` G, Manjunath Kondaiah -1 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-21 10:24 UTC (permalink / raw) To: Grant Likely Cc: Kevin Hilman, Tony Lindgren, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hi Grant, On Wed, Jul 20, 2011 at 3:07 AM, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: > On Tue, Jul 19, 2011 at 11:28:53AM +0530, G, Manjunath Kondaiah wrote: >> Grant/Kevin, >> >> On Sun, Jul 17, 2011 at 10:43 AM, Grant Likely >> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote: >> > Hi Manjunath, >> > >> > Comments below. I left in a lot of context for the new folks that >> > I've cc'd (Tony and Kevin). >> > >> > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk-l0cyMroinI0@public.gmane.org> wrote: >> >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> >>>> > +static void __init omap3_init(void) >> >>>> > +{ >> [...] >> >> + omap_register_i2c_bus(id, speed, i2c_board_info, 1); >> > >> > While this does in a sense work, and creates an omap_device for the >> > i2c bus that does get probed, it ends up encoding an awful lot of >> > device specific details into the generic devicetree support code. The >> > i2c bus driver itself must be responsible for decoding the speed and >> > child nodes, and in fact it can easily be modified to do so (I've >> > already demonstrated how to do so). The real problem is making sure >> > the platform_device is created in a way that attaches the correct >> > hwmod data. For this context, the current omap_register_i2c_bus() >> > isn't a useful method for doing so. >> > >> > So what is to be done? omap_register_i2c_bus() does three things; >> > 1) register an i2c board info for the bus with i2c_register_board_info(), >> > 2) fill platform_data for the device, and >> > 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. >> > >> > Of these three, 1 & 2 must not be done when using the DT. Only >> > omap_i2c_add_bus() does something useful, but that is still specific >> > to the i2c device. >> > >> > omap_i2c_add_bus() splits to omap{1,2}_add_bus(). >> > >> > omap1_i2c_add_bus() sets up pinmux and calls platform_device register. >> > pinmux setup needs to be factored out anyway for generic DT platform >> > device registration, which just leaves platform_device creation which >> > is already handled by of_platform_populate(). >> > >> > omap2_i2c_add_bus() does the same thing, except it also looks up the >> > hwmod data (*oh) and uses it to call omap_device_build(). >> > omap_device_build() or something equivalent needs to be called for >> > every omap device in the system, which is to create platform_devices >> > with hwmod attached. Now we're starting to hit generic code. :-) >> > >> > The way I see it, you've got two options: >> > >> > 1) modify the of_platform_bus_create() to call some kind of >> > of_platform_bus_create_omap() for devices that match "ti,omap3-device" >> > or something. >> > >> > 2) Leave of_platform_bus_create(), and instead us a notifier to attach >> > hwmod data to normal platform devices. omap_device_build() is >> > actually pretty simple. It allocated a device, it attaches >> > platform_data and hwmod pointers to the device and registers it. >> > omap_device_register() is just a wrapper around >> > platform_device_register(). >> > >> > My preference is definitely #2, but there is a wrinkle in this >> > approach. Unfortunately omap_devices are not simply plain >> > platform_devices with extra data attached, an omap_device actually >> > embeds the platform_device inside it, which cannot be attached after >> > the fact. I think I had talked with Kevin (cc'd) about eliminating >> > the embedding, but I cannot remember clearly on this point. As long >> > as platform_device remains embedded inside struct omap_device, #2 >> > won't work. >> >> Can you please elaborate more on this issue? > > Look at the of_platform_populate() call path (in devicetree/next) and > see how it handles amba devices. Do the same thing for omap_devices. As per the discussion so far, I am trying to take following approach: |--->of_platform_populate(...) |--->of_platform_bus_create_omap()-->Here notifiers are added |--->platform_device_register() ---> calls registered notifiers |--->notifier_callback - look up hwmod entry by name - call omap_device_build(need to pass platform_device pointer) - get all the details such as resource structures, irqs,platform_data etc from hwmod database and auxdata and assign it to platform_device pointer received from notifier callback function. - there will be no platform_device_register from omap_device_build since it is already called from "of_platform_bus_create_omap" Please note that, with the above approach, the platform_device pointer which is embedded inside omap_device is not used and omap_device_build api will expect platform device pointer. -M ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 10:24 ` G, Manjunath Kondaiah 0 siblings, 0 replies; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-21 10:24 UTC (permalink / raw) To: linux-arm-kernel Hi Grant, On Wed, Jul 20, 2011 at 3:07 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Tue, Jul 19, 2011 at 11:28:53AM +0530, G, Manjunath Kondaiah wrote: >> Grant/Kevin, >> >> On Sun, Jul 17, 2011 at 10:43 AM, Grant Likely >> <grant.likely@secretlab.ca> wrote: >> > Hi Manjunath, >> > >> > Comments below. ?I left in a lot of context for the new folks that >> > I've cc'd (Tony and Kevin). >> > >> > On Sat, Jul 16, 2011 at 2:07 PM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: >> >> On Thu, Jul 14, 2011 at 4:45 AM, Grant Likely <grant.likely@secretlab.ca> wrote: >> >>>> > +static void __init omap3_init(void) >> >>>> > +{ >> [...] >> >> + ? ? ? omap_register_i2c_bus(id, speed, i2c_board_info, 1); >> > >> > While this does in a sense work, and creates an omap_device for the >> > i2c bus that does get probed, it ends up encoding an awful lot of >> > device specific details into the generic devicetree support code. ?The >> > i2c bus driver itself must be responsible for decoding the speed and >> > child nodes, and in fact it can easily be modified to do so (I've >> > already demonstrated how to do so). ?The real problem is making sure >> > the platform_device is created in a way that attaches the correct >> > hwmod data. For this context, the current omap_register_i2c_bus() >> > isn't a useful method for doing so. >> > >> > So what is to be done? ?omap_register_i2c_bus() does three things; >> > 1) register an i2c board info for the bus with i2c_register_board_info(), >> > 2) fill platform_data for the device, and >> > 3) use omap_i2c_add_bus to create the platform_device with attached hwmod. >> > >> > Of these three, 1 & 2 must not be done when using the DT. Only >> > omap_i2c_add_bus() does something useful, but that is still specific >> > to the i2c device. >> > >> > omap_i2c_add_bus() splits to omap{1,2}_add_bus(). >> > >> > omap1_i2c_add_bus() sets up pinmux and calls platform_device register. >> > ?pinmux setup needs to be factored out anyway for generic DT platform >> > device registration, which just leaves platform_device creation which >> > is already handled by of_platform_populate(). >> > >> > omap2_i2c_add_bus() does the same thing, except it also looks up the >> > hwmod data (*oh) and uses it to call omap_device_build(). >> > omap_device_build() or something equivalent needs to be called for >> > every omap device in the system, which is to create platform_devices >> > with hwmod attached. ?Now we're starting to hit generic code. ?:-) >> > >> > The way I see it, you've got two options: >> > >> > 1) modify the of_platform_bus_create() to call some kind of >> > of_platform_bus_create_omap() for devices that match "ti,omap3-device" >> > or something. >> > >> > 2) Leave of_platform_bus_create(), and instead us a notifier to attach >> > hwmod data to normal platform devices. ?omap_device_build() is >> > actually pretty simple. ?It allocated a device, it attaches >> > platform_data and hwmod pointers to the device and registers it. >> > omap_device_register() is just a wrapper around >> > platform_device_register(). >> > >> > My preference is definitely #2, but there is a wrinkle in this >> > approach. ?Unfortunately omap_devices are not simply plain >> > platform_devices with extra data attached, an omap_device actually >> > embeds the platform_device inside it, which cannot be attached after >> > the fact. ?I think I had talked with Kevin (cc'd) about eliminating >> > the embedding, but I cannot remember clearly on this point. ?As long >> > as platform_device remains embedded inside struct omap_device, #2 >> > won't work. >> >> Can you please elaborate more on this issue? > > Look at the of_platform_populate() call path (in devicetree/next) and > see how it handles amba devices. ?Do the same thing for omap_devices. As per the discussion so far, I am trying to take following approach: |--->of_platform_populate(...) |--->of_platform_bus_create_omap()-->Here notifiers are added |--->platform_device_register() ---> calls registered notifiers |--->notifier_callback - look up hwmod entry by name - call omap_device_build(need to pass platform_device pointer) - get all the details such as resource structures, irqs,platform_data etc from hwmod database and auxdata and assign it to platform_device pointer received from notifier callback function. - there will be no platform_device_register from omap_device_build since it is already called from "of_platform_bus_create_omap" Please note that, with the above approach, the platform_device pointer which is embedded inside omap_device is not used and omap_device_build api will expect platform device pointer. -M ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 3/4] dt: omap3: add generic board file for dt support 2011-07-17 5:13 ` Grant Likely @ 2011-07-21 23:53 ` Kevin Hilman -1 siblings, 0 replies; 60+ messages in thread From: Kevin Hilman @ 2011-07-21 23:53 UTC (permalink / raw) To: Grant Likely Cc: G, Manjunath Kondaiah, Tony Lindgren, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux Grant Likely <grant.likely@secretlab.ca> writes: [...] > The way I see it, you've got two options: > > 1) modify the of_platform_bus_create() to call some kind of > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > or something. > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > hwmod data to normal platform devices. omap_device_build() is > actually pretty simple. It allocated a device, it attaches > platform_data and hwmod pointers to the device and registers it. > omap_device_register() is just a wrapper around > platform_device_register(). > > My preference is definitely #2, but there is a wrinkle in this > approach. Unfortunately omap_devices are not simply plain > platform_devices with extra data attached, an omap_device actually > embeds the platform_device inside it, which cannot be attached after > the fact. I think I had talked with Kevin (cc'd) about eliminating > the embedding, but I cannot remember clearly on this point. As long > as platform_device remains embedded inside struct omap_device, #2 > won't work. I agree with #2, and I think we need to go down this de-coupling route also. I just sent an RFC series that starts down this path to at least demonstrate that it's possible. Kevin ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/4] dt: omap3: add generic board file for dt support @ 2011-07-21 23:53 ` Kevin Hilman 0 siblings, 0 replies; 60+ messages in thread From: Kevin Hilman @ 2011-07-21 23:53 UTC (permalink / raw) To: linux-arm-kernel Grant Likely <grant.likely@secretlab.ca> writes: [...] > The way I see it, you've got two options: > > 1) modify the of_platform_bus_create() to call some kind of > of_platform_bus_create_omap() for devices that match "ti,omap3-device" > or something. > > 2) Leave of_platform_bus_create(), and instead us a notifier to attach > hwmod data to normal platform devices. omap_device_build() is > actually pretty simple. It allocated a device, it attaches > platform_data and hwmod pointers to the device and registers it. > omap_device_register() is just a wrapper around > platform_device_register(). > > My preference is definitely #2, but there is a wrinkle in this > approach. Unfortunately omap_devices are not simply plain > platform_devices with extra data attached, an omap_device actually > embeds the platform_device inside it, which cannot be attached after > the fact. I think I had talked with Kevin (cc'd) about eliminating > the embedding, but I cannot remember clearly on this point. As long > as platform_device remains embedded inside struct omap_device, #2 > won't work. I agree with #2, and I think we need to go down this de-coupling route also. I just sent an RFC series that starts down this path to at least demonstrate that it's possible. Kevin ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree 2011-07-13 22:06 [PATCH 0/4] dt: omap3: add device tree support G, Manjunath Kondaiah ` (2 preceding siblings ...) 2011-07-13 22:06 ` [PATCH 3/4] dt: omap3: add generic board file for dt support G, Manjunath Kondaiah @ 2011-07-13 22:06 ` G, Manjunath Kondaiah 2011-07-13 23:20 ` Grant Likely 3 siblings, 1 reply; 60+ messages in thread From: G, Manjunath Kondaiah @ 2011-07-13 22:06 UTC (permalink / raw) To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r The i2c-omap driver is converted for supporting both dt and non dt builds and driver is modified to use dt data partially. Tested on OMAP3 beagle board. Signed-off-by: G, Manjunath Kondaiah <manjugk-l0cyMroinI0@public.gmane.org> --- drivers/i2c/busses/i2c-omap.c | 48 ++++++++++++++++++++++++++++++++++++++++- 1 files changed, 47 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index ae1545b..6d11a13 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -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,10 @@ static const struct i2c_algorithm omap_i2c_algo = { .functionality = omap_i2c_func, }; +#if defined(CONFIG_OF) +static const struct of_device_id omap_i2c_of_match[]; +#endif + static int __devinit omap_i2c_probe(struct platform_device *pdev) { @@ -979,10 +987,17 @@ 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; +#if defined(CONFIG_OF) + const struct of_device_id *match; +#endif irq_handler_t isr; int r; u32 speed = 0; +#if defined(CONFIG_OF) + match = of_match_device(omap_i2c_of_match, &pdev->dev); +#endif + /* NOTE: driver uses the static register mapping */ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!mem) { @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) if (pdata != NULL) { speed = pdata->clkrate; dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; +#if defined(CONFIG_OF) + } else if (pdev->dev.of_node) { + u32 prop; + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", + &prop)) + speed = prop/100; + else + speed = 100; +#else } else { speed = 100; /* Default speed */ - dev->set_mpu_wkup_lat = NULL; +#endif } +#if defined(CONFIG_OF) + /* TODO: remove this after DT depencies with hwmod are resolved */ + if (match) + return 0; +#endif dev->speed = speed; dev->idle = 1; dev->dev = &pdev->dev; @@ -1096,7 +1125,9 @@ omap_i2c_probe(struct platform_device *pdev) strlcpy(adap->name, "OMAP I2C adapter", sizeof(adap->name)); adap->algo = &omap_i2c_algo; adap->dev.parent = &pdev->dev; +#if defined(CONFIG_OF) adap->dev.of_node = pdev->dev.of_node; +#endif /* i2c device drivers may be active on return from add_adapter() */ adap->nr = pdev->id; @@ -1106,7 +1137,9 @@ omap_i2c_probe(struct platform_device *pdev) goto err_free_irq; } +#if defined(CONFIG_OF) of_i2c_register_devices(adap); +#endif return 0; @@ -1162,6 +1195,16 @@ static int omap_i2c_resume(struct device *dev) return 0; } +#if defined(CONFIG_OF) +static const struct of_device_id omap_i2c_of_match[] = { + {.compatible = "ti,omap3-i2c", }, + {}, +} +MODULE_DEVICE_TABLE(of, omap_i2c_of_match); +#else +#define omap_i2c_of_match NULL +#endif + static struct dev_pm_ops omap_i2c_pm_ops = { .suspend = omap_i2c_suspend, .resume = omap_i2c_resume, @@ -1178,6 +1221,9 @@ static struct platform_driver omap_i2c_driver = { .name = "omap_i2c", .owner = THIS_MODULE, .pm = OMAP_I2C_PM_OPS, +#if defined(CONFIG_OF) + .of_match_table = omap_i2c_of_match, +#endif }, }; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree 2011-07-13 22:06 ` [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree G, Manjunath Kondaiah @ 2011-07-13 23:20 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 23:20 UTC (permalink / raw) To: G, Manjunath Kondaiah Cc: devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > The i2c-omap driver is converted for supporting both > dt and non dt builds and driver is modified to use dt > data partially. > > Tested on OMAP3 beagle board. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > drivers/i2c/busses/i2c-omap.c | 48 ++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 47 insertions(+), 1 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index ae1545b..6d11a13 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -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,10 @@ static const struct i2c_algorithm omap_i2c_algo = { > .functionality = omap_i2c_func, > }; > > +#if defined(CONFIG_OF) > +static const struct of_device_id omap_i2c_of_match[]; > +#endif > + > static int __devinit > omap_i2c_probe(struct platform_device *pdev) > { > @@ -979,10 +987,17 @@ 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; > +#if defined(CONFIG_OF) > + const struct of_device_id *match; > +#endif > irq_handler_t isr; > int r; > u32 speed = 0; > > +#if defined(CONFIG_OF) > + match = of_match_device(omap_i2c_of_match, &pdev->dev); > +#endif of_match_device() is an empty inline when CONFIG_OF is not defined. You can drop the #if defined() protection around this statement. > + > /* NOTE: driver uses the static register mapping */ > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!mem) { > @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) > if (pdata != NULL) { > speed = pdata->clkrate; > dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; > +#if defined(CONFIG_OF) > + } else if (pdev->dev.of_node) { > + u32 prop; > + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > + &prop)) > + speed = prop/100; > + else > + speed = 100; If you move the 'speed = 100' statement above the if(pdata != NULL) test, then this whole block can become simpler for both the pdata and DT situations. > +#else > } else { > speed = 100; /* Default speed */ > - dev->set_mpu_wkup_lat = NULL; > +#endif > } > > +#if defined(CONFIG_OF) > + /* TODO: remove this after DT depencies with hwmod are resolved */ > + if (match) > + return 0; > +#endif > dev->speed = speed; > dev->idle = 1; > dev->dev = &pdev->dev; > @@ -1096,7 +1125,9 @@ omap_i2c_probe(struct platform_device *pdev) > strlcpy(adap->name, "OMAP I2C adapter", sizeof(adap->name)); > adap->algo = &omap_i2c_algo; > adap->dev.parent = &pdev->dev; > +#if defined(CONFIG_OF) > adap->dev.of_node = pdev->dev.of_node; > +#endif The #if defined() can be safely removed here. > > /* i2c device drivers may be active on return from add_adapter() */ > adap->nr = pdev->id; > @@ -1106,7 +1137,9 @@ omap_i2c_probe(struct platform_device *pdev) > goto err_free_irq; > } > > +#if defined(CONFIG_OF) > of_i2c_register_devices(adap); > +#endif Ditto here. of_i2c_register_devices() is an empty inline when !CONFIG_OF > > return 0; > > @@ -1162,6 +1195,16 @@ static int omap_i2c_resume(struct device *dev) > return 0; > } > > +#if defined(CONFIG_OF) > +static const struct of_device_id omap_i2c_of_match[] = { > + {.compatible = "ti,omap3-i2c", }, > + {}, > +} > +MODULE_DEVICE_TABLE(of, omap_i2c_of_match); > +#else > +#define omap_i2c_of_match NULL > +#endif You can move this whole block up to where omap_i2c_of_match is forward declared, which will make the patch smaller. > + > static struct dev_pm_ops omap_i2c_pm_ops = { > .suspend = omap_i2c_suspend, > .resume = omap_i2c_resume, > @@ -1178,6 +1221,9 @@ static struct platform_driver omap_i2c_driver = { > .name = "omap_i2c", > .owner = THIS_MODULE, > .pm = OMAP_I2C_PM_OPS, > +#if defined(CONFIG_OF) > + .of_match_table = omap_i2c_of_match, > +#endif Drop the #if defined() protection. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree @ 2011-07-13 23:20 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-13 23:20 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah <manjugk@ti.com> wrote: > > The i2c-omap driver is converted for supporting both > dt and non dt builds and driver is modified to use dt > data partially. > > Tested on OMAP3 beagle board. > > Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com> > --- > ?drivers/i2c/busses/i2c-omap.c | ? 48 ++++++++++++++++++++++++++++++++++++++++- > ?1 files changed, 47 insertions(+), 1 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index ae1545b..6d11a13 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -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,10 @@ static const struct i2c_algorithm omap_i2c_algo = { > ? ? ? ?.functionality ?= omap_i2c_func, > ?}; > > +#if defined(CONFIG_OF) > +static const struct of_device_id omap_i2c_of_match[]; > +#endif > + > ?static int __devinit > ?omap_i2c_probe(struct platform_device *pdev) > ?{ > @@ -979,10 +987,17 @@ 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; > +#if defined(CONFIG_OF) > + ? ? ? const struct of_device_id *match; > +#endif > ? ? ? ?irq_handler_t isr; > ? ? ? ?int r; > ? ? ? ?u32 speed = 0; > > +#if defined(CONFIG_OF) > + ? ? ? match = of_match_device(omap_i2c_of_match, &pdev->dev); > +#endif of_match_device() is an empty inline when CONFIG_OF is not defined. You can drop the #if defined() protection around this statement. > + > ? ? ? ?/* NOTE: driver uses the static register mapping */ > ? ? ? ?mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > ? ? ? ?if (!mem) { > @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) > ? ? ? ?if (pdata != NULL) { > ? ? ? ? ? ? ? ?speed = pdata->clkrate; > ? ? ? ? ? ? ? ?dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; > +#if defined(CONFIG_OF) > + ? ? ? } else if (pdev->dev.of_node) { > + ? ? ? ? ? ? ? u32 prop; > + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &prop)) > + ? ? ? ? ? ? ? ? ? ? ? speed = prop/100; > + ? ? ? ? ? ? ? else > + ? ? ? ? ? ? ? ? ? ? ? speed = 100; If you move the 'speed = 100' statement above the if(pdata != NULL) test, then this whole block can become simpler for both the pdata and DT situations. > +#else > ? ? ? ?} else { > ? ? ? ? ? ? ? ?speed = 100; ? ?/* Default speed */ > - ? ? ? ? ? ? ? dev->set_mpu_wkup_lat = NULL; > +#endif > ? ? ? ?} > > +#if defined(CONFIG_OF) > + ? ? ? /* TODO: remove this after DT depencies with hwmod are resolved */ > + ? ? ? if (match) > + ? ? ? ? ? ? ? return 0; > +#endif > ? ? ? ?dev->speed = speed; > ? ? ? ?dev->idle = 1; > ? ? ? ?dev->dev = &pdev->dev; > @@ -1096,7 +1125,9 @@ omap_i2c_probe(struct platform_device *pdev) > ? ? ? ?strlcpy(adap->name, "OMAP I2C adapter", sizeof(adap->name)); > ? ? ? ?adap->algo = &omap_i2c_algo; > ? ? ? ?adap->dev.parent = &pdev->dev; > +#if defined(CONFIG_OF) > ? ? ? ?adap->dev.of_node = pdev->dev.of_node; > +#endif The #if defined() can be safely removed here. > > ? ? ? ?/* i2c device drivers may be active on return from add_adapter() */ > ? ? ? ?adap->nr = pdev->id; > @@ -1106,7 +1137,9 @@ omap_i2c_probe(struct platform_device *pdev) > ? ? ? ? ? ? ? ?goto err_free_irq; > ? ? ? ?} > > +#if defined(CONFIG_OF) > ? ? ? ?of_i2c_register_devices(adap); > +#endif Ditto here. of_i2c_register_devices() is an empty inline when !CONFIG_OF > > ? ? ? ?return 0; > > @@ -1162,6 +1195,16 @@ static int omap_i2c_resume(struct device *dev) > ? ? ? ?return 0; > ?} > > +#if defined(CONFIG_OF) > +static const struct of_device_id omap_i2c_of_match[] = { > + ? ? ? {.compatible = "ti,omap3-i2c", }, > + ? ? ? {}, > +} > +MODULE_DEVICE_TABLE(of, omap_i2c_of_match); > +#else > +#define omap_i2c_of_match NULL > +#endif You can move this whole block up to where omap_i2c_of_match is forward declared, which will make the patch smaller. > + > ?static struct dev_pm_ops omap_i2c_pm_ops = { > ? ? ? ?.suspend = omap_i2c_suspend, > ? ? ? ?.resume = omap_i2c_resume, > @@ -1178,6 +1221,9 @@ static struct platform_driver omap_i2c_driver = { > ? ? ? ? ? ? ? ?.name ? = "omap_i2c", > ? ? ? ? ? ? ? ?.owner ?= THIS_MODULE, > ? ? ? ? ? ? ? ?.pm ? ? = OMAP_I2C_PM_OPS, > +#if defined(CONFIG_OF) > + ? ? ? ? ? ? ? .of_match_table = omap_i2c_of_match, > +#endif Drop the #if defined() protection. g. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree 2011-07-13 23:20 ` Grant Likely @ 2011-07-28 17:34 ` Cousson, Benoit -1 siblings, 0 replies; 60+ messages in thread From: Cousson, Benoit @ 2011-07-28 17:34 UTC (permalink / raw) To: Grant Likely Cc: G, Manjunath Kondaiah, devicetree-discuss, linux-arm-kernel, linux-omap, ben-linux Hi Grant, On 7/14/2011 1:20 AM, Grant Likely wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah<manjugk@ti.com> wrote: >> >> The i2c-omap driver is converted for supporting both >> dt and non dt builds and driver is modified to use dt >> data partially. [...] >> /* NOTE: driver uses the static register mapping */ >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!mem) { >> @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) >> if (pdata != NULL) { >> speed = pdata->clkrate; >> dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; >> +#if defined(CONFIG_OF) >> + } else if (pdev->dev.of_node) { >> + u32 prop; >> + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", >> +&prop)) >> + speed = prop/100; >> + else >> + speed = 100; Do we have to modify every drivers in order to take advantage of the DT bus? Cannot we init the already existing pdata during device creation and let the driver untouched? I think it is a pity to add all that #ifdefry in the driver to be able to support two kinds of bus. Cannot we have an intermediate phase that will deal only with the device creation with DT? That will allow us to clean the omap_device creation from hwmod we have today spread everywhere in plat-omap and mach-omap2. Regards Benoit ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree @ 2011-07-28 17:34 ` Cousson, Benoit 0 siblings, 0 replies; 60+ messages in thread From: Cousson, Benoit @ 2011-07-28 17:34 UTC (permalink / raw) To: linux-arm-kernel Hi Grant, On 7/14/2011 1:20 AM, Grant Likely wrote: > On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah<manjugk@ti.com> wrote: >> >> The i2c-omap driver is converted for supporting both >> dt and non dt builds and driver is modified to use dt >> data partially. [...] >> /* NOTE: driver uses the static register mapping */ >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!mem) { >> @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) >> if (pdata != NULL) { >> speed = pdata->clkrate; >> dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; >> +#if defined(CONFIG_OF) >> + } else if (pdev->dev.of_node) { >> + u32 prop; >> + if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency", >> +&prop)) >> + speed = prop/100; >> + else >> + speed = 100; Do we have to modify every drivers in order to take advantage of the DT bus? Cannot we init the already existing pdata during device creation and let the driver untouched? I think it is a pity to add all that #ifdefry in the driver to be able to support two kinds of bus. Cannot we have an intermediate phase that will deal only with the device creation with DT? That will allow us to clean the omap_device creation from hwmod we have today spread everywhere in plat-omap and mach-omap2. Regards Benoit ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree 2011-07-28 17:34 ` Cousson, Benoit @ 2011-07-28 21:32 ` Grant Likely -1 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-28 21:32 UTC (permalink / raw) To: Cousson, Benoit Cc: G, Manjunath Kondaiah, devicetree-discuss, linux-omap, ben-linux, linux-arm-kernel On Thu, Jul 28, 2011 at 11:34 AM, Cousson, Benoit <b-cousson@ti.com> wrote: > Hi Grant, > > On 7/14/2011 1:20 AM, Grant Likely wrote: >> >> On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah<manjugk@ti.com> >> wrote: >>> >>> The i2c-omap driver is converted for supporting both >>> dt and non dt builds and driver is modified to use dt >>> data partially. > > [...] > >>> /* NOTE: driver uses the static register mapping */ >>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> if (!mem) { >>> @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) >>> if (pdata != NULL) { >>> speed = pdata->clkrate; >>> dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; >>> +#if defined(CONFIG_OF) >>> + } else if (pdev->dev.of_node) { >>> + u32 prop; >>> + if (!of_property_read_u32(pdev->dev.of_node, >>> "clock-frequency", >>> +&prop)) >>> + speed = prop/100; >>> + else >>> + speed = 100; > > Do we have to modify every drivers in order to take advantage of the DT bus? > Cannot we init the already existing pdata during device creation and let the > driver untouched? > > I think it is a pity to add all that #ifdefry in the driver to be able to > support two kinds of bus. It's not supporting 2 different kinds of bus. There is no such think as a dt bus type (anymore. there used to be and of_platform_bus_type, but that was a mistake). DT is only a different configuration representation. > Cannot we have an intermediate phase that will deal only with the device > creation with DT? > > That will allow us to clean the omap_device creation from hwmod we have > today spread everywhere in plat-omap and mach-omap2. Consider what you're suggesting... Pretty much every driver that needs platform data has as a unique platform_data structure. That means that for every driver there needs to be a separate block of code to translate from DT data into struct <driver>_platform_data. There is no way to make that generic. That translation code needs to live somewhere, and something needs to be responsible for executing it when relevant devices appear. It can either be built into the driver, or it can be done sometime before the driver is probed. Option 1: If it is part of the driver, the sequence looks like this: ...early boot: 1) allocate and register platform_devices for DT nodes (generic code, attach device_node) ... boot progresses to initcalls... 2) probe platform_driver (DT translation code to obtain <device>_platform_data from device_node) Option 2: If it is part of platform_device registration, the sequence looks something like: ...early boot: 1) allocate and register platform_devices for DT nodes (call device specific translation code for each device to create <device>_platform_data) ...boot progresses to initcalls 2) probe platform_driver (use platform_data, nothing changes here) Option 3: decoupled from device registration and the device driver: ...early boot: 1) allocate and register platform_devices for DT nodes (generic code, attach device_node) ...boot progresses to initcalls 2) hook into device model *before* drivers get probed to call device specific translation code. 3) probe platform_driver (use platform_data, nothing changes here) I understand the desire to not change the drivers, but going with either option 2 or 3 causes a giant mess in figuring out how to make sure the correct translation code gets called. Option 2 is a disaster because the translation code for any device that may possibly be attached to the kernel must be statically linked in. It cannot be in modules because it must be available during early init. Option 2 is similarly a no-go because it requires a new hunk of infrastructure to take care of finding and executing translation code before the device can get probed by the device driver. Besides, no matter how you look at it, the DT translation code is always device-driver-specific. It isn't something that can be handled by generic code for all devices, and the *whole point* of the DT transition is to get away from board specific code during early init. Why wouldn't DT translation code live inside the one device driver that it supports? g. Note: This is only an issue if a driver requires platform data. Drivers that only need register and irq resources work without any additional code. As for the #ifdef issue, yes it can look ugly if it is done in the wrong way. Generally, I split the translation code out into a separate function that can be made an empty static inline when CONFIG_OF is not selected so that all the DT specific code can be factored out into a single #ifdef block. g. > > Regards > Benoit > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree @ 2011-07-28 21:32 ` Grant Likely 0 siblings, 0 replies; 60+ messages in thread From: Grant Likely @ 2011-07-28 21:32 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 28, 2011 at 11:34 AM, Cousson, Benoit <b-cousson@ti.com> wrote: > Hi Grant, > > On 7/14/2011 1:20 AM, Grant Likely wrote: >> >> On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah<manjugk@ti.com> >> ?wrote: >>> >>> The i2c-omap driver is converted for supporting both >>> dt and non dt builds and driver is modified to use dt >>> data partially. > > [...] > >>> ? ? ? ?/* NOTE: driver uses the static register mapping */ >>> ? ? ? ?mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> ? ? ? ?if (!mem) { >>> @@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev) >>> ? ? ? ?if (pdata != NULL) { >>> ? ? ? ? ? ? ? ?speed = pdata->clkrate; >>> ? ? ? ? ? ? ? ?dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; >>> +#if defined(CONFIG_OF) >>> + ? ? ? } else if (pdev->dev.of_node) { >>> + ? ? ? ? ? ? ? u32 prop; >>> + ? ? ? ? ? ? ? if (!of_property_read_u32(pdev->dev.of_node, >>> "clock-frequency", >>> +&prop)) >>> + ? ? ? ? ? ? ? ? ? ? ? speed = prop/100; >>> + ? ? ? ? ? ? ? else >>> + ? ? ? ? ? ? ? ? ? ? ? speed = 100; > > Do we have to modify every drivers in order to take advantage of the DT bus? > Cannot we init the already existing pdata during device creation and let the > driver untouched? > > I think it is a pity to add all that #ifdefry in the driver to be able to > support two kinds of bus. It's not supporting 2 different kinds of bus. There is no such think as a dt bus type (anymore. there used to be and of_platform_bus_type, but that was a mistake). DT is only a different configuration representation. > Cannot we have an intermediate phase that will deal only with the device > creation with DT? > > That will allow us to clean the omap_device creation from hwmod we have > today spread everywhere in plat-omap and mach-omap2. Consider what you're suggesting... Pretty much every driver that needs platform data has as a unique platform_data structure. That means that for every driver there needs to be a separate block of code to translate from DT data into struct <driver>_platform_data. There is no way to make that generic. That translation code needs to live somewhere, and something needs to be responsible for executing it when relevant devices appear. It can either be built into the driver, or it can be done sometime before the driver is probed. Option 1: If it is part of the driver, the sequence looks like this: ...early boot: 1) allocate and register platform_devices for DT nodes (generic code, attach device_node) ... boot progresses to initcalls... 2) probe platform_driver (DT translation code to obtain <device>_platform_data from device_node) Option 2: If it is part of platform_device registration, the sequence looks something like: ...early boot: 1) allocate and register platform_devices for DT nodes (call device specific translation code for each device to create <device>_platform_data) ...boot progresses to initcalls 2) probe platform_driver (use platform_data, nothing changes here) Option 3: decoupled from device registration and the device driver: ...early boot: 1) allocate and register platform_devices for DT nodes (generic code, attach device_node) ...boot progresses to initcalls 2) hook into device model *before* drivers get probed to call device specific translation code. 3) probe platform_driver (use platform_data, nothing changes here) I understand the desire to not change the drivers, but going with either option 2 or 3 causes a giant mess in figuring out how to make sure the correct translation code gets called. Option 2 is a disaster because the translation code for any device that may possibly be attached to the kernel must be statically linked in. It cannot be in modules because it must be available during early init. Option 2 is similarly a no-go because it requires a new hunk of infrastructure to take care of finding and executing translation code before the device can get probed by the device driver. Besides, no matter how you look at it, the DT translation code is always device-driver-specific. It isn't something that can be handled by generic code for all devices, and the *whole point* of the DT transition is to get away from board specific code during early init. Why wouldn't DT translation code live inside the one device driver that it supports? g. Note: This is only an issue if a driver requires platform data. Drivers that only need register and irq resources work without any additional code. As for the #ifdef issue, yes it can look ugly if it is done in the wrong way. Generally, I split the translation code out into a separate function that can be made an empty static inline when CONFIG_OF is not selected so that all the DT specific code can be factored out into a single #ifdef block. g. > > Regards > Benoit > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree 2011-07-28 21:32 ` Grant Likely @ 2011-08-03 12:56 ` Cousson, Benoit -1 siblings, 0 replies; 60+ messages in thread From: Cousson, Benoit @ 2011-08-03 12:56 UTC (permalink / raw) To: Grant Likely Cc: G, Manjunath Kondaiah, devicetree-discuss, linux-omap, ben-linux, linux-arm-kernel On 7/28/2011 11:32 PM, Grant Likely wrote: > On Thu, Jul 28, 2011 at 11:34 AM, Cousson, Benoit<b-cousson@ti.com> wrote: [...] >> Do we have to modify every drivers in order to take advantage of the DT bus? >> Cannot we init the already existing pdata during device creation and let the >> driver untouched? >> >> I think it is a pity to add all that #ifdefry in the driver to be able to >> support two kinds of bus. > > It's not supporting 2 different kinds of bus. There is no such think > as a dt bus type (anymore. there used to be and of_platform_bus_type, > but that was a mistake). DT is only a different configuration > representation. That's the point, sorry for my confusing sentence. We are creating almost the exact same platform_device in both cases, but we still have to modify the platform_driver due to a change in the device creation method. No doubt that DT will clean up the messy devices creation from board file we have today in OMAP. But, for a smooth migration to DT, I would have defined a legacy mode that will avoid hacking every drivers just because we are using a different "bus enumeration" method. And then later we will be able to use DT for the configuration if needed. >> Cannot we have an intermediate phase that will deal only with the device >> creation with DT? >> >> That will allow us to clean the omap_device creation from hwmod we have >> today spread everywhere in plat-omap and mach-omap2. > > Consider what you're suggesting... Pretty much every driver that > needs platform data has as a unique platform_data structure. That > means that for every driver there needs to be a separate block of code > to translate from DT data into struct<driver>_platform_data. There > is no way to make that generic. That translation code needs to live > somewhere, and something needs to be responsible for executing it when > relevant devices appear. It can either be built into the driver, or > it can be done sometime before the driver is probed. > > Option 1: If it is part of the driver, the sequence looks like this: > ...early boot: > 1) allocate and register platform_devices for DT nodes (generic code, > attach device_node) > ... boot progresses to initcalls... > 2) probe platform_driver (DT translation code to obtain > <device>_platform_data from device_node) > > Option 2: If it is part of platform_device registration, the sequence > looks something like: > ...early boot: > 1) allocate and register platform_devices for DT nodes (call device > specific translation code for each device to create > <device>_platform_data) > ...boot progresses to initcalls > 2) probe platform_driver (use platform_data, nothing changes here) > > Option 3: decoupled from device registration and the device driver: > ...early boot: > 1) allocate and register platform_devices for DT nodes (generic code, > attach device_node) > ...boot progresses to initcalls > 2) hook into device model *before* drivers get probed to call device > specific translation code. > 3) probe platform_driver (use platform_data, nothing changes here) > > I understand the desire to not change the drivers, but going with > either option 2 or 3 causes a giant mess in figuring out how to make > sure the correct translation code gets called. Option 2 is a disaster > because the translation code for any device that may possibly be > attached to the kernel must be statically linked in. It cannot be in > modules because it must be available during early init. Option 2 is > similarly a no-go because it requires a new hunk of infrastructure to > take care of finding and executing translation code before the device > can get probed by the device driver. Mmm, cannot we have an option #4? It seems to me that you already have in place some hooks to provide pdata thanks to OF_DEV_AUXDATA. Adding a callback will potentially allow a custom board code to translate configuration from DT into legacy pdata. static struct omap_pdata i2c_pdata = { .clkrate= 400000, }; void init_i2c_pdata(...) { of_property_read_u32(pdev->dev.of_node, pdata->clkrate); } static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA_CB("ti,omap-i2c", XXX, "i2c.1", &i2c_pdata, init_i2c_pdata), {} }; In legacy mode you get the pdata from the static board. If CONFIG_OF is set you can override the value from the callback. The driver remains the same. That being said, does it worth the effort? I'm not sure, but the idea was to allow focusing on the device creation first without hacking every drivers. Using a regular OF_DEV_AUXDATA with pdata, seems a good tradeoff already. > Besides, no matter how you look at it, the DT translation code is > always device-driver-specific. It isn't something that can be handled > by generic code for all devices, and the *whole point* of the DT > transition is to get away from board specific code during early init. Well, AFAIK, the main motivation, at least for OMAP, was to get rid of static data in the arm directory. Getting rid of board code is, I agree, quite interesting as well. > Why wouldn't DT translation code live inside the one device driver > that it supports? Because, it is still optional and force us to maintain two different methods in the driver code when pdata is needed. Ideally "of_property_xxx" will be a platform_property_xxx call always available, and DT will be one of the potential source of configuration. Drivers will then be able to use one and only one standard Linux API, and various implementation will allow the board static stuff for non-DT case or DT blob when available. That is just my .2 euros. Regards, Benoit ^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree @ 2011-08-03 12:56 ` Cousson, Benoit 0 siblings, 0 replies; 60+ messages in thread From: Cousson, Benoit @ 2011-08-03 12:56 UTC (permalink / raw) To: linux-arm-kernel On 7/28/2011 11:32 PM, Grant Likely wrote: > On Thu, Jul 28, 2011 at 11:34 AM, Cousson, Benoit<b-cousson@ti.com> wrote: [...] >> Do we have to modify every drivers in order to take advantage of the DT bus? >> Cannot we init the already existing pdata during device creation and let the >> driver untouched? >> >> I think it is a pity to add all that #ifdefry in the driver to be able to >> support two kinds of bus. > > It's not supporting 2 different kinds of bus. There is no such think > as a dt bus type (anymore. there used to be and of_platform_bus_type, > but that was a mistake). DT is only a different configuration > representation. That's the point, sorry for my confusing sentence. We are creating almost the exact same platform_device in both cases, but we still have to modify the platform_driver due to a change in the device creation method. No doubt that DT will clean up the messy devices creation from board file we have today in OMAP. But, for a smooth migration to DT, I would have defined a legacy mode that will avoid hacking every drivers just because we are using a different "bus enumeration" method. And then later we will be able to use DT for the configuration if needed. >> Cannot we have an intermediate phase that will deal only with the device >> creation with DT? >> >> That will allow us to clean the omap_device creation from hwmod we have >> today spread everywhere in plat-omap and mach-omap2. > > Consider what you're suggesting... Pretty much every driver that > needs platform data has as a unique platform_data structure. That > means that for every driver there needs to be a separate block of code > to translate from DT data into struct<driver>_platform_data. There > is no way to make that generic. That translation code needs to live > somewhere, and something needs to be responsible for executing it when > relevant devices appear. It can either be built into the driver, or > it can be done sometime before the driver is probed. > > Option 1: If it is part of the driver, the sequence looks like this: > ...early boot: > 1) allocate and register platform_devices for DT nodes (generic code, > attach device_node) > ... boot progresses to initcalls... > 2) probe platform_driver (DT translation code to obtain > <device>_platform_data from device_node) > > Option 2: If it is part of platform_device registration, the sequence > looks something like: > ...early boot: > 1) allocate and register platform_devices for DT nodes (call device > specific translation code for each device to create > <device>_platform_data) > ...boot progresses to initcalls > 2) probe platform_driver (use platform_data, nothing changes here) > > Option 3: decoupled from device registration and the device driver: > ...early boot: > 1) allocate and register platform_devices for DT nodes (generic code, > attach device_node) > ...boot progresses to initcalls > 2) hook into device model *before* drivers get probed to call device > specific translation code. > 3) probe platform_driver (use platform_data, nothing changes here) > > I understand the desire to not change the drivers, but going with > either option 2 or 3 causes a giant mess in figuring out how to make > sure the correct translation code gets called. Option 2 is a disaster > because the translation code for any device that may possibly be > attached to the kernel must be statically linked in. It cannot be in > modules because it must be available during early init. Option 2 is > similarly a no-go because it requires a new hunk of infrastructure to > take care of finding and executing translation code before the device > can get probed by the device driver. Mmm, cannot we have an option #4? It seems to me that you already have in place some hooks to provide pdata thanks to OF_DEV_AUXDATA. Adding a callback will potentially allow a custom board code to translate configuration from DT into legacy pdata. static struct omap_pdata i2c_pdata = { .clkrate= 400000, }; void init_i2c_pdata(...) { of_property_read_u32(pdev->dev.of_node, pdata->clkrate); } static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA_CB("ti,omap-i2c", XXX, "i2c.1", &i2c_pdata, init_i2c_pdata), {} }; In legacy mode you get the pdata from the static board. If CONFIG_OF is set you can override the value from the callback. The driver remains the same. That being said, does it worth the effort? I'm not sure, but the idea was to allow focusing on the device creation first without hacking every drivers. Using a regular OF_DEV_AUXDATA with pdata, seems a good tradeoff already. > Besides, no matter how you look at it, the DT translation code is > always device-driver-specific. It isn't something that can be handled > by generic code for all devices, and the *whole point* of the DT > transition is to get away from board specific code during early init. Well, AFAIK, the main motivation, at least for OMAP, was to get rid of static data in the arm directory. Getting rid of board code is, I agree, quite interesting as well. > Why wouldn't DT translation code live inside the one device driver > that it supports? Because, it is still optional and force us to maintain two different methods in the driver code when pdata is needed. Ideally "of_property_xxx" will be a platform_property_xxx call always available, and DT will be one of the potential source of configuration. Drivers will then be able to use one and only one standard Linux API, and various implementation will allow the board static stuff for non-DT case or DT blob when available. That is just my .2 euros. Regards, Benoit ^ permalink raw reply [flat|nested] 60+ messages in thread
end of thread, other threads:[~2011-08-03 12:56 UTC | newest] Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-13 22:06 [PATCH 0/4] dt: omap3: add device tree support G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 1/4] dt: omap3: add SoC file for handling i2c controllers G, Manjunath Kondaiah 2011-07-13 22:57 ` Grant Likely 2011-07-13 22:57 ` Grant Likely 2011-07-13 22:58 ` Grant Likely 2011-07-13 22:58 ` Grant Likely 2011-07-14 3:51 ` G, Manjunath Kondaiah 2011-07-14 3:51 ` G, Manjunath Kondaiah 2011-07-14 3:34 ` G, Manjunath Kondaiah 2011-07-14 3:34 ` G, Manjunath Kondaiah 2011-07-13 22:06 ` [PATCH 2/4] dt: OMAP3: Beagle board: set clock freq for i2c devices G, Manjunath Kondaiah 2011-07-13 23:04 ` Grant Likely 2011-07-13 23:04 ` Grant Likely 2011-07-13 22:06 ` [PATCH 3/4] dt: omap3: add generic board file for dt support G, Manjunath Kondaiah 2011-07-13 23:15 ` Grant Likely 2011-07-13 23:15 ` Grant Likely 2011-07-14 8:53 ` [PATCH] omap2+: Use Kconfig symbol in Makefile instead of obj-y Tony Lindgren 2011-07-14 8:53 ` Tony Lindgren [not found] ` <CACxGe6sELS=C==16TZ2pfrxJDDyqjS_qOwJUfwzwMO8hqo8Xag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-07-16 19:54 ` [PATCH 3/4] dt: omap3: add generic board file for dt support G, Manjunath Kondaiah 2011-07-16 20:07 ` G, Manjunath Kondaiah 2011-07-16 20:07 ` G, Manjunath Kondaiah 2011-07-17 5:13 ` Grant Likely 2011-07-17 5:13 ` Grant Likely 2011-07-18 9:07 ` Tony Lindgren 2011-07-18 9:07 ` Tony Lindgren 2011-07-19 21:34 ` Grant Likely 2011-07-19 21:34 ` Grant Likely 2011-07-21 7:18 ` Tony Lindgren 2011-07-21 7:18 ` Tony Lindgren 2011-07-21 8:55 ` Rajendra Nayak 2011-07-21 8:55 ` Rajendra Nayak [not found] ` <4E27E967.7090501-l0cyMroinI0@public.gmane.org> 2011-07-21 9:09 ` Felipe Balbi 2011-07-21 9:09 ` Felipe Balbi 2011-07-21 9:33 ` Rajendra Nayak 2011-07-21 9:33 ` Rajendra Nayak [not found] ` <4E27F264.4040409-l0cyMroinI0@public.gmane.org> 2011-07-21 9:39 ` Felipe Balbi 2011-07-21 9:39 ` Felipe Balbi 2011-07-28 18:20 ` Cousson, Benoit 2011-07-28 18:20 ` Cousson, Benoit 2011-07-18 10:15 ` G, Manjunath Kondaiah 2011-07-18 10:15 ` G, Manjunath Kondaiah 2011-07-19 21:36 ` Grant Likely 2011-07-19 21:36 ` Grant Likely 2011-07-19 5:58 ` G, Manjunath Kondaiah 2011-07-19 5:58 ` G, Manjunath Kondaiah 2011-07-19 21:37 ` Grant Likely 2011-07-19 21:37 ` Grant Likely [not found] ` <20110719213737.GQ6848-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org> 2011-07-21 10:24 ` G, Manjunath Kondaiah 2011-07-21 10:24 ` G, Manjunath Kondaiah 2011-07-21 23:53 ` Kevin Hilman 2011-07-21 23:53 ` Kevin Hilman 2011-07-13 22:06 ` [PATCH 4/4] dt: i2c-omap: Convert i2c driver to use device tree G, Manjunath Kondaiah 2011-07-13 23:20 ` Grant Likely 2011-07-13 23:20 ` Grant Likely 2011-07-28 17:34 ` Cousson, Benoit 2011-07-28 17:34 ` Cousson, Benoit 2011-07-28 21:32 ` Grant Likely 2011-07-28 21:32 ` Grant Likely 2011-08-03 12:56 ` Cousson, Benoit 2011-08-03 12:56 ` Cousson, Benoit
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.