Re: [PATCH 09/11] ARM: dts: mediatek: add basic support for MT6572 SoC

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 20/06/25 17:40, Max Shevchenko via B4 Relay ha scritto:
> From: Max Shevchenko <wctrl@proton.me>
> 
> Add basic support for the MediaTek MT6572 SoC.
> 
> Signed-off-by: Max Shevchenko <wctrl@proton.me>
> ---
>   arch/arm/boot/dts/mediatek/mt6572.dtsi | 105 +++++++++++++++++++++++++++++++++
>   1 file changed, 105 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/mediatek/mt6572.dtsi b/arch/arm/boot/dts/mediatek/mt6572.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..dd12231ca745be7455e99391abd2d708f2f1a8a9
> --- /dev/null
> +++ b/arch/arm/boot/dts/mediatek/mt6572.dtsi
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Max Shevchenko <wctrl@proton.me>
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	compatible = "mediatek,mt6572";
> +	interrupt-parent = <&sysirq>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		enable-method = "mediatek,mt6589-smp";
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0x0>;
> +		};
> +		cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0x1>;
> +		};
> +	};
> +
> +	system_clk: dummy13m {
> +		compatible = "fixed-clock";
> +		clock-frequency = <13000000>;
> +		#clock-cells = <0>;
> +	};
> +
> +	rtc_clk: dummy32k {
> +		compatible = "fixed-clock";
> +		clock-frequency = <32000>;
> +		#clock-cells = <0>;
> +	};
> +
> +	uart_clk: dummy26m {
> +		compatible = "fixed-clock";
> +		clock-frequency = <26000000>;
> +		#clock-cells = <0>;
> +	};
> +

Anything that has an MMIO address shall be child of "soc".

soc {
	watchdog@....

	timer@....

	etc.
};

> +	watchdog: watchdog@10007000 {
> +		compatible = "mediatek,mt6572-wdt",
> +			     "mediatek,mt6589-wdt";

those fit in one line:

compatible = "mediatek,mt6572-wdt", "mediatek,mt6589-wdt";

> +		reg = <0x10007000 0x100>;
> +		interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_LOW>;
> +		timeout-sec = <15>;
> +		#reset-cells = <1>;
> +	};
> +
> +	timer: timer@10008000 {
> +		compatible = "mediatek,mt6572-timer",
> +			     "mediatek,mt6577-timer";

same

> +		reg = <0x10008000 0x80>;
> +		interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&system_clk>, <&rtc_clk>;
> +		clock-names = "system-clk", "rtc-clk";
> +	};
> +
> +	sysirq: interrupt-controller@10200100 {
> +		compatible = "mediatek,mt6572-sysirq",
> +			     "mediatek,mt6577-sysirq";

same; and reg goes after compatible.

> +		interrupt-controller;
> +		#interrupt-cells = <3>;
> +		interrupt-parent = <&gic>;

are you sure that interrupt-parent is gic?

> +		reg = <0x10200100 0x1c>;
> +	};
> +
> +	gic: interrupt-controller@10211000 {
> +		compatible = "arm,cortex-a7-gic";

reg here.


> +		interrupt-controller;
> +		#interrupt-cells = <3>;
> +		interrupt-parent = <&gic>;

are you sure that the interrupt parent isn't sysirq here? :-)

> +		reg = <0x10211000 0x1000>,
> +		      <0x10212000 0x2000>,
> +		      <0x10214000 0x2000>,
> +		      <0x10216000 0x2000>;
> +	};
> +
> +	uart0: serial@11005000 {
> +		compatible = "mediatek,mt6572-uart",
> +			     "mediatek,mt6577-uart";

fits in one line

> +		reg = <0x11005000 0x400>;
> +		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&uart_clk>;

clock-names = .....

> +		status = "disabled";
> +	};
> +
> +	uart1: serial@11006000 {
> +		compatible = "mediatek,mt6572-uart",

...again.

> +			     "mediatek,mt6577-uart";
> +		reg = <0x11006000 0x400>;
> +		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&uart_clk>;
> +		status = "disabled";
> +	};
> +};
> 

Cheers,
Angelo