On Mon, Sep 22, 2014 at 03:45:48PM +0800, Jingchang Lu wrote:

I think leds are board level devices and do not have to be under node
"soc"?

+			compatible = "pwm-leds";

Please have a new line between property list and device node ...

+			led0 {

Usually, the instance number shouldn't be directly encoded in node name,
but be part of node name in form of unit-address, i.e. led@0.  That also
means we will need a 'reg' property for the node and the following for
the parent node.

	#address-cells = <1>;
	#size-cells = <0>;

+				label = "led0";
+				pwms = <&pwm3 0 150000 0>;
+				max-brightness = <100>;
+			};

Please also put a new line between device nodes.

+			led1 {
+				label = "led1";
+				pwms = <&pwm3 1 150000 0>;
+				max-brightness = <100>;
+			};
+			led2 {
+				label = "led2";
+				pwms = <&pwm3 2 150000 0>;
+				max-brightness = <100>;
+			};
+			led3 {
+				label = "led3";
+				pwms = <&pwm3 3 150000 0>;
+				max-brightness = <100>;
+			};
+			led4 {
+				label = "led4";
+				pwms = <&pwm3 4 150000 0>;
+				max-brightness = <100>;
+			};
+			led5 {
+				label = "led5";
+				pwms = <&pwm3 5 150000 0>;
+				max-brightness = <100>;
+			};
+			led6 {
+				label = "led6";
+				pwms = <&pwm3 6 150000 0>;
+				max-brightness = <100>;
+			};
+			led7 {
+				label = "led7";
+				pwms = <&pwm3 7 150000 0>;
+				max-brightness = <100>;
+			};
+		};
+	};
+};
+
+&dspi0 {
+	bus-num = <0>;
+	status = "okay";
+
+	dspiflash: at45db021d@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "atmel,at45db021d", "atmel,at45", "atmel,dataflash";
+		spi-max-frequency = <16000000>;
+		spi-cpol;
+		spi-cpha;
+		reg = <0>;
+	};
+};
+
+&enet0 {
+	tbi-handle = <&tbi0>;

I cannot find this property in any binding doc.

+	phy-handle = <&sgmii_phy1c>;
+	phy-connection-type = "sgmii";
+	status = "okay";
+};
+
+&enet1 {
+	tbi-handle = <&tbi0>;
+	phy-handle = <&sgmii_phy1d>;
+	phy-connection-type = "sgmii";
+	status = "okay";
+};
+
+&enet2 {
+	phy-handle = <&rgmii_phy3>;
+	phy-connection-type = "rgmii-id";
+	status = "okay";
+};
+
+&i2c0 {
+	status = "okay";

Add a new line.

+	pca9547@77 {
+		compatible = "philips,pca9547";

Undocumented compatible.

+		reg = <0x77>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x0>;
+
+			rtc@68 {
+				compatible = "dallas,ds3232";
+				reg = <0x68>;
+				interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+			};
+		};
+
+		i2c@2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x2>;
+
+			ina220@40 {
+				compatible = "ti,ina220";
+				reg = <0x40>;
+				shunt-resistor = <1000>;
+			};
+
+			ina220@41 {
+				compatible = "ti,ina220";
+				reg = <0x41>;
+				shunt-resistor = <1000>;
+			};
+		};
+
+		i2c@3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x3>;
+
+			eeprom@56 {
+				compatible = "at24,24c512";

Undocumented property.

+				reg = <0x56>;
+			};
+
+			eeprom@57 {
+				compatible = "at24,24c512";
+				reg = <0x57>;
+			};
+
+			adt7461a@4c {
+				compatible = "adt7461a";

Shouldn't it be "adi,adt7461a"?  And if that's case, per
Documentation/devicetree/bindings/hwmon/lm90.txt, vcc-supply is a
required property.

+				reg = <0x4c>;
+			};
+		};
+
+		i2c@4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x4>;
+		};

What's this device?  Drop it and add it only when you actually need it.

+	};
+};
+
+&ifc {
+	status = "okay";

I generally prefer to put 'status' at the bottom of the property list.

+	#address-cells = <2>;
+	#size-cells = <1>;
+	/* NOR, NAND Flashes and FPGA on board */
+	ranges = <0x0 0x0 0x0 0x60000000 0x08000000
+		0x2 0x0 0x0 0x7e800000 0x00010000
+		0x3 0x0 0x0 0x7fb00000 0x00000100>;
+
+		nor@0,0 {

Drop one level of indentation.

+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "cfi-flash";
+			reg = <0x0 0x0 0x8000000>;
+			bank-width = <2>;
+			device-width = <1>;
+		};
+
+		nand@2,0 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "fsl,ifc-nand";

"fsl,ifc" is a documented property, but "fsl,ifc-nand" is not.

+			reg = <0x2 0x0 0x10000>;
+		};
+
+		fpga: board-control@3,0 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-bus";
+			reg = <0x3 0x0 0x0000100>;
+			bank-width = <1>;
+			device-width = <1>;
+			ranges = <0 3 0 0x100>;
+
+			mdio-mux-emi1 {
+				compatible = "mdio-mux-mmioreg";
+				mdio-parent-bus = <&mdio0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x54 1>; /* BRDCFG4 */
+				mux-mask = <0xe0>; /* EMI1[2:0] */
+
+				/* Onboard PHYs */
+				ls1021amdio0: mdio@0 {
+					reg = <0>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					rgmii_phy1: ethernet-phy@1 {
+						reg = <0x1>;
+					};
+				};

Have a new line.

Shawn

+				ls1021amdio1: mdio@20 {
+					reg = <0x20>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					rgmii_phy2: ethernet-phy@2 {
+						reg = <0x2>;
+					};
+				};
+				ls1021amdio2: mdio@40 {
+					reg = <0x40>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					rgmii_phy3: ethernet-phy@3 {
+						reg = <0x3>;
+					};
+				};
+				ls1021amdio3: mdio@60 {
+					reg = <0x60>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					sgmii_phy1c: ethernet-phy@1c {
+						reg = <0x1c>;
+					};
+				};
+				ls1021amdio4: mdio@80 {
+					reg = <0x80>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					sgmii_phy1d: ethernet-phy@1d {
+						reg = <0x1d>;
+					};
+				};
+			};
+		};
+};
+
+&lpuart0 {
+	status = "okay";
+};
+
+&mdio0 {
+	tbi0: tbi-phy@8 {
+		reg = <0x8>;
+		device_type = "tbi-phy";
+	};
+};
+
+&pwm3 {
+	status = "okay";
+};
+
+&pwm7 {
+	status = "okay";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&uart1 {
+	status = "okay";
+};
-- 
1.8.0