2018-09-05 14:12:10

by Ryder Lee

[permalink] [raw]
Subject: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

Add display subsystem related device nodes for MT7623.

Cc: CK Hu <[email protected]>
Signed-off-by: chunhui dai <[email protected]>
Signed-off-by: Bibby Hsieh <[email protected]>
Signed-off-by: Ryder Lee <[email protected]>
---
I forgot to sort nodes in my previous mail. Sorry for the inconvenience.

This patch depends on the series: https://lkml.org/lkml/2018/9/5/223

@Matthias,
I know you're working on broken MMSYS - just want to ask whether it's possible
to let the patch to go to your tree (if others are okay with it), and send a
fixup one for MT7623 MMSYS later?
---
arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
3 files changed, 343 insertions(+)

diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
index d01bdee..fdf9078 100644
--- a/arch/arm/boot/dts/mt7623.dtsi
+++ b/arch/arm/boot/dts/mt7623.dtsi
@@ -23,6 +23,11 @@
#address-cells = <2>;
#size-cells = <2>;

+ aliases {
+ rdma0 = &rdma0;
+ rdma1 = &rdma1;
+ };
+
cpu_opp_table: opp-table {
compatible = "operating-points-v2";
opp-shared;
@@ -311,6 +316,25 @@
clock-names = "spi", "wrap";
};

+ mipi_tx0: mipi-dphy@10010000 {
+ compatible = "mediatek,mt7623-mipi-tx",
+ "mediatek,mt2701-mipi-tx";
+ reg = <0 0x10010000 0 0x90>;
+ clocks = <&clk26m>;
+ clock-output-names = "mipi_tx0_pll";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ };
+
+ cec: cec@10012000 {
+ compatible = "mediatek,mt7623-cec",
+ "mediatek,mt8173-cec";
+ reg = <0 0x10012000 0 0xbc>;
+ interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&infracfg CLK_INFRA_CEC>;
+ status = "disabled";
+ };
+
cir: cir@10013000 {
compatible = "mediatek,mt7623-cir";
reg = <0 0x10013000 0 0x1000>;
@@ -359,6 +383,18 @@
#clock-cells = <1>;
};

+ hdmi_phy: phy@10209100 {
+ compatible = "mediatek,mt7623-hdmi-phy",
+ "mediatek,mt2701-hdmi-phy";
+ reg = <0 0x10209100 0 0x24>;
+ clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
+ clock-names = "pll_ref";
+ clock-output-names = "hdmitx_dig_cts";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
rng: rng@1020f000 {
compatible = "mediatek,mt7623-rng";
reg = <0 0x1020f000 0 0x1000>;
@@ -558,6 +594,16 @@
status = "disabled";
};

+ hdmiddc0: i2c@11013000 {
+ compatible = "mediatek,mt7623-hdmi-ddc",
+ "mediatek,mt8173-hdmi-ddc";
+ interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
+ reg = <0 0x11013000 0 0x1C>;
+ clocks = <&pericfg CLK_PERI_I2C3>;
+ clock-names = "ddc-i2c";
+ status = "disabled";
+ };
+
nor_flash: spi@11014000 {
compatible = "mediatek,mt7623-nor",
"mediatek,mt8173-nor";
@@ -732,6 +778,84 @@
#clock-cells = <1>;
};

+ display_components: dispsys@14000000 {
+ compatible = "mediatek,mt7623-mmsys",
+ "mediatek,mt2701-mmsys";
+ reg = <0 0x14000000 0 0x1000>;
+ power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
+ };
+
+ ovl@14007000 {
+ compatible = "mediatek,mt7623-disp-ovl",
+ "mediatek,mt2701-disp-ovl";
+ reg = <0 0x14007000 0 0x1000>;
+ interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_OVL>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
+ mediatek,larb = <&larb0>;
+ };
+
+ rdma0: rdma@14008000 {
+ compatible = "mediatek,mt7623-disp-rdma",
+ "mediatek,mt2701-disp-rdma";
+ reg = <0 0x14008000 0 0x1000>;
+ interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_RDMA>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
+ mediatek,larb = <&larb0>;
+ };
+
+ wdma@14009000 {
+ compatible = "mediatek,mt7623-disp-wdma",
+ "mediatek,mt2701-disp-wdma";
+ reg = <0 0x14009000 0 0x1000>;
+ interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_WDMA>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
+ mediatek,larb = <&larb0>;
+ };
+
+ bls: pwm@1400a000 {
+ compatible = "mediatek,mt7623-disp-pwm",
+ "mediatek,mt2701-disp-pwm";
+ reg = <0 0x1400a000 0 0x1000>;
+ #pwm-cells = <2>;
+ clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
+ <&mmsys CLK_MM_DISP_BLS>;
+ clock-names = "main", "mm";
+ status = "disabled";
+ };
+
+ color@1400b000 {
+ compatible = "mediatek,mt7623-disp-color",
+ "mediatek,mt2701-disp-color";
+ reg = <0 0x1400b000 0 0x1000>;
+ interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_COLOR>;
+ };
+
+ dsi: dsi@1400c000 {
+ compatible = "mediatek,mt7623-dsi",
+ "mediatek,mt2701-dsi";
+ reg = <0 0x1400c000 0 0x1000>;
+ interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DSI_ENGINE>,
+ <&mmsys CLK_MM_DSI_DIG>,
+ <&mipi_tx0>;
+ clock-names = "engine", "digital", "hs";
+ phys = <&mipi_tx0>;
+ phy-names = "dphy";
+ status = "disabled";
+ };
+
+ mutex: mutex@1400e000 {
+ compatible = "mediatek,mt7623-disp-mutex",
+ "mediatek,mt2701-disp-mutex";
+ reg = <0 0x1400e000 0 0x1000>;
+ interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_MUTEX_32K>;
+ };
+
larb0: larb@14010000 {
compatible = "mediatek,mt7623-smi-larb",
"mediatek,mt2701-smi-larb";
@@ -744,6 +868,44 @@
power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
};

+ rdma1: rdma@14012000 {
+ compatible = "mediatek,mt7623-disp-rdma",
+ "mediatek,mt2701-disp-rdma";
+ reg = <0 0x14012000 0 0x1000>;
+ interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DISP_RDMA1>;
+ iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
+ mediatek,larb = <&larb0>;
+ };
+
+ dpi0: dpi@14014000 {
+ compatible = "mediatek,mt7623-dpi",
+ "mediatek,mt2701-dpi";
+ reg = <0 0x14014000 0 0x1000>;
+ interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&mmsys CLK_MM_DPI1_DIGL>,
+ <&mmsys CLK_MM_DPI1_ENGINE>,
+ <&topckgen CLK_TOP_TVDPLL>;
+ clock-names = "pixel", "engine", "pll";
+ status = "disabled";
+ };
+
+ hdmi0: hdmi@14015000 {
+ compatible = "mediatek,mt7623-hdmi",
+ "mediatek,mt8173-hdmi";
+ reg = <0 0x14015000 0 0x400>;
+ clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
+ <&mmsys CLK_MM_HDMI_PLL>,
+ <&mmsys CLK_MM_HDMI_AUDIO>,
+ <&mmsys CLK_MM_HDMI_SPDIF>;
+ clock-names = "pixel", "pll", "bclk", "spdif";
+ phys = <&hdmi_phy>;
+ phy-names = "hdmi";
+ mediatek,syscon-hdmi = <&mmsys 0x900>;
+ cec = <&cec>;
+ status = "disabled";
+ };
+
imgsys: syscon@15000000 {
compatible = "mediatek,mt7623-imgsys",
"mediatek,mt2701-imgsys",
@@ -1071,6 +1233,21 @@
};
};

+ hdmi_pins_a: hdmi-default {
+ pins-hdmi {
+ pinmux = <MT7623_PIN_123_HTPLG_FUNC_HTPLG>;
+ input-enable;
+ bias-pull-down;
+ };
+ };
+
+ hdmi_ddc_pins_a: hdmi_ddc-default {
+ pins-hdmi-ddc {
+ pinmux = <MT7623_PIN_124_GPIO124_FUNC_HDMISCK>,
+ <MT7623_PIN_125_GPIO125_FUNC_HDMISD>;
+ };
+ };
+
i2c0_pins_a: i2c0-default {
pins-i2c0 {
pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
index 2b760f9..dbce86b 100644
--- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
+++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
@@ -21,6 +21,19 @@
stdout-path = "serial2:115200n8";
};

+ connector {
+ compatible = "hdmi-connector";
+ label = "hdmi";
+ type = "d";
+ ddc-i2c-bus = <&hdmiddc0>;
+
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&hdmi0_out>;
+ };
+ };
+ };
+
cpus {
cpu@0 {
proc-supply = <&mt6323_vproc_reg>;
@@ -114,10 +127,24 @@
};
};

+&bls {
+ status = "okay";
+
+ port {
+ bls_out: endpoint {
+ remote-endpoint = <&dpi0_in>;
+ };
+ };
+};
+
&btif {
status = "okay";
};

+&cec {
+ status = "okay";
+};
+
&cir {
pinctrl-names = "default";
pinctrl-0 = <&cir_pins_a>;
@@ -128,6 +155,26 @@
status = "okay";
};

+&dpi0 {
+ status = "okay";
+
+ ports {
+ port@0 {
+ reg = <0>;
+ dpi0_out: endpoint {
+ remote-endpoint = <&hdmi0_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dpi0_in: endpoint {
+ remote-endpoint = <&bls_out>;
+ };
+ };
+ };
+};
+
&eth {
status = "okay";

@@ -199,6 +246,42 @@
};
};

+&hdmi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_pins_a>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ hdmi0_in: endpoint {
+ remote-endpoint = <&dpi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ hdmi0_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+ };
+ };
+};
+
+&hdmiddc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_ddc_pins_a>;
+ status = "okay";
+};
+
+&hdmi_phy {
+ mediatek,ibias = <0xa>;
+ mediatek,ibias_up = <0x1c>;
+ status = "okay";
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c0_pins_a>;
diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
index b760613..bdf9010 100644
--- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
+++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
@@ -24,6 +24,19 @@
stdout-path = "serial2:115200n8";
};

+ connector {
+ compatible = "hdmi-connector";
+ label = "hdmi";
+ type = "d";
+ ddc-i2c-bus = <&hdmiddc0>;
+
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&hdmi0_out>;
+ };
+ };
+ };
+
cpus {
cpu@0 {
proc-supply = <&mt6323_vproc_reg>;
@@ -106,10 +119,24 @@
};
};

+&bls {
+ status = "okay";
+
+ port {
+ bls_out: endpoint {
+ remote-endpoint = <&dpi0_in>;
+ };
+ };
+};
+
&btif {
status = "okay";
};

+&cec {
+ status = "okay";
+};
+
&cir {
pinctrl-names = "default";
pinctrl-0 = <&cir_pins_a>;
@@ -120,6 +147,26 @@
status = "okay";
};

+&dpi0 {
+ status = "okay";
+
+ ports {
+ port@0 {
+ reg = <0>;
+ dpi0_out: endpoint {
+ remote-endpoint = <&hdmi0_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dpi0_in: endpoint {
+ remote-endpoint = <&bls_out>;
+ };
+ };
+ };
+};
+
&eth {
status = "okay";

@@ -202,6 +249,42 @@
};
};

+&hdmi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_pins_a>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ hdmi0_in: endpoint {
+ remote-endpoint = <&dpi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ hdmi0_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+ };
+ };
+};
+
+&hdmiddc0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&hdmi_ddc_pins_a>;
+ status = "okay";
+};
+
+&hdmi_phy {
+ mediatek,ibias = <0xa>;
+ mediatek,ibias_up = <0x1c>;
+ status = "okay";
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c0_pins_a>;
--
1.9.1



2018-09-25 15:49:06

by Matthias Brugger

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes



On 05/09/2018 16:09, Ryder Lee wrote:
> Add display subsystem related device nodes for MT7623.
>
> Cc: CK Hu <[email protected]>
> Signed-off-by: chunhui dai <[email protected]>
> Signed-off-by: Bibby Hsieh <[email protected]>
> Signed-off-by: Ryder Lee <[email protected]>
> ---
> I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
>
> This patch depends on the series: https://lkml.org/lkml/2018/9/5/223

That's not yet merged, right?

>
> @Matthias,
> I know you're working on broken MMSYS - just want to ask whether it's possible
> to let the patch to go to your tree (if others are okay with it), and send a
> fixup one for MT7623 MMSYS later?

I know I'm a big blocker here, sorry for the inconvenience.
I'll do my best to get that in for the next release. If I make no progress on
that then we should find a different solution.

Regards,
Matthias

> ---
> arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> 3 files changed, 343 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> index d01bdee..fdf9078 100644
> --- a/arch/arm/boot/dts/mt7623.dtsi
> +++ b/arch/arm/boot/dts/mt7623.dtsi
> @@ -23,6 +23,11 @@
> #address-cells = <2>;
> #size-cells = <2>;
>
> + aliases {
> + rdma0 = &rdma0;
> + rdma1 = &rdma1;
> + };
> +
> cpu_opp_table: opp-table {
> compatible = "operating-points-v2";
> opp-shared;
> @@ -311,6 +316,25 @@
> clock-names = "spi", "wrap";
> };
>
> + mipi_tx0: mipi-dphy@10010000 {
> + compatible = "mediatek,mt7623-mipi-tx",
> + "mediatek,mt2701-mipi-tx";
> + reg = <0 0x10010000 0 0x90>;
> + clocks = <&clk26m>;
> + clock-output-names = "mipi_tx0_pll";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + };
> +
> + cec: cec@10012000 {
> + compatible = "mediatek,mt7623-cec",
> + "mediatek,mt8173-cec";
> + reg = <0 0x10012000 0 0xbc>;
> + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&infracfg CLK_INFRA_CEC>;
> + status = "disabled";
> + };
> +
> cir: cir@10013000 {
> compatible = "mediatek,mt7623-cir";
> reg = <0 0x10013000 0 0x1000>;
> @@ -359,6 +383,18 @@
> #clock-cells = <1>;
> };
>
> + hdmi_phy: phy@10209100 {
> + compatible = "mediatek,mt7623-hdmi-phy",
> + "mediatek,mt2701-hdmi-phy";
> + reg = <0 0x10209100 0 0x24>;
> + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> + clock-names = "pll_ref";
> + clock-output-names = "hdmitx_dig_cts";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> rng: rng@1020f000 {
> compatible = "mediatek,mt7623-rng";
> reg = <0 0x1020f000 0 0x1000>;
> @@ -558,6 +594,16 @@
> status = "disabled";
> };
>
> + hdmiddc0: i2c@11013000 {
> + compatible = "mediatek,mt7623-hdmi-ddc",
> + "mediatek,mt8173-hdmi-ddc";
> + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> + reg = <0 0x11013000 0 0x1C>;
> + clocks = <&pericfg CLK_PERI_I2C3>;
> + clock-names = "ddc-i2c";
> + status = "disabled";
> + };
> +
> nor_flash: spi@11014000 {
> compatible = "mediatek,mt7623-nor",
> "mediatek,mt8173-nor";
> @@ -732,6 +778,84 @@
> #clock-cells = <1>;
> };
>
> + display_components: dispsys@14000000 {
> + compatible = "mediatek,mt7623-mmsys",
> + "mediatek,mt2701-mmsys";
> + reg = <0 0x14000000 0 0x1000>;
> + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> + };
> +
> + ovl@14007000 {
> + compatible = "mediatek,mt7623-disp-ovl",
> + "mediatek,mt2701-disp-ovl";
> + reg = <0 0x14007000 0 0x1000>;
> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_OVL>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + rdma0: rdma@14008000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14008000 0 0x1000>;
> + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + wdma@14009000 {
> + compatible = "mediatek,mt7623-disp-wdma",
> + "mediatek,mt2701-disp-wdma";
> + reg = <0 0x14009000 0 0x1000>;
> + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + bls: pwm@1400a000 {
> + compatible = "mediatek,mt7623-disp-pwm",
> + "mediatek,mt2701-disp-pwm";
> + reg = <0 0x1400a000 0 0x1000>;
> + #pwm-cells = <2>;
> + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> + <&mmsys CLK_MM_DISP_BLS>;
> + clock-names = "main", "mm";
> + status = "disabled";
> + };
> +
> + color@1400b000 {
> + compatible = "mediatek,mt7623-disp-color",
> + "mediatek,mt2701-disp-color";
> + reg = <0 0x1400b000 0 0x1000>;
> + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> + };
> +
> + dsi: dsi@1400c000 {
> + compatible = "mediatek,mt7623-dsi",
> + "mediatek,mt2701-dsi";
> + reg = <0 0x1400c000 0 0x1000>;
> + interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DSI_ENGINE>,
> + <&mmsys CLK_MM_DSI_DIG>,
> + <&mipi_tx0>;
> + clock-names = "engine", "digital", "hs";
> + phys = <&mipi_tx0>;
> + phy-names = "dphy";
> + status = "disabled";
> + };
> +
> + mutex: mutex@1400e000 {
> + compatible = "mediatek,mt7623-disp-mutex",
> + "mediatek,mt2701-disp-mutex";
> + reg = <0 0x1400e000 0 0x1000>;
> + interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_MUTEX_32K>;
> + };
> +
> larb0: larb@14010000 {
> compatible = "mediatek,mt7623-smi-larb",
> "mediatek,mt2701-smi-larb";
> @@ -744,6 +868,44 @@
> power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> };
>
> + rdma1: rdma@14012000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14012000 0 0x1000>;
> + interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA1>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + dpi0: dpi@14014000 {
> + compatible = "mediatek,mt7623-dpi",
> + "mediatek,mt2701-dpi";
> + reg = <0 0x14014000 0 0x1000>;
> + interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DPI1_DIGL>,
> + <&mmsys CLK_MM_DPI1_ENGINE>,
> + <&topckgen CLK_TOP_TVDPLL>;
> + clock-names = "pixel", "engine", "pll";
> + status = "disabled";
> + };
> +
> + hdmi0: hdmi@14015000 {
> + compatible = "mediatek,mt7623-hdmi",
> + "mediatek,mt8173-hdmi";
> + reg = <0 0x14015000 0 0x400>;
> + clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
> + <&mmsys CLK_MM_HDMI_PLL>,
> + <&mmsys CLK_MM_HDMI_AUDIO>,
> + <&mmsys CLK_MM_HDMI_SPDIF>;
> + clock-names = "pixel", "pll", "bclk", "spdif";
> + phys = <&hdmi_phy>;
> + phy-names = "hdmi";
> + mediatek,syscon-hdmi = <&mmsys 0x900>;
> + cec = <&cec>;
> + status = "disabled";
> + };
> +
> imgsys: syscon@15000000 {
> compatible = "mediatek,mt7623-imgsys",
> "mediatek,mt2701-imgsys",
> @@ -1071,6 +1233,21 @@
> };
> };
>
> + hdmi_pins_a: hdmi-default {
> + pins-hdmi {
> + pinmux = <MT7623_PIN_123_HTPLG_FUNC_HTPLG>;
> + input-enable;
> + bias-pull-down;
> + };
> + };
> +
> + hdmi_ddc_pins_a: hdmi_ddc-default {
> + pins-hdmi-ddc {
> + pinmux = <MT7623_PIN_124_GPIO124_FUNC_HDMISCK>,
> + <MT7623_PIN_125_GPIO125_FUNC_HDMISD>;
> + };
> + };
> +
> i2c0_pins_a: i2c0-default {
> pins-i2c0 {
> pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
> diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> index 2b760f9..dbce86b 100644
> --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> @@ -21,6 +21,19 @@
> stdout-path = "serial2:115200n8";
> };
>
> + connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "d";
> + ddc-i2c-bus = <&hdmiddc0>;
> +
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&hdmi0_out>;
> + };
> + };
> + };
> +
> cpus {
> cpu@0 {
> proc-supply = <&mt6323_vproc_reg>;
> @@ -114,10 +127,24 @@
> };
> };
>
> +&bls {
> + status = "okay";
> +
> + port {
> + bls_out: endpoint {
> + remote-endpoint = <&dpi0_in>;
> + };
> + };
> +};
> +
> &btif {
> status = "okay";
> };
>
> +&cec {
> + status = "okay";
> +};
> +
> &cir {
> pinctrl-names = "default";
> pinctrl-0 = <&cir_pins_a>;
> @@ -128,6 +155,26 @@
> status = "okay";
> };
>
> +&dpi0 {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + dpi0_out: endpoint {
> + remote-endpoint = <&hdmi0_in>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dpi0_in: endpoint {
> + remote-endpoint = <&bls_out>;
> + };
> + };
> + };
> +};
> +
> &eth {
> status = "okay";
>
> @@ -199,6 +246,42 @@
> };
> };
>
> +&hdmi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_pins_a>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + hdmi0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi0_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> +};
> +
> +&hdmiddc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_ddc_pins_a>;
> + status = "okay";
> +};
> +
> +&hdmi_phy {
> + mediatek,ibias = <0xa>;
> + mediatek,ibias_up = <0x1c>;
> + status = "okay";
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> index b760613..bdf9010 100644
> --- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> +++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> @@ -24,6 +24,19 @@
> stdout-path = "serial2:115200n8";
> };
>
> + connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "d";
> + ddc-i2c-bus = <&hdmiddc0>;
> +
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&hdmi0_out>;
> + };
> + };
> + };
> +
> cpus {
> cpu@0 {
> proc-supply = <&mt6323_vproc_reg>;
> @@ -106,10 +119,24 @@
> };
> };
>
> +&bls {
> + status = "okay";
> +
> + port {
> + bls_out: endpoint {
> + remote-endpoint = <&dpi0_in>;
> + };
> + };
> +};
> +
> &btif {
> status = "okay";
> };
>
> +&cec {
> + status = "okay";
> +};
> +
> &cir {
> pinctrl-names = "default";
> pinctrl-0 = <&cir_pins_a>;
> @@ -120,6 +147,26 @@
> status = "okay";
> };
>
> +&dpi0 {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + dpi0_out: endpoint {
> + remote-endpoint = <&hdmi0_in>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dpi0_in: endpoint {
> + remote-endpoint = <&bls_out>;
> + };
> + };
> + };
> +};
> +
> &eth {
> status = "okay";
>
> @@ -202,6 +249,42 @@
> };
> };
>
> +&hdmi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_pins_a>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + hdmi0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi0_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> +};
> +
> +&hdmiddc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_ddc_pins_a>;
> + status = "okay";
> +};
> +
> +&hdmi_phy {
> + mediatek,ibias = <0xa>;
> + mediatek,ibias_up = <0x1c>;
> + status = "okay";
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
>

2018-09-26 01:38:36

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

Hi, Ryder:

On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
> Add display subsystem related device nodes for MT7623.
>
> Cc: CK Hu <[email protected]>
> Signed-off-by: chunhui dai <[email protected]>
> Signed-off-by: Bibby Hsieh <[email protected]>
> Signed-off-by: Ryder Lee <[email protected]>
> ---
> I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
>
> This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
>
> @Matthias,
> I know you're working on broken MMSYS - just want to ask whether it's possible
> to let the patch to go to your tree (if others are okay with it), and send a
> fixup one for MT7623 MMSYS later?
> ---
> arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> 3 files changed, 343 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> index d01bdee..fdf9078 100644
> --- a/arch/arm/boot/dts/mt7623.dtsi
> +++ b/arch/arm/boot/dts/mt7623.dtsi
> @@ -23,6 +23,11 @@
> #address-cells = <2>;
> #size-cells = <2>;
>
> + aliases {
> + rdma0 = &rdma0;
> + rdma1 = &rdma1;
> + };
> +
> cpu_opp_table: opp-table {
> compatible = "operating-points-v2";
> opp-shared;
> @@ -311,6 +316,25 @@
> clock-names = "spi", "wrap";
> };
>
> + mipi_tx0: mipi-dphy@10010000 {
> + compatible = "mediatek,mt7623-mipi-tx",
> + "mediatek,mt2701-mipi-tx";
> + reg = <0 0x10010000 0 0x90>;
> + clocks = <&clk26m>;
> + clock-output-names = "mipi_tx0_pll";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + };
> +
> + cec: cec@10012000 {
> + compatible = "mediatek,mt7623-cec",
> + "mediatek,mt8173-cec";
> + reg = <0 0x10012000 0 0xbc>;
> + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&infracfg CLK_INFRA_CEC>;
> + status = "disabled";
> + };
> +
> cir: cir@10013000 {
> compatible = "mediatek,mt7623-cir";
> reg = <0 0x10013000 0 0x1000>;
> @@ -359,6 +383,18 @@
> #clock-cells = <1>;
> };
>
> + hdmi_phy: phy@10209100 {
> + compatible = "mediatek,mt7623-hdmi-phy",
> + "mediatek,mt2701-hdmi-phy";
> + reg = <0 0x10209100 0 0x24>;
> + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> + clock-names = "pll_ref";
> + clock-output-names = "hdmitx_dig_cts";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> rng: rng@1020f000 {
> compatible = "mediatek,mt7623-rng";
> reg = <0 0x1020f000 0 0x1000>;
> @@ -558,6 +594,16 @@
> status = "disabled";
> };
>
> + hdmiddc0: i2c@11013000 {
> + compatible = "mediatek,mt7623-hdmi-ddc",
> + "mediatek,mt8173-hdmi-ddc";
> + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> + reg = <0 0x11013000 0 0x1C>;
> + clocks = <&pericfg CLK_PERI_I2C3>;
> + clock-names = "ddc-i2c";
> + status = "disabled";
> + };
> +
> nor_flash: spi@11014000 {
> compatible = "mediatek,mt7623-nor",
> "mediatek,mt8173-nor";
> @@ -732,6 +778,84 @@
> #clock-cells = <1>;
> };
>
> + display_components: dispsys@14000000 {
> + compatible = "mediatek,mt7623-mmsys",

Checkpatch warning:

WARNING: DT compatible string "mediatek,mt7623-mmsys" appears
un-documented -- check ./Documentation/devicetree/bindings/
#101: FILE: arch/arm/boot/dts/mt7623.dtsi:782:
+ compatible = "mediatek,mt7623-mmsys",


> + "mediatek,mt2701-mmsys";
> + reg = <0 0x14000000 0 0x1000>;
> + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> + };
> +
> + ovl@14007000 {
> + compatible = "mediatek,mt7623-disp-ovl",

I think this is also un-documented, but I don't know why checkpatch does
not show any warning.

Regards,
CK

> + "mediatek,mt2701-disp-ovl";
> + reg = <0 0x14007000 0 0x1000>;
> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_OVL>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + rdma0: rdma@14008000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14008000 0 0x1000>;
> + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + wdma@14009000 {
> + compatible = "mediatek,mt7623-disp-wdma",
> + "mediatek,mt2701-disp-wdma";
> + reg = <0 0x14009000 0 0x1000>;
> + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + bls: pwm@1400a000 {
> + compatible = "mediatek,mt7623-disp-pwm",
> + "mediatek,mt2701-disp-pwm";
> + reg = <0 0x1400a000 0 0x1000>;
> + #pwm-cells = <2>;
> + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> + <&mmsys CLK_MM_DISP_BLS>;
> + clock-names = "main", "mm";
> + status = "disabled";
> + };
> +
> + color@1400b000 {
> + compatible = "mediatek,mt7623-disp-color",
> + "mediatek,mt2701-disp-color";
> + reg = <0 0x1400b000 0 0x1000>;
> + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> + };
> +
> + dsi: dsi@1400c000 {
> + compatible = "mediatek,mt7623-dsi",
> + "mediatek,mt2701-dsi";
> + reg = <0 0x1400c000 0 0x1000>;
> + interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DSI_ENGINE>,
> + <&mmsys CLK_MM_DSI_DIG>,
> + <&mipi_tx0>;
> + clock-names = "engine", "digital", "hs";
> + phys = <&mipi_tx0>;
> + phy-names = "dphy";
> + status = "disabled";
> + };
> +
> + mutex: mutex@1400e000 {
> + compatible = "mediatek,mt7623-disp-mutex",
> + "mediatek,mt2701-disp-mutex";
> + reg = <0 0x1400e000 0 0x1000>;
> + interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_MUTEX_32K>;
> + };
> +
> larb0: larb@14010000 {
> compatible = "mediatek,mt7623-smi-larb",
> "mediatek,mt2701-smi-larb";
> @@ -744,6 +868,44 @@
> power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> };
>
> + rdma1: rdma@14012000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14012000 0 0x1000>;
> + interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA1>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + dpi0: dpi@14014000 {
> + compatible = "mediatek,mt7623-dpi",
> + "mediatek,mt2701-dpi";
> + reg = <0 0x14014000 0 0x1000>;
> + interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DPI1_DIGL>,
> + <&mmsys CLK_MM_DPI1_ENGINE>,
> + <&topckgen CLK_TOP_TVDPLL>;
> + clock-names = "pixel", "engine", "pll";
> + status = "disabled";
> + };
> +
> + hdmi0: hdmi@14015000 {
> + compatible = "mediatek,mt7623-hdmi",
> + "mediatek,mt8173-hdmi";
> + reg = <0 0x14015000 0 0x400>;
> + clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
> + <&mmsys CLK_MM_HDMI_PLL>,
> + <&mmsys CLK_MM_HDMI_AUDIO>,
> + <&mmsys CLK_MM_HDMI_SPDIF>;
> + clock-names = "pixel", "pll", "bclk", "spdif";
> + phys = <&hdmi_phy>;
> + phy-names = "hdmi";
> + mediatek,syscon-hdmi = <&mmsys 0x900>;
> + cec = <&cec>;
> + status = "disabled";
> + };
> +
> imgsys: syscon@15000000 {
> compatible = "mediatek,mt7623-imgsys",
> "mediatek,mt2701-imgsys",
> @@ -1071,6 +1233,21 @@
> };
> };
>
> + hdmi_pins_a: hdmi-default {
> + pins-hdmi {
> + pinmux = <MT7623_PIN_123_HTPLG_FUNC_HTPLG>;
> + input-enable;
> + bias-pull-down;
> + };
> + };
> +
> + hdmi_ddc_pins_a: hdmi_ddc-default {
> + pins-hdmi-ddc {
> + pinmux = <MT7623_PIN_124_GPIO124_FUNC_HDMISCK>,
> + <MT7623_PIN_125_GPIO125_FUNC_HDMISD>;
> + };
> + };
> +
> i2c0_pins_a: i2c0-default {
> pins-i2c0 {
> pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
> diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> index 2b760f9..dbce86b 100644
> --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> @@ -21,6 +21,19 @@
> stdout-path = "serial2:115200n8";
> };
>
> + connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "d";
> + ddc-i2c-bus = <&hdmiddc0>;
> +
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&hdmi0_out>;
> + };
> + };
> + };
> +
> cpus {
> cpu@0 {
> proc-supply = <&mt6323_vproc_reg>;
> @@ -114,10 +127,24 @@
> };
> };
>
> +&bls {
> + status = "okay";
> +
> + port {
> + bls_out: endpoint {
> + remote-endpoint = <&dpi0_in>;
> + };
> + };
> +};
> +
> &btif {
> status = "okay";
> };
>
> +&cec {
> + status = "okay";
> +};
> +
> &cir {
> pinctrl-names = "default";
> pinctrl-0 = <&cir_pins_a>;
> @@ -128,6 +155,26 @@
> status = "okay";
> };
>
> +&dpi0 {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + dpi0_out: endpoint {
> + remote-endpoint = <&hdmi0_in>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dpi0_in: endpoint {
> + remote-endpoint = <&bls_out>;
> + };
> + };
> + };
> +};
> +
> &eth {
> status = "okay";
>
> @@ -199,6 +246,42 @@
> };
> };
>
> +&hdmi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_pins_a>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + hdmi0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi0_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> +};
> +
> +&hdmiddc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_ddc_pins_a>;
> + status = "okay";
> +};
> +
> +&hdmi_phy {
> + mediatek,ibias = <0xa>;
> + mediatek,ibias_up = <0x1c>;
> + status = "okay";
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> index b760613..bdf9010 100644
> --- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> +++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> @@ -24,6 +24,19 @@
> stdout-path = "serial2:115200n8";
> };
>
> + connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "d";
> + ddc-i2c-bus = <&hdmiddc0>;
> +
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&hdmi0_out>;
> + };
> + };
> + };
> +
> cpus {
> cpu@0 {
> proc-supply = <&mt6323_vproc_reg>;
> @@ -106,10 +119,24 @@
> };
> };
>
> +&bls {
> + status = "okay";
> +
> + port {
> + bls_out: endpoint {
> + remote-endpoint = <&dpi0_in>;
> + };
> + };
> +};
> +
> &btif {
> status = "okay";
> };
>
> +&cec {
> + status = "okay";
> +};
> +
> &cir {
> pinctrl-names = "default";
> pinctrl-0 = <&cir_pins_a>;
> @@ -120,6 +147,26 @@
> status = "okay";
> };
>
> +&dpi0 {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + dpi0_out: endpoint {
> + remote-endpoint = <&hdmi0_in>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dpi0_in: endpoint {
> + remote-endpoint = <&bls_out>;
> + };
> + };
> + };
> +};
> +
> &eth {
> status = "okay";
>
> @@ -202,6 +249,42 @@
> };
> };
>
> +&hdmi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_pins_a>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + hdmi0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi0_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> +};
> +
> +&hdmiddc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_ddc_pins_a>;
> + status = "okay";
> +};
> +
> +&hdmi_phy {
> + mediatek,ibias = <0xa>;
> + mediatek,ibias_up = <0x1c>;
> + status = "okay";
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;



2018-09-26 02:38:51

by Ryder Lee

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

On Wed, 2018-09-26 at 09:37 +0800, CK Hu wrote:
> Hi, Ryder:
>
> On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
> > Add display subsystem related device nodes for MT7623.
> >
> > Cc: CK Hu <[email protected]>
> > Signed-off-by: chunhui dai <[email protected]>
> > Signed-off-by: Bibby Hsieh <[email protected]>
> > Signed-off-by: Ryder Lee <[email protected]>
> > ---
> > I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
> >
> > This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
> >
> > @Matthias,
> > I know you're working on broken MMSYS - just want to ask whether it's possible
> > to let the patch to go to your tree (if others are okay with it), and send a
> > fixup one for MT7623 MMSYS later?
> > ---
> > arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> > arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> > arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> > 3 files changed, 343 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> > index d01bdee..fdf9078 100644
> > --- a/arch/arm/boot/dts/mt7623.dtsi
> > +++ b/arch/arm/boot/dts/mt7623.dtsi
> > @@ -23,6 +23,11 @@
> > #address-cells = <2>;
> > #size-cells = <2>;
> >
> > + aliases {
> > + rdma0 = &rdma0;
> > + rdma1 = &rdma1;
> > + };
> > +
> > cpu_opp_table: opp-table {
> > compatible = "operating-points-v2";
> > opp-shared;
> > @@ -311,6 +316,25 @@
> > clock-names = "spi", "wrap";
> > };
> >
> > + mipi_tx0: mipi-dphy@10010000 {
> > + compatible = "mediatek,mt7623-mipi-tx",
> > + "mediatek,mt2701-mipi-tx";
> > + reg = <0 0x10010000 0 0x90>;
> > + clocks = <&clk26m>;
> > + clock-output-names = "mipi_tx0_pll";
> > + #clock-cells = <0>;
> > + #phy-cells = <0>;
> > + };
> > +
> > + cec: cec@10012000 {
> > + compatible = "mediatek,mt7623-cec",
> > + "mediatek,mt8173-cec";
> > + reg = <0 0x10012000 0 0xbc>;
> > + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&infracfg CLK_INFRA_CEC>;
> > + status = "disabled";
> > + };
> > +
> > cir: cir@10013000 {
> > compatible = "mediatek,mt7623-cir";
> > reg = <0 0x10013000 0 0x1000>;
> > @@ -359,6 +383,18 @@
> > #clock-cells = <1>;
> > };
> >
> > + hdmi_phy: phy@10209100 {
> > + compatible = "mediatek,mt7623-hdmi-phy",
> > + "mediatek,mt2701-hdmi-phy";
> > + reg = <0 0x10209100 0 0x24>;
> > + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> > + clock-names = "pll_ref";
> > + clock-output-names = "hdmitx_dig_cts";
> > + #clock-cells = <0>;
> > + #phy-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > rng: rng@1020f000 {
> > compatible = "mediatek,mt7623-rng";
> > reg = <0 0x1020f000 0 0x1000>;
> > @@ -558,6 +594,16 @@
> > status = "disabled";
> > };
> >
> > + hdmiddc0: i2c@11013000 {
> > + compatible = "mediatek,mt7623-hdmi-ddc",
> > + "mediatek,mt8173-hdmi-ddc";
> > + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> > + reg = <0 0x11013000 0 0x1C>;
> > + clocks = <&pericfg CLK_PERI_I2C3>;
> > + clock-names = "ddc-i2c";
> > + status = "disabled";
> > + };
> > +
> > nor_flash: spi@11014000 {
> > compatible = "mediatek,mt7623-nor",
> > "mediatek,mt8173-nor";
> > @@ -732,6 +778,84 @@
> > #clock-cells = <1>;
> > };
> >
> > + display_components: dispsys@14000000 {
> > + compatible = "mediatek,mt7623-mmsys",
>
> Checkpatch warning:
>
> WARNING: DT compatible string "mediatek,mt7623-mmsys" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #101: FILE: arch/arm/boot/dts/mt7623.dtsi:782:
> + compatible = "mediatek,mt7623-mmsys",
>
>
> > + "mediatek,mt2701-mmsys";
> > + reg = <0 0x14000000 0 0x1000>;
> > + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > + };
> > +
> > + ovl@14007000 {
> > + compatible = "mediatek,mt7623-disp-ovl",
>
> I think this is also un-documented, but I don't know why checkpatch does
> not show any warning.
>
> Regards,
> CK
> > + "mediatek,mt2701-disp-ovl";
> > + reg = <0 0x14007000 0 0x1000>;
> > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_OVL>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +

I fallback to use the MT2701's compatible string here and there, but I
could add a new one for MT7623.

BTW, I've had this question for a long time - should I add a new
compatible for the very same IPs, or could we just use the old one in
DTS?

Ryder


2018-09-26 02:52:02

by Ryder Lee

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

On Tue, 2018-09-25 at 17:48 +0200, Matthias Brugger wrote:
>
> On 05/09/2018 16:09, Ryder Lee wrote:
> > Add display subsystem related device nodes for MT7623.
> >
> > Cc: CK Hu <[email protected]>
> > Signed-off-by: chunhui dai <[email protected]>
> > Signed-off-by: Bibby Hsieh <[email protected]>
> > Signed-off-by: Ryder Lee <[email protected]>
> > ---
> > I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
> >
> > This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
>
> That's not yet merged, right?

Yes.

> >
> > @Matthias,
> > I know you're working on broken MMSYS - just want to ask whether it's possible
> > to let the patch to go to your tree (if others are okay with it), and send a
> > fixup one for MT7623 MMSYS later?
>
> I know I'm a big blocker here, sorry for the inconvenience.
> I'll do my best to get that in for the next release. If I make no progress on
> that then we should find a different solution.

No problem. Thanks for your great effort :)

> Regards,
> Matthias
>
> > ---
> > arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> > arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> > arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> > 3 files changed, 343 insertions(+)



2018-09-26 05:22:32

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

Hi, Ryder:

On Wed, 2018-09-26 at 10:38 +0800, Ryder Lee wrote:
> On Wed, 2018-09-26 at 09:37 +0800, CK Hu wrote:
> > Hi, Ryder:
> >
> > On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
> > > Add display subsystem related device nodes for MT7623.
> > >
> > > Cc: CK Hu <[email protected]>
> > > Signed-off-by: chunhui dai <[email protected]>
> > > Signed-off-by: Bibby Hsieh <[email protected]>
> > > Signed-off-by: Ryder Lee <[email protected]>
> > > ---
> > > I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
> > >
> > > This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
> > >
> > > @Matthias,
> > > I know you're working on broken MMSYS - just want to ask whether it's possible
> > > to let the patch to go to your tree (if others are okay with it), and send a
> > > fixup one for MT7623 MMSYS later?
> > > ---
> > > arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> > > arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> > > arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> > > 3 files changed, 343 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> > > index d01bdee..fdf9078 100644
> > > --- a/arch/arm/boot/dts/mt7623.dtsi
> > > +++ b/arch/arm/boot/dts/mt7623.dtsi
> > > @@ -23,6 +23,11 @@
> > > #address-cells = <2>;
> > > #size-cells = <2>;
> > >
> > > + aliases {
> > > + rdma0 = &rdma0;
> > > + rdma1 = &rdma1;
> > > + };
> > > +
> > > cpu_opp_table: opp-table {
> > > compatible = "operating-points-v2";
> > > opp-shared;
> > > @@ -311,6 +316,25 @@
> > > clock-names = "spi", "wrap";
> > > };
> > >
> > > + mipi_tx0: mipi-dphy@10010000 {
> > > + compatible = "mediatek,mt7623-mipi-tx",
> > > + "mediatek,mt2701-mipi-tx";
> > > + reg = <0 0x10010000 0 0x90>;
> > > + clocks = <&clk26m>;
> > > + clock-output-names = "mipi_tx0_pll";
> > > + #clock-cells = <0>;
> > > + #phy-cells = <0>;
> > > + };
> > > +
> > > + cec: cec@10012000 {
> > > + compatible = "mediatek,mt7623-cec",
> > > + "mediatek,mt8173-cec";
> > > + reg = <0 0x10012000 0 0xbc>;
> > > + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> > > + clocks = <&infracfg CLK_INFRA_CEC>;
> > > + status = "disabled";
> > > + };
> > > +
> > > cir: cir@10013000 {
> > > compatible = "mediatek,mt7623-cir";
> > > reg = <0 0x10013000 0 0x1000>;
> > > @@ -359,6 +383,18 @@
> > > #clock-cells = <1>;
> > > };
> > >
> > > + hdmi_phy: phy@10209100 {
> > > + compatible = "mediatek,mt7623-hdmi-phy",
> > > + "mediatek,mt2701-hdmi-phy";
> > > + reg = <0 0x10209100 0 0x24>;
> > > + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> > > + clock-names = "pll_ref";
> > > + clock-output-names = "hdmitx_dig_cts";
> > > + #clock-cells = <0>;
> > > + #phy-cells = <0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > rng: rng@1020f000 {
> > > compatible = "mediatek,mt7623-rng";
> > > reg = <0 0x1020f000 0 0x1000>;
> > > @@ -558,6 +594,16 @@
> > > status = "disabled";
> > > };
> > >
> > > + hdmiddc0: i2c@11013000 {
> > > + compatible = "mediatek,mt7623-hdmi-ddc",
> > > + "mediatek,mt8173-hdmi-ddc";
> > > + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> > > + reg = <0 0x11013000 0 0x1C>;
> > > + clocks = <&pericfg CLK_PERI_I2C3>;
> > > + clock-names = "ddc-i2c";
> > > + status = "disabled";
> > > + };
> > > +
> > > nor_flash: spi@11014000 {
> > > compatible = "mediatek,mt7623-nor",
> > > "mediatek,mt8173-nor";
> > > @@ -732,6 +778,84 @@
> > > #clock-cells = <1>;
> > > };
> > >
> > > + display_components: dispsys@14000000 {
> > > + compatible = "mediatek,mt7623-mmsys",
> >
> > Checkpatch warning:
> >
> > WARNING: DT compatible string "mediatek,mt7623-mmsys" appears
> > un-documented -- check ./Documentation/devicetree/bindings/
> > #101: FILE: arch/arm/boot/dts/mt7623.dtsi:782:
> > + compatible = "mediatek,mt7623-mmsys",
> >
> >
> > > + "mediatek,mt2701-mmsys";
> > > + reg = <0 0x14000000 0 0x1000>;
> > > + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > > + };
> > > +
> > > + ovl@14007000 {
> > > + compatible = "mediatek,mt7623-disp-ovl",
> >
> > I think this is also un-documented, but I don't know why checkpatch does
> > not show any warning.
> >
> > Regards,
> > CK
> > > + "mediatek,mt2701-disp-ovl";
> > > + reg = <0 0x14007000 0 0x1000>;
> > > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> > > + clocks = <&mmsys CLK_MM_DISP_OVL>;
> > > + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> > > + mediatek,larb = <&larb0>;
> > > + };
> > > +
>
> I fallback to use the MT2701's compatible string here and there, but I
> could add a new one for MT7623.
>
> BTW, I've had this question for a long time - should I add a new
> compatible for the very same IPs, or could we just use the old one in
> DTS?
>
> Ryder
>

From [1], there is a description for the multiple compatible string
scheme:

====
The reasoning behind this scheme is the observation that in the majority
of cases, a single machine_desc can support a large number of boards
if they all use the same SoC, or same family of SoCs. However,
invariably there will be some exceptions where a specific board will
require special setup code that is not useful in the generic case.
Special cases could be handled by explicitly checking for the
troublesome board(s) in generic setup code, but doing so very quickly
becomes ugly and/or unmaintainable if it is more than just a couple of
cases.

Instead, the compatible list allows a generic machine_desc to provide
support for a wide common set of boards by specifying "less
compatible" values in the dt_compat list. In the example above,
generic board support can claim compatibility with "ti,omap3" or
"ti,omap3450". If a bug was discovered on the original beagleboard
that required special workaround code during early boot, then a new
machine_desc could be added which implements the workarounds and only
matches on "ti,omap3-beagleboard".
====

I don't know would there be a problem happened only in MT7623 even
though this hardware is identical in both MT2701 and MT7623. Maybe the
system-wide configuration would influence this driver. So the easy way
is to add the compatible string for MT7623. If you could prove that
this driver would never be influenced by system-wide configuration,
using only MT2701 compatible string is acceptable for me.

This is my opinion, but the most important is maintainer's opinion.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/Documentation/devicetree/usage-model.txt?id=HEAD

Regards,
CK




2018-09-26 08:52:11

by Matthias Brugger

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes



On 26/09/2018 07:21, CK Hu wrote:
> Hi, Ryder:
>
> On Wed, 2018-09-26 at 10:38 +0800, Ryder Lee wrote:
>> On Wed, 2018-09-26 at 09:37 +0800, CK Hu wrote:
>>> Hi, Ryder:
>>>
>>> On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
>>>> Add display subsystem related device nodes for MT7623.
>>>>
>>>> Cc: CK Hu <[email protected]>
>>>> Signed-off-by: chunhui dai <[email protected]>
>>>> Signed-off-by: Bibby Hsieh <[email protected]>
>>>> Signed-off-by: Ryder Lee <[email protected]>
>>>> ---
>>>> I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
>>>>
>>>> This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
>>>>
>>>> @Matthias,
>>>> I know you're working on broken MMSYS - just want to ask whether it's possible
>>>> to let the patch to go to your tree (if others are okay with it), and send a
>>>> fixup one for MT7623 MMSYS later?
>>>> ---
>>>> arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
>>>> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
>>>> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
>>>> 3 files changed, 343 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
>>>> index d01bdee..fdf9078 100644
>>>> --- a/arch/arm/boot/dts/mt7623.dtsi
>>>> +++ b/arch/arm/boot/dts/mt7623.dtsi
>>>> @@ -23,6 +23,11 @@
>>>> #address-cells = <2>;
>>>> #size-cells = <2>;
>>>>
>>>> + aliases {
>>>> + rdma0 = &rdma0;
>>>> + rdma1 = &rdma1;
>>>> + };
>>>> +
>>>> cpu_opp_table: opp-table {
>>>> compatible = "operating-points-v2";
>>>> opp-shared;
>>>> @@ -311,6 +316,25 @@
>>>> clock-names = "spi", "wrap";
>>>> };
>>>>
>>>> + mipi_tx0: mipi-dphy@10010000 {
>>>> + compatible = "mediatek,mt7623-mipi-tx",
>>>> + "mediatek,mt2701-mipi-tx";
>>>> + reg = <0 0x10010000 0 0x90>;
>>>> + clocks = <&clk26m>;
>>>> + clock-output-names = "mipi_tx0_pll";
>>>> + #clock-cells = <0>;
>>>> + #phy-cells = <0>;
>>>> + };
>>>> +
>>>> + cec: cec@10012000 {
>>>> + compatible = "mediatek,mt7623-cec",
>>>> + "mediatek,mt8173-cec";
>>>> + reg = <0 0x10012000 0 0xbc>;
>>>> + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
>>>> + clocks = <&infracfg CLK_INFRA_CEC>;
>>>> + status = "disabled";
>>>> + };
>>>> +
>>>> cir: cir@10013000 {
>>>> compatible = "mediatek,mt7623-cir";
>>>> reg = <0 0x10013000 0 0x1000>;
>>>> @@ -359,6 +383,18 @@
>>>> #clock-cells = <1>;
>>>> };
>>>>
>>>> + hdmi_phy: phy@10209100 {
>>>> + compatible = "mediatek,mt7623-hdmi-phy",
>>>> + "mediatek,mt2701-hdmi-phy";
>>>> + reg = <0 0x10209100 0 0x24>;
>>>> + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
>>>> + clock-names = "pll_ref";
>>>> + clock-output-names = "hdmitx_dig_cts";
>>>> + #clock-cells = <0>;
>>>> + #phy-cells = <0>;
>>>> + status = "disabled";
>>>> + };
>>>> +
>>>> rng: rng@1020f000 {
>>>> compatible = "mediatek,mt7623-rng";
>>>> reg = <0 0x1020f000 0 0x1000>;
>>>> @@ -558,6 +594,16 @@
>>>> status = "disabled";
>>>> };
>>>>
>>>> + hdmiddc0: i2c@11013000 {
>>>> + compatible = "mediatek,mt7623-hdmi-ddc",
>>>> + "mediatek,mt8173-hdmi-ddc";
>>>> + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
>>>> + reg = <0 0x11013000 0 0x1C>;
>>>> + clocks = <&pericfg CLK_PERI_I2C3>;
>>>> + clock-names = "ddc-i2c";
>>>> + status = "disabled";
>>>> + };
>>>> +
>>>> nor_flash: spi@11014000 {
>>>> compatible = "mediatek,mt7623-nor",
>>>> "mediatek,mt8173-nor";
>>>> @@ -732,6 +778,84 @@
>>>> #clock-cells = <1>;
>>>> };
>>>>
>>>> + display_components: dispsys@14000000 {
>>>> + compatible = "mediatek,mt7623-mmsys",
>>>
>>> Checkpatch warning:
>>>
>>> WARNING: DT compatible string "mediatek,mt7623-mmsys" appears
>>> un-documented -- check ./Documentation/devicetree/bindings/
>>> #101: FILE: arch/arm/boot/dts/mt7623.dtsi:782:
>>> + compatible = "mediatek,mt7623-mmsys",
>>>
>>>
>>>> + "mediatek,mt2701-mmsys";
>>>> + reg = <0 0x14000000 0 0x1000>;
>>>> + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
>>>> + };
>>>> +
>>>> + ovl@14007000 {
>>>> + compatible = "mediatek,mt7623-disp-ovl",
>>>
>>> I think this is also un-documented, but I don't know why checkpatch does
>>> not show any warning.
>>>
>>> Regards,
>>> CK
>>>> + "mediatek,mt2701-disp-ovl";
>>>> + reg = <0 0x14007000 0 0x1000>;
>>>> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
>>>> + clocks = <&mmsys CLK_MM_DISP_OVL>;
>>>> + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
>>>> + mediatek,larb = <&larb0>;
>>>> + };
>>>> +
>>
>> I fallback to use the MT2701's compatible string here and there, but I
>> could add a new one for MT7623.
>>
>> BTW, I've had this question for a long time - should I add a new
>> compatible for the very same IPs, or could we just use the old one in
>> DTS?
>>
>> Ryder
>>
>
> From [1], there is a description for the multiple compatible string
> scheme:
>
> ====
> The reasoning behind this scheme is the observation that in the majority
> of cases, a single machine_desc can support a large number of boards
> if they all use the same SoC, or same family of SoCs. However,
> invariably there will be some exceptions where a specific board will
> require special setup code that is not useful in the generic case.
> Special cases could be handled by explicitly checking for the
> troublesome board(s) in generic setup code, but doing so very quickly
> becomes ugly and/or unmaintainable if it is more than just a couple of
> cases.
>
> Instead, the compatible list allows a generic machine_desc to provide
> support for a wide common set of boards by specifying "less
> compatible" values in the dt_compat list. In the example above,
> generic board support can claim compatibility with "ti,omap3" or
> "ti,omap3450". If a bug was discovered on the original beagleboard
> that required special workaround code during early boot, then a new
> machine_desc could be added which implements the workarounds and only
> matches on "ti,omap3-beagleboard".
> ====
>
> I don't know would there be a problem happened only in MT7623 even
> though this hardware is identical in both MT2701 and MT7623. Maybe the
> system-wide configuration would influence this driver. So the easy way
> is to add the compatible string for MT7623. If you could prove that
> this driver would never be influenced by system-wide configuration,
> using only MT2701 compatible string is acceptable for me.
>
> This is my opinion, but the most important is maintainer's opinion.
>

The compatible string looks good. But you should update the binding documentation :)

Regards,
Matthias

2018-09-26 09:20:53

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

Hi, Ryder:

More inline comment.

On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
> Add display subsystem related device nodes for MT7623.
>
> Cc: CK Hu <[email protected]>
> Signed-off-by: chunhui dai <[email protected]>
> Signed-off-by: Bibby Hsieh <[email protected]>
> Signed-off-by: Ryder Lee <[email protected]>
> ---
> I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
>
> This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
>
> @Matthias,
> I know you're working on broken MMSYS - just want to ask whether it's possible
> to let the patch to go to your tree (if others are okay with it), and send a
> fixup one for MT7623 MMSYS later?
> ---
> arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> 3 files changed, 343 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> index d01bdee..fdf9078 100644
> --- a/arch/arm/boot/dts/mt7623.dtsi
> +++ b/arch/arm/boot/dts/mt7623.dtsi
> @@ -23,6 +23,11 @@
> #address-cells = <2>;
> #size-cells = <2>;
>
> + aliases {
> + rdma0 = &rdma0;
> + rdma1 = &rdma1;
> + };
> +
> cpu_opp_table: opp-table {
> compatible = "operating-points-v2";
> opp-shared;
> @@ -311,6 +316,25 @@
> clock-names = "spi", "wrap";
> };
>
> + mipi_tx0: mipi-dphy@10010000 {
> + compatible = "mediatek,mt7623-mipi-tx",
> + "mediatek,mt2701-mipi-tx";
> + reg = <0 0x10010000 0 0x90>;
> + clocks = <&clk26m>;
> + clock-output-names = "mipi_tx0_pll";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + };
> +
> + cec: cec@10012000 {
> + compatible = "mediatek,mt7623-cec",
> + "mediatek,mt8173-cec";
> + reg = <0 0x10012000 0 0xbc>;
> + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&infracfg CLK_INFRA_CEC>;
> + status = "disabled";
> + };
> +
> cir: cir@10013000 {
> compatible = "mediatek,mt7623-cir";
> reg = <0 0x10013000 0 0x1000>;
> @@ -359,6 +383,18 @@
> #clock-cells = <1>;
> };
>
> + hdmi_phy: phy@10209100 {
> + compatible = "mediatek,mt7623-hdmi-phy",
> + "mediatek,mt2701-hdmi-phy";
> + reg = <0 0x10209100 0 0x24>;
> + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> + clock-names = "pll_ref";
> + clock-output-names = "hdmitx_dig_cts";
> + #clock-cells = <0>;
> + #phy-cells = <0>;
> + status = "disabled";
> + };
> +
> rng: rng@1020f000 {
> compatible = "mediatek,mt7623-rng";
> reg = <0 0x1020f000 0 0x1000>;
> @@ -558,6 +594,16 @@
> status = "disabled";
> };
>
> + hdmiddc0: i2c@11013000 {
> + compatible = "mediatek,mt7623-hdmi-ddc",
> + "mediatek,mt8173-hdmi-ddc";
> + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> + reg = <0 0x11013000 0 0x1C>;
> + clocks = <&pericfg CLK_PERI_I2C3>;
> + clock-names = "ddc-i2c";
> + status = "disabled";
> + };
> +
> nor_flash: spi@11014000 {
> compatible = "mediatek,mt7623-nor",
> "mediatek,mt8173-nor";
> @@ -732,6 +778,84 @@
> #clock-cells = <1>;
> };
>
> + display_components: dispsys@14000000 {
> + compatible = "mediatek,mt7623-mmsys",
> + "mediatek,mt2701-mmsys";
> + reg = <0 0x14000000 0 0x1000>;
> + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> + };
> +
> + ovl@14007000 {
> + compatible = "mediatek,mt7623-disp-ovl",
> + "mediatek,mt2701-disp-ovl";
> + reg = <0 0x14007000 0 0x1000>;
> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_OVL>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + rdma0: rdma@14008000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14008000 0 0x1000>;
> + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + wdma@14009000 {
> + compatible = "mediatek,mt7623-disp-wdma",
> + "mediatek,mt2701-disp-wdma";
> + reg = <0 0x14009000 0 0x1000>;
> + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + bls: pwm@1400a000 {
> + compatible = "mediatek,mt7623-disp-pwm",
> + "mediatek,mt2701-disp-pwm";
> + reg = <0 0x1400a000 0 0x1000>;
> + #pwm-cells = <2>;
> + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> + <&mmsys CLK_MM_DISP_BLS>;
> + clock-names = "main", "mm";
> + status = "disabled";
> + };
> +
> + color@1400b000 {
> + compatible = "mediatek,mt7623-disp-color",
> + "mediatek,mt2701-disp-color";
> + reg = <0 0x1400b000 0 0x1000>;
> + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> + };
> +
> + dsi: dsi@1400c000 {
> + compatible = "mediatek,mt7623-dsi",
> + "mediatek,mt2701-dsi";
> + reg = <0 0x1400c000 0 0x1000>;
> + interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DSI_ENGINE>,
> + <&mmsys CLK_MM_DSI_DIG>,
> + <&mipi_tx0>;
> + clock-names = "engine", "digital", "hs";
> + phys = <&mipi_tx0>;
> + phy-names = "dphy";
> + status = "disabled";
> + };
> +
> + mutex: mutex@1400e000 {
> + compatible = "mediatek,mt7623-disp-mutex",
> + "mediatek,mt2701-disp-mutex";
> + reg = <0 0x1400e000 0 0x1000>;
> + interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_MUTEX_32K>;
> + };
> +
> larb0: larb@14010000 {
> compatible = "mediatek,mt7623-smi-larb",
> "mediatek,mt2701-smi-larb";
> @@ -744,6 +868,44 @@
> power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> };
>
> + rdma1: rdma@14012000 {
> + compatible = "mediatek,mt7623-disp-rdma",
> + "mediatek,mt2701-disp-rdma";
> + reg = <0 0x14012000 0 0x1000>;
> + interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DISP_RDMA1>;
> + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
> + mediatek,larb = <&larb0>;
> + };
> +
> + dpi0: dpi@14014000 {
> + compatible = "mediatek,mt7623-dpi",
> + "mediatek,mt2701-dpi";
> + reg = <0 0x14014000 0 0x1000>;
> + interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&mmsys CLK_MM_DPI1_DIGL>,
> + <&mmsys CLK_MM_DPI1_ENGINE>,
> + <&topckgen CLK_TOP_TVDPLL>;
> + clock-names = "pixel", "engine", "pll";
> + status = "disabled";
> + };
> +
> + hdmi0: hdmi@14015000 {
> + compatible = "mediatek,mt7623-hdmi",
> + "mediatek,mt8173-hdmi";
> + reg = <0 0x14015000 0 0x400>;
> + clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
> + <&mmsys CLK_MM_HDMI_PLL>,
> + <&mmsys CLK_MM_HDMI_AUDIO>,
> + <&mmsys CLK_MM_HDMI_SPDIF>;
> + clock-names = "pixel", "pll", "bclk", "spdif";
> + phys = <&hdmi_phy>;
> + phy-names = "hdmi";
> + mediatek,syscon-hdmi = <&mmsys 0x900>;
> + cec = <&cec>;
> + status = "disabled";
> + };
> +
> imgsys: syscon@15000000 {
> compatible = "mediatek,mt7623-imgsys",
> "mediatek,mt2701-imgsys",
> @@ -1071,6 +1233,21 @@
> };
> };
>
> + hdmi_pins_a: hdmi-default {
> + pins-hdmi {
> + pinmux = <MT7623_PIN_123_HTPLG_FUNC_HTPLG>;
> + input-enable;
> + bias-pull-down;
> + };
> + };
> +
> + hdmi_ddc_pins_a: hdmi_ddc-default {
> + pins-hdmi-ddc {
> + pinmux = <MT7623_PIN_124_GPIO124_FUNC_HDMISCK>,
> + <MT7623_PIN_125_GPIO125_FUNC_HDMISD>;
> + };
> + };
> +
> i2c0_pins_a: i2c0-default {
> pins-i2c0 {
> pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
> diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> index 2b760f9..dbce86b 100644
> --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> @@ -21,6 +21,19 @@
> stdout-path = "serial2:115200n8";
> };
>
> + connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "d";
> + ddc-i2c-bus = <&hdmiddc0>;
> +
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&hdmi0_out>;
> + };
> + };
> + };
> +
> cpus {
> cpu@0 {
> proc-supply = <&mt6323_vproc_reg>;
> @@ -114,10 +127,24 @@
> };
> };
>
> +&bls {
> + status = "okay";
> +
> + port {
> + bls_out: endpoint {
> + remote-endpoint = <&dpi0_in>;
> + };
> + };
> +};
> +
> &btif {
> status = "okay";
> };
>
> +&cec {
> + status = "okay";
> +};
> +
> &cir {
> pinctrl-names = "default";
> pinctrl-0 = <&cir_pins_a>;
> @@ -128,6 +155,26 @@
> status = "okay";
> };
>
> +&dpi0 {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + dpi0_out: endpoint {
> + remote-endpoint = <&hdmi0_in>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dpi0_in: endpoint {
> + remote-endpoint = <&bls_out>;
> + };
> + };
> + };

This part is different with binding
(Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt), I
think you should modify the binding document.

> +};
> +
> &eth {
> status = "okay";
>
> @@ -199,6 +246,42 @@
> };
> };
>
> +&hdmi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_pins_a>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + hdmi0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi0_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> +};
> +
> +&hdmiddc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_ddc_pins_a>;
> + status = "okay";
> +};
> +
> +&hdmi_phy {
> + mediatek,ibias = <0xa>;
> + mediatek,ibias_up = <0x1c>;
> + status = "okay";
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> index b760613..bdf9010 100644
> --- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> +++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> @@ -24,6 +24,19 @@
> stdout-path = "serial2:115200n8";
> };
>
> + connector {
> + compatible = "hdmi-connector";
> + label = "hdmi";
> + type = "d";
> + ddc-i2c-bus = <&hdmiddc0>;
> +
> + port {
> + hdmi_connector_in: endpoint {
> + remote-endpoint = <&hdmi0_out>;
> + };
> + };
> + };
> +
> cpus {
> cpu@0 {
> proc-supply = <&mt6323_vproc_reg>;
> @@ -106,10 +119,24 @@
> };
> };
>
> +&bls {
> + status = "okay";
> +
> + port {
> + bls_out: endpoint {
> + remote-endpoint = <&dpi0_in>;
> + };
> + };

This part is different with binding
(Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt),
I think you should modify the binding document.

Regards,
CK

> +};
> +
> &btif {
> status = "okay";
> };
>
> +&cec {
> + status = "okay";
> +};
> +
> &cir {
> pinctrl-names = "default";
> pinctrl-0 = <&cir_pins_a>;
> @@ -120,6 +147,26 @@
> status = "okay";
> };
>
> +&dpi0 {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + dpi0_out: endpoint {
> + remote-endpoint = <&hdmi0_in>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dpi0_in: endpoint {
> + remote-endpoint = <&bls_out>;
> + };
> + };
> + };
> +};
> +
> &eth {
> status = "okay";
>
> @@ -202,6 +249,42 @@
> };
> };
>
> +&hdmi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_pins_a>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + hdmi0_in: endpoint {
> + remote-endpoint = <&dpi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi0_out: endpoint {
> + remote-endpoint = <&hdmi_connector_in>;
> + };
> + };
> + };
> +};
> +
> +&hdmiddc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hdmi_ddc_pins_a>;
> + status = "okay";
> +};
> +
> +&hdmi_phy {
> + mediatek,ibias = <0xa>;
> + mediatek,ibias_up = <0x1c>;
> + status = "okay";
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;



2018-09-26 09:35:08

by Ryder Lee

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

On Wed, 2018-09-26 at 10:50 +0200, Matthias Brugger wrote:
>
> On 26/09/2018 07:21, CK Hu wrote:
> > Hi, Ryder:
> >
> > On Wed, 2018-09-26 at 10:38 +0800, Ryder Lee wrote:
> >> On Wed, 2018-09-26 at 09:37 +0800, CK Hu wrote:
> >>> Hi, Ryder:
> >>>
> >>> On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
> >>>> Add display subsystem related device nodes for MT7623.
> >>>>
> >>>> Cc: CK Hu <[email protected]>
> >>>> Signed-off-by: chunhui dai <[email protected]>
> >>>> Signed-off-by: Bibby Hsieh <[email protected]>
> >>>> Signed-off-by: Ryder Lee <[email protected]>
> >>>> ---
> >>>> I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
> >>>>
> >>>> This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
> >>>>
> >>>> @Matthias,
> >>>> I know you're working on broken MMSYS - just want to ask whether it's possible
> >>>> to let the patch to go to your tree (if others are okay with it), and send a
> >>>> fixup one for MT7623 MMSYS later?
> >>>> ---
> >>>> arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> >>>> arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> >>>> arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> >>>> 3 files changed, 343 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> >>>> index d01bdee..fdf9078 100644
> >>>> --- a/arch/arm/boot/dts/mt7623.dtsi
> >>>> +++ b/arch/arm/boot/dts/mt7623.dtsi
> >>>> @@ -23,6 +23,11 @@
> >>>> #address-cells = <2>;
> >>>> #size-cells = <2>;
> >>>>
> >>>> + aliases {
> >>>> + rdma0 = &rdma0;
> >>>> + rdma1 = &rdma1;
> >>>> + };
> >>>> +
> >>>> cpu_opp_table: opp-table {
> >>>> compatible = "operating-points-v2";
> >>>> opp-shared;
> >>>> @@ -311,6 +316,25 @@
> >>>> clock-names = "spi", "wrap";
> >>>> };
> >>>>
> >>>> + mipi_tx0: mipi-dphy@10010000 {
> >>>> + compatible = "mediatek,mt7623-mipi-tx",
> >>>> + "mediatek,mt2701-mipi-tx";
> >>>> + reg = <0 0x10010000 0 0x90>;
> >>>> + clocks = <&clk26m>;
> >>>> + clock-output-names = "mipi_tx0_pll";
> >>>> + #clock-cells = <0>;
> >>>> + #phy-cells = <0>;
> >>>> + };
> >>>> +
> >>>> + cec: cec@10012000 {
> >>>> + compatible = "mediatek,mt7623-cec",
> >>>> + "mediatek,mt8173-cec";
> >>>> + reg = <0 0x10012000 0 0xbc>;
> >>>> + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> >>>> + clocks = <&infracfg CLK_INFRA_CEC>;
> >>>> + status = "disabled";
> >>>> + };
> >>>> +
> >>>> cir: cir@10013000 {
> >>>> compatible = "mediatek,mt7623-cir";
> >>>> reg = <0 0x10013000 0 0x1000>;
> >>>> @@ -359,6 +383,18 @@
> >>>> #clock-cells = <1>;
> >>>> };
> >>>>
> >>>> + hdmi_phy: phy@10209100 {
> >>>> + compatible = "mediatek,mt7623-hdmi-phy",
> >>>> + "mediatek,mt2701-hdmi-phy";
> >>>> + reg = <0 0x10209100 0 0x24>;
> >>>> + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> >>>> + clock-names = "pll_ref";
> >>>> + clock-output-names = "hdmitx_dig_cts";
> >>>> + #clock-cells = <0>;
> >>>> + #phy-cells = <0>;
> >>>> + status = "disabled";
> >>>> + };
> >>>> +
> >>>> rng: rng@1020f000 {
> >>>> compatible = "mediatek,mt7623-rng";
> >>>> reg = <0 0x1020f000 0 0x1000>;
> >>>> @@ -558,6 +594,16 @@
> >>>> status = "disabled";
> >>>> };
> >>>>
> >>>> + hdmiddc0: i2c@11013000 {
> >>>> + compatible = "mediatek,mt7623-hdmi-ddc",
> >>>> + "mediatek,mt8173-hdmi-ddc";
> >>>> + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> >>>> + reg = <0 0x11013000 0 0x1C>;
> >>>> + clocks = <&pericfg CLK_PERI_I2C3>;
> >>>> + clock-names = "ddc-i2c";
> >>>> + status = "disabled";
> >>>> + };
> >>>> +
> >>>> nor_flash: spi@11014000 {
> >>>> compatible = "mediatek,mt7623-nor",
> >>>> "mediatek,mt8173-nor";
> >>>> @@ -732,6 +778,84 @@
> >>>> #clock-cells = <1>;
> >>>> };
> >>>>
> >>>> + display_components: dispsys@14000000 {
> >>>> + compatible = "mediatek,mt7623-mmsys",
> >>>
> >>> Checkpatch warning:
> >>>
> >>> WARNING: DT compatible string "mediatek,mt7623-mmsys" appears
> >>> un-documented -- check ./Documentation/devicetree/bindings/
> >>> #101: FILE: arch/arm/boot/dts/mt7623.dtsi:782:
> >>> + compatible = "mediatek,mt7623-mmsys",
> >>>
> >>>
> >>>> + "mediatek,mt2701-mmsys";
> >>>> + reg = <0 0x14000000 0 0x1000>;
> >>>> + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> >>>> + };
> >>>> +
> >>>> + ovl@14007000 {
> >>>> + compatible = "mediatek,mt7623-disp-ovl",
> >>>
> >>> I think this is also un-documented, but I don't know why checkpatch does
> >>> not show any warning.
> >>>
> >>> Regards,
> >>> CK
> >>>> + "mediatek,mt2701-disp-ovl";
> >>>> + reg = <0 0x14007000 0 0x1000>;
> >>>> + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> >>>> + clocks = <&mmsys CLK_MM_DISP_OVL>;
> >>>> + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> >>>> + mediatek,larb = <&larb0>;
> >>>> + };
> >>>> +
> >>
> >> I fallback to use the MT2701's compatible string here and there, but I
> >> could add a new one for MT7623.
> >>
> >> BTW, I've had this question for a long time - should I add a new
> >> compatible for the very same IPs, or could we just use the old one in
> >> DTS?
> >>
> >> Ryder
> >>
> >
> > From [1], there is a description for the multiple compatible string
> > scheme:
> >
> > ====
> > The reasoning behind this scheme is the observation that in the majority
> > of cases, a single machine_desc can support a large number of boards
> > if they all use the same SoC, or same family of SoCs. However,
> > invariably there will be some exceptions where a specific board will
> > require special setup code that is not useful in the generic case.
> > Special cases could be handled by explicitly checking for the
> > troublesome board(s) in generic setup code, but doing so very quickly
> > becomes ugly and/or unmaintainable if it is more than just a couple of
> > cases.
> >
> > Instead, the compatible list allows a generic machine_desc to provide
> > support for a wide common set of boards by specifying "less
> > compatible" values in the dt_compat list. In the example above,
> > generic board support can claim compatibility with "ti,omap3" or
> > "ti,omap3450". If a bug was discovered on the original beagleboard
> > that required special workaround code during early boot, then a new
> > machine_desc could be added which implements the workarounds and only
> > matches on "ti,omap3-beagleboard".
> > ====
> >
> > I don't know would there be a problem happened only in MT7623 even
> > though this hardware is identical in both MT2701 and MT7623. Maybe the
> > system-wide configuration would influence this driver. So the easy way
> > is to add the compatible string for MT7623. If you could prove that
> > this driver would never be influenced by system-wide configuration,
> > using only MT2701 compatible string is acceptable for me.
> >
> > This is my opinion, but the most important is maintainer's opinion.
> >
>
> The compatible string looks good. But you should update the binding documentation :)

Okay, I will update it.

> Regards,
> Matthias



2018-09-26 09:37:10

by Ryder Lee

[permalink] [raw]
Subject: Re: [Resend PATCH 5/5] arm: dts: mt7623: add display subsystem related device nodes

On Wed, 2018-09-26 at 17:20 +0800, CK Hu wrote:
> Hi, Ryder:
>
> More inline comment.
>
> On Wed, 2018-09-05 at 22:09 +0800, Ryder Lee wrote:
> > Add display subsystem related device nodes for MT7623.
> >
> > Cc: CK Hu <[email protected]>
> > Signed-off-by: chunhui dai <[email protected]>
> > Signed-off-by: Bibby Hsieh <[email protected]>
> > Signed-off-by: Ryder Lee <[email protected]>
> > ---
> > I forgot to sort nodes in my previous mail. Sorry for the inconvenience.
> >
> > This patch depends on the series: https://lkml.org/lkml/2018/9/5/223
> >
> > @Matthias,
> > I know you're working on broken MMSYS - just want to ask whether it's possible
> > to let the patch to go to your tree (if others are okay with it), and send a
> > fixup one for MT7623 MMSYS later?
> > ---
> > arch/arm/boot/dts/mt7623.dtsi | 177 ++++++++++++++++++++++++++
> > arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 83 ++++++++++++
> > arch/arm/boot/dts/mt7623n-rfb-emmc.dts | 83 ++++++++++++
> > 3 files changed, 343 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi
> > index d01bdee..fdf9078 100644
> > --- a/arch/arm/boot/dts/mt7623.dtsi
> > +++ b/arch/arm/boot/dts/mt7623.dtsi
> > @@ -23,6 +23,11 @@
> > #address-cells = <2>;
> > #size-cells = <2>;
> >
> > + aliases {
> > + rdma0 = &rdma0;
> > + rdma1 = &rdma1;
> > + };
> > +
> > cpu_opp_table: opp-table {
> > compatible = "operating-points-v2";
> > opp-shared;
> > @@ -311,6 +316,25 @@
> > clock-names = "spi", "wrap";
> > };
> >
> > + mipi_tx0: mipi-dphy@10010000 {
> > + compatible = "mediatek,mt7623-mipi-tx",
> > + "mediatek,mt2701-mipi-tx";
> > + reg = <0 0x10010000 0 0x90>;
> > + clocks = <&clk26m>;
> > + clock-output-names = "mipi_tx0_pll";
> > + #clock-cells = <0>;
> > + #phy-cells = <0>;
> > + };
> > +
> > + cec: cec@10012000 {
> > + compatible = "mediatek,mt7623-cec",
> > + "mediatek,mt8173-cec";
> > + reg = <0 0x10012000 0 0xbc>;
> > + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&infracfg CLK_INFRA_CEC>;
> > + status = "disabled";
> > + };
> > +
> > cir: cir@10013000 {
> > compatible = "mediatek,mt7623-cir";
> > reg = <0 0x10013000 0 0x1000>;
> > @@ -359,6 +383,18 @@
> > #clock-cells = <1>;
> > };
> >
> > + hdmi_phy: phy@10209100 {
> > + compatible = "mediatek,mt7623-hdmi-phy",
> > + "mediatek,mt2701-hdmi-phy";
> > + reg = <0 0x10209100 0 0x24>;
> > + clocks = <&apmixedsys CLK_APMIXED_HDMI_REF>;
> > + clock-names = "pll_ref";
> > + clock-output-names = "hdmitx_dig_cts";
> > + #clock-cells = <0>;
> > + #phy-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > rng: rng@1020f000 {
> > compatible = "mediatek,mt7623-rng";
> > reg = <0 0x1020f000 0 0x1000>;
> > @@ -558,6 +594,16 @@
> > status = "disabled";
> > };
> >
> > + hdmiddc0: i2c@11013000 {
> > + compatible = "mediatek,mt7623-hdmi-ddc",
> > + "mediatek,mt8173-hdmi-ddc";
> > + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_LOW>;
> > + reg = <0 0x11013000 0 0x1C>;
> > + clocks = <&pericfg CLK_PERI_I2C3>;
> > + clock-names = "ddc-i2c";
> > + status = "disabled";
> > + };
> > +
> > nor_flash: spi@11014000 {
> > compatible = "mediatek,mt7623-nor",
> > "mediatek,mt8173-nor";
> > @@ -732,6 +778,84 @@
> > #clock-cells = <1>;
> > };
> >
> > + display_components: dispsys@14000000 {
> > + compatible = "mediatek,mt7623-mmsys",
> > + "mediatek,mt2701-mmsys";
> > + reg = <0 0x14000000 0 0x1000>;
> > + power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > + };
> > +
> > + ovl@14007000 {
> > + compatible = "mediatek,mt7623-disp-ovl",
> > + "mediatek,mt2701-disp-ovl";
> > + reg = <0 0x14007000 0 0x1000>;
> > + interrupts = <GIC_SPI 153 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_OVL>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_OVL_0>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + rdma0: rdma@14008000 {
> > + compatible = "mediatek,mt7623-disp-rdma",
> > + "mediatek,mt2701-disp-rdma";
> > + reg = <0 0x14008000 0 0x1000>;
> > + interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_RDMA>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + wdma@14009000 {
> > + compatible = "mediatek,mt7623-disp-wdma",
> > + "mediatek,mt2701-disp-wdma";
> > + reg = <0 0x14009000 0 0x1000>;
> > + interrupts = <GIC_SPI 154 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_WDMA>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_WDMA>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + bls: pwm@1400a000 {
> > + compatible = "mediatek,mt7623-disp-pwm",
> > + "mediatek,mt2701-disp-pwm";
> > + reg = <0 0x1400a000 0 0x1000>;
> > + #pwm-cells = <2>;
> > + clocks = <&mmsys CLK_MM_MDP_BLS_26M>,
> > + <&mmsys CLK_MM_DISP_BLS>;
> > + clock-names = "main", "mm";
> > + status = "disabled";
> > + };
> > +
> > + color@1400b000 {
> > + compatible = "mediatek,mt7623-disp-color",
> > + "mediatek,mt2701-disp-color";
> > + reg = <0 0x1400b000 0 0x1000>;
> > + interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_COLOR>;
> > + };
> > +
> > + dsi: dsi@1400c000 {
> > + compatible = "mediatek,mt7623-dsi",
> > + "mediatek,mt2701-dsi";
> > + reg = <0 0x1400c000 0 0x1000>;
> > + interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DSI_ENGINE>,
> > + <&mmsys CLK_MM_DSI_DIG>,
> > + <&mipi_tx0>;
> > + clock-names = "engine", "digital", "hs";
> > + phys = <&mipi_tx0>;
> > + phy-names = "dphy";
> > + status = "disabled";
> > + };
> > +
> > + mutex: mutex@1400e000 {
> > + compatible = "mediatek,mt7623-disp-mutex",
> > + "mediatek,mt2701-disp-mutex";
> > + reg = <0 0x1400e000 0 0x1000>;
> > + interrupts = <GIC_SPI 161 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_MUTEX_32K>;
> > + };
> > +
> > larb0: larb@14010000 {
> > compatible = "mediatek,mt7623-smi-larb",
> > "mediatek,mt2701-smi-larb";
> > @@ -744,6 +868,44 @@
> > power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> > };
> >
> > + rdma1: rdma@14012000 {
> > + compatible = "mediatek,mt7623-disp-rdma",
> > + "mediatek,mt2701-disp-rdma";
> > + reg = <0 0x14012000 0 0x1000>;
> > + interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DISP_RDMA1>;
> > + iommus = <&iommu MT2701_M4U_PORT_DISP_RDMA1>;
> > + mediatek,larb = <&larb0>;
> > + };
> > +
> > + dpi0: dpi@14014000 {
> > + compatible = "mediatek,mt7623-dpi",
> > + "mediatek,mt2701-dpi";
> > + reg = <0 0x14014000 0 0x1000>;
> > + interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&mmsys CLK_MM_DPI1_DIGL>,
> > + <&mmsys CLK_MM_DPI1_ENGINE>,
> > + <&topckgen CLK_TOP_TVDPLL>;
> > + clock-names = "pixel", "engine", "pll";
> > + status = "disabled";
> > + };
> > +
> > + hdmi0: hdmi@14015000 {
> > + compatible = "mediatek,mt7623-hdmi",
> > + "mediatek,mt8173-hdmi";
> > + reg = <0 0x14015000 0 0x400>;
> > + clocks = <&mmsys CLK_MM_HDMI_PIXEL>,
> > + <&mmsys CLK_MM_HDMI_PLL>,
> > + <&mmsys CLK_MM_HDMI_AUDIO>,
> > + <&mmsys CLK_MM_HDMI_SPDIF>;
> > + clock-names = "pixel", "pll", "bclk", "spdif";
> > + phys = <&hdmi_phy>;
> > + phy-names = "hdmi";
> > + mediatek,syscon-hdmi = <&mmsys 0x900>;
> > + cec = <&cec>;
> > + status = "disabled";
> > + };
> > +
> > imgsys: syscon@15000000 {
> > compatible = "mediatek,mt7623-imgsys",
> > "mediatek,mt2701-imgsys",
> > @@ -1071,6 +1233,21 @@
> > };
> > };
> >
> > + hdmi_pins_a: hdmi-default {
> > + pins-hdmi {
> > + pinmux = <MT7623_PIN_123_HTPLG_FUNC_HTPLG>;
> > + input-enable;
> > + bias-pull-down;
> > + };
> > + };
> > +
> > + hdmi_ddc_pins_a: hdmi_ddc-default {
> > + pins-hdmi-ddc {
> > + pinmux = <MT7623_PIN_124_GPIO124_FUNC_HDMISCK>,
> > + <MT7623_PIN_125_GPIO125_FUNC_HDMISD>;
> > + };
> > + };
> > +
> > i2c0_pins_a: i2c0-default {
> > pins-i2c0 {
> > pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
> > diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> > index 2b760f9..dbce86b 100644
> > --- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> > +++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
> > @@ -21,6 +21,19 @@
> > stdout-path = "serial2:115200n8";
> > };
> >
> > + connector {
> > + compatible = "hdmi-connector";
> > + label = "hdmi";
> > + type = "d";
> > + ddc-i2c-bus = <&hdmiddc0>;
> > +
> > + port {
> > + hdmi_connector_in: endpoint {
> > + remote-endpoint = <&hdmi0_out>;
> > + };
> > + };
> > + };
> > +
> > cpus {
> > cpu@0 {
> > proc-supply = <&mt6323_vproc_reg>;
> > @@ -114,10 +127,24 @@
> > };
> > };
> >
> > +&bls {
> > + status = "okay";
> > +
> > + port {
> > + bls_out: endpoint {
> > + remote-endpoint = <&dpi0_in>;
> > + };
> > + };
> > +};
> > +
> > &btif {
> > status = "okay";
> > };
> >
> > +&cec {
> > + status = "okay";
> > +};
> > +
> > &cir {
> > pinctrl-names = "default";
> > pinctrl-0 = <&cir_pins_a>;
> > @@ -128,6 +155,26 @@
> > status = "okay";
> > };
> >
> > +&dpi0 {
> > + status = "okay";
> > +
> > + ports {
> > + port@0 {
> > + reg = <0>;
> > + dpi0_out: endpoint {
> > + remote-endpoint = <&hdmi0_in>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + dpi0_in: endpoint {
> > + remote-endpoint = <&bls_out>;
> > + };
> > + };
> > + };
>
> This part is different with binding
> (Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.txt), I
> think you should modify the binding document.
> > +};
> > +
> > &eth {
> > status = "okay";
> >
> > @@ -199,6 +246,42 @@
> > };
> > };
> >
> > +&hdmi0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&hdmi_pins_a>;
> > + status = "okay";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + hdmi0_in: endpoint {
> > + remote-endpoint = <&dpi0_out>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + hdmi0_out: endpoint {
> > + remote-endpoint = <&hdmi_connector_in>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +&hdmiddc0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&hdmi_ddc_pins_a>;
> > + status = "okay";
> > +};
> > +
> > +&hdmi_phy {
> > + mediatek,ibias = <0xa>;
> > + mediatek,ibias_up = <0x1c>;
> > + status = "okay";
> > +};
> > +
> > &i2c0 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&i2c0_pins_a>;
> > diff --git a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> > index b760613..bdf9010 100644
> > --- a/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> > +++ b/arch/arm/boot/dts/mt7623n-rfb-emmc.dts
> > @@ -24,6 +24,19 @@
> > stdout-path = "serial2:115200n8";
> > };
> >
> > + connector {
> > + compatible = "hdmi-connector";
> > + label = "hdmi";
> > + type = "d";
> > + ddc-i2c-bus = <&hdmiddc0>;
> > +
> > + port {
> > + hdmi_connector_in: endpoint {
> > + remote-endpoint = <&hdmi0_out>;
> > + };
> > + };
> > + };
> > +
> > cpus {
> > cpu@0 {
> > proc-supply = <&mt6323_vproc_reg>;
> > @@ -106,10 +119,24 @@
> > };
> > };
> >
> > +&bls {
> > + status = "okay";
> > +
> > + port {
> > + bls_out: endpoint {
> > + remote-endpoint = <&dpi0_in>;
> > + };
> > + };
>
> This part is different with binding
> (Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt),
> I think you should modify the binding document.
>
> Regards,
> CK

Okay, we will update mediatek,disp.txt.

Ryder