lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 21 Jan 2020 15:16:17 -0800 From: Stephen Boyd <swboyd@...omium.org> To: Harigovindan P <harigovi@...eaurora.org>, devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org Cc: Harigovindan P <harigovi@...eaurora.org>, linux-kernel@...r.kernel.org, robdclark@...il.com, seanpaul@...omium.org, hoegsberg@...omium.org, kalyan_t@...eaurora.org, nganji@...eaurora.org Subject: Re: [v1] arm64: dts: sc7180: add display dt nodes 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@...0000 { 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@...1000 { 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@...4000 { 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@...4400 { 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@...0000 { > compatible = "qcom,sc7180-pdc", "qcom,pdc"; > reg = <0 0x0b220000 0 0x30000>;
Powered by blists - more mailing lists