Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20200630195151.347-1-tanmay@codeaurora.org>
References: <20200630195151.347-1-tanmay@codeaurora.org>
Subject: Re: [PATCH v3] arm64: dts: qcom: sc7180: Add Display Port dt node
From:   Stephen Boyd <swboyd@chromium.org>
Cc:     agross@kernel.org, bjorn.andersson@linaro.org, robh+dt@kernel.org,
        sam@ravnborg.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
        robdclark@gmail.com, aravindh@codeaurora.org,
        abhinavk@codeaurora.org, chandanu@codeaurora.org,
        varar@codeaurora.org, Tanmay Shah <tanmay@codeaurora.org>
To:     Tanmay Shah <tanmay@codeaurora.org>, seanpaul@chromium.org
Date:   Mon, 20 Jul 2020 13:22:16 -0700
Message-ID: <159527653610.1987609.18177358790741857860@swboyd.mtv.corp.google.com>
User-Agent: alot/0.9
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Quoting Tanmay Shah (2020-06-30 12:51:51)
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/q=
com/sc7180.dtsi
> index 31b9217bb5bf..271d55db62ab 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -2440,6 +2447,44 @@ dsi_phy: dsi-phy@ae94400 {
> =20
>                                 status =3D "disabled";
>                         };
> +
> +                       msm_dp: displayport-controller@ae90000{
> +                               status =3D "disabled";
> +                               compatible =3D "qcom,sc7180-dp";
> +
> +                               reg =3D <0 0xae90000 0 0x1400>;

Please use full 8 digits in address, i.e. 0x0ae90000.

> +                               reg-names =3D "dp_controller";

reg-names is pretty useless here. Please remove it.

> +
> +                               interrupt-parent =3D <&mdss>;
> +                               interrupts =3D <12 0>;

Please use irq flags instead of 0.

> +
> +                               clocks =3D <&dispcc DISP_CC_MDSS_AHB_CLK>,
> +                                        <&dispcc DISP_CC_MDSS_DP_AUX_CLK=
>,
> +                                        <&dispcc DISP_CC_MDSS_DP_LINK_CL=
K>,
> +                                        <&dispcc DISP_CC_MDSS_DP_LINK_IN=
TF_CLK>,
> +                                        <&dispcc DISP_CC_MDSS_DP_PIXEL_C=
LK>;
> +                               clock-names =3D "core_iface", "core_aux",=
 "ctrl_link",
> +                                             "ctrl_link_iface", "stream_=
pixel";
> +                               #clock-cells =3D <1>;
> +                               assigned-clocks =3D <&dispcc DISP_CC_MDSS=
_DP_PIXEL_CLK_SRC>;
> +                               assigned-clock-parents =3D <&msm_dp 1>;

Why don't we assign both clk parents here (&msm_dp 0) so that we don't
need any clk_set_parent() calls in the code?

> +
> +                               ports {
> +                                       #address-cells =3D <1>;
> +                                       #size-cells =3D <0>;
> +                                       port@0 {
> +                                               reg =3D <0>;
> +                                               dp_in: endpoint {
> +                                                       remote-endpoint =
=3D <&dpu_intf0_out>;
> +                                               };
> +                                       };
> +
> +                                       port@1 {
> +                                               reg =3D <1>;
> +                                               dp_out: endpoint { };
> +                                       };
> +                               };
> +                       };
>                 };
> =20
>                 dispcc: clock-controller@af00000 {