[PATCH v7 0/3] Add basic SoC support for mediatek mt7986

[PATCH v7 0/3] Add basic SoC support for mediatek mt7986

[Sam Shih]

This patch adds basic SoC support for Mediatek's new 4-core SoC,
MT7986, which is mainly for wifi-router application.

---
v7: added memory node back to dts
v6: separate basic part into a single patch series

According to the maintainer’s suggestion, this patch splits the previous
thread into independent patch series.
This patch include basic device support.

Original thread:
https://lore.kernel.org/all/20210914085137.31761-1-sam.shih@mediatek.com/
---

Sam Shih (3):
  dt-bindings: arm64: dts: mediatek: Add mt7986 series
  arm64: dts: mediatek: add basic mt7986a support
  arm64: dts: mediatek: add basic mt7986b support

 .../devicetree/bindings/arm/mediatek.yaml     |   8 +
 arch/arm64/boot/dts/mediatek/Makefile         |   2 +
 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts  |  38 +++++
 arch/arm64/boot/dts/mediatek/mt7986a.dtsi     | 149 ++++++++++++++++++
 arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts  |  30 ++++
 arch/arm64/boot/dts/mediatek/mt7986b.dtsi     | 149 ++++++++++++++++++
 6 files changed, 376 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986b.dtsi

--
2.29.2

[PATCH v7 1/3] dt-bindings: arm64: dts: mediatek: Add mt7986 series

[Sam Shih]

MT7986 series is Mediatek's new 4-core SoC, which is mainly
for wifi-router application. The difference between mt7986a and mt7986b
is that some pins do not exist on mt7986b.

Signed-off-by: Sam Shih <sam.shih@mediatek.com>
Acked-by: Rob Herring <robh@kernel.org>

---
v6: separate basic part into a single patch series

Original thread:
https://lore.kernel.org/all/315d7823aa108c909a3d36464fe54763b76ab2f4.camel@mediatek.com/

v3: changed 'MT7986' to 'MT7986 series' in the commit message
v2: added an Acked-by tag
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index 80a05f6fee85..a9a778269684 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -76,6 +76,14 @@ properties:
           - enum:
               - mediatek,mt7629-rfb
           - const: mediatek,mt7629
+      - items:
+          - enum:
+              - mediatek,mt7986a-rfb
+          - const: mediatek,mt7986a
+      - items:
+          - enum:
+              - mediatek,mt7986b-rfb
+          - const: mediatek,mt7986b
       - items:
           - enum:
               - mediatek,mt8127-moose
-- 
2.29.2

[PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Sam Shih]

Add basic chip support for Mediatek mt7986a, include
basic uart nodes, rng node and watchdog node.

Add cpu node, timer node, gic node, psci and reserved-memory node
for ARM Trusted Firmware.

Signed-off-by: Sam Shih <sam.shih@mediatek.com>

---
v7: added memory node back to dts
v6: removed clock and pinctrl node, to separate basic part into a single
    patch series

Original thread:
https://lore.kernel.org/all/20211004124155.1404-1-sam.shih@mediatek.com/

v5: follow reviewr's comment: removed clock freqency node in timer due to
    we have set CNTFRQ_EL0 in ATF firmware, and also corrected GICD range
v4: added missing gic register bases, and fixed range of GICR
v3: used the stdout-path instead of console=ttyS0
v2: modified clock and uart node due to clock driver updated
---
 arch/arm64/boot/dts/mediatek/Makefile        |   1 +
 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts |  38 +++++
 arch/arm64/boot/dts/mediatek/mt7986a.dtsi    | 149 +++++++++++++++++++
 3 files changed, 188 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 4f68ebed2e31..e6c3a73b9e4a 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
new file mode 100644
index 000000000000..ca074cf8e578
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2021 MediaTek Inc.
+ * Author: Sam.Shih <sam.shih@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt7986a.dtsi"
+
+/ {
+	model = "MediaTek MT7986a RFB";
+	compatible = "mediatek,mt7986a-rfb";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+		bootargs = "earlycon=uart8250,mmio32,0x11002000 swiotlb=512";
+	};
+
+	memory {
+		reg = <0 0x40000000 0 0x40000000>;
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&uart1 {
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a.dtsi b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
new file mode 100644
index 000000000000..75912bcf6c9c
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2021 MediaTek Inc.
+ * Author: Sam.Shih <sam.shih@mediatek.com>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "mediatek,mt7986a";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	system_clk: dummy40m {
+		compatible = "fixed-clock";
+		clock-frequency = <40000000>;
+		#clock-cells = <0>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x0>;
+			#cooling-cells = <2>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x1>;
+			#cooling-cells = <2>;
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x2>;
+			#cooling-cells = <2>;
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			enable-method = "psci";
+			compatible = "arm,cortex-a53";
+			reg = <0x3>;
+			#cooling-cells = <2>;
+		};
+	};
+
+	psci {
+		compatible  = "arm,psci-0.2";
+		method      = "smc";
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
+		secmon_reserved: secmon@43000000 {
+			reg = <0 0x43000000 0 0x30000>;
+			no-map;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		ranges;
+
+		gic: interrupt-controller@c000000 {
+			compatible = "arm,gic-v3";
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			reg = <0 0x0c000000 0 0x10000>,  /* GICD */
+			      <0 0x0c080000 0 0x80000>,  /* GICR */
+			      <0 0x0c400000 0 0x2000>,   /* GICC */
+			      <0 0x0c410000 0 0x1000>,   /* GICH */
+			      <0 0x0c420000 0 0x2000>;   /* GICV */
+			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		watchdog: watchdog@1001c000 {
+			compatible = "mediatek,mt7986-wdt",
+				     "mediatek,mt6589-wdt";
+			reg = <0 0x1001c000 0 0x1000>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+			#reset-cells = <1>;
+			status = "disabled";
+		};
+
+		trng: trng@1020f000 {
+			compatible = "mediatek,mt7986-rng",
+				     "mediatek,mt7623-rng";
+			reg = <0 0x1020f000 0 0x100>;
+			clocks = <&system_clk>;
+			clock-names = "rng";
+			status = "disabled";
+		};
+
+		uart0: serial@11002000 {
+			compatible = "mediatek,mt7986-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11002000 0 0x400>;
+			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&system_clk>;
+			status = "disabled";
+		};
+
+		uart1: serial@11003000 {
+			compatible = "mediatek,mt7986-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11003000 0 0x400>;
+			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&system_clk>;
+			status = "disabled";
+		};
+
+		uart2: serial@11004000 {
+			compatible = "mediatek,mt7986-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11004000 0 0x400>;
+			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&system_clk>;
+			status = "disabled";
+		};
+
+	};
+
+};
-- 
2.29.2

[PATCH v7 3/3] arm64: dts: mediatek: add basic mt7986b support

[Sam Shih]

Add basic chip support for Mediatek mt7986b, include
basic uart nodes, rng node and watchdog node.

Add cpu node, timer node, gic node, psci and reserved-memory node
for ARM Trusted Firmware.

Signed-off-by: Sam Shih <sam.shih@mediatek.com>

---
v7: added memory node back to dts
v6: separate basic part into a single patch series
v5: follow reviewr's comment: removed clock freqency node in timer due to
    we have set CNTFRQ_EL0 in ATF firmware, and also corrected GICD range
v4: added missing gic register bases, and fixed range of GICR
v3: used the stdout-path instead of console=ttyS0
v2: modified clock and uart node due to clock driver updated
---
 arch/arm64/boot/dts/mediatek/Makefile        |   1 +
 arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts |  30 ++++
 arch/arm64/boot/dts/mediatek/mt7986b.dtsi    | 149 +++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986b.dtsi

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index e6c3a73b9e4a..d555e43d1ccc 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986b-rfb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
new file mode 100644
index 000000000000..0f2f337bef4e
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2021 MediaTek Inc.
+ * Author: Sam.Shih <sam.shih@mediatek.com>
+ */
+
+/dts-v1/;
+#include "mt7986b.dtsi"
+
+/ {
+	model = "MediaTek MT7986b RFB";
+	compatible = "mediatek,mt7986b-rfb";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+		bootargs = "earlycon=uart8250,mmio32,0x11002000 swiotlb=512";
+	};
+
+	memory {
+		reg = <0 0x40000000 0 0x10000000>;
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/mediatek/mt7986b.dtsi b/arch/arm64/boot/dts/mediatek/mt7986b.dtsi
new file mode 100644
index 000000000000..2b8e0a382398
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986b.dtsi
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2021 MediaTek Inc.
+ * Author: Sam.Shih <sam.shih@mediatek.com>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	compatible = "mediatek,mt7986b";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	system_clk: dummy40m {
+		compatible = "fixed-clock";
+		clock-frequency = <40000000>;
+		#clock-cells = <0>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x0>;
+			#cooling-cells = <2>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x1>;
+			#cooling-cells = <2>;
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a53";
+			enable-method = "psci";
+			reg = <0x2>;
+			#cooling-cells = <2>;
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			enable-method = "psci";
+			compatible = "arm,cortex-a53";
+			reg = <0x3>;
+			#cooling-cells = <2>;
+		};
+	};
+
+	psci {
+		compatible  = "arm,psci-0.2";
+		method      = "smc";
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
+		secmon_reserved: secmon@43000000 {
+			reg = <0 0x43000000 0 0x30000>;
+			no-map;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupt-parent = <&gic>;
+		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		ranges;
+
+		gic: interrupt-controller@c000000 {
+			compatible = "arm,gic-v3";
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			reg = <0 0x0c000000 0 0x10000>,  /* GICD */
+			      <0 0x0c080000 0 0x80000>,  /* GICR */
+			      <0 0x0c400000 0 0x2000>,   /* GICC */
+			      <0 0x0c410000 0 0x1000>,   /* GICH */
+			      <0 0x0c420000 0 0x2000>;   /* GICV */
+			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		watchdog: watchdog@1001c000 {
+			compatible = "mediatek,mt7986-wdt",
+				     "mediatek,mt6589-wdt";
+			reg = <0 0x1001c000 0 0x1000>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+			#reset-cells = <1>;
+			status = "disabled";
+		};
+
+		trng: trng@1020f000 {
+			compatible = "mediatek,mt7986-rng",
+				     "mediatek,mt7623-rng";
+			reg = <0 0x1020f000 0 0x100>;
+			clocks = <&system_clk>;
+			clock-names = "rng";
+			status = "disabled";
+		};
+
+		uart0: serial@11002000 {
+			compatible = "mediatek,mt7986-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11002000 0 0x400>;
+			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&system_clk>;
+			status = "disabled";
+		};
+
+		uart1: serial@11003000 {
+			compatible = "mediatek,mt7986-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11003000 0 0x400>;
+			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&system_clk>;
+			status = "disabled";
+		};
+
+		uart2: serial@11004000 {
+			compatible = "mediatek,mt7986-uart",
+				     "mediatek,mt6577-uart";
+			reg = <0 0x11004000 0 0x400>;
+			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&system_clk>;
+			status = "disabled";
+		};
+
+	};
+
+};
-- 
2.29.2

Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Matthias Brugger]

Hi,

On 18/10/2021 13:40, Sam Shih wrote:
> Add basic chip support for Mediatek mt7986a, include
> basic uart nodes, rng node and watchdog node.
> 
> Add cpu node, timer node, gic node, psci and reserved-memory node
> for ARM Trusted Firmware.
> 

What is the exact difference between mt7986a and mt7986b? Right now, it's only 
the compatible, for that it makes no sense to split them.

It would be good to see what the exact differences are, so that we can see if it 
makes sense to have one of the alternatives:
1) use a common mt7986.dtsi which get included by mt7986[a,b].dtsi
2) Use on mt7986.dtsi and only add one mt7986a.dtsi or mt7986b.dtsi which has 
add-ons.

> Signed-off-by: Sam Shih <sam.shih@mediatek.com>
> 
> ---
> v7: added memory node back to dts
> v6: removed clock and pinctrl node, to separate basic part into a single
>      patch series
> 
> Original thread:
> https://lore.kernel.org/all/20211004124155.1404-1-sam.shih@mediatek.com/
> 
> v5: follow reviewr's comment: removed clock freqency node in timer due to
>      we have set CNTFRQ_EL0 in ATF firmware, and also corrected GICD range
> v4: added missing gic register bases, and fixed range of GICR
> v3: used the stdout-path instead of console=ttyS0
> v2: modified clock and uart node due to clock driver updated
> ---
>   arch/arm64/boot/dts/mediatek/Makefile        |   1 +
>   arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts |  38 +++++
>   arch/arm64/boot/dts/mediatek/mt7986a.dtsi    | 149 +++++++++++++++++++
>   3 files changed, 188 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 4f68ebed2e31..e6c3a73b9e4a 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> new file mode 100644
> index 000000000000..ca074cf8e578
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Author: Sam.Shih <sam.shih@mediatek.com>
> + */
> +
> +/dts-v1/;
> +#include "mt7986a.dtsi"
> +
> +/ {
> +	model = "MediaTek MT7986a RFB";
> +	compatible = "mediatek,mt7986a-rfb";
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +		bootargs = "earlycon=uart8250,mmio32,0x11002000 swiotlb=512";

We normally don't add earlycon parameter to the normal bootargs, as it's only 
for debugging. Also what do we need the swiotlb? Are there any limitation in the 
HW that makes us need it?

Regards,
Matthias

> +	};
> +
> +	memory {
> +		reg = <0 0x40000000 0 0x40000000>;
> +	};
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&uart1 {
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a.dtsi b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> new file mode 100644
> index 000000000000..75912bcf6c9c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Author: Sam.Shih <sam.shih@mediatek.com>
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> +	compatible = "mediatek,mt7986a";
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	system_clk: dummy40m {
> +		compatible = "fixed-clock";
> +		clock-frequency = <40000000>;
> +		#clock-cells = <0>;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			enable-method = "psci";
> +			reg = <0x0>;
> +			#cooling-cells = <2>;
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			enable-method = "psci";
> +			reg = <0x1>;
> +			#cooling-cells = <2>;
> +		};
> +
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a53";
> +			enable-method = "psci";
> +			reg = <0x2>;
> +			#cooling-cells = <2>;
> +		};
> +
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			enable-method = "psci";
> +			compatible = "arm,cortex-a53";
> +			reg = <0x3>;
> +			#cooling-cells = <2>;
> +		};
> +	};
> +
> +	psci {
> +		compatible  = "arm,psci-0.2";
> +		method      = "smc";
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
> +		secmon_reserved: secmon@43000000 {
> +			reg = <0 0x43000000 0 0x30000>;
> +			no-map;
> +		};
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupt-parent = <&gic>;
> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +
> +	soc {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		gic: interrupt-controller@c000000 {
> +			compatible = "arm,gic-v3";
> +			#interrupt-cells = <3>;
> +			interrupt-parent = <&gic>;
> +			interrupt-controller;
> +			reg = <0 0x0c000000 0 0x10000>,  /* GICD */
> +			      <0 0x0c080000 0 0x80000>,  /* GICR */
> +			      <0 0x0c400000 0 0x2000>,   /* GICC */
> +			      <0 0x0c410000 0 0x1000>,   /* GICH */
> +			      <0 0x0c420000 0 0x2000>;   /* GICV */
> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +
> +		watchdog: watchdog@1001c000 {
> +			compatible = "mediatek,mt7986-wdt",
> +				     "mediatek,mt6589-wdt";
> +			reg = <0 0x1001c000 0 0x1000>;
> +			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> +			#reset-cells = <1>;
> +			status = "disabled";
> +		};
> +
> +		trng: trng@1020f000 {
> +			compatible = "mediatek,mt7986-rng",
> +				     "mediatek,mt7623-rng";
> +			reg = <0 0x1020f000 0 0x100>;
> +			clocks = <&system_clk>;
> +			clock-names = "rng";
> +			status = "disabled";
> +		};
> +
> +		uart0: serial@11002000 {
> +			compatible = "mediatek,mt7986-uart",
> +				     "mediatek,mt6577-uart";
> +			reg = <0 0x11002000 0 0x400>;
> +			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&system_clk>;
> +			status = "disabled";
> +		};
> +
> +		uart1: serial@11003000 {
> +			compatible = "mediatek,mt7986-uart",
> +				     "mediatek,mt6577-uart";
> +			reg = <0 0x11003000 0 0x400>;
> +			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&system_clk>;
> +			status = "disabled";
> +		};
> +
> +		uart2: serial@11004000 {
> +			compatible = "mediatek,mt7986-uart",
> +				     "mediatek,mt6577-uart";
> +			reg = <0 0x11004000 0 0x400>;
> +			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&system_clk>;
> +			status = "disabled";
> +		};
> +
> +	};
> +
> +};
> 

Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Sam Shih]

Hi,

On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
> Hi,
> 
> On 18/10/2021 13:40, Sam Shih wrote:
> > Add basic chip support for Mediatek mt7986a, include
> > basic uart nodes, rng node and watchdog node.
> > 
> > Add cpu node, timer node, gic node, psci and reserved-memory node
> > for ARM Trusted Firmware.
> > 
> 
> What is the exact difference between mt7986a and mt7986b? Right now,
> it's only 
> the compatible, for that it makes no sense to split them.
> 

The difference between mt7986a and mt7986b is pinout which described
in our pinctrl patch series 
https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/

You are right, in this "basic SoC support" patch series, only show
compatible differences

> It would be good to see what the exact differences are, so that we
> can see if it 
> makes sense to have one of the alternatives:
> 1) use a common mt7986.dtsi which get included by mt7986[a,b].dtsi
> 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or mt7986b.dtsi
> which has 
> add-ons.
> 

In this case, can we use solution (1) to create a generic mt7986.dtsi
in this patch series, and add mt7986[a,b].dtsi to the dts part of the
pinctrl patch series to separate the difference nodes?

> > Signed-off-by: Sam Shih <sam.shih@mediatek.com>
> > 
> > ---
> > v7: added memory node back to dts
> > v6: removed clock and pinctrl node, to separate basic part into a
> > single
> >      patch series
> > 
> > Original thread:
> > 
https://urldefense.com/v3/__https://lore.kernel.org/all/20211004124155.1404-1-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!xCEW0lwTKx4k272sWASoi90y_yRyQdAx0oNJC-jSAIWnIEkprJD-gAc1ugCvo0ex$
> >  
> > 
> > v5: follow reviewr's comment: removed clock freqency node in timer
> > due to
> >      we have set CNTFRQ_EL0 in ATF firmware, and also corrected
> > GICD range
> > v4: added missing gic register bases, and fixed range of GICR
> > v3: used the stdout-path instead of console=ttyS0
> > v2: modified clock and uart node due to clock driver updated
> > ---
> >   arch/arm64/boot/dts/mediatek/Makefile        |   1 +
> >   arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts |  38 +++++
> >   arch/arm64/boot/dts/mediatek/mt7986a.dtsi    | 149
> > +++++++++++++++++++
> >   3 files changed, 188 insertions(+)
> >   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> >   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/Makefile
> > b/arch/arm64/boot/dts/mediatek/Makefile
> > index 4f68ebed2e31..e6c3a73b9e4a 100644
> > --- a/arch/arm64/boot/dts/mediatek/Makefile
> > +++ b/arch/arm64/boot/dts/mediatek/Makefile
> > @@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
> >   dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
> >   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> >   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
> >   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
> >   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
> >   dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
> > diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> > b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> > new file mode 100644
> > index 000000000000..ca074cf8e578
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> > @@ -0,0 +1,38 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (C) 2021 MediaTek Inc.
> > + * Author: Sam.Shih <sam.shih@mediatek.com>
> > + */
> > +
> > +/dts-v1/;
> > +#include "mt7986a.dtsi"
> > +
> > +/ {
> > +	model = "MediaTek MT7986a RFB";
> > +	compatible = "mediatek,mt7986a-rfb";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +		bootargs = "earlycon=uart8250,mmio32,0x11002000
> > swiotlb=512";
> 
> We normally don't add earlycon parameter to the normal bootargs, as
> it's only 
> for debugging. Also what do we need the swiotlb? Are there any
> limitation in the 
> HW that makes us need it?
> 

Thank you for your suggestion, as far as I know, it should not have
hardware limitations. This bootargs is just inherent from mt7622-
rfb.dts, I will delete it and test again on our development board.

> Regards,
> Matthias
> 
> > +	};
> > +
> > +	memory {
> > +		reg = <0 0x40000000 0 0x40000000>;
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	status = "okay";
> > +};
> > +
> > +&uart1 {
> > +	status = "okay";
> > +};
> > +
> > +&uart2 {
> > +	status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > new file mode 100644
> > index 000000000000..75912bcf6c9c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > @@ -0,0 +1,149 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (C) 2021 MediaTek Inc.
> > + * Author: Sam.Shih <sam.shih@mediatek.com>
> > + */
> > +
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +/ {
> > +	compatible = "mediatek,mt7986a";
> > +	interrupt-parent = <&gic>;
> > +	#address-cells = <2>;
> > +	#size-cells = <2>;
> > +
> > +	system_clk: dummy40m {
> > +		compatible = "fixed-clock";
> > +		clock-frequency = <40000000>;
> > +		#clock-cells = <0>;
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		cpu0: cpu@0 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			enable-method = "psci";
> > +			reg = <0x0>;
> > +			#cooling-cells = <2>;
> > +		};
> > +
> > +		cpu1: cpu@1 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			enable-method = "psci";
> > +			reg = <0x1>;
> > +			#cooling-cells = <2>;
> > +		};
> > +
> > +		cpu2: cpu@2 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			enable-method = "psci";
> > +			reg = <0x2>;
> > +			#cooling-cells = <2>;
> > +		};
> > +
> > +		cpu3: cpu@3 {
> > +			device_type = "cpu";
> > +			enable-method = "psci";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x3>;
> > +			#cooling-cells = <2>;
> > +		};
> > +	};
> > +
> > +	psci {
> > +		compatible  = "arm,psci-0.2";
> > +		method      = "smc";
> > +	};
> > +
> > +	reserved-memory {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
> > +		secmon_reserved: secmon@43000000 {
> > +			reg = <0 0x43000000 0 0x30000>;
> > +			no-map;
> > +		};
> > +	};
> > +
> > +	timer {
> > +		compatible = "arm,armv8-timer";
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> > +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> > +	};
> > +
> > +	soc {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		compatible = "simple-bus";
> > +		ranges;
> > +
> > +		gic: interrupt-controller@c000000 {
> > +			compatible = "arm,gic-v3";
> > +			#interrupt-cells = <3>;
> > +			interrupt-parent = <&gic>;
> > +			interrupt-controller;
> > +			reg = <0 0x0c000000 0 0x10000>,  /* GICD */
> > +			      <0 0x0c080000 0 0x80000>,  /* GICR */
> > +			      <0 0x0c400000 0 0x2000>,   /* GICC */
> > +			      <0 0x0c410000 0 0x1000>,   /* GICH */
> > +			      <0 0x0c420000 0 0x2000>;   /* GICV */
> > +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > +		};
> > +
> > +		watchdog: watchdog@1001c000 {
> > +			compatible = "mediatek,mt7986-wdt",
> > +				     "mediatek,mt6589-wdt";
> > +			reg = <0 0x1001c000 0 0x1000>;
> > +			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> > +			#reset-cells = <1>;
> > +			status = "disabled";
> > +		};
> > +
> > +		trng: trng@1020f000 {
> > +			compatible = "mediatek,mt7986-rng",
> > +				     "mediatek,mt7623-rng";
> > +			reg = <0 0x1020f000 0 0x100>;
> > +			clocks = <&system_clk>;
> > +			clock-names = "rng";
> > +			status = "disabled";
> > +		};
> > +
> > +		uart0: serial@11002000 {
> > +			compatible = "mediatek,mt7986-uart",
> > +				     "mediatek,mt6577-uart";
> > +			reg = <0 0x11002000 0 0x400>;
> > +			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&system_clk>;
> > +			status = "disabled";
> > +		};
> > +
> > +		uart1: serial@11003000 {
> > +			compatible = "mediatek,mt7986-uart",
> > +				     "mediatek,mt6577-uart";
> > +			reg = <0 0x11003000 0 0x400>;
> > +			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&system_clk>;
> > +			status = "disabled";
> > +		};
> > +
> > +		uart2: serial@11004000 {
> > +			compatible = "mediatek,mt7986-uart",
> > +				     "mediatek,mt6577-uart";
> > +			reg = <0 0x11004000 0 0x400>;
> > +			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&system_clk>;
> > +			status = "disabled";
> > +		};
> > +
> > +	};
> > +
> > +};
> > 

Regards,
Sam

Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Matthias Brugger]

On 16/11/2021 02:39, Sam Shih wrote:
> Hi,
> 
> On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
>> Hi,
>>
>> On 18/10/2021 13:40, Sam Shih wrote:
>>> Add basic chip support for Mediatek mt7986a, include
>>> basic uart nodes, rng node and watchdog node.
>>>
>>> Add cpu node, timer node, gic node, psci and reserved-memory node
>>> for ARM Trusted Firmware.
>>>
>>
>> What is the exact difference between mt7986a and mt7986b? Right now,
>> it's only
>> the compatible, for that it makes no sense to split them.
>>
> 
> The difference between mt7986a and mt7986b is pinout which described
> in our pinctrl patch series
> https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/
> 
> You are right, in this "basic SoC support" patch series, only show
> compatible differences
> 
>> It would be good to see what the exact differences are, so that we
>> can see if it
>> makes sense to have one of the alternatives:
>> 1) use a common mt7986.dtsi which get included by mt7986[a,b].dtsi
>> 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or mt7986b.dtsi
>> which has
>> add-ons.
>>
> 
> In this case, can we use solution (1) to create a generic mt7986.dtsi
> in this patch series, and add mt7986[a,b].dtsi to the dts part of the
> pinctrl patch series to separate the difference nodes?
> 

If the only difference is the GPIO controller then why not go with solution 2. 
Create a mt7986.dtsi which holds e.g. the node for pincontroller mt7986a and 
then create a mt7986b.dtsi that just changes compatible and gpio-ranges:

&pio {
    compatible = "mediatek,mt7986b-pinctrl";
    gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
}

What do you think?

Regards,
Matthias

>>> Signed-off-by: Sam Shih <sam.shih@mediatek.com>
>>>
>>> ---
>>> v7: added memory node back to dts
>>> v6: removed clock and pinctrl node, to separate basic part into a
>>> single
>>>       patch series
>>>
>>> Original thread:
>>>
> https://urldefense.com/v3/__https://lore.kernel.org/all/20211004124155.1404-1-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!xCEW0lwTKx4k272sWASoi90y_yRyQdAx0oNJC-jSAIWnIEkprJD-gAc1ugCvo0ex$
>>>   
>>>
>>> v5: follow reviewr's comment: removed clock freqency node in timer
>>> due to
>>>       we have set CNTFRQ_EL0 in ATF firmware, and also corrected
>>> GICD range
>>> v4: added missing gic register bases, and fixed range of GICR
>>> v3: used the stdout-path instead of console=ttyS0
>>> v2: modified clock and uart node due to clock driver updated
>>> ---
>>>    arch/arm64/boot/dts/mediatek/Makefile        |   1 +
>>>    arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts |  38 +++++
>>>    arch/arm64/boot/dts/mediatek/mt7986a.dtsi    | 149
>>> +++++++++++++++++++
>>>    3 files changed, 188 insertions(+)
>>>    create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>>    create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile
>>> b/arch/arm64/boot/dts/mediatek/Makefile
>>> index 4f68ebed2e31..e6c3a73b9e4a 100644
>>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>>> @@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> new file mode 100644
>>> index 000000000000..ca074cf8e578
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> @@ -0,0 +1,38 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2021 MediaTek Inc.
>>> + * Author: Sam.Shih <sam.shih@mediatek.com>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "mt7986a.dtsi"
>>> +
>>> +/ {
>>> +	model = "MediaTek MT7986a RFB";
>>> +	compatible = "mediatek,mt7986a-rfb";
>>> +
>>> +	aliases {
>>> +		serial0 = &uart0;
>>> +	};
>>> +
>>> +	chosen {
>>> +		stdout-path = "serial0:115200n8";
>>> +		bootargs = "earlycon=uart8250,mmio32,0x11002000
>>> swiotlb=512";
>>
>> We normally don't add earlycon parameter to the normal bootargs, as
>> it's only
>> for debugging. Also what do we need the swiotlb? Are there any
>> limitation in the
>> HW that makes us need it?
>>
> 
> Thank you for your suggestion, as far as I know, it should not have
> hardware limitations. This bootargs is just inherent from mt7622-
> rfb.dts, I will delete it and test again on our development board.
> 
>> Regards,
>> Matthias
>>
>>> +	};
>>> +
>>> +	memory {
>>> +		reg = <0 0x40000000 0 0x40000000>;
>>> +	};
>>> +};
>>> +
>>> +&uart0 {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&uart1 {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&uart2 {
>>> +	status = "okay";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>> new file mode 100644
>>> index 000000000000..75912bcf6c9c
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>> @@ -0,0 +1,149 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2021 MediaTek Inc.
>>> + * Author: Sam.Shih <sam.shih@mediatek.com>
>>> + */
>>> +
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/ {
>>> +	compatible = "mediatek,mt7986a";
>>> +	interrupt-parent = <&gic>;
>>> +	#address-cells = <2>;
>>> +	#size-cells = <2>;
>>> +
>>> +	system_clk: dummy40m {
>>> +		compatible = "fixed-clock";
>>> +		clock-frequency = <40000000>;
>>> +		#clock-cells = <0>;
>>> +	};
>>> +
>>> +	cpus {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +		cpu0: cpu@0 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a53";
>>> +			enable-method = "psci";
>>> +			reg = <0x0>;
>>> +			#cooling-cells = <2>;
>>> +		};
>>> +
>>> +		cpu1: cpu@1 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a53";
>>> +			enable-method = "psci";
>>> +			reg = <0x1>;
>>> +			#cooling-cells = <2>;
>>> +		};
>>> +
>>> +		cpu2: cpu@2 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a53";
>>> +			enable-method = "psci";
>>> +			reg = <0x2>;
>>> +			#cooling-cells = <2>;
>>> +		};
>>> +
>>> +		cpu3: cpu@3 {
>>> +			device_type = "cpu";
>>> +			enable-method = "psci";
>>> +			compatible = "arm,cortex-a53";
>>> +			reg = <0x3>;
>>> +			#cooling-cells = <2>;
>>> +		};
>>> +	};
>>> +
>>> +	psci {
>>> +		compatible  = "arm,psci-0.2";
>>> +		method      = "smc";
>>> +	};
>>> +
>>> +	reserved-memory {
>>> +		#address-cells = <2>;
>>> +		#size-cells = <2>;
>>> +		ranges;
>>> +		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
>>> +		secmon_reserved: secmon@43000000 {
>>> +			reg = <0 0x43000000 0 0x30000>;
>>> +			no-map;
>>> +		};
>>> +	};
>>> +
>>> +	timer {
>>> +		compatible = "arm,armv8-timer";
>>> +		interrupt-parent = <&gic>;
>>> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
>>> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>>> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>>> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
>>> +	};
>>> +
>>> +	soc {
>>> +		#address-cells = <2>;
>>> +		#size-cells = <2>;
>>> +		compatible = "simple-bus";
>>> +		ranges;
>>> +
>>> +		gic: interrupt-controller@c000000 {
>>> +			compatible = "arm,gic-v3";
>>> +			#interrupt-cells = <3>;
>>> +			interrupt-parent = <&gic>;
>>> +			interrupt-controller;
>>> +			reg = <0 0x0c000000 0 0x10000>,  /* GICD */
>>> +			      <0 0x0c080000 0 0x80000>,  /* GICR */
>>> +			      <0 0x0c400000 0 0x2000>,   /* GICC */
>>> +			      <0 0x0c410000 0 0x1000>,   /* GICH */
>>> +			      <0 0x0c420000 0 0x2000>;   /* GICV */
>>> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>>> +		};
>>> +
>>> +		watchdog: watchdog@1001c000 {
>>> +			compatible = "mediatek,mt7986-wdt",
>>> +				     "mediatek,mt6589-wdt";
>>> +			reg = <0 0x1001c000 0 0x1000>;
>>> +			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
>>> +			#reset-cells = <1>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		trng: trng@1020f000 {
>>> +			compatible = "mediatek,mt7986-rng",
>>> +				     "mediatek,mt7623-rng";
>>> +			reg = <0 0x1020f000 0 0x100>;
>>> +			clocks = <&system_clk>;
>>> +			clock-names = "rng";
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		uart0: serial@11002000 {
>>> +			compatible = "mediatek,mt7986-uart",
>>> +				     "mediatek,mt6577-uart";
>>> +			reg = <0 0x11002000 0 0x400>;
>>> +			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
>>> +			clocks = <&system_clk>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		uart1: serial@11003000 {
>>> +			compatible = "mediatek,mt7986-uart",
>>> +				     "mediatek,mt6577-uart";
>>> +			reg = <0 0x11003000 0 0x400>;
>>> +			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
>>> +			clocks = <&system_clk>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		uart2: serial@11004000 {
>>> +			compatible = "mediatek,mt7986-uart",
>>> +				     "mediatek,mt6577-uart";
>>> +			reg = <0 0x11004000 0 0x400>;
>>> +			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
>>> +			clocks = <&system_clk>;
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +	};
>>> +
>>> +};
>>>
> 
> Regards,
> Sam
> 

Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Sam Shih]

Hi

On Tue, 2021-11-16 at 12:18 +0100, Matthias Brugger wrote:
> 
> On 16/11/2021 02:39, Sam Shih wrote:
> > Hi,
> > 
> > On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
> > > Hi,
> > > 
> > > On 18/10/2021 13:40, Sam Shih wrote:
> > > > Add basic chip support for Mediatek mt7986a, include
> > > > basic uart nodes, rng node and watchdog node.
> > > > 
> > > > Add cpu node, timer node, gic node, psci and reserved-memory
> > > > node
> > > > for ARM Trusted Firmware.
> > > > 
> > > 
> > > What is the exact difference between mt7986a and mt7986b? Right
> > > now,
> > > it's only
> > > the compatible, for that it makes no sense to split them.
> > > 
> > 
> > The difference between mt7986a and mt7986b is pinout which
> > described
> > in our pinctrl patch series
> > 
https://urldefense.com/v3/__https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!0kseU8x1KnHHXDErh6Yj6MKqecufPEfGyeumtTBism47e99UFO2Gs-HfWjL1_jUv$
> >  
> > 
> > You are right, in this "basic SoC support" patch series, only show
> > compatible differences
> > 
> > > It would be good to see what the exact differences are, so that
> > > we
> > > can see if it
> > > makes sense to have one of the alternatives:
> > > 1) use a common mt7986.dtsi which get included by
> > > mt7986[a,b].dtsi
> > > 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or
> > > mt7986b.dtsi
> > > which has
> > > add-ons.
> > > 
> > 
> > In this case, can we use solution (1) to create a generic
> > mt7986.dtsi
> > in this patch series, and add mt7986[a,b].dtsi to the dts part of
> > the
> > pinctrl patch series to separate the difference nodes?
> > 
> 
> If the only difference is the GPIO controller then why not go with
> solution 2. 
> Create a mt7986.dtsi which holds e.g. the node for pincontroller
> mt7986a and 
> then create a mt7986b.dtsi that just changes compatible and gpio-
> ranges:
> 
> &pio {
>     compatible = "mediatek,mt7986b-pinctrl";
>     gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
> }
> 
> What do you think?

Ok,

For this basic patch series DTS, I will send the next version:
- Use "mt7986.dtsi" instead of "mt7986[a,b].dtsi",
  And make"mt7986.dtsi" get included by "mt7986[a,b]-rfb.dts" 
  (No dedicated uart1/uart2 pinout for mt7986b-rfb, status of dts node
shoud be set to "disabled")


For the pinctrl patch series DTS, I will send th next version:
- Add "mt7986b.dtsi" according to your suggestion,
  the new include
chain will be:
  mt7986a-rfb.dts <-- mt7986.dtsi (mt7986a pinctrl)
 
mt7986b-rfb.dts <-- mt7986b.dtsi (mt7986b pinctrl) <-- mt7986.dtsi
(mt7986a pinctrl)

Do you agree above proposal?

Regards,
Sam

> 
> Regards,
> Matthias
> 
> > > > Signed-off-by: Sam Shih <sam.shih@mediatek.com>
> > > > 
> > > > ---
> > > > v7: added memory node back to dts
> > > > v6: removed clock and pinctrl node, to separate basic part into
> > > > a
> > > > single
> > > >       patch series
> > > > 
> > > > Original thread:
> > > > 
> > 
> > 
https://urldefense.com/v3/__https://lore.kernel.org/all/20211004124155.1404-1-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!xCEW0lwTKx4k272sWASoi90y_yRyQdAx0oNJC-jSAIWnIEkprJD-gAc1ugCvo0ex$
> > > >   
> > > > 
> > > > v5: follow reviewr's comment: removed clock freqency node in
> > > > timer
> > > > due to
> > > >       we have set CNTFRQ_EL0 in ATF firmware, and also
> > > > corrected
> > > > GICD range
> > > > v4: added missing gic register bases, and fixed range of GICR
> > > > v3: used the stdout-path instead of console=ttyS0
> > > > v2: modified clock and uart node due to clock driver updated
> > > > ---
> > > >    arch/arm64/boot/dts/mediatek/Makefile        |   1 +
> > > >    arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts |  38 +++++
> > > >    arch/arm64/boot/dts/mediatek/mt7986a.dtsi    | 149
> > > > +++++++++++++++++++
> > > >    3 files changed, 188 insertions(+)
> > > >    create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-
> > > > rfb.dts
> > > >    create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/mediatek/Makefile
> > > > b/arch/arm64/boot/dts/mediatek/Makefile
> > > > index 4f68ebed2e31..e6c3a73b9e4a 100644
> > > > --- a/arch/arm64/boot/dts/mediatek/Makefile
> > > > +++ b/arch/arm64/boot/dts/mediatek/Makefile
> > > > @@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
> > > >    dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
> > > >    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> > > >    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> > > > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
> > > >    dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
> > > >    dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
> > > >    dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
> > > > diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> > > > b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> > > > new file mode 100644
> > > > index 000000000000..ca074cf8e578
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
> > > > @@ -0,0 +1,38 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > > > +/*
> > > > + * Copyright (C) 2021 MediaTek Inc.
> > > > + * Author: Sam.Shih <sam.shih@mediatek.com>
> > > > + */
> > > > +
> > > > +/dts-v1/;
> > > > +#include "mt7986a.dtsi"
> > > > +
> > > > +/ {
> > > > +	model = "MediaTek MT7986a RFB";
> > > > +	compatible = "mediatek,mt7986a-rfb";
> > > > +
> > > > +	aliases {
> > > > +		serial0 = &uart0;
> > > > +	};
> > > > +
> > > > +	chosen {
> > > > +		stdout-path = "serial0:115200n8";
> > > > +		bootargs = "earlycon=uart8250,mmio32,0x11002000
> > > > swiotlb=512";
> > > 
> > > We normally don't add earlycon parameter to the normal bootargs,
> > > as
> > > it's only
> > > for debugging. Also what do we need the swiotlb? Are there any
> > > limitation in the
> > > HW that makes us need it?
> > > 
> > 
> > Thank you for your suggestion, as far as I know, it should not have
> > hardware limitations. This bootargs is just inherent from mt7622-
> > rfb.dts, I will delete it and test again on our development board.
> > 
> > > Regards,
> > > Matthias
> > > 
> > > > +	};
> > > > +
> > > > +	memory {
> > > > +		reg = <0 0x40000000 0 0x40000000>;
> > > > +	};
> > > > +};
> > > > +
> > > > +&uart0 {
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +&uart1 {
> > > > +	status = "okay";
> > > > +};
> > > > +
> > > > +&uart2 {
> > > > +	status = "okay";
> > > > +};
> > > > diff --git a/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > > > b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > > > new file mode 100644
> > > > index 000000000000..75912bcf6c9c
> > > > --- /dev/null
> > > > +++ b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
> > > > @@ -0,0 +1,149 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > > > +/*
> > > > + * Copyright (C) 2021 MediaTek Inc.
> > > > + * Author: Sam.Shih <sam.shih@mediatek.com>
> > > > + */
> > > > +
> > > > +#include <dt-bindings/interrupt-controller/irq.h>
> > > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +
> > > > +/ {
> > > > +	compatible = "mediatek,mt7986a";
> > > > +	interrupt-parent = <&gic>;
> > > > +	#address-cells = <2>;
> > > > +	#size-cells = <2>;
> > > > +
> > > > +	system_clk: dummy40m {
> > > > +		compatible = "fixed-clock";
> > > > +		clock-frequency = <40000000>;
> > > > +		#clock-cells = <0>;
> > > > +	};
> > > > +
> > > > +	cpus {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +		cpu0: cpu@0 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			enable-method = "psci";
> > > > +			reg = <0x0>;
> > > > +			#cooling-cells = <2>;
> > > > +		};
> > > > +
> > > > +		cpu1: cpu@1 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			enable-method = "psci";
> > > > +			reg = <0x1>;
> > > > +			#cooling-cells = <2>;
> > > > +		};
> > > > +
> > > > +		cpu2: cpu@2 {
> > > > +			device_type = "cpu";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			enable-method = "psci";
> > > > +			reg = <0x2>;
> > > > +			#cooling-cells = <2>;
> > > > +		};
> > > > +
> > > > +		cpu3: cpu@3 {
> > > > +			device_type = "cpu";
> > > > +			enable-method = "psci";
> > > > +			compatible = "arm,cortex-a53";
> > > > +			reg = <0x3>;
> > > > +			#cooling-cells = <2>;
> > > > +		};
> > > > +	};
> > > > +
> > > > +	psci {
> > > > +		compatible  = "arm,psci-0.2";
> > > > +		method      = "smc";
> > > > +	};
> > > > +
> > > > +	reserved-memory {
> > > > +		#address-cells = <2>;
> > > > +		#size-cells = <2>;
> > > > +		ranges;
> > > > +		/* 192 KiB reserved for ARM Trusted Firmware
> > > > (BL31) */
> > > > +		secmon_reserved: secmon@43000000 {
> > > > +			reg = <0 0x43000000 0 0x30000>;
> > > > +			no-map;
> > > > +		};
> > > > +	};
> > > > +
> > > > +	timer {
> > > > +		compatible = "arm,armv8-timer";
> > > > +		interrupt-parent = <&gic>;
> > > > +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> > > > +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> > > > +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> > > > +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> > > > +	};
> > > > +
> > > > +	soc {
> > > > +		#address-cells = <2>;
> > > > +		#size-cells = <2>;
> > > > +		compatible = "simple-bus";
> > > > +		ranges;
> > > > +
> > > > +		gic: interrupt-controller@c000000 {
> > > > +			compatible = "arm,gic-v3";
> > > > +			#interrupt-cells = <3>;
> > > > +			interrupt-parent = <&gic>;
> > > > +			interrupt-controller;
> > > > +			reg = <0 0x0c000000 0 0x10000>,  /*
> > > > GICD */
> > > > +			      <0 0x0c080000 0 0x80000>,  /*
> > > > GICR */
> > > > +			      <0 0x0c400000 0 0x2000>,   /*
> > > > GICC */
> > > > +			      <0 0x0c410000 0 0x1000>,   /*
> > > > GICH */
> > > > +			      <0 0x0c420000 0 0x2000>;   /*
> > > > GICV */
> > > > +			interrupts = <GIC_PPI 9
> > > > IRQ_TYPE_LEVEL_HIGH>;
> > > > +		};
> > > > +
> > > > +		watchdog: watchdog@1001c000 {
> > > > +			compatible = "mediatek,mt7986-wdt",
> > > > +				     "mediatek,mt6589-wdt";
> > > > +			reg = <0 0x1001c000 0 0x1000>;
> > > > +			interrupts = <GIC_SPI 110
> > > > IRQ_TYPE_LEVEL_HIGH>;
> > > > +			#reset-cells = <1>;
> > > > +			status = "disabled";
> > > > +		};
> > > > +
> > > > +		trng: trng@1020f000 {
> > > > +			compatible = "mediatek,mt7986-rng",
> > > > +				     "mediatek,mt7623-rng";
> > > > +			reg = <0 0x1020f000 0 0x100>;
> > > > +			clocks = <&system_clk>;
> > > > +			clock-names = "rng";
> > > > +			status = "disabled";
> > > > +		};
> > > > +
> > > > +		uart0: serial@11002000 {
> > > > +			compatible = "mediatek,mt7986-uart",
> > > > +				     "mediatek,mt6577-uart";
> > > > +			reg = <0 0x11002000 0 0x400>;
> > > > +			interrupts = <GIC_SPI 123
> > > > IRQ_TYPE_LEVEL_HIGH>;
> > > > +			clocks = <&system_clk>;
> > > > +			status = "disabled";
> > > > +		};
> > > > +
> > > > +		uart1: serial@11003000 {
> > > > +			compatible = "mediatek,mt7986-uart",
> > > > +				     "mediatek,mt6577-uart";
> > > > +			reg = <0 0x11003000 0 0x400>;
> > > > +			interrupts = <GIC_SPI 124
> > > > IRQ_TYPE_LEVEL_HIGH>;
> > > > +			clocks = <&system_clk>;
> > > > +			status = "disabled";
> > > > +		};
> > > > +
> > > > +		uart2: serial@11004000 {
> > > > +			compatible = "mediatek,mt7986-uart",
> > > > +				     "mediatek,mt6577-uart";
> > > > +			reg = <0 0x11004000 0 0x400>;
> > > > +			interrupts = <GIC_SPI 125
> > > > IRQ_TYPE_LEVEL_HIGH>;
> > > > +			clocks = <&system_clk>;
> > > > +			status = "disabled";
> > > > +		};
> > > > +
> > > > +	};
> > > > +
> > > > +};
> > > > 
> > 
> > Regards,
> > Sam
> > 

Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Matthias Brugger]

	

On 18/11/2021 04:48, Sam Shih wrote:
> Hi
> 
> On Tue, 2021-11-16 at 12:18 +0100, Matthias Brugger wrote:
>>
>> On 16/11/2021 02:39, Sam Shih wrote:
>>> Hi,
>>>
>>> On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
>>>> Hi,
>>>>
>>>> On 18/10/2021 13:40, Sam Shih wrote:
>>>>> Add basic chip support for Mediatek mt7986a, include
>>>>> basic uart nodes, rng node and watchdog node.
>>>>>
>>>>> Add cpu node, timer node, gic node, psci and reserved-memory
>>>>> node
>>>>> for ARM Trusted Firmware.
>>>>>
>>>>
>>>> What is the exact difference between mt7986a and mt7986b? Right
>>>> now,
>>>> it's only
>>>> the compatible, for that it makes no sense to split them.
>>>>
>>>
>>> The difference between mt7986a and mt7986b is pinout which
>>> described
>>> in our pinctrl patch series
>>>
> https://urldefense.com/v3/__https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!0kseU8x1KnHHXDErh6Yj6MKqecufPEfGyeumtTBism47e99UFO2Gs-HfWjL1_jUv$
>>>   
>>>
>>> You are right, in this "basic SoC support" patch series, only show
>>> compatible differences
>>>
>>>> It would be good to see what the exact differences are, so that
>>>> we
>>>> can see if it
>>>> makes sense to have one of the alternatives:
>>>> 1) use a common mt7986.dtsi which get included by
>>>> mt7986[a,b].dtsi
>>>> 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or
>>>> mt7986b.dtsi
>>>> which has
>>>> add-ons.
>>>>
>>>
>>> In this case, can we use solution (1) to create a generic
>>> mt7986.dtsi
>>> in this patch series, and add mt7986[a,b].dtsi to the dts part of
>>> the
>>> pinctrl patch series to separate the difference nodes?
>>>
>>
>> If the only difference is the GPIO controller then why not go with
>> solution 2.
>> Create a mt7986.dtsi which holds e.g. the node for pincontroller
>> mt7986a and
>> then create a mt7986b.dtsi that just changes compatible and gpio-
>> ranges:
>>
>> &pio {
>>      compatible = "mediatek,mt7986b-pinctrl";
>>      gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
>> }
>>
>> What do you think?
> 
> Ok,
> 
> For this basic patch series DTS, I will send the next version:
> - Use "mt7986.dtsi" instead of "mt7986[a,b].dtsi",
>    And make"mt7986.dtsi" get included by "mt7986[a,b]-rfb.dts"
>    (No dedicated uart1/uart2 pinout for mt7986b-rfb, status of dts node
> shoud be set to "disabled")
> 
> 
> For the pinctrl patch series DTS, I will send th next version:
> - Add "mt7986b.dtsi" according to your suggestion,
>    the new include
> chain will be:
>    mt7986a-rfb.dts <-- mt7986.dtsi (mt7986a pinctrl)
>   
> mt7986b-rfb.dts <-- mt7986b.dtsi (mt7986b pinctrl) <-- mt7986.dtsi
> (mt7986a pinctrl)
> 
> Do you agree above proposal?
> 

I mean something like this:
mt7986a.dtsi:
pio: pinctrl@1001f000 {
	compatible = "mediatek,mt7986a-pinctrl";
	reg = <0 0x1001f000 0 0x1000>,
	      <0 0x11c30000 0 0x1000>,
	      <0 0x11c40000 0 0x1000>,
	      <0 0x11e20000 0 0x1000>,
	      <0 0x11e30000 0 0x1000>,
	      <0 0x11f00000 0 0x1000>,
	      <0 0x11f10000 0 0x1000>,
	      <0 0x1000b000 0 0x1000>;
	reg-names = "gpio", "iocfg_rt", "iocfg_rb", "iocfg_lt",
		    "iocfg_lb", "iocfg_tr", "iocfg_tl", "eint";
	gpio-controller;
	#gpio-cells = <2>;
	gpio-ranges = <&pio 0 0 100>;
	interrupt-controller;
	interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
	interrupt-parent = <&gic>;
	#interrupt-cells = <2>;
};

mt7986b.dtsi:
#include "mt7986a.dtsi"

&pio {
      compatible = "mediatek,mt7986b-pinctrl";
      gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
}

mt7986b-rfb.dts:
#include "mt7986b.dtsi"

&pio {
	uart1_pins: uart1-pins {
		mux { [...]


mt7986a-rfb.dts:
#include "mt7986a.dtsi"

&pio {
	uart1_pins: uart1-pins {
		mux { [...]


Makes sense?

Regards,
Matthias

Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support

[Sam Shih]

Hi,

On Fri, 2021-11-19 at 11:31 +0100, Matthias Brugger wrote:
> 	
> 
> On 18/11/2021 04:48, Sam Shih wrote:
> > Hi
> > 
> > On Tue, 2021-11-16 at 12:18 +0100, Matthias Brugger wrote:
> > > 
> > > On 16/11/2021 02:39, Sam Shih wrote:
> > > > Hi,
> > > > 
> > > > On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
> > > > > Hi,
> > > > > 
> > > > > On 18/10/2021 13:40, Sam Shih wrote:
> > > > > > Add basic chip support for Mediatek mt7986a, include
> > > > > > basic uart nodes, rng node and watchdog node.
> > > > > > 
> > > > > > Add cpu node, timer node, gic node, psci and reserved-
> > > > > > memory
> > > > > > node
> > > > > > for ARM Trusted Firmware.
> > > > > > 
> > > > > 
> > > > > What is the exact difference between mt7986a and mt7986b?
> > > > > Right
> > > > > now,
> > > > > it's only
> > > > > the compatible, for that it makes no sense to split them.
> > > > > 
> > > > 
> > > > The difference between mt7986a and mt7986b is pinout which
> > > > described
> > > > in our pinctrl patch series
> > > > 
> > 
> > 
https://urldefense.com/v3/__https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!0kseU8x1KnHHXDErh6Yj6MKqecufPEfGyeumtTBism47e99UFO2Gs-HfWjL1_jUv$
> > > >   
> > > > 
> > > > You are right, in this "basic SoC support" patch series, only
> > > > show
> > > > compatible differences
> > > > 
> > > > > It would be good to see what the exact differences are, so
> > > > > that
> > > > > we
> > > > > can see if it
> > > > > makes sense to have one of the alternatives:
> > > > > 1) use a common mt7986.dtsi which get included by
> > > > > mt7986[a,b].dtsi
> > > > > 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or
> > > > > mt7986b.dtsi
> > > > > which has
> > > > > add-ons.
> > > > > 
> > > > 
> > > > In this case, can we use solution (1) to create a generic
> > > > mt7986.dtsi
> > > > in this patch series, and add mt7986[a,b].dtsi to the dts part
> > > > of
> > > > the
> > > > pinctrl patch series to separate the difference nodes?
> > > > 
> > > 
> > > If the only difference is the GPIO controller then why not go
> > > with
> > > solution 2.
> > > Create a mt7986.dtsi which holds e.g. the node for pincontroller
> > > mt7986a and
> > > then create a mt7986b.dtsi that just changes compatible and gpio-
> > > ranges:
> > > 
> > > &pio {
> > >      compatible = "mediatek,mt7986b-pinctrl";
> > >      gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
> > > }
> > > 
> > > What do you think?
> > 
> > Ok,
> > 
> > For this basic patch series DTS, I will send the next version:
> > - Use "mt7986.dtsi" instead of "mt7986[a,b].dtsi",
> >    And make"mt7986.dtsi" get included by "mt7986[a,b]-rfb.dts"
> >    (No dedicated uart1/uart2 pinout for mt7986b-rfb, status of dts
> > node
> > shoud be set to "disabled")
> > 
> > 
> > For the pinctrl patch series DTS, I will send th next version:
> > - Add "mt7986b.dtsi" according to your suggestion,
> >    the new include
> > chain will be:
> >    mt7986a-rfb.dts <-- mt7986.dtsi (mt7986a pinctrl)
> >   
> > mt7986b-rfb.dts <-- mt7986b.dtsi (mt7986b pinctrl) <-- mt7986.dtsi
> > (mt7986a pinctrl)
> > 
> > Do you agree above proposal?
> > 
> 
> I mean something like this:
> mt7986a.dtsi:
> pio: pinctrl@1001f000 {
> 	compatible = "mediatek,mt7986a-pinctrl";
> 	reg = <0 0x1001f000 0 0x1000>,
> 	      <0 0x11c30000 0 0x1000>,
> 	      <0 0x11c40000 0 0x1000>,
> 	      <0 0x11e20000 0 0x1000>,
> 	      <0 0x11e30000 0 0x1000>,
> 	      <0 0x11f00000 0 0x1000>,
> 	      <0 0x11f10000 0 0x1000>,
> 	      <0 0x1000b000 0 0x1000>;
> 	reg-names = "gpio", "iocfg_rt", "iocfg_rb", "iocfg_lt",
> 		    "iocfg_lb", "iocfg_tr", "iocfg_tl", "eint";
> 	gpio-controller;
> 	#gpio-cells = <2>;
> 	gpio-ranges = <&pio 0 0 100>;
> 	interrupt-controller;
> 	interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
> 	interrupt-parent = <&gic>;
> 	#interrupt-cells = <2>;
> };
> 
> mt7986b.dtsi:
> #include "mt7986a.dtsi"
> 
> &pio {
>       compatible = "mediatek,mt7986b-pinctrl";
>       gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
> }
> 
> mt7986b-rfb.dts:
> #include "mt7986b.dtsi"
> 
> &pio {
> 	uart1_pins: uart1-pins {
> 		mux { [...]
> 
> 
> mt7986a-rfb.dts:
> #include "mt7986a.dtsi"
> 
> &pio {
> 	uart1_pins: uart1-pins {
> 		mux { [...]
> 
> 
> Makes sense?
> 

Okay,

I have sent new patch set based on your suggestion,

Basic Part:

https://lore.kernel.org/all/20211122123222.8016-1-sam.shih@mediatek.com/

Pinctrl:

https://lore.kernel.org/all/20211122123552.8218-1-sam.shih@mediatek.com/

Please take a look at those patches when you are free.

Regards,
Sam

> Regards,
> Matthias
>