Quoting Harigovindan P (2020-01-21 07:52:08) > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > old mode 100644 > new mode 100755 > index 8011c5f..963f5c1 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -1151,6 +1151,131 @@ > }; > }; > > + mdss: mdss@ae00000 { Is there a better node name for this? display-subsystem perhaps? > + compatible = "qcom,sc7180-mdss"; > + reg = <0 0x0ae00000 0 0x1000>; > + reg-names = "mdss"; > + > + power-domains = <&dispcc MDSS_GDSC>; > + > + clocks = <&gcc GCC_DISP_AHB_CLK>, > + <&gcc GCC_DISP_HF_AXI_CLK>, > + <&dispcc DISP_CC_MDSS_MDP_CLK>; > + clock-names = "iface", "gcc_bus", "core"; > + > + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>; > + assigned-clock-rates = <300000000>; > + > + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-controller; > + #interrupt-cells = <1>; > + > + iommus = <&apps_smmu 0x800 0x2>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + mdss_mdp: mdp@ae01000 { Is there a better node name for this? display-controller perhaps? Also, first reg property is supposed to be the one after the @ sign. In this case that would be ae00000. > + compatible = "qcom,sc7180-dpu"; > + reg = <0 0x0ae00000 0 0x1000>, > + <0 0x0ae01000 0 0x8f000>, > + <0 0x0aeb0000 0 0x2008>, > + <0 0x0af03000 0 0x16>; > + reg-names = "mdss","mdp", "vbif", "disp_cc"; ^ Nitpick: Add a space here after the comma. > + > + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, > + <&dispcc DISP_CC_MDSS_ROT_CLK>, > + <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>, > + <&dispcc DISP_CC_MDSS_MDP_CLK>, > + <&dispcc DISP_CC_MDSS_VSYNC_CLK>; > + clock-names = "iface", "rot", "lut", "core", > + "vsync"; Nitpick: Tabbing seems weird here. The clocks property is aligned but not the clock-names. > + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>, > + <&dispcc DISP_CC_MDSS_VSYNC_CLK>; > + assigned-clock-rates = <300000000>, > + <19200000>; > + > + interrupt-parent = <&mdss>; > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dpu_intf1_out: endpoint { > + remote-endpoint = <&dsi0_in>; > + }; > + }; > + }; > + }; > + > + dsi0: qcom,mdss_dsi_ctrl0@ae94000 { Is there a better node name for this? dsi-controller perhaps? > + compatible = "qcom,mdss-dsi-ctrl"; > + reg = <0 0x0ae94000 0 0x400>; > + reg-names = "dsi_ctrl"; > + > + interrupt-parent = <&mdss>; > + interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; > + > + clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>, > + <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>, > + <&dispcc DISP_CC_MDSS_PCLK0_CLK>, > + <&dispcc DISP_CC_MDSS_ESC0_CLK>, > + <&dispcc DISP_CC_MDSS_AHB_CLK>, > + <&gcc GCC_DISP_HF_AXI_CLK>; > + clock-names = "byte", > + "byte_intf", > + "pixel", > + "core", > + "iface", > + "bus"; Nitpick: Tabbing is all of here too. > + > + phys = <&dsi0_phy>; > + phy-names = "dsi"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dsi0_in: endpoint { > + remote-endpoint = <&dpu_intf1_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + dsi0_out: endpoint { > + }; > + }; > + }; > + }; > + > + dsi0_phy: dsi-phy0@ae94400 { Just call it 'dsi-phy' or 'phy' please. The address differentiates it and the phandle can call it 0. > + compatible = "qcom,dsi-phy-10nm"; > + reg = <0 0x0ae94400 0 0x200>, > + <0 0x0ae94600 0 0x280>, > + <0 0x0ae94a00 0 0x1e0>; > + reg-names = "dsi_phy", > + "dsi_phy_lane", > + "dsi_pll"; > + > + #clock-cells = <1>; > + #phy-cells = <0>; > + > + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>; Do you need the XO or reference clk here too? So that the PLL can generate a clk with the reference clk? > + clock-names = "iface"; > + Nitpick: Why the extra newline? Please remove. > + }; > + }; > + > pdc: interrupt-controller@b220000 { > compatible = "qcom,sc7180-pdc", "qcom,pdc"; > reg = <0 0x0b220000 0 0x30000>;