Re: [PATCH v2 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes

On 2021-10-21 11:44, Stephen Boyd wrote:

Quoting Krishna Manikandan (2021-10-20 06:58:53)

...

From: Sankeerth Billakanti quic_sbillaka@quicinc.com

Add edp controller and phy DT nodes for sc7280.

Signed-off-by: Sankeerth Billakanti quic_sbillaka@quicinc.com Signed-off-by: Krishna Manikandan quic_mkrishn@quicinc.com

Some comments below

Reviewed-by: Stephen Boyd swboyd@chromium.org

...

Changes in v2: - Move regulator definitions to board file (Matthias Kaehlcke) - Move the gpio definitions to board file (Matthias Kaehlcke) - Move the pinconf to board file (Matthias Kaehlcke) - Move status property (Stephen Boyd) - Drop flags from interrupts (Stephen Boyd) - Add clock names one per line for readability (Stephen Boyd) - Rename edp-opp-table (Stephen Boyd)

---

arch/arm64/boot/dts/qcom/sc7280.dtsi | 107 ++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index dd35882..4450277 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -2575,7 +2575,7 @@ reg = <0 0xaf00000 0 0x20000>; clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_DISP_GPLL0_CLK_SRC>,

•                            <0>, <0>, <0>, <0>, <0>, <0>;
  

•                            <0>, <0>, <0>, <0>, <&edp_phy 0>, 
  

<&edp_phy 1>;

I can already tell this is going to be a merge mess! Can this also be one cell per line?

where are dsi phy? (<&dsi_phy 0>, <&dsi_phy 1>)

...

                    clock-names = "bi_tcxo", "gcc_disp_gpll0_clk",
                                  "dsi0_phy_pll_out_byteclk",
                                  "dsi0_phy_pll_out_dsiclk",

@@ -2777,6 +2784,103 @@

                            status = "disabled";
                    };

•                   msm_edp: edp@aea0000 {
  

•                           compatible = "qcom,sc7280-edp";
  

•                           reg = <0 0xaea0000 0 0x200>,
  

•                                 <0 0xaea0200 0 0x200>,
  

•                                 <0 0xaea0400 0 0xc00>,
  

•                                 <0 0xaea1000 0 0x400>;
  

•                           interrupt-parent = <&mdss>;
  

•                           interrupts = <14>;
  

•                           clocks = <&rpmhcc RPMH_CXO_CLK>,
  

•                                    <&gcc GCC_EDP_CLKREF_EN>,
  

•                                    <&dispcc 
  

DISP_CC_MDSS_AHB_CLK>,

•                                    <&dispcc 
  

DISP_CC_MDSS_EDP_AUX_CLK>,

•                                    <&dispcc 
  

DISP_CC_MDSS_EDP_LINK_CLK>,

•                                    <&dispcc 
  

DISP_CC_MDSS_EDP_LINK_INTF_CLK>,

•                                    <&dispcc 
  

DISP_CC_MDSS_EDP_PIXEL_CLK>;

•                           clock-names = "core_xo",
  

•                                         "core_ref",
  

•                                         "core_iface",
  

•                                         "core_aux",
  

•                                         "ctrl_link",
  

•                                         "ctrl_link_iface",
  

•                                         "stream_pixel";
  

•                           #clock-cells = <1>;
  

•                           assigned-clocks = <&dispcc 
  

DISP_CC_MDSS_EDP_LINK_CLK_SRC>,

•                                             <&dispcc 
  

DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>;

•                           assigned-clock-parents = <&edp_phy 0>, 
  

<&edp_phy 1>;

•                           phys = <&edp_phy>;
  

•                           phy-names = "dp";
  

•                           operating-points-v2 = 
  

<&edp_opp_table>;

•                           power-domains = <&rpmhpd SC7280_CX>;
  

•                           #address-cells = <1>;
  

•                           #size-cells = <0>;
  

•                           status = "disabled";
  

•                           ports {
  

•                                   #address-cells = <1>;
  

•                                   #size-cells = <0>;
  

•                                   port@0 {
  

•                                           reg = <0>;
  

•                                           edp_in: endpoint {
  

remote-endpoint = <&dpu_intf5_out>;

•                                           };
  

•                                   };
  

•                           };
  

•                           edp_opp_table: opp-table {
  

•                                   compatible = 
  

"operating-points-v2";

•                                   opp-160000000 {
  

•                                           opp-hz = /bits/ 64 
  

<160000000>;

•                                           required-opps = 
  

<&rpmhpd_opp_low_svs>;

•                                   };
  

•                                   opp-270000000 {
  

•                                           opp-hz = /bits/ 64 
  

<270000000>;

•                                           required-opps = 
  

<&rpmhpd_opp_svs>;

•                                   };
  

•                                   opp-540000000 {
  

•                                           opp-hz = /bits/ 64 
  

<540000000>;

•                                           required-opps = 
  

<&rpmhpd_opp_nom>;

•                                   };
  

•                                   opp-810000000 {
  

•                                           opp-hz = /bits/ 64 
  

<810000000>;

•                                           required-opps = 
  

<&rpmhpd_opp_nom>;

•                                   };
  

•                           };
  

•                   };
  

•                   edp_phy: phy@aec2000 {
  

unit address needs to match first reg property. This should be

	edp_phy: phy@aec2a00

...

•                           compatible = "qcom,sc7280-edp-phy";
  

•                           reg = <0 0xaec2a00 0 0x19c>,
  

•                                 <0 0xaec2200 0 0xa0>,
  

•                                 <0 0xaec2600 0 0xa0>,
  

•                                 <0 0xaec2000 0 0x1c0>;
  

•                           clocks = <&rpmhcc RPMH_CXO_CLK>,
  

•                                    <&gcc GCC_EDP_CLKREF_EN>;
  

•                           clock-names = "aux",
  

•                                         "cfg_ahb";
  

•                           #clock-cells = <1>;
  

•                           #phy-cells = <0>;
  

•                           status = "disabled";
  

•                   };
            };
  
            pdc: interrupt-controller@b220000 {
  

@@ -3932,6 +4036,7 @@ <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; }; };

Drop this?