Re: [PATCH v3 2/3] arm64: dts: qcom: sc7280: Add lpass cpu node

[Srinivasa Rao Mandadapu]

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@xxxxxxxxxxx>
> > Co-developed-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
> > Signed-off-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
> > ---
> >   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>;