Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <0f75ebf7-b893-a1a0-1581-6ec29e0be8c6@quicinc.com>
Date:   Tue, 15 Mar 2022 21:45:26 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.1
Subject: Re: [PATCH v3 2/3] arm64: dts: qcom: sc7280: Add lpass cpu node
Content-Language: en-US
To:     Stephen Boyd <swboyd@chromium.org>, <agross@kernel.org>,
        <bjorn.andersson@linaro.org>, <devicetree@vger.kernel.org>,
        <dianders@chromium.org>, <judyhsiao@chromium.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <robh+dt@kernel.org>, <rohitkr@codeaurora.org>,
        <srinivas.kandagatla@linaro.org>
CC:     Venkata Prasad Potturu <quic_potturu@quicinc.com>
References: <1643887981-31011-1-git-send-email-quic_srivasam@quicinc.com>
 <1643887981-31011-3-git-send-email-quic_srivasam@quicinc.com>
 <CAE-0n53X-+gaspxgdVtnr8FW6S5VhXPJaAxLd+vikCnYf9aF6w@mail.gmail.com>
From:   Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Organization: Qualcomm
In-Reply-To: <CAE-0n53X-+gaspxgdVtnr8FW6S5VhXPJaAxLd+vikCnYf9aF6w@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/1/2022 3:01 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-03 03:33:00)
>> Add lpass cpu node for audio on sc7280 based platforms.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 28 +++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi     | 59 ++++++++++++++++++++++++++++++++
>>   2 files changed, 87 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index 2806888..a76b2d1 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -691,3 +691,31 @@
>>   &vamacro {
>>          vdd-micb-supply = <&vreg_bob>;
>>   };
>> +
>> +&lpass_cpu {
>> +       status = "okay";
>> +
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&sec_mi2s_active>;
> Is it a reset gpio? If so, make it a reset-gpios property. I couldn't
> find the definition.

It's not reset gpio. it's for I2S clk, data and ws. It is there in 
previous patch set, which is not applied yet.

We did splitting this node as per functionality. will change here 
accordingly.

>
>> +
>> +       mi2s-secondary@1 {
>> +               reg = <MI2S_SECONDARY>;
>> +               qcom,playback-sd-lines = <0>;
>> +       };
>> +
>> +       hdmi-primary@5 {
>> +               reg = <LPASS_DP_RX>;
>> +       };
>> +
>> +       wcd-rx@6 {
>> +               reg = <LPASS_CDC_DMA_RX0>;
>> +       };
>> +
>> +       wcd-tx@19 {
>> +               reg = <LPASS_CDC_DMA_TX3>;
>> +       };
>> +
>> +       va-tx@25 {
>> +               reg = <LPASS_CDC_DMA_VA_TX0>;
>> +       };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 946eb01..c2da5ce 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -17,6 +17,7 @@
>>   #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>>   #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>>   #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>> +#include <dt-bindings/sound/qcom,lpass.h>
>>   #include <dt-bindings/thermal/thermal.h>
>>
>>   / {
>> @@ -1847,6 +1848,64 @@
>>                          #size-cells = <0>;
>>                  };
>>
>> +               lpass_cpu: audio-subsystem@3260000 {
>> +                       compatible = "qcom,sc7280-lpass-cpu";
>> +                       reg = <0 0x3260000 0 0xC000>,
>> +                             <0 0x3280000 0 0x29000>,
>> +                             <0 0x3340000 0 0x29000>,
>> +                             <0 0x336C000 0 0x3000>,
>> +                             <0 0x3987000 0 0x68000>,
>> +                             <0 0x3B00000 0 0x29000>;
> Lowercase hex. Pad out reg to 8 digits.
Okay.
>
>> +                       reg-names = "lpass-rxtx-cdc-dma-lpm",
>> +                                   "lpass-rxtx-lpaif",
>> +                                   "lpass-va-lpaif",
>> +                                   "lpass-va-cdc-dma-lpm",
>> +                                   "lpass-hdmiif",
>> +                                   "lpass-lpaif";
> That 'lpass' prefix looks very redundant.
Okay. Currently driver and documentation has mentioned similarly. Will 
take care from next time.
>
>> +
>> +                       iommus = <&apps_smmu 0x1820 0>,
>> +                                <&apps_smmu 0x1821 0>,
>> +                                <&apps_smmu 0x1832 0>;