From: Stephen Boyd <swboyd@chromium.org>
To: Harigovindan P <harigovi@codeaurora.org>,
devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Cc: Harigovindan P <harigovi@codeaurora.org>,
linux-kernel@vger.kernel.org, robdclark@gmail.com,
seanpaul@chromium.org, hoegsberg@chromium.org,
kalyan_t@codeaurora.org, nganji@codeaurora.org
Subject: Re: [v1] arm64: dts: sc7180: add display dt nodes
Date: Tue, 21 Jan 2020 15:16:17 -0800 [thread overview]
Message-ID: <5e278642.1c69fb81.5a8db.80b4@mx.google.com> (raw)
In-Reply-To: <1579621928-18619-1-git-send-email-harigovi@codeaurora.org>
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>;
prev parent reply other threads:[~2020-01-21 23:16 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-21 15:52 [v1] arm64: dts: sc7180: add display dt nodes Harigovindan P
2020-01-21 23:16 ` Stephen Boyd [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5e278642.1c69fb81.5a8db.80b4@mx.google.com \
--to=swboyd@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=harigovi@codeaurora.org \
--cc=hoegsberg@chromium.org \
--cc=kalyan_t@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nganji@codeaurora.org \
--cc=robdclark@gmail.com \
--cc=seanpaul@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox