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;
Sender: mkrishn=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Thu, 30 Sep 2021 17:26:59 +0530
From:   mkrishn@codeaurora.org
To:     Stephen Boyd <swboyd@chromium.org>
Cc:     devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org, kalyan_t@codeaurora.org,
        sbillaka@codeaurora.org, abhinavk@codeaurora.org,
        robdclark@gmail.com, bjorn.andersson@linaro.org,
        khsieh@codeaurora.org, rajeevny@codeaurora.org,
        freedreno@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        robh+dt@kernel.org
Subject: Re: [PATCH v1 2/4] arm64: dts: qcom: sc7280: add display dt nodes
In-Reply-To: <CAE-0n50b=pX=1MFwGPDvDR=O03tUAkAgyMonGm2+SXBft=16KQ@mail.gmail.com>
References: <1629282424-4070-1-git-send-email-mkrishn@codeaurora.org>
 <1629282424-4070-2-git-send-email-mkrishn@codeaurora.org>
 <CAE-0n50b=pX=1MFwGPDvDR=O03tUAkAgyMonGm2+SXBft=16KQ@mail.gmail.com>
Message-ID: <5adf2ab2c2a162272509d253bd797721@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2021-08-19 01:27, Stephen Boyd wrote:
> Quoting Krishna Manikandan (2021-08-18 03:27:02)
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 53a21d0..fd7ff1c 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -5,6 +5,7 @@
>>   * Copyright (c) 2020-2021, The Linux Foundation. All rights 
>> reserved.
>>   */
>> 
>> +#include <dt-bindings/clock/qcom,dispcc-sc7280.h>
>>  #include <dt-bindings/clock/qcom,gcc-sc7280.h>
>>  #include <dt-bindings/clock/qcom,rpmh.h>
>>  #include <dt-bindings/interconnect/qcom,sc7280.h>
>> @@ -1424,6 +1425,90 @@
>>                         #power-domain-cells = <1>;
>>                 };
>> 
>> +               mdss: mdss@ae00000 {
> 
> subsystem@ae00000
> 
>> +                       compatible = "qcom,sc7280-mdss";
>> +                       reg = <0 0x0ae00000 0 0x1000>;
>> +                       reg-names = "mdss";
>> +
>> +                       power-domains = <&dispcc 
>> DISP_CC_MDSS_CORE_GDSC>;
>> +
>> +                       clocks = <&gcc GCC_DISP_AHB_CLK>,
>> +                                <&dispcc DISP_CC_MDSS_AHB_CLK>,
>> +                               <&dispcc DISP_CC_MDSS_MDP_CLK>;
>> +                       clock-names = "iface", "ahb", "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>;
>> +
>> +                       interconnects = <&mmss_noc MASTER_MDP0 0 
>> &mc_virt SLAVE_EBI1 0>;
>> +                       interconnect-names = "mdp0-mem";
>> +
>> +                       iommus = <&apps_smmu 0x900 0x402>;
>> +
>> +                       #address-cells = <2>;
>> +                       #size-cells = <2>;
>> +                       ranges;
>> +
>> +                       status = "disabled";
>> +
>> +                       mdp: mdp@ae01000 {
> 
> display-controller@ae01000

Stephen,
    In the current driver code, there is a substring comparison for "mdp" 
in device node name as part of probe sequence. If "mdp" is not present 
in the node name, it will
    return an error resulting in probe failure. Can we continue using mdp 
as nodename instead of display controller?

Thanks,
Krishna


> 
>> +                               compatible = "qcom,sc7280-dpu";
>> +                               reg = <0 0x0ae01000 0 0x8f030>,
>> +                                       <0 0x0aeb0000 0 0x2008>;
>> +                               reg-names = "mdp", "vbif";
>> +
>> +                               clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
>> +                                       <&gcc GCC_DISP_SF_AXI_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_AHB_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_MDP_LUT_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_MDP_CLK>,
>> +                                       <&dispcc 
>> DISP_CC_MDSS_VSYNC_CLK>;
>> +                               clock-names = "bus", "nrt_bus", 
>> "iface", "lut", "core",
>> +                                             "vsync";
> 
> One line per string please.
> 
>> +                               assigned-clocks = <&dispcc 
>> DISP_CC_MDSS_MDP_CLK>,
>> +                                               <&dispcc 
>> DISP_CC_MDSS_VSYNC_CLK>,
>> +                                               <&dispcc 
>> DISP_CC_MDSS_AHB_CLK>;
>> +                               assigned-clock-rates = <300000000>,
>> +                                                       <19200000>,
>> +                                                       <19200000>;
>> +                               operating-points-v2 = 
>> <&mdp_opp_table>;
>> +                               power-domains = <&rpmhpd SC7280_CX>;
>> +
>> +                               interrupt-parent = <&mdss>;
>> +                               interrupts = <0>;
>> +
>> +                               status = "disabled";
>> +
>> +                               mdp_opp_table: mdp-opp-table {
> 
> mdp_opp_table: opp-table {
> 
>> +                                       compatible = 
>> "operating-points-v2";
>> +
>> +                                       opp-200000000 {
>> +                                               opp-hz = /bits/ 64 
>> <200000000>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_low_svs>;
>> +                                       };
>> +
>> +                                       opp-300000000 {
>> +                                               opp-hz = /bits/ 64 
>> <300000000>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_svs>;
>> +                                       };
>> +
>> +                                       opp-380000000 {
>> +                                               opp-hz = /bits/ 64 
>> <380000000>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_svs_l1>;
>> +                                       };
>> +
>> +                                       opp-506666667 {
>> +                                               opp-hz = /bits/ 64 
>> <506666667>;
>> +                                               required-opps = 
>> <&rpmhpd_opp_nom>;
>> +                                       };
>> +                               };
>> +                       };
>> +               };
>> +