LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes
@ 2018-05-07 12:37 Adam Ford
  2018-05-10  9:10 ` Sekhar Nori
  0 siblings, 1 reply; 5+ messages in thread
From: Adam Ford @ 2018-05-07 12:37 UTC (permalink / raw)
  To: adam.ford
  Cc: Adam Ford, Sekhar Nori, Kevin Hilman, Rob Herring, Mark Rutland,
	linux-arm-kernel, devicetree, linux-kernel

Many node labels in the device tree (like serial0, serial1, etc) are being
redefined, so let's modernize the device tree by using phandles to
extend the existing nodes.  This helps reduce the whitespace.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V3:  Rebase against 4.18/dt and fix subject.

V2: Rename subject per feedback from Sekhar.
    Extend notes from tps6507x
    Add empty line after each node
    
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 339cae353302..3c486d8675b8 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -27,143 +27,6 @@
 		spi0 = &spi1;
 	};
 
-	soc@1c00000 {
-		pmx_core: pinmux@14120 {
-			status = "okay";
-
-			mcasp0_pins: pinmux_mcasp0_pins {
-				pinctrl-single,bits = <
-					/*
-					 * AHCLKX, ACLKX, AFSX, AHCLKR, ACLKR,
-					 * AFSR, AMUTE
-					 */
-					0x00 0x11111111 0xffffffff
-					/* AXR11, AXR12 */
-					0x04 0x00011000 0x000ff000
-				>;
-			};
-			nand_pins: nand_pins {
-				pinctrl-single,bits = <
-					/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
-					0x1c 0x10110110  0xf0ff0ff0
-					/*
-					 * EMA_D[0], EMA_D[1], EMA_D[2],
-					 * EMA_D[3], EMA_D[4], EMA_D[5],
-					 * EMA_D[6], EMA_D[7]
-					 */
-					0x24 0x11111111  0xffffffff
-					/* EMA_A[1], EMA_A[2] */
-					0x30 0x01100000  0x0ff00000
-				>;
-			};
-		};
-		serial0: serial@42000 {
-			status = "okay";
-		};
-		serial1: serial@10c000 {
-			status = "okay";
-		};
-		serial2: serial@10d000 {
-			status = "okay";
-		};
-		rtc0: rtc@23000 {
-			status = "okay";
-		};
-		i2c0: i2c@22000 {
-			status = "okay";
-			clock-frequency = <100000>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&i2c0_pins>;
-
-			tps: tps@48 {
-				reg = <0x48>;
-			};
-			tlv320aic3106: tlv320aic3106@18 {
-				#sound-dai-cells = <0>;
-				compatible = "ti,tlv320aic3106";
-				reg = <0x18>;
-				status = "okay";
-
-				/* Regulators */
-				IOVDD-supply = <&vdcdc2_reg>;
-				/* Derived from VBAT: Baseboard 3.3V / 1.8V */
-				AVDD-supply = <&vbat>;
-				DRVDD-supply = <&vbat>;
-				DVDD-supply = <&vbat>;
-			};
-			tca6416: gpio@20 {
-				compatible = "ti,tca6416";
-				reg = <0x20>;
-				gpio-controller;
-				#gpio-cells = <2>;
-			};
-		};
-		wdt: wdt@21000 {
-			status = "okay";
-		};
-		mmc0: mmc@40000 {
-			max-frequency = <50000000>;
-			bus-width = <4>;
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&mmc0_pins>;
-		};
-		spi1: spi@30e000 {
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&spi1_pins &spi1_cs0_pin>;
-			flash: m25p80@0 {
-				#address-cells = <1>;
-				#size-cells = <1>;
-				compatible = "m25p64";
-				spi-max-frequency = <30000000>;
-				m25p,fast-read;
-				reg = <0>;
-				partition@0 {
-					label = "U-Boot-SPL";
-					reg = <0x00000000 0x00010000>;
-					read-only;
-				};
-				partition@1 {
-					label = "U-Boot";
-					reg = <0x00010000 0x00080000>;
-					read-only;
-				};
-				partition@2 {
-					label = "U-Boot-Env";
-					reg = <0x00090000 0x00010000>;
-					read-only;
-				};
-				partition@3 {
-					label = "Kernel";
-					reg = <0x000a0000 0x00280000>;
-				};
-				partition@4 {
-					label = "Filesystem";
-					reg = <0x00320000 0x00400000>;
-				};
-				partition@5 {
-					label = "MAC-Address";
-					reg = <0x007f0000 0x00010000>;
-					read-only;
-				};
-			};
-		};
-		mdio: mdio@224000 {
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&mdio_pins>;
-			bus_freq = <2200000>;
-		};
-		eth0: ethernet@220000 {
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&mii_pins>;
-		};
-		gpio: gpio@226000 {
-			status = "okay";
-		};
-	};
 	vbat: fixedregulator0 {
 		compatible = "regulator-fixed";
 		regulator-name = "vbat";
@@ -200,56 +63,200 @@
 	};
 };
 
+&pmx_core {
+	status = "okay";
+
+	mcasp0_pins: pinmux_mcasp0_pins {
+		pinctrl-single,bits = <
+			/*
+			 * AHCLKX, ACLKX, AFSX, AHCLKR, ACLKR,
+			 * AFSR, AMUTE
+			 */
+			0x00 0x11111111 0xffffffff
+			/* AXR11, AXR12 */
+			0x04 0x00011000 0x000ff000
+		>;
+	};
+	nand_pins: nand_pins {
+		pinctrl-single,bits = <
+			/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
+			0x1c 0x10110110  0xf0ff0ff0
+			/*
+			 * EMA_D[0], EMA_D[1], EMA_D[2],
+			 * EMA_D[3], EMA_D[4], EMA_D[5],
+			 * EMA_D[6], EMA_D[7]
+			 */
+			0x24 0x11111111  0xffffffff
+			/* EMA_A[1], EMA_A[2] */
+			0x30 0x01100000  0x0ff00000
+		>;
+	};
+};
+
+&serial0 {
+	status = "okay";
+};
+
+&serial1 {
+	status = "okay";
+};
+
+&serial2 {
+	status = "okay";
+};
+
+&rtc0 {
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins>;
+
+	tps: tps@48 {
+		reg = <0x48>;
+	};
+	tlv320aic3106: tlv320aic3106@18 {
+		#sound-dai-cells = <0>;
+		compatible = "ti,tlv320aic3106";
+		reg = <0x18>;
+		status = "okay";
+
+		/* Regulators */
+		IOVDD-supply = <&vdcdc2_reg>;
+		/* Derived from VBAT: Baseboard 3.3V / 1.8V */
+		AVDD-supply = <&vbat>;
+		DRVDD-supply = <&vbat>;
+		DVDD-supply = <&vbat>;
+	};
+	tca6416: gpio@20 {
+		compatible = "ti,tca6416";
+		reg = <0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+};
+
+&wdt {
+	status = "okay";
+};
+
+&mmc0 {
+	max-frequency = <50000000>;
+	bus-width = <4>;
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins>;
+};
+
+&spi1 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi1_pins &spi1_cs0_pin>;
+	flash: m25p80@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "m25p64";
+		spi-max-frequency = <30000000>;
+		m25p,fast-read;
+		reg = <0>;
+		partition@0 {
+			label = "U-Boot-SPL";
+			reg = <0x00000000 0x00010000>;
+			read-only;
+		};
+		partition@1 {
+			label = "U-Boot";
+			reg = <0x00010000 0x00080000>;
+			read-only;
+		};
+		partition@2 {
+			label = "U-Boot-Env";
+			reg = <0x00090000 0x00010000>;
+			read-only;
+		};
+		partition@3 {
+			label = "Kernel";
+			reg = <0x000a0000 0x00280000>;
+		};
+		partition@4 {
+			label = "Filesystem";
+			reg = <0x00320000 0x00400000>;
+		};
+		partition@5 {
+			label = "MAC-Address";
+			reg = <0x007f0000 0x00010000>;
+			read-only;
+		};
+	};
+};
+&mdio {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mdio_pins>;
+	bus_freq = <2200000>;
+};
+
+&eth0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mii_pins>;
+};
+
+&gpio {
+	status = "okay";
+};
+
 /include/ "tps6507x.dtsi"
 
 &tps {
 	vdcdc1_2-supply = <&vbat>;
 	vdcdc3-supply = <&vbat>;
 	vldo1_2-supply = <&vbat>;
+};
 
-	regulators {
-		vdcdc1_reg: regulator@0 {
-			regulator-name = "VDCDC1_3.3V";
-			regulator-min-microvolt = <3150000>;
-			regulator-max-microvolt = <3450000>;
-			regulator-always-on;
-			regulator-boot-on;
-		};
+&vdcdc1_reg {
+	regulator-name = "VDCDC1_3.3V";
+	regulator-min-microvolt = <3150000>;
+	regulator-max-microvolt = <3450000>;
+	regulator-always-on;
+	regulator-boot-on;
+};
 
-		vdcdc2_reg: regulator@1 {
-			regulator-name = "VDCDC2_3.3V";
-			regulator-min-microvolt = <1710000>;
-			regulator-max-microvolt = <3450000>;
-			regulator-always-on;
-			regulator-boot-on;
-			ti,defdcdc_default = <1>;
-		};
+&vdcdc2_reg {
+	regulator-name = "VDCDC2_3.3V";
+	regulator-min-microvolt = <1710000>;
+	regulator-max-microvolt = <3450000>;
+	regulator-always-on;
+	regulator-boot-on;
+	ti,defdcdc_default = <1>;
+};
 
-		vdcdc3_reg: regulator@2 {
-			regulator-name = "VDCDC3_1.2V";
-			regulator-min-microvolt = <950000>;
-			regulator-max-microvolt = <1350000>;
-			regulator-always-on;
-			regulator-boot-on;
-			ti,defdcdc_default = <1>;
-		};
+&vdcdc3_reg {
+	regulator-name = "VDCDC3_1.2V";
+	regulator-min-microvolt = <950000>;
+	regulator-max-microvolt = <1350000>;
+	regulator-always-on;
+	regulator-boot-on;
+	ti,defdcdc_default = <1>;
+};
 
-		ldo1_reg: regulator@3 {
-			regulator-name = "LDO1_1.8V";
-			regulator-min-microvolt = <1710000>;
-			regulator-max-microvolt = <1890000>;
-			regulator-always-on;
-			regulator-boot-on;
-		};
+&ldo1_reg {
+	regulator-name = "LDO1_1.8V";
+	regulator-min-microvolt = <1710000>;
+	regulator-max-microvolt = <1890000>;
+	regulator-always-on;
+	regulator-boot-on;
+};
 
-		ldo2_reg: regulator@4 {
-			regulator-name = "LDO2_1.2V";
-			regulator-min-microvolt = <1140000>;
-			regulator-max-microvolt = <1320000>;
-			regulator-always-on;
-			regulator-boot-on;
-		};
-	};
+&ldo2_reg {
+	regulator-name = "LDO2_1.2V";
+	regulator-min-microvolt = <1140000>;
+	regulator-max-microvolt = <1320000>;
+	regulator-always-on;
+	regulator-boot-on;
 };
 
 &mcasp0 {
-- 
2.17.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes
  2018-05-07 12:37 [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes Adam Ford
@ 2018-05-10  9:10 ` Sekhar Nori
  2018-05-10 15:08   ` Adam Ford
  0 siblings, 1 reply; 5+ messages in thread
From: Sekhar Nori @ 2018-05-10  9:10 UTC (permalink / raw)
  To: Adam Ford, adam.ford
  Cc: Kevin Hilman, Rob Herring, Mark Rutland, linux-arm-kernel,
	devicetree, linux-kernel

On Monday 07 May 2018 06:07 PM, Adam Ford wrote:
> Many node labels in the device tree (like serial0, serial1, etc) are being
> redefined, so let's modernize the device tree by using phandles to
> extend the existing nodes.  This helps reduce the whitespace.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

I applied this without the pmic changes. I am not convinced about those. 
The tps node is already being referred to as phandle. I am not sure 
referring to each individual regulator using phandle is needed. Other 
files like am335x-evm.dts don't do it as well.

Another thing is whether we really need the tp6507x.dtsi file. It does 
not seem to contain much and also da850-evm.dts is the only file 
including it. So it seems pretty pointless to me.

Here is what I committed.

Thanks,
Sekhar

---8<---
commit ae62a32d6019a8225e2c32e631b8b0d039151131 (refs/heads/v4.18/dt)
Author:     Adam Ford <aford173@gmail.com>
AuthorDate: Mon May 7 07:37:21 2018 -0500
Commit:     Sekhar Nori <nsekhar@ti.com>
CommitDate: Thu May 10 14:01:15 2018 +0530

    ARM: dts: da850-evm: use phandles to extend nodes
    
    Many node labels in the device tree (like serial0, serial1, etc) are being
    redefined, so let's modernize the device tree by using phandles to
    extend the existing nodes.  This helps reduce the whitespace.
    
    Signed-off-by: Adam Ford <aford173@gmail.com>
    [nsekhar@ti.com: drop tps6507x related changes]
    Signed-off-by: Sekhar Nori <nsekhar@ti.com>

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 339cae353302..9389f95f4094 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -27,143 +27,6 @@
 		spi0 = &spi1;
 	};
 
-	soc@1c00000 {
-		pmx_core: pinmux@14120 {
-			status = "okay";
-
-			mcasp0_pins: pinmux_mcasp0_pins {
-				pinctrl-single,bits = <
-					/*
-					 * AHCLKX, ACLKX, AFSX, AHCLKR, ACLKR,
-					 * AFSR, AMUTE
-					 */
-					0x00 0x11111111 0xffffffff
-					/* AXR11, AXR12 */
-					0x04 0x00011000 0x000ff000
-				>;
-			};
-			nand_pins: nand_pins {
-				pinctrl-single,bits = <
-					/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
-					0x1c 0x10110110  0xf0ff0ff0
-					/*
-					 * EMA_D[0], EMA_D[1], EMA_D[2],
-					 * EMA_D[3], EMA_D[4], EMA_D[5],
-					 * EMA_D[6], EMA_D[7]
-					 */
-					0x24 0x11111111  0xffffffff
-					/* EMA_A[1], EMA_A[2] */
-					0x30 0x01100000  0x0ff00000
-				>;
-			};
-		};
-		serial0: serial@42000 {
-			status = "okay";
-		};
-		serial1: serial@10c000 {
-			status = "okay";
-		};
-		serial2: serial@10d000 {
-			status = "okay";
-		};
-		rtc0: rtc@23000 {
-			status = "okay";
-		};
-		i2c0: i2c@22000 {
-			status = "okay";
-			clock-frequency = <100000>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&i2c0_pins>;
-
-			tps: tps@48 {
-				reg = <0x48>;
-			};
-			tlv320aic3106: tlv320aic3106@18 {
-				#sound-dai-cells = <0>;
-				compatible = "ti,tlv320aic3106";
-				reg = <0x18>;
-				status = "okay";
-
-				/* Regulators */
-				IOVDD-supply = <&vdcdc2_reg>;
-				/* Derived from VBAT: Baseboard 3.3V / 1.8V */
-				AVDD-supply = <&vbat>;
-				DRVDD-supply = <&vbat>;
-				DVDD-supply = <&vbat>;
-			};
-			tca6416: gpio@20 {
-				compatible = "ti,tca6416";
-				reg = <0x20>;
-				gpio-controller;
-				#gpio-cells = <2>;
-			};
-		};
-		wdt: wdt@21000 {
-			status = "okay";
-		};
-		mmc0: mmc@40000 {
-			max-frequency = <50000000>;
-			bus-width = <4>;
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&mmc0_pins>;
-		};
-		spi1: spi@30e000 {
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&spi1_pins &spi1_cs0_pin>;
-			flash: m25p80@0 {
-				#address-cells = <1>;
-				#size-cells = <1>;
-				compatible = "m25p64";
-				spi-max-frequency = <30000000>;
-				m25p,fast-read;
-				reg = <0>;
-				partition@0 {
-					label = "U-Boot-SPL";
-					reg = <0x00000000 0x00010000>;
-					read-only;
-				};
-				partition@1 {
-					label = "U-Boot";
-					reg = <0x00010000 0x00080000>;
-					read-only;
-				};
-				partition@2 {
-					label = "U-Boot-Env";
-					reg = <0x00090000 0x00010000>;
-					read-only;
-				};
-				partition@3 {
-					label = "Kernel";
-					reg = <0x000a0000 0x00280000>;
-				};
-				partition@4 {
-					label = "Filesystem";
-					reg = <0x00320000 0x00400000>;
-				};
-				partition@5 {
-					label = "MAC-Address";
-					reg = <0x007f0000 0x00010000>;
-					read-only;
-				};
-			};
-		};
-		mdio: mdio@224000 {
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&mdio_pins>;
-			bus_freq = <2200000>;
-		};
-		eth0: ethernet@220000 {
-			status = "okay";
-			pinctrl-names = "default";
-			pinctrl-0 = <&mii_pins>;
-		};
-		gpio: gpio@226000 {
-			status = "okay";
-		};
-	};
 	vbat: fixedregulator0 {
 		compatible = "regulator-fixed";
 		regulator-name = "vbat";
@@ -200,6 +63,153 @@
 	};
 };
 
+&pmx_core {
+	status = "okay";
+
+	mcasp0_pins: pinmux_mcasp0_pins {
+		pinctrl-single,bits = <
+			/*
+			 * AHCLKX, ACLKX, AFSX, AHCLKR, ACLKR,
+			 * AFSR, AMUTE
+			 */
+			0x00 0x11111111 0xffffffff
+			/* AXR11, AXR12 */
+			0x04 0x00011000 0x000ff000
+		>;
+	};
+	nand_pins: nand_pins {
+		pinctrl-single,bits = <
+			/* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
+			0x1c 0x10110110  0xf0ff0ff0
+			/*
+			 * EMA_D[0], EMA_D[1], EMA_D[2],
+			 * EMA_D[3], EMA_D[4], EMA_D[5],
+			 * EMA_D[6], EMA_D[7]
+			 */
+			0x24 0x11111111  0xffffffff
+			/* EMA_A[1], EMA_A[2] */
+			0x30 0x01100000  0x0ff00000
+		>;
+	};
+};
+
+&serial0 {
+	status = "okay";
+};
+
+&serial1 {
+	status = "okay";
+};
+
+&serial2 {
+	status = "okay";
+};
+
+&rtc0 {
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins>;
+
+	tps: tps@48 {
+		reg = <0x48>;
+	};
+	tlv320aic3106: tlv320aic3106@18 {
+		#sound-dai-cells = <0>;
+		compatible = "ti,tlv320aic3106";
+		reg = <0x18>;
+		status = "okay";
+
+		/* Regulators */
+		IOVDD-supply = <&vdcdc2_reg>;
+		/* Derived from VBAT: Baseboard 3.3V / 1.8V */
+		AVDD-supply = <&vbat>;
+		DRVDD-supply = <&vbat>;
+		DVDD-supply = <&vbat>;
+	};
+	tca6416: gpio@20 {
+		compatible = "ti,tca6416";
+		reg = <0x20>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+};
+
+&wdt {
+	status = "okay";
+};
+
+&mmc0 {
+	max-frequency = <50000000>;
+	bus-width = <4>;
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins>;
+};
+
+&spi1 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi1_pins &spi1_cs0_pin>;
+	flash: m25p80@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "m25p64";
+		spi-max-frequency = <30000000>;
+		m25p,fast-read;
+		reg = <0>;
+		partition@0 {
+			label = "U-Boot-SPL";
+			reg = <0x00000000 0x00010000>;
+			read-only;
+		};
+		partition@1 {
+			label = "U-Boot";
+			reg = <0x00010000 0x00080000>;
+			read-only;
+		};
+		partition@2 {
+			label = "U-Boot-Env";
+			reg = <0x00090000 0x00010000>;
+			read-only;
+		};
+		partition@3 {
+			label = "Kernel";
+			reg = <0x000a0000 0x00280000>;
+		};
+		partition@4 {
+			label = "Filesystem";
+			reg = <0x00320000 0x00400000>;
+		};
+		partition@5 {
+			label = "MAC-Address";
+			reg = <0x007f0000 0x00010000>;
+			read-only;
+		};
+	};
+};
+
+&mdio {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mdio_pins>;
+	bus_freq = <2200000>;
+};
+
+&eth0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mii_pins>;
+};
+
+&gpio {
+	status = "okay";
+};
+
 /include/ "tps6507x.dtsi"
 
 &tps {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes
  2018-05-10  9:10 ` Sekhar Nori
@ 2018-05-10 15:08   ` Adam Ford
  2018-05-10 15:19     ` Sekhar Nori
  0 siblings, 1 reply; 5+ messages in thread
From: Adam Ford @ 2018-05-10 15:08 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Adam Ford, Kevin Hilman, Rob Herring, Mark Rutland, arm-soc,
	devicetree, Linux Kernel Mailing List

On Thu, May 10, 2018 at 4:10 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Monday 07 May 2018 06:07 PM, Adam Ford wrote:
>> Many node labels in the device tree (like serial0, serial1, etc) are being
>> redefined, so let's modernize the device tree by using phandles to
>> extend the existing nodes.  This helps reduce the whitespace.
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> I applied this without the pmic changes. I am not convinced about those.
> The tps node is already being referred to as phandle. I am not sure
> referring to each individual regulator using phandle is needed. Other
> files like am335x-evm.dts don't do it as well.

I tested the regulator values and names after booting to see if the
names and values matched the expected values. They did, so I am
fairly confident it would have worked.

>
> Another thing is whether we really need the tp6507x.dtsi file. It does
> not seem to contain much and also da850-evm.dts is the only file
> including it. So it seems pretty pointless to me.

Do you want me to do a patch that removes the  tp6507x.dtsi file and
just sets up the
PMIC from scratch within the da850-evm file?

>
> Here is what I committed.

Thank you.  I think looks cleaner this way, and more consistent with
many of the other platforms and boards.

adam
>
> Thanks,
> Sekhar
>
> ---8<---
> commit ae62a32d6019a8225e2c32e631b8b0d039151131 (refs/heads/v4.18/dt)
> Author:     Adam Ford <aford173@gmail.com>
> AuthorDate: Mon May 7 07:37:21 2018 -0500
> Commit:     Sekhar Nori <nsekhar@ti.com>
> CommitDate: Thu May 10 14:01:15 2018 +0530
>
>     ARM: dts: da850-evm: use phandles to extend nodes
>
>     Many node labels in the device tree (like serial0, serial1, etc) are being
>     redefined, so let's modernize the device tree by using phandles to
>     extend the existing nodes.  This helps reduce the whitespace.
>
>     Signed-off-by: Adam Ford <aford173@gmail.com>
>     [nsekhar@ti.com: drop tps6507x related changes]
>     Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 339cae353302..9389f95f4094 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -27,143 +27,6 @@
>                 spi0 = &spi1;
>         };
>
> -       soc@1c00000 {
> -               pmx_core: pinmux@14120 {
> -                       status = "okay";
> -
> -                       mcasp0_pins: pinmux_mcasp0_pins {
> -                               pinctrl-single,bits = <
> -                                       /*
> -                                        * AHCLKX, ACLKX, AFSX, AHCLKR, ACLKR,
> -                                        * AFSR, AMUTE
> -                                        */
> -                                       0x00 0x11111111 0xffffffff
> -                                       /* AXR11, AXR12 */
> -                                       0x04 0x00011000 0x000ff000
> -                               >;
> -                       };
> -                       nand_pins: nand_pins {
> -                               pinctrl-single,bits = <
> -                                       /* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
> -                                       0x1c 0x10110110  0xf0ff0ff0
> -                                       /*
> -                                        * EMA_D[0], EMA_D[1], EMA_D[2],
> -                                        * EMA_D[3], EMA_D[4], EMA_D[5],
> -                                        * EMA_D[6], EMA_D[7]
> -                                        */
> -                                       0x24 0x11111111  0xffffffff
> -                                       /* EMA_A[1], EMA_A[2] */
> -                                       0x30 0x01100000  0x0ff00000
> -                               >;
> -                       };
> -               };
> -               serial0: serial@42000 {
> -                       status = "okay";
> -               };
> -               serial1: serial@10c000 {
> -                       status = "okay";
> -               };
> -               serial2: serial@10d000 {
> -                       status = "okay";
> -               };
> -               rtc0: rtc@23000 {
> -                       status = "okay";
> -               };
> -               i2c0: i2c@22000 {
> -                       status = "okay";
> -                       clock-frequency = <100000>;
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&i2c0_pins>;
> -
> -                       tps: tps@48 {
> -                               reg = <0x48>;
> -                       };
> -                       tlv320aic3106: tlv320aic3106@18 {
> -                               #sound-dai-cells = <0>;
> -                               compatible = "ti,tlv320aic3106";
> -                               reg = <0x18>;
> -                               status = "okay";
> -
> -                               /* Regulators */
> -                               IOVDD-supply = <&vdcdc2_reg>;
> -                               /* Derived from VBAT: Baseboard 3.3V / 1.8V */
> -                               AVDD-supply = <&vbat>;
> -                               DRVDD-supply = <&vbat>;
> -                               DVDD-supply = <&vbat>;
> -                       };
> -                       tca6416: gpio@20 {
> -                               compatible = "ti,tca6416";
> -                               reg = <0x20>;
> -                               gpio-controller;
> -                               #gpio-cells = <2>;
> -                       };
> -               };
> -               wdt: wdt@21000 {
> -                       status = "okay";
> -               };
> -               mmc0: mmc@40000 {
> -                       max-frequency = <50000000>;
> -                       bus-width = <4>;
> -                       status = "okay";
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&mmc0_pins>;
> -               };
> -               spi1: spi@30e000 {
> -                       status = "okay";
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&spi1_pins &spi1_cs0_pin>;
> -                       flash: m25p80@0 {
> -                               #address-cells = <1>;
> -                               #size-cells = <1>;
> -                               compatible = "m25p64";
> -                               spi-max-frequency = <30000000>;
> -                               m25p,fast-read;
> -                               reg = <0>;
> -                               partition@0 {
> -                                       label = "U-Boot-SPL";
> -                                       reg = <0x00000000 0x00010000>;
> -                                       read-only;
> -                               };
> -                               partition@1 {
> -                                       label = "U-Boot";
> -                                       reg = <0x00010000 0x00080000>;
> -                                       read-only;
> -                               };
> -                               partition@2 {
> -                                       label = "U-Boot-Env";
> -                                       reg = <0x00090000 0x00010000>;
> -                                       read-only;
> -                               };
> -                               partition@3 {
> -                                       label = "Kernel";
> -                                       reg = <0x000a0000 0x00280000>;
> -                               };
> -                               partition@4 {
> -                                       label = "Filesystem";
> -                                       reg = <0x00320000 0x00400000>;
> -                               };
> -                               partition@5 {
> -                                       label = "MAC-Address";
> -                                       reg = <0x007f0000 0x00010000>;
> -                                       read-only;
> -                               };
> -                       };
> -               };
> -               mdio: mdio@224000 {
> -                       status = "okay";
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&mdio_pins>;
> -                       bus_freq = <2200000>;
> -               };
> -               eth0: ethernet@220000 {
> -                       status = "okay";
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&mii_pins>;
> -               };
> -               gpio: gpio@226000 {
> -                       status = "okay";
> -               };
> -       };
>         vbat: fixedregulator0 {
>                 compatible = "regulator-fixed";
>                 regulator-name = "vbat";
> @@ -200,6 +63,153 @@
>         };
>  };
>
> +&pmx_core {
> +       status = "okay";
> +
> +       mcasp0_pins: pinmux_mcasp0_pins {
> +               pinctrl-single,bits = <
> +                       /*
> +                        * AHCLKX, ACLKX, AFSX, AHCLKR, ACLKR,
> +                        * AFSR, AMUTE
> +                        */
> +                       0x00 0x11111111 0xffffffff
> +                       /* AXR11, AXR12 */
> +                       0x04 0x00011000 0x000ff000
> +               >;
> +       };
> +       nand_pins: nand_pins {
> +               pinctrl-single,bits = <
> +                       /* EMA_WAIT[0], EMA_OE, EMA_WE, EMA_CS[4], EMA_CS[3] */
> +                       0x1c 0x10110110  0xf0ff0ff0
> +                       /*
> +                        * EMA_D[0], EMA_D[1], EMA_D[2],
> +                        * EMA_D[3], EMA_D[4], EMA_D[5],
> +                        * EMA_D[6], EMA_D[7]
> +                        */
> +                       0x24 0x11111111  0xffffffff
> +                       /* EMA_A[1], EMA_A[2] */
> +                       0x30 0x01100000  0x0ff00000
> +               >;
> +       };
> +};
> +
> +&serial0 {
> +       status = "okay";
> +};
> +
> +&serial1 {
> +       status = "okay";
> +};
> +
> +&serial2 {
> +       status = "okay";
> +};
> +
> +&rtc0 {
> +       status = "okay";
> +};
> +
> +&i2c0 {
> +       status = "okay";
> +       clock-frequency = <100000>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&i2c0_pins>;
> +
> +       tps: tps@48 {
> +               reg = <0x48>;
> +       };
> +       tlv320aic3106: tlv320aic3106@18 {
> +               #sound-dai-cells = <0>;
> +               compatible = "ti,tlv320aic3106";
> +               reg = <0x18>;
> +               status = "okay";
> +
> +               /* Regulators */
> +               IOVDD-supply = <&vdcdc2_reg>;
> +               /* Derived from VBAT: Baseboard 3.3V / 1.8V */
> +               AVDD-supply = <&vbat>;
> +               DRVDD-supply = <&vbat>;
> +               DVDD-supply = <&vbat>;
> +       };
> +       tca6416: gpio@20 {
> +               compatible = "ti,tca6416";
> +               reg = <0x20>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +       };
> +};
> +
> +&wdt {
> +       status = "okay";
> +};
> +
> +&mmc0 {
> +       max-frequency = <50000000>;
> +       bus-width = <4>;
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&mmc0_pins>;
> +};
> +
> +&spi1 {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&spi1_pins &spi1_cs0_pin>;
> +       flash: m25p80@0 {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "m25p64";
> +               spi-max-frequency = <30000000>;
> +               m25p,fast-read;
> +               reg = <0>;
> +               partition@0 {
> +                       label = "U-Boot-SPL";
> +                       reg = <0x00000000 0x00010000>;
> +                       read-only;
> +               };
> +               partition@1 {
> +                       label = "U-Boot";
> +                       reg = <0x00010000 0x00080000>;
> +                       read-only;
> +               };
> +               partition@2 {
> +                       label = "U-Boot-Env";
> +                       reg = <0x00090000 0x00010000>;
> +                       read-only;
> +               };
> +               partition@3 {
> +                       label = "Kernel";
> +                       reg = <0x000a0000 0x00280000>;
> +               };
> +               partition@4 {
> +                       label = "Filesystem";
> +                       reg = <0x00320000 0x00400000>;
> +               };
> +               partition@5 {
> +                       label = "MAC-Address";
> +                       reg = <0x007f0000 0x00010000>;
> +                       read-only;
> +               };
> +       };
> +};
> +
> +&mdio {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&mdio_pins>;
> +       bus_freq = <2200000>;
> +};
> +
> +&eth0 {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&mii_pins>;
> +};
> +
> +&gpio {
> +       status = "okay";
> +};
> +
>  /include/ "tps6507x.dtsi"
>
>  &tps {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes
  2018-05-10 15:08   ` Adam Ford
@ 2018-05-10 15:19     ` Sekhar Nori
  2018-05-10 15:39       ` Adam Ford
  0 siblings, 1 reply; 5+ messages in thread
From: Sekhar Nori @ 2018-05-10 15:19 UTC (permalink / raw)
  To: Adam Ford
  Cc: Adam Ford, Kevin Hilman, Rob Herring, Mark Rutland, arm-soc,
	devicetree, Linux Kernel Mailing List

On Thursday 10 May 2018 08:38 PM, Adam Ford wrote:
> On Thu, May 10, 2018 at 4:10 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>> On Monday 07 May 2018 06:07 PM, Adam Ford wrote:
>>> Many node labels in the device tree (like serial0, serial1, etc) are being
>>> redefined, so let's modernize the device tree by using phandles to
>>> extend the existing nodes.  This helps reduce the whitespace.
>>>
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>
>> I applied this without the pmic changes. I am not convinced about those.
>> The tps node is already being referred to as phandle. I am not sure
>> referring to each individual regulator using phandle is needed. Other
>> files like am335x-evm.dts don't do it as well.
> 
> I tested the regulator values and names after booting to see if the
> names and values matched the expected values. They did, so I am
> fairly confident it would have worked.

Not doubting that. But I am not sure if thats the "norm". Do you see any
other device-tree file doing this?

> 
>>
>> Another thing is whether we really need the tp6507x.dtsi file. It does
>> not seem to contain much and also da850-evm.dts is the only file
>> including it. So it seems pretty pointless to me.
> 
> Do you want me to do a patch that removes the  tp6507x.dtsi file and
> just sets up the
> PMIC from scratch within the da850-evm file?

I am fine with the plan, but not something urgent, IMO.

> 
>>
>> Here is what I committed.
> 
> Thank you.  I think looks cleaner this way, and more consistent with
> many of the other platforms and boards.

Yes.

Thanks,
Sekhar

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes
  2018-05-10 15:19     ` Sekhar Nori
@ 2018-05-10 15:39       ` Adam Ford
  0 siblings, 0 replies; 5+ messages in thread
From: Adam Ford @ 2018-05-10 15:39 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Adam Ford, Kevin Hilman, Rob Herring, Mark Rutland, arm-soc,
	devicetree, Linux Kernel Mailing List

On Thu, May 10, 2018 at 10:19 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Thursday 10 May 2018 08:38 PM, Adam Ford wrote:
>> On Thu, May 10, 2018 at 4:10 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>>> On Monday 07 May 2018 06:07 PM, Adam Ford wrote:
>>>> Many node labels in the device tree (like serial0, serial1, etc) are being
>>>> redefined, so let's modernize the device tree by using phandles to
>>>> extend the existing nodes.  This helps reduce the whitespace.
>>>>
>>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>>
>>> I applied this without the pmic changes. I am not convinced about those.
>>> The tps node is already being referred to as phandle. I am not sure
>>> referring to each individual regulator using phandle is needed. Other
>>> files like am335x-evm.dts don't do it as well.
>>
>> I tested the regulator values and names after booting to see if the
>> names and values matched the expected values. They did, so I am
>> fairly confident it would have worked.
>
> Not doubting that. But I am not sure if thats the "norm". Do you see any
> other device-tree file doing this?

The omap3 boards do this.  For example, the beagle-xm board includes
the twl4030 files

#include "twl4030.dtsi"
#include "twl4030_omap3.dtsi"

These files setup the PMIC regulators, but the beagle-xm modifies the
PMIC settings phandles.

&vaux2 {
regulator-name = "usb_1v8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
};

I was trying to mimic this behavior when I did it for the da850-evm,
however, it seems like a moot point we merge the
 tp6507x.dtsi contents into the da850-evm.dts.

>
>>
>>>
>>> Another thing is whether we really need the tp6507x.dtsi file. It does
>>> not seem to contain much and also da850-evm.dts is the only file
>>> including it. So it seems pretty pointless to me.
>>
>> Do you want me to do a patch that removes the  tp6507x.dtsi file and
>> just sets up the
>> PMIC from scratch within the da850-evm file?
>
> I am fine with the plan, but not something urgent, IMO.

I'm going to try and revisit the LCD again first, but I'll try to do
them both this weekend if I can get some time.

>
>>
>>>
>>> Here is what I committed.
>>
>> Thank you.  I think looks cleaner this way, and more consistent with
>> many of the other platforms and boards.
>
> Yes.
>
> Thanks,
> Sekhar

Thank you,

adam

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-10 15:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 12:37 [PATCH V3] ARM: dts: da850-evm: use phandles to extend nodes Adam Ford
2018-05-10  9:10 ` Sekhar Nori
2018-05-10 15:08   ` Adam Ford
2018-05-10 15:19     ` Sekhar Nori
2018-05-10 15:39       ` Adam Ford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).