dt: omap3: add device tree support

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 <[email protected]>

---
?arch/arm/boot/dts/omap3-beagle-nunchuck.dts | ? 11 +--------- ?arch/arm/boot/dts/omap3-beagle.dts ? ? ? ? ?| ? 29 +++++++++++++++++++++++++- ?2 files changed, 28 insertions(+), 12 deletions(-)

arch/arm/boot/dts/omap3-beagle-nunchuck.dts +0 -0

index 2607be5..324ff86 100644
@@ -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 >;

arch/arm/boot/dts/omap3-beagle.dts +0 -0

index 4439466..b45fbc9 100644

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 <[email protected]>

---
?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

arch/arm/boot/dts/omap3-soc.dtsi new file +7 -0

new file mode 100644
index 0000000..f186a32
@@ -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

arch/arm/boot/dts/omap3-soc.dtsi new file +26 -1

+ ? ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? ? 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

Quoted 29 lines · Manjunath Kondaiah

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 <[email protected]>

---
?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

arch/arm/boot/dts/omap3-soc.dtsi new file +7 -0

new file mode 100644
index 0000000..f186a32
@@ -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

arch/arm/boot/dts/omap3-soc.dtsi line 17

Quoted 10 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +10 -0

+ * 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.

arch/arm/boot/dts/omap3-soc.dtsi line 21

Quoted 4 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +4 -0

+ ? ? ? 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.

arch/arm/boot/dts/omap3-soc.dtsi line 28

Quoted 7 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +7 -0

+ ? ? ? ? ? ? ? interrupt-controller;
+ ? ? ? ? ? ? ? #interrupt-cells = <1>;
+ ? ? ? ? ? ? ? reg = <0x48200000 0x1000>;
+ ? ? ? };
+
+ ? ? ? l4-core {
+ ? ? ? ? ? ? ? compatible = "ti,l4-core";

Probably should be "ti,omap3-l4-core"?

Quoted 5 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +5 -0

+ ? ? ? ? ? ? ? #address-cells = <1>;
+ ? ? ? ? ? ? ? #size-cells = <1>;
+ ? ? ? ? ? ? ? ranges = <0 0x48000000 0x1000000>;
+
+ ? ? ? ? ? ? ? i2c at 1 {

arch/arm/boot/dts/omap3-soc.dtsi line 39

Quoted 6 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +6 -0

+ ? ? ? ? ? ? ? ? ? ? ? #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.

Quoted 27 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +26 -1

+ ? ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? ? 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>;
+ ? ? ? };
+};
--

Quoted 3 lines

1.7.4.1

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Quoted 60 lines · Grant Likely

Quoted 29 lines · Manjunath Kondaiah

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 <[email protected]>

---
?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

arch/arm/boot/dts/omap3-soc.dtsi new file +7 -0

new file mode 100644
index 0000000..f186a32
@@ -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

arch/arm/boot/dts/omap3-soc.dtsi line 17

Quoted 10 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +10 -0

+ * 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.

arch/arm/boot/dts/omap3-soc.dtsi line 21

Quoted 4 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +4 -0

+ ? ? ? 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.

arch/arm/boot/dts/omap3-soc.dtsi line 28

Quoted 7 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +7 -0

+ ? ? ? ? ? ? ? interrupt-controller;
+ ? ? ? ? ? ? ? #interrupt-cells = <1>;
+ ? ? ? ? ? ? ? reg = <0x48200000 0x1000>;
+ ? ? ? };
+
+ ? ? ? l4-core {
+ ? ? ? ? ? ? ? compatible = "ti,l4-core";

Quoted 7 lines

Probably should be "ti,omap3-l4-core"?

Quoted 5 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +5 -0

+ ? ? ? ? ? ? ? #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".

Quoted 10 lines

arch/arm/boot/dts/omap3-soc.dtsi line 39

Quoted 6 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +6 -0

+ ? ? ? ? ? ? ? ? ? ? ? #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.

Quoted 33 lines · Manjunath Kondaiah

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 <[email protected]>

---
?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

arch/arm/boot/dts/omap3-soc.dtsi new file +11 -0

new file mode 100644
index 0000000..f186a32
@@ -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/;

arch/arm/boot/dts/omap3-soc.dtsi

Quoted 6 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +6 -0

+/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.

arch/arm/boot/dts/omap3-soc.dtsi line 21

Quoted 5 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +4 -0

+ ? ? ? 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.

arch/arm/boot/dts/omap3-soc.dtsi line 28

Quoted 8 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +7 -0

+ ? ? ? ? ? ? ? interrupt-controller;
+ ? ? ? ? ? ? ? #interrupt-cells = <1>;
+ ? ? ? ? ? ? ? reg = <0x48200000 0x1000>;
+ ? ? ? };
+
+ ? ? ? l4-core {
+ ? ? ? ? ? ? ? compatible = "ti,l4-core";

Probably should be "ti,omap3-l4-core"?

arch/arm/boot/dts/omap3-soc.dtsi line 39

Quoted 12 lines

arch/arm/boot/dts/omap3-soc.dtsi new file +11 -0

+ ? ? ? ? ? ? ? #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.

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 <[email protected]>

---

?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

arch/arm/mach-omap2/Kconfig +0 -0

index 19d5891..174f6d1 100644
@@ -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

arch/arm/mach-omap2/Kconfig +6 -0

+
+ ? ? ? 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

arch/arm/mach-omap2/Makefile +0 -0

index b148077..86e66f7 100644
@@ -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

arch/arm/mach-omap2/Makefile +26 -0

+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));
+

+ ? ? ? .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,

arch/arm/mach-omap2/Makefile +1 -0

+MACHINE_END

arch/arm/mach-omap2/board-omap3beagle.c +0 -0

index 213c4cd..7efb178 100644
@@ -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>

arch/arm/mach-omap2/board-omap3beagle.c +0 -0

@@ -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;
?}

arch/arm/mach-omap2/board-omap3beagle.c +0 -9

@@ -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,

arch/arm/mach-omap2/board-omap3beagle.c +0 -1

@@ -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

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?

Quoted 8 lines · Manjunath Kondaiah

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.

Quoted 7 lines

Signed-off-by G, Manjunath Kondaiah <[email protected]>

---
 arch/arm/mach-omap2/Kconfig             |   12 +
 arch/arm/mach-omap2/Makefile            |    2 +
 arch/arm/mach-omap2/board-omap3-dt.c    |  623

+++++++++++++++++++++++++++++++

Quoted 12 lines

 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

arch/arm/mach-omap2/Kconfig +0 -0

index 19d5891..174f6d1 100644
@@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP
       default y
       select OMAP_PACKAGE_CBP
 

arch/arm/mach-omap2/Kconfig config MACH_OMAP_3630SDP · line 310

Quoted 6 lines

arch/arm/mach-omap2/Kconfig +6 -0

+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

arch/arm/mach-omap2/Kconfig config MACH_OMAP_3630SDP · line 313

Quoted 4 lines

arch/arm/mach-omap2/Kconfig +3 -0

+
+       help
+         Support for generic TI OMAP3 boards using Flattened Device

Tree.

Quoted 11 lines

arch/arm/mach-omap2/Kconfig +3 -0

+         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

arch/arm/mach-omap2/Makefile +0 -0

index b148077..86e66f7 100644

arch/arm/mach-omap2/Makefile obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o · line 184

Quoted 7 lines

arch/arm/mach-omap2/Makefile +2 -0

@@ -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?

arch/arm/mach-omap2/Makefile obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o

Quoted 5 lines

arch/arm/mach-omap2/Makefile +5 -0

+static void __init omap3_init(void)
+{
+       struct device_node *node;
+
+       node = of_find_matching_node_by_address(NULL,

omap3_dt_intc_match,

Quoted 9 lines

arch/arm/mach-omap2/Makefile +9 -0

+                                               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));

arch/arm/mach-omap2/Makefile

Quoted 10 lines

arch/arm/mach-omap2/Makefile +10 -0

+       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,

arch/arm/mach-omap2/Makefile

Quoted 4 lines

arch/arm/mach-omap2/Makefile +4 -0

+                            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);

arch/arm/mach-omap2/Makefile

Quoted 8 lines

arch/arm/mach-omap2/Makefile +8 -0

+
+       /* 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.

Quoted 11 lines · Manjunath Kondaiah

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 19 lines

Quoted 19 lines

Signed-off-by G, Manjunath Kondaiah <[email protected]>

---
 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

arch/arm/mach-omap2/Kconfig +0 -0

index 19d5891..174f6d1 100644
@@ -302,6 +302,18 @@ config MACH_OMAP_3630SDP
       default y
       select OMAP_PACKAGE_CBP
 

Quoted 8 lines

arch/arm/mach-omap2/Kconfig config MACH_OMAP_3630SDP · line 310

Quoted 6 lines

arch/arm/mach-omap2/Kconfig +6 -0

+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

Quoted 61 lines

Quoted 15 lines

arch/arm/mach-omap2/Kconfig +6 -0

+
+       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

arch/arm/mach-omap2/Makefile +0 -0

index b148077..86e66f7 100644

arch/arm/mach-omap2/Makefile obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o · line 184

Quoted 7 lines

arch/arm/mach-omap2/Makefile +2 -0

@@ -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?

Quoted 35 lines

arch/arm/mach-omap2/Makefile +35 -0

+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();

Quoted 8 lines

Quoted 1 line

+ 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.

Quoted 35 lines · Grant Likely

Quoted 35 lines

Quoted 35 lines

(quoted diff) +35 -0

+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();

Quoted 9 lines

Quoted 8 lines

Quoted 1 line

+ ? ? ? 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.

drivers/of/platform.c +2 -0

index c1773456..104ef31 100644
@@ -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,

drivers/of/platform.c +36 -1

@@ -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",

drivers/of/platform.c +2 -0

+}
+

?/**
?* of_platform_bus_create() - Create a device for a node and its children. ?* @bus: device node of the bus to instantiate

drivers/of/platform.c +5 -0

@@ -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

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.

Quoted 23 lines · Grant Likely

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.

Quoted 8 lines

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?

Quoted 33 lines · Tony Lindgren

Quoted 23 lines · Grant Likely

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 2 lines

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.

Quoted 33 lines · Tony Lindgren

Quoted 23 lines · Grant Likely

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 2 lines

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 43 lines · Grant Likely

Quoted 33 lines · Tony Lindgren

Quoted 23 lines · Grant Likely

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

Quoted 2 lines

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?

Hi,

Quoted 63 lines · Rajendra Nayak

Quoted 43 lines · Grant Likely

Quoted 33 lines · Tony Lindgren

Quoted 23 lines · Grant Likely

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

Quoted 2 lines

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().

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 43 lines · Grant Likely

Quoted 33 lines · Tony Lindgren

Quoted 23 lines · Grant Likely

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.

Quoted 8 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

Quoted 2 lines

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?

Hi Manjunath,

Comments below. ?I left in a lot of context for the new folks that I've cc'd (Tony and Kevin).

Quoted 5 lines · Manjunath Kondaiah

Quoted 4 lines · Grant Likely

Quoted 4 lines

Quoted 4 lines

(quoted diff) +4 -0

+static void __init omap3_init(void)
+{
+ ? ? ? struct device_node *node;
+

Quoted 43 lines

(quoted diff) +43 -0

+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));

Quoted 2 lines

+ ? ? ? }
+ ? ? ? 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

Hi Grant,

Quoted 11 lines · Grant Likely

Hi Manjunath,

Comments below. ?I left in a lot of context for the new folks that I've cc'd (Tony and Kevin).

Quoted 5 lines · Manjunath Kondaiah

Quoted 4 lines · Grant Likely

Quoted 4 lines

Quoted 4 lines

(quoted diff) +4 -0

+static void __init omap3_init(void)
+{
+ ? ? ? struct device_node *node;
+

[...]

Quoted 43 lines

Quoted 43 lines

(quoted diff) +43 -0

+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));

Quoted 8 lines

Quoted 2 lines

+ ? ? ? }
+ ? ? ? 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?

Hi Manjunath,

Comments below. ?I left in a lot of context for the new folks that I've cc'd (Tony and Kevin).

Quoted 3 lines · Manjunath Kondaiah

Quoted 2 lines · Grant Likely

Quoted 2 lines

Quoted 2 lines

(quoted diff) +2 -0

+static void __init omap3_init(void)
+{

Quoted 1 line

+ ? ? ? 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.

Grant/Kevin,

Quoted 9 lines · Grant Likely

Hi Manjunath,

Comments below. ?I left in a lot of context for the new folks that I've cc'd (Tony and Kevin).

Quoted 3 lines · Manjunath Kondaiah

Quoted 2 lines · Grant Likely

Quoted 2 lines

Quoted 2 lines

(quoted diff) +2 -0

+static void __init omap3_init(void)
+{

[...]

Quoted 47 lines

Quoted 1 line

+ ? ? ? 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().

Quoted 8 lines

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?

Quoted 70 lines · Manjunath Kondaiah

Grant/Kevin,

Quoted 9 lines · Grant Likely

Hi Manjunath,

Comments below. ?I left in a lot of context for the new folks that I've cc'd (Tony and Kevin).

Quoted 3 lines · Manjunath Kondaiah

Quoted 2 lines · Grant Likely

Quoted 2 lines

Quoted 2 lines

(quoted diff) +2 -0

+static void __init omap3_init(void)
+{

[...]

Quoted 55 lines

Quoted 1 line

+ ? ? ? 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.

Quoted 1 line

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.

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.

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 <[email protected]>

---
?drivers/i2c/busses/i2c-omap.c | ? 48 ++++++++++++++++++++++++++++++++++++++++- ?1 files changed, 47 insertions(+), 1 deletions(-)

drivers/i2c/busses/i2c-omap.c +0 -0

index ae1545b..6d11a13 100644
@@ -38,9 +38,13 @@

?#include <linux/clk.h>
?#include <linux/io.h>
?#include <linux/of_i2c.h>

drivers/i2c/busses/i2c-omap.c +3 -0

+#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

drivers/i2c/busses/i2c-omap.c +4 -0

@@ -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)
?{

drivers/i2c/busses/i2c-omap.c +0 -0

@@ -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;

drivers/i2c/busses/i2c-omap.c +1 -0

+

? ? ? ?/* NOTE: driver uses the static register mapping */ ? ? ? ?mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); ? ? ? ?if (!mem) {

drivers/i2c/busses/i2c-omap.c +0 -0

@@ -1011,11 +1026,25 @@ omap_i2c_probe(struct platform_device *pdev)
 ? ? ? ?if (pdata != NULL) {

drivers/i2c/busses/i2c-omap.c +1 -0

+#else
 ? ? ? ?} else {

? ? ? ? ? ? ? ?speed = 100; ? ?/* Default speed */
- ? ? ? ? ? ? ? dev->set_mpu_wkup_lat = NULL; +#endif
? ? ? ?}

drivers/i2c/busses/i2c-omap.c +5 -0

+#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;

drivers/i2c/busses/i2c-omap.c +0 -0

 ? ? ? ?return 0;
 

@@ -1162,6 +1195,16 @@ static int omap_i2c_resume(struct device *dev)
 ? ? ? ?return 0;
 ?}

Quoted 4 lines · Manjunath Kondaiah

The i2c-omap driver is converted for supporting both dt and non dt builds and driver is modified to use dt data partially.

Quoted 7 lines

        /* NOTE: driver uses the static register mapping */
        mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        if (!mem) {

drivers/i2c/busses/i2c-omap.c +0 -0

@@ -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;

drivers/i2c/busses/i2c-omap.c omap_i2c_probe(struct platform_device *pdev) · line 1036

Quoted 8 lines

drivers/i2c/busses/i2c-omap.c +8 -0

+#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;

Hi Grant,

Quoted 7 lines · Grant Likely

Quoted 4 lines · On Thu, Jul 14, 2011 at 7:06 AM, G, Manjunath Kondaiah<[email protected]> ?wrote:

The i2c-omap driver is converted for supporting both dt and non dt builds and driver is modified to use dt data partially.

[...]

Quoted 8 lines

Quoted 8 lines

? ? ? ?/* NOTE: driver uses the static register mapping */ ? ? ? ?mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); ? ? ? ?if (!mem) {

(quoted diff) +1 -0

@@ -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)

Quoted 8 lines

Review omap_i2c_probe(struct platform_device *pdev) · line 1037

Quoted 8 lines

(quoted diff) +7 -0

+ ? ? ? } 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Jul 28, 2011 at 11:34 AM, Cousson, Benoit<[email protected]> wrote:

Quoted 6 lines

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.

Quoted 5 lines

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?