Re: [PATCH v2 1/3] arm64: dts: imx: Add i.mx8mm dtsi support

[Shawn Guo <shawnguo@kernel.org>]

On Tue, Mar 12, 2019 at 05:44:53AM +0000, Jacky Bai wrote:
> The i.MX8M Mini is new SOC of the i.MX8M family. it is
> focused on delivering the latest and greatest video and
> audio experience combining state-of-the-art media-specific
> features with high-performance processing while optimized
> for lowest power consumption. The i.MX 8M Mini Media Applications
> Processor is  14nm FinFET product of the growing i.MX8M family
> targeting the consumer & industrial market. It is built in 14LPP
> to achieve both high performance and low power consumption
> and relies on a powerful fully coherent core complex based on
> a quad Cortex-A53 cluster with video and graphics accelerators
> 
> This patch adds the basic dtsi support for i.MX8MM.
> 
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>  change v1->v2:
>  - removed unnecessary 'okay' status
>  - change AIPS's address-cells and size-cells to '1'
>  - remove sdma's undocumented property
> ---
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 717 ++++++++++++++++++++++++++++++
>  1 file changed, 717 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mm.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> new file mode 100644
> index 0000000..47740d6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -0,0 +1,717 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2019 NXP
> + */
> +
> +#include <dt-bindings/clock/imx8mm-clock.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +#include "imx8mm-pinfunc.h"
> +
> +/ {
> +	compatible = "fsl,imx8mm";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		ethernet0 = &fec1;
> +		i2c0 = &i2c1;
> +		i2c1 = &i2c2;
> +		i2c2 = &i2c3;
> +		i2c3 = &i2c4;
> +		serial0 = &uart1;
> +		serial1 = &uart2;
> +		serial2 = &uart3;
> +		serial3 = &uart4;
> +		spi0 = &ecspi1;
> +		spi1 = &ecspi2;
> +		spi2 = &ecspi3;
> +		mmc0 = &usdhc1;
> +		mmc1 = &usdhc2;
> +		mmc2 = &usdhc3;
> +		gpio0 = &gpio1;
> +		gpio1 = &gpio2;
> +		gpio2 = &gpio3;
> +		gpio3 = &gpio4;
> +		gpio4 = &gpio5;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		A53_0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x0>;
> +			enable-method = "psci";
> +			next-level-cache = <&A53_L2>;
> +		};
> +
> +		A53_1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x1>;
> +			enable-method = "psci";
> +			next-level-cache = <&A53_L2>;
> +		};
> +
> +		A53_2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x2>;
> +			enable-method = "psci";
> +			next-level-cache = <&A53_L2>;
> +		};
> +
> +		A53_3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x3>;
> +			enable-method = "psci";
> +			next-level-cache = <&A53_L2>;
> +		};
> +
> +		A53_L2: l2-cache0 {
> +			compatible = "cache";
> +		};
> +	};
> +
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x40000000 0 0x80000000>;
> +	};
> +
> +	osc_32k: clock-osc-32k {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "osc_32k";
> +	};
> +
> +	osc_24m: clock-osc-24m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		clock-output-names = "osc_24m";
> +	};
> +
> +	clk_ext1: clock-ext1 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <133000000>;
> +		clock-output-names = "clk_ext1";
> +	};
> +
> +	clk_ext2: clock-ext2 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <133000000>;
> +		clock-output-names = "clk_ext2";
> +	};
> +
> +	clk_ext3: clock-ext3 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <133000000>;
> +		clock-output-names = "clk_ext3";
> +	};
> +
> +	clk_ext4: clock-ext4 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency= <133000000>;
> +		clock-output-names = "clk_ext4";
> +	};
> +
> +	gic: interrupt-controller@38800000 {
> +		compatible = "arm,gic-v3";
> +		reg = <0x0 0x38800000 0 0x10000>, /* GIC Dist */
> +		      <0x0 0x38880000 0 0xC0000>; /* GICR (RD_base + SGI_base) */
> +		#interrupt-cells = <3>;
> +		interrupt-controller;
> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0";
> +		method = "smc";
> +	};
> +
> +	pmu {
> +		compatible = "arm,armv8-pmuv3";
> +		interrupts = <GIC_PPI 7
> +			     (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
> +		interrupt-affinity = <&A53_0>, <&A53_1>, <&A53_2>, <&A53_3>;
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>, /* Physical Secure */
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>, /* Physical Non-Secure */
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>, /* Virtual */
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>; /* Hypervisor */
> +		clock-frequency = <8000000>;
> +		arm,no-tick-in-suspend;
> +	};
> +
> +	soc@0 {

The unit-address 0 makes no sense here, right?

> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0x0 0x0 0x0 0x3e000000>;
> +
> +		aips1: bus@30000000 {
> +			compatible = "fsl,aips-bus", "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			gpio1: gpio@30200000 {
> +				compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio";
> +				reg = <0x30200000 0x10000>;
> +				interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +
> +			gpio2: gpio@30210000 {
> +				compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio";
> +				reg = <0x30210000 0x10000>;
> +				interrupts = <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +
> +			gpio3: gpio@30220000 {
> +				compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio";
> +				reg = <0x30220000 0x10000>;
> +				interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +
> +			gpio4: gpio@30230000 {
> +				compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio";
> +				reg = <0x30230000 0x10000>;
> +				interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +
> +			gpio5: gpio@30240000 {
> +				compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio";
> +				reg = <0x30240000 0x10000>;
> +				interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
> +
> +			wdog1: wdog@30280000 {

watchdog for node name.

> +				compatible = "fsl,imx21-wdt";

Don't you need a "fsl,imx8mm-wdt"?

> +				reg = <0x30280000 0x10000>;
> +				interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_WDOG1_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			wdog2: wdog@30290000 {
> +				compatible = "fsl,imx21-wdt";
> +				reg = <0x30290000 0x10000>;
> +				interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_WDOG2_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			wdog3: wdog@302a0000 {
> +				compatible = "fsl,imx21-wdt";
> +				reg = <0x302a0000 0x10000>;
> +				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_WDOG3_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			sdma2: dma-controller@302c0000 {
> +				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";

Should be fsl,imx8mm-sdma?

> +				reg = <0x302c0000 0x10000>;
> +				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_SDMA2_ROOT>,
> +					 <&clk IMX8MM_CLK_SDMA2_ROOT>;
> +				clock-names = "ipg", "ahb";
> +				#dma-cells = <3>;
> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";

i.mx8mm reuses the exactly same SDMA RAM script as imx7d?

> +			};
> +
> +			sdma3: dma-controller@302b0000 {
> +				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";
> +				reg = <0x302b0000 0x10000>;
> +				interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_SDMA3_ROOT>,
> +				 <&clk IMX8MM_CLK_SDMA3_ROOT>;
> +				clock-names = "ipg", "ahb";
> +				#dma-cells = <3>;
> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> +			};
> +
> +			iomuxc: pinctrl@30330000 {
> +				compatible = "fsl,imx8mm-iomuxc";
> +				reg = <0x30330000 0x10000>;
> +			};
> +
> +			gpr: iomuxc-gpr@30340000 {
> +				compatible = "fsl,imx8mm-iomuxc-gpr", "syscon";
> +				reg = <0x30340000 0x10000>;
> +			};
> +
> +			ocotp: ocotp-ctrl@30350000 {
> +				compatible = "fsl,imx8mq-ocotp", "fsl,imx7d-ocotp", "syscon";

Should be fsl,imx8mm-ocotp?

> +				reg = <0x30350000 0x10000>;
> +				clocks = <&clk IMX8MM_CLK_OCOTP_ROOT>;
> +				/* For nvmem subnodes */
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +			};
> +
> +			anatop: anatop@30360000 {
> +				compatible = "fsl,imx8mm-anatop", "syscon", "simple-bus";
> +				reg = <0x30360000 0x10000>;
> +			};
> +
> +			snvs: snvs@30370000 {
> +				compatible = "fsl,sec-v4.0-mon","syscon", "simple-mfd";
> +				reg = <0x30370000 0x10000>;
> +
> +				snvs_rtc: snvs-rtc-lp{

Miss one space before {.

> +					compatible = "fsl,sec-v4.0-mon-rtc-lp";
> +					regmap =<&snvs>;

Miss one space after =.

> +					offset = <0x34>;
> +					interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>,
> +						     <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
> +				};
> +
> +				snvs_pwrkey: snvs-powerkey {
> +					compatible = "fsl,sec-v4.0-pwrkey";
> +					regmap = <&snvs>;
> +					interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> +					linux,keycode = <KEY_POWER>;
> +					wakeup-source;
> +				};
> +			};
> +
> +			clk: clock-controller@30380000 {
> +				compatible = "fsl,imx8mm-ccm";
> +				reg = <0x30380000 0x10000>;
> +				#clock-cells = <1>;
> +				clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>,
> +					 <&clk_ext3>, <&clk_ext4>;
> +				clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2",
> +					      "clk_ext3", "clk_ext4";
> +			};
> +
> +			src: src@30390000 {

reset-controller for node name?

> +				compatible = "fsl,imx8mm-src", "syscon";
> +				reg = <0x30390000 0x10000>;
> +				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> +				#reset-cells = <1>;
> +			};
> +
> +			gpc: gpc@303a0000 {

interrupt-controller for node name?

> +				compatible = "fsl,imx8mm-gpc", "syscon";
> +				reg = <0x303a0000 0x10000>;
> +				interrupt-controller;
> +				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +				#interrupt-cells = <3>;
> +			};
> +		};
> +
> +		aips2: bus@30400000 {
> +			compatible = "fsl,aips-bus", "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			pwm1: pwm@30660000 {
> +				compatible = "fsl,imx8mm-pwm", "fsl,imx27-pwm";
> +				reg = <0x30660000 0x10000>;
> +				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_PWM1_ROOT>,
> +					<&clk IMX8MM_CLK_PWM1_ROOT>;
> +				clock-names = "ipg", "per";
> +				#pwm-cells = <2>;
> +				status = "disabled";
> +			};
> +
> +			pwm2: pwm@30670000 {
> +				compatible = "fsl,imx8mm-pwm", "fsl,imx27-pwm";
> +				reg = <0x30670000 0x10000>;
> +				interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_PWM2_ROOT>,
> +					 <&clk IMX8MM_CLK_PWM2_ROOT>;
> +				clock-names = "ipg", "per";
> +				#pwm-cells = <2>;
> +				status = "disabled";
> +			};
> +
> +			pwm3: pwm@30680000 {
> +				compatible = "fsl,imx8mm-pwm", "fsl,imx27-pwm";
> +				reg = <0x30680000 0x10000>;
> +				interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_PWM3_ROOT>,
> +					 <&clk IMX8MM_CLK_PWM3_ROOT>;
> +				clock-names = "ipg", "per";
> +				#pwm-cells = <2>;
> +				status = "disabled";
> +			};
> +
> +			pwm4: pwm@30690000 {
> +				compatible = "fsl,imx8mm-pwm", "fsl,imx27-pwm";
> +				reg = <0x30690000 0x10000>;
> +				interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_PWM4_ROOT>,
> +					 <&clk IMX8MM_CLK_PWM4_ROOT>;
> +				clock-names = "ipg", "per";
> +				#pwm-cells = <2>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		aips3: bus@30800000 {
> +			compatible = "fsl,aips-bus", "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			ecspi1: ecspi@30820000 {

spi for node name?

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

We usually start properties with compatible.  Can these be moved
somewhere after compatible?

> +				compatible = "fsl,imx8mm-ecspi", "fsl,imx51-ecspi";
> +				reg = <0x30820000 0x10000>;
> +				interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_ECSPI1_ROOT>,
> +					 <&clk IMX8MM_CLK_ECSPI1_ROOT>;
> +				clock-names = "ipg", "per";
> +				dmas = <&sdma1 0 7 1>, <&sdma1 1 7 2>;
> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
> +			ecspi2: ecspi@30830000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx8mm-ecspi", "fsl,imx51-ecspi";
> +				reg = <0x30830000 0x10000>;
> +				interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_ECSPI2_ROOT>,
> +					 <&clk IMX8MM_CLK_ECSPI2_ROOT>;
> +				clock-names = "ipg", "per";
> +				dmas = <&sdma1 2 7 1>, <&sdma1 3 7 2>;
> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
> +			ecspi3: ecspi@30840000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx8mm-ecspi", "fsl,imx51-ecspi";
> +				reg = <0x30840000 0x10000>;
> +				interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_ECSPI3_ROOT>,
> +					 <&clk IMX8MM_CLK_ECSPI3_ROOT>;
> +				clock-names = "ipg", "per";
> +				dmas = <&sdma1 4 7 1>, <&sdma1 5 7 2>;
> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
> +			uart1: serial@30860000 {
> +				compatible = "fsl,imx8mm-uart",
> +					     "fsl,imx6q-uart", "fsl,imx21-uart";

I think "fsl,imx21-uart" can be dropped in this case?

> +				reg = <0x30860000 0x10000>;
> +				interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_UART1_ROOT>,
> +					 <&clk IMX8MM_CLK_UART1_ROOT>;
> +				clock-names = "ipg", "per";
> +				dmas = <&sdma1 22 4 0>, <&sdma1 23 4 0>;
> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
> +			uart3: serial@30880000 {
> +				compatible = "fsl,imx8mm-uart",
> +					     "fsl,imx6q-uart", "fsl,imx21-uart";
> +				reg = <0x30880000 0x10000>;
> +				interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_UART3_ROOT>,
> +					 <&clk IMX8MM_CLK_UART3_ROOT>;
> +				clock-names = "ipg", "per";
> +				dmas = <&sdma1 26 4 0>, <&sdma1 27 4 0>;
> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
> +			uart2: serial@30890000 {
> +				compatible = "fsl,imx8mm-uart",
> +					     "fsl,imx6q-uart", "fsl,imx21-uart";
> +				reg = <0x30890000 0x10000>;
> +				interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_UART2_ROOT>,
> +					 <&clk IMX8MM_CLK_UART2_ROOT>;
> +				clock-names = "ipg", "per";
> +				status = "disabled";
> +			};
> +
> +			i2c1: i2c@30a20000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx8mm-i2c", "fsl,imx21-i2c";
> +				reg = <0x30a20000 0x10000>;
> +				interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_I2C1_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			i2c2: i2c@30a30000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx8mm-i2c", "fsl,imx21-i2c";
> +				reg = <0x30a30000 0x10000>;
> +				interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_I2C2_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			i2c3: i2c@30a40000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx8mm-i2c", "fsl,imx21-i2c";
> +				reg = <0x30a40000 0x10000>;
> +				interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_I2C3_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			i2c4: i2c@30a50000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx8mm-i2c", "fsl,imx21-i2c";
> +				reg = <0x30a50000 0x10000>;
> +				interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_I2C4_ROOT>;
> +				status = "disabled";
> +			};
> +
> +			uart4: serial@30a60000 {
> +				compatible = "fsl,imx8mq-uart",
> +					     "fsl,imx6q-uart", "fsl,imx21-uart";
> +				reg = <0x30a60000 0x10000>;
> +				interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_UART4_ROOT>,
> +					 <&clk IMX8MM_CLK_UART4_ROOT>;
> +				clock-names = "ipg", "per";
> +				dmas = <&sdma1 28 4 0>, <&sdma1 29 4 0>;
> +				dma-names = "rx", "tx";
> +				status = "disabled";
> +			};
> +
> +			usdhc1: mmc@30b40000 {
> +				compatible = "fsl,imx8mq-usdhc", "fsl,imx7d-usdhc";

Should be fsl,imx8mm-usdhc?

> +				reg = <0x30b40000 0x10000>;
> +				interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_DUMMY>,
> +					 <&clk IMX8MM_CLK_NAND_USDHC_BUS>,
> +					 <&clk IMX8MM_CLK_USDHC1_ROOT>;
> +				clock-names = "ipg", "ahb", "per";
> +				assigned-clocks = <&clk IMX8MM_CLK_USDHC1>;
> +				assigned-clock-rates = <400000000>;
> +				fsl,tuning-start-tap = <20>;
> +				fsl,tuning-step= <2>;
> +				bus-width = <4>;
> +				status = "disabled";
> +			};
> +
> +			usdhc2: mmc@30b50000 {
> +				compatible = "fsl,imx8mq-usdhc", "fsl,imx7d-usdhc";
> +				reg = <0x30b50000 0x10000>;
> +				interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_DUMMY>,
> +					 <&clk IMX8MM_CLK_NAND_USDHC_BUS>,
> +					 <&clk IMX8MM_CLK_USDHC2_ROOT>;
> +				clock-names = "ipg", "ahb", "per";
> +				fsl,tuning-start-tap = <20>;
> +				fsl,tuning-step= <2>;
> +				bus-width = <4>;
> +				status = "disabled";
> +			};
> +
> +			usdhc3: mmc@30b60000 {
> +				compatible = "fsl,imx8mq-usdhc", "fsl,imx7d-usdhc";
> +				reg = <0x30b60000 0x10000>;
> +				interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_DUMMY>,
> +					 <&clk IMX8MM_CLK_NAND_USDHC_BUS>,
> +					 <&clk IMX8MM_CLK_USDHC3_ROOT>;
> +				clock-names = "ipg", "ahb", "per";
> +				assigned-clocks = <&clk IMX8MM_CLK_USDHC3_ROOT>;
> +				assigned-clock-rates = <400000000>;
> +				fsl,tuning-start-tap = <20>;
> +				fsl,tuning-step= <2>;
> +				bus-width = <4>;
> +				status = "disabled";
> +			};
> +
> +			sdma1: dma-controller@30bd0000 {
> +				compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma";

Should be fsl,imx8mm-sdma?

> +				reg = <0x30bd0000 0x10000>;
> +				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_SDMA1_ROOT>,
> +					 <&clk IMX8MM_CLK_SDMA1_ROOT>;
> +				clock-names = "ipg", "ahb";
> +				#dma-cells = <3>;
> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> +			};
> +
> +			fec1: ethernet@30be0000 {
> +				compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";

Should be fsl,imx8mm-fec?

> +				reg = <0x30be0000 0x10000>;
> +				interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_ENET1_ROOT>,
> +					 <&clk IMX8MM_CLK_ENET1_ROOT>,
> +					 <&clk IMX8MM_CLK_ENET_TIMER>,
> +					 <&clk IMX8MM_CLK_ENET_REF>,
> +					 <&clk IMX8MM_CLK_ENET_PHY_REF>;
> +				clock-names = "ipg", "ahb", "ptp",
> +					      "enet_clk_ref", "enet_out";
> +				assigned-clocks = <&clk IMX8MM_CLK_ENET_AXI>,
> +						  <&clk IMX8MM_CLK_ENET_TIMER>,
> +						  <&clk IMX8MM_CLK_ENET_REF>,
> +						  <&clk IMX8MM_CLK_ENET_TIMER>;
> +				assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_266M>,
> +							 <&clk IMX8MM_SYS_PLL2_100M>,
> +							 <&clk IMX8MM_SYS_PLL2_125M>;
> +				assigned-clock-rates = <0>, <0>, <125000000>, <100000000>;
> +				stop-mode = <&gpr 0x10 3>;

vendor property?

> +				fsl,num-tx-queues=<3>;
> +				fsl,num-rx-queues=<3>;

Miss space before and after =.

> +				fsl,wakeup_irq = <2>;

vendor property?

> +				status = "disabled";
> +			};
> +
> +		};
> +
> +		aips4: bus@32c00000 {
> +			compatible = "fsl,aips-bus", "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			usbotg1: usb@32e40000 {
> +				compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb", "fsl,imx27-usb";

fsl,imx27-usb can be saved?

> +				reg = <0x32e40000 0x200>;
> +				interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_USB1_CTRL_ROOT>;
> +				clock-names = "usb1_ctrl_root_clk";
> +				assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>,
> +						  <&clk IMX8MM_CLK_USB_CORE_REF>;
> +				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>,
> +							 <&clk IMX8MM_SYS_PLL1_100M>;
> +				fsl,usbphy = <&usbphynop1>;
> +				fsl,usbmisc = <&usbmisc1 0>;
> +				status = "disabled";
> +			};
> +
> +			usbphynop1: usbphynop1 {
> +				compatible = "usb-nop-xceiv";
> +				clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> +				assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> +				assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>;
> +				clock-names = "main_clk";
> +			};
> +
> +			usbmisc1: usbmisc@32e40200 {
> +				#index-cells = <1>;

Move it afterwards?

> +				compatible = "fsl,imx7d-usbmisc", "fsl,imx6q-usbmisc";

Should be compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc"?

> +				reg = <0x32e40200 0x200>;
> +			};
> +
> +			usbotg2: usb@32e50000 {
> +				compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb", "fsl,imx27-usb";
> +				reg = <0x32e50000 0x200>;
> +				interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MM_CLK_USB1_CTRL_ROOT>;
> +				clock-names = "usb1_ctrl_root_clk";
> +				assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>,
> +						  <&clk IMX8MM_CLK_USB_CORE_REF>;
> +				assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>,
> +							 <&clk IMX8MM_SYS_PLL1_100M>;
> +				fsl,usbphy = <&usbphynop2>;
> +				fsl,usbmisc = <&usbmisc2 0>;
> +				status = "disabled";
> +			};
> +
> +			usbphynop2: usbphynop2 {
> +				compatible = "usb-nop-xceiv";
> +				clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> +				assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
> +				assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>;
> +				clock-names = "main_clk";
> +			};
> +
> +			usbmisc2: usbmisc@32e50200 {
> +				#index-cells = <1>;
> +				compatible = "fsl,imx7d-usbmisc", "fsl,imx6q-usbmisc";
> +				reg = <0x32e50200 0x200>;
> +			};
> +
> +		};
> +
> +		dma_apbh: dma-apbh@33000000 {

dma-controller for node name.

> +			compatible = "fsl,imx7d-dma-apbh", "fsl,imx28-dma-apbh";
> +			reg = <0x33000000 0x2000>;
> +			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "gpmi0", "gpmi1", "gpmi2", "gpmi3";
> +			#dma-cells = <1>;
> +			dma-channels = <4>;
> +			clocks = <&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>;
> +		};
> +
> +		gpmi: gpmi-nand@33002000{

nand-controller for node name.

> +			compatible = "fsl,imx7d-gpmi-nand";

fsl,imx8mm-gpmi-nand before it?

> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			reg = <0x33002000 0x2000>, <0x33004000 0x4000>;
> +			reg-names = "gpmi-nand", "bch";
> +			interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "bch";
> +			clocks = <&clk IMX8MM_CLK_NAND_ROOT>,
> +			<&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>;

Miss indentation.

Shawn

> +			clock-names = "gpmi_io", "gpmi_bch_apb";
> +			dmas = <&dma_apbh 0>;
> +			dma-names = "rx-tx";
> +			status = "disabled";
> +		};
> +	};
> +};
> -- 
> 1.9.1
>