Skip to contentSkip to conversation

ARM: dts: AM33XX: node definition cleanups

PATCH New Javier Martinez Canillas 4 participants 7 messages Aug 10, 2013 – Aug 16, 2013 Device tree bindings

lore
Share & tools
lore

Javier Martinez Canillas

author
#0 lore raw

cover ARM: dts: AM33XX: node definition cleanups

Hi,

While reviewing the submission of a DT file for a new board based on the am3352 SoC, I noticed that am33xx*.dts files needed some cleanups. All am33xx based board files redefine device nodes that are already defined in the included am33xx.dtsi file.

This not only makes the DTS harder to maintain without any benefit but also leaks SoC internal details to board files such as how the OCP interconnect is modeled (currently just a flat representation) and the addresses of some IP modules that are part of the SoC and are not going to change from board to board.

This patch-set cleans these board files and is composed of the following patches:

[PATCH 1/2] ARM: dts: AM33XX: use pinmux node defined in included [PATCH 2/2] ARM: dts: AM33XX: don't redefine OCP bus and device

NOTE: I don't own any of these am33xx based boards so I was only able to build test the changes. However I compared the generated dtb before and after the changes and the disassembled DTS with dtc -I dtb -O dts foo.dtb > bar.dts are identical.

Even though the binary dtb is the same and there shouldn't be any visible changes, boot testing will be highly appreciated.

Thanks a lot and best regards,
Javier

author
#1 lore raw

PATCH 1/2 ARM: dts: AM33XX: use pinmux node defined in included file

am33xx boards DTS include the am33xx.dtsi Device Tree source file that already define a pinmux device node for the AM33XX SoC Pin Multiplex.

Redefining this for each board makes the Device Tree files harder to modify and maintain so let's just use what is already defined in the included .dtsi file.

Signed-off-by Javier Martinez Canillas <[email protected]>
---
 arch/arm/boot/dts/am335x-bone.dts  |  176 ++++++++++++------------
 arch/arm/boot/dts/am335x-evm.dts   |  254 ++++++++++++++++++------------------
 arch/arm/boot/dts/am335x-evmsk.dts |  258 ++++++++++++++++++------------------
 3 files changed, 344 insertions(+), 344 deletions(-)
arch/arm/boot/dts/am335x-bone.dts +88 -88
index 04feaf8..f0d53fe 100644
@@ -24,94 +24,6 @@
 		reg = <0x80000000 0x10000000>; /* 256 MB */
 	};
 
-	am33xx_pinmux: pinmux at 44e10800 {
-		pinctrl-names = "default";
-		pinctrl-0 = <&clkout2_pin>;
-
-		user_leds_s0: user_leds_s0 {
-			pinctrl-single,pins = <
-				0x54 (PIN_OUTPUT_PULLDOWN | MUX_MODE7)	/* gpmc_a5.gpio1_21 */
-				0x58 (PIN_OUTPUT_PULLUP | MUX_MODE7)	/* gpmc_a6.gpio1_22 */
-				0x5c (PIN_OUTPUT_PULLDOWN | MUX_MODE7)	/* gpmc_a7.gpio1_23 */
-				0x60 (PIN_OUTPUT_PULLUP | MUX_MODE7)	/* gpmc_a8.gpio1_24 */
-			>;
-		};
-
-		i2c0_pins: pinmux_i2c0_pins {
-			pinctrl-single,pins = <
-				0x188 (PIN_INPUT_PULLUP | MUX_MODE0)	/* i2c0_sda.i2c0_sda */
-				0x18c (PIN_INPUT_PULLUP | MUX_MODE0)	/* i2c0_scl.i2c0_scl */
-			>;
-		};
-
-		uart0_pins: pinmux_uart0_pins {
-			pinctrl-single,pins = <
-				0x170 (PIN_INPUT_PULLUP | MUX_MODE0)	/* uart0_rxd.uart0_rxd */
-				0x174 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* uart0_txd.uart0_txd */
-			>;
-		};
-
-		clkout2_pin: pinmux_clkout2_pin {
-			pinctrl-single,pins = <
-				0x1b4 (PIN_OUTPUT_PULLDOWN | MUX_MODE3)	/* xdma_event_intr1.clkout2 */
-			>;
-		};
-
-		cpsw_default: cpsw_default {
-			pinctrl-single,pins = <
-				/* Slave 1 */
-				0x110 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxerr.mii1_rxerr */
-				0x114 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txen.mii1_txen */
-				0x118 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxdv.mii1_rxdv */
-				0x11c (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd3.mii1_txd3 */
-				0x120 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd2.mii1_txd2 */
-				0x124 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd1.mii1_txd1 */
-				0x128 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd0.mii1_txd0 */
-				0x12c (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_txclk.mii1_txclk */
-				0x130 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxclk.mii1_rxclk */
-				0x134 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd3.mii1_rxd3 */
-				0x138 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd2.mii1_rxd2 */
-				0x13c (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd1.mii1_rxd1 */
-				0x140 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd0.mii1_rxd0 */
-			>;
-		};
-
-		cpsw_sleep: cpsw_sleep {
-			pinctrl-single,pins = <
-				/* Slave 1 reset value */
-				0x110 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x114 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x118 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x11c (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x120 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x124 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x128 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x12c (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x130 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x134 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x138 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x13c (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x140 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-			>;
-		};
-
-		davinci_mdio_default: davinci_mdio_default {
-			pinctrl-single,pins = <
-				/* MDIO */
-				0x148 (PIN_INPUT_PULLUP | SLEWCTRL_FAST | MUX_MODE0)	/* mdio_data.mdio_data */
-				0x14c (PIN_OUTPUT_PULLUP | MUX_MODE0)			/* mdio_clk.mdio_clk */
-			>;
-		};
-
-		davinci_mdio_sleep: davinci_mdio_sleep {
-			pinctrl-single,pins = <
-				/* MDIO reset value */
-				0x148 (PIN_INPUT_PULLDOWN | MUX_MODE7)
-				0x14c (PIN_INPUT_PULLDOWN | MUX_MODE7)
-			>;
-		};
-	};
-
 	ocp {
 		uart0: serial at 44e09000 {
 			pinctrl-names = "default";

@@ -168,6 +80,94 @@
 	};
 };
 
+&am33xx_pinmux {
+	pinctrl-names = "default";
+	pinctrl-0 = <&clkout2_pin>;
+
+	user_leds_s0: user_leds_s0 {
+		pinctrl-single,pins = <
+			0x54 (PIN_OUTPUT_PULLDOWN | MUX_MODE7)	/* gpmc_a5.gpio1_21 */
+			0x58 (PIN_OUTPUT_PULLUP | MUX_MODE7)	/* gpmc_a6.gpio1_22 */
+			0x5c (PIN_OUTPUT_PULLDOWN | MUX_MODE7)	/* gpmc_a7.gpio1_23 */
+			0x60 (PIN_OUTPUT_PULLUP | MUX_MODE7)	/* gpmc_a8.gpio1_24 */
+		>;
+	};
+
+	i2c0_pins: pinmux_i2c0_pins {
+		pinctrl-single,pins = <
+			0x188 (PIN_INPUT_PULLUP | MUX_MODE0)	/* i2c0_sda.i2c0_sda */
+			0x18c (PIN_INPUT_PULLUP | MUX_MODE0)	/* i2c0_scl.i2c0_scl */
+		>;
+	};
+
+	uart0_pins: pinmux_uart0_pins {
+		pinctrl-single,pins = <
+			0x170 (PIN_INPUT_PULLUP | MUX_MODE0)	/* uart0_rxd.uart0_rxd */
+			0x174 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* uart0_txd.uart0_txd */
+		>;
+	};
+
+	clkout2_pin: pinmux_clkout2_pin {
+		pinctrl-single,pins = <
+			0x1b4 (PIN_OUTPUT_PULLDOWN | MUX_MODE3)	/* xdma_event_intr1.clkout2 */
+		>;
+	};
+
+	cpsw_default: cpsw_default {
+		pinctrl-single,pins = <
+			/* Slave 1 */
+			0x110 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxerr.mii1_rxerr */
+			0x114 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txen.mii1_txen */
+			0x118 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxdv.mii1_rxdv */
+			0x11c (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd3.mii1_txd3 */
+			0x120 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd2.mii1_txd2 */
+			0x124 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd1.mii1_txd1 */
+			0x128 (PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mii1_txd0.mii1_txd0 */
+			0x12c (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_txclk.mii1_txclk */
+			0x130 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxclk.mii1_rxclk */
+			0x134 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd3.mii1_rxd3 */
+			0x138 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd2.mii1_rxd2 */
+			0x13c (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd1.mii1_rxd1 */
+			0x140 (PIN_INPUT_PULLUP | MUX_MODE0)	/* mii1_rxd0.mii1_rxd0 */
+		>;
+	};
+
+	cpsw_sleep: cpsw_sleep {
+		pinctrl-single,pins = <
+			/* Slave 1 reset value */
+			0x110 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x114 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x118 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x11c (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x120 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x124 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x128 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x12c (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x130 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x134 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x138 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x13c (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x140 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+		>;
+	};
+
+	davinci_mdio_default: davinci_mdio_default {
+		pinctrl-single,pins = <
+			/* MDIO */
+			0x148 (PIN_INPUT_PULLUP | SLEWCTRL_FAST | MUX_MODE0)	/* mdio_data.mdio_data */
+			0x14c (PIN_OUTPUT_PULLUP | MUX_MODE0)			/* mdio_clk.mdio_clk */
+		>;
+	};
+
+	davinci_mdio_sleep: davinci_mdio_sleep {
+		pinctrl-single,pins = <
+			/* MDIO reset value */
+			0x148 (PIN_INPUT_PULLDOWN | MUX_MODE7)
+			0x14c (PIN_INPUT_PULLDOWN | MUX_MODE7)
+		>;
+	};
+};
+
 /include/ "tps65217.dtsi"
 
 &tps {
arch/arm/boot/dts/am335x-evm.dts +127 -127

266 lines not on this page · raw message

arch/arm/boot/dts/am335x-evmsk.dts +129 -129

270 lines not on this page · raw message

Signature · 1 line
1.7.7.6
author
#2 lore raw

PATCH 2/2 ARM: dts: AM33XX: don't redefine OCP bus and device nodes

In reply to Javier Martinez Canillas

The On Chip Peripherals (OCP) device node is a simplified representation of the AM33XX SoC interconnect. An OCP dev node is already defined in the am33xx.dtsi Device Tree source file included by am33xx based boards so there is no need to redefe this on each board DT file.

Also, the OCP and IP modules directly connected to it are SoC internal details that is better to keep outside of board files.

Signed-off-by Javier Martinez Canillas <[email protected]>
---
 arch/arm/boot/dts/am335x-bone.dts  |   41 ++---
 arch/arm/boot/dts/am335x-evm.dts   |  322 ++++++++++++++++++------------------
 arch/arm/boot/dts/am335x-evmsk.dts |  116 +++++++-------
 3 files changed, 236 insertions(+), 243 deletions(-)
arch/arm/boot/dts/am335x-bone.dts +19 -22
index f0d53fe..721cad2 100644
@@ -24,28 +24,6 @@
 		reg = <0x80000000 0x10000000>; /* 256 MB */
 	};
 
-	ocp {
-		uart0: serial at 44e09000 {
-			pinctrl-names = "default";
-			pinctrl-0 = <&uart0_pins>;
-
-			status = "okay";
-		};
-
-		i2c0: i2c at 44e0b000 {
-			pinctrl-names = "default";
-			pinctrl-0 = <&i2c0_pins>;
-
-			status = "okay";
-			clock-frequency = <400000>;
-
-			tps: tps at 24 {
-				reg = <0x24>;
-			};
-
-		};
-	};
-
 	leds {
 		pinctrl-names = "default";
 		pinctrl-0 = <&user_leds_s0>;

@@ -168,6 +146,25 @@
 	};
 };
 
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins>;
+
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins>;
+
+	status = "okay";
+	clock-frequency = <400000>;
+
+	tps: tps at 24 {
+		reg = <0x24>;
+	};
+};
+
 /include/ "tps65217.dtsi"
 
 &tps {
arch/arm/boot/dts/am335x-evm.dts +160 -162

334 lines not on this page · raw message

arch/arm/boot/dts/am335x-evmsk.dts +57 -59

128 lines not on this page · raw message

Signature · 1 line
1.7.7.6
#3 lore raw

cover ARM: dts: AM33XX: node definition cleanups

In reply to Javier Martinez Canillas

+ Few TI folks to review and test.

Quoted 19 lines · Javier Martinez Canillas

Hi,

While reviewing the submission of a DT file for a new board based on the am3352 SoC, I noticed that am33xx*.dts files needed some cleanups. All am33xx based board files redefine device nodes that are already defined in the included am33xx.dtsi file.

This not only makes the DTS harder to maintain without any benefit but also leaks SoC internal details to board files such as how the OCP interconnect is modeled (currently just a flat representation) and the addresses of some IP modules that are part of the SoC and are not going to change from board to board.

This patch-set cleans these board files and is composed of the following patches:

[PATCH 1/2] ARM: dts: AM33XX: use pinmux node defined in included [PATCH 2/2] ARM: dts: AM33XX: don't redefine OCP bus and device

Quoted 7 lines

NOTE: I don't own any of these am33xx based boards so I was only able to build test the changes. However I compared the generated dtb before and after the changes and the disassembled DTS with dtc -I dtb -O dts foo.dtb > bar.dts are identical.

Even though the binary dtb is the same and there shouldn't be any visible changes, boot testing will be highly appreciated.

Thanks it looks good to me.

Regards,
Benoit

#4 lore raw

cover ARM: dts: AM33XX: node definition cleanups

Hi Benoit,

* Benoit Cousson, August 13, 2013 5:15 PM:

Quoted 1 line

+ Few TI folks to review and test.

I am out of office, will be back early next week and then test this.

Regards
Afzal

#5 lore raw

cover ARM: dts: AM33XX: node definition cleanups

In reply to Benoit Cousson
Quoted 4 lines · Benoit Cousson

+ Few TI folks to review and test.

Quoted 1 line · Javier Martinez Canillas

Hi,

[]..

Quoted 3 lines
Quoted 3 lines

Even though the binary dtb is the same and there shouldn't be any visible changes, boot testing will be highly appreciated.

I just boot tested these on my BoneBlack and it boots just fine [1] Feel free to add,

Tested-by Rajendra Nayak <[email protected]>

[1] http://pastebin.com/Z2j6pymp

Quoted 11 lines

Thanks it looks good to me.

Regards,
Benoit

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

#6 lore raw

cover ARM: dts: AM33XX: node definition cleanups

Quoted 8 lines · Rajendra Nayak
Quoted 4 lines · Benoit Cousson

+ Few TI folks to review and test.

Quoted 1 line · Javier Martinez Canillas

Hi,

[]..

Quoted 1 line
Quoted 1 line
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines

Even though the binary dtb is the same and there shouldn't be any visible changes, boot testing will be highly appreciated.

I just boot tested these on my BoneBlack and it boots just fine [1] Feel free to add,

Tested-by Rajendra Nayak <[email protected]>

Thanks Rajendra.

Javier,
I've just applied it for 3.12 with Rajendra's T-by.

Regards,
Benoit