Re: [PATCH v2 3/3] arm64: dts: qcom: sc7280: add sound card support

[Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx>]

On 1/6/2022 2:40 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-01-03 03:13:00)
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > index 3449d56..63b1184 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > @@ -637,6 +637,99 @@
> >          };
> >   };
> >
> > +&sound {
> > +       compatible = "google,sc7280-herobrine";
> > +       model = "sc7280-wcd938x-max98360a-1mic";
> > +       status = "okay";
> > +       audio-routing =
> > +               "IN1_HPHL", "HPHL_OUT",
> > +               "IN2_HPHR", "HPHR_OUT",
> > +               "AMIC1", "MIC BIAS1",
> > +               "AMIC2", "MIC BIAS2",
> > +               "VA DMIC0", "MIC BIAS3",
> > +               "VA DMIC1", "MIC BIAS3",
> > +               "VA DMIC2", "MIC BIAS1",
> > +               "VA DMIC3", "MIC BIAS1",
> > +               "TX SWR_ADC0", "ADC1_OUTPUT",
> > +               "TX SWR_ADC1", "ADC2_OUTPUT",
> > +               "TX SWR_ADC2", "ADC3_OUTPUT",
> > +               "TX SWR_DMIC0", "DMIC1_OUTPUT",
> > +               "TX SWR_DMIC1", "DMIC2_OUTPUT",
> > +               "TX SWR_DMIC2", "DMIC3_OUTPUT",
> > +               "TX SWR_DMIC3", "DMIC4_OUTPUT",
> > +               "TX SWR_DMIC4", "DMIC5_OUTPUT",
> > +               "TX SWR_DMIC5", "DMIC6_OUTPUT",
> > +               "TX SWR_DMIC6", "DMIC7_OUTPUT",
> > +               "TX SWR_DMIC7", "DMIC8_OUTPUT";
> > +
> > +               qcom,msm-mbhc-hphl-swh = <1>;
> > +               qcom,msm-mbhc-gnd-swh = <1>;
> Why are these last extra tabbed?
Okay. Will remove it.
> > +
> > +       #address-cells = <1>;
> > +       #size-cells = <0>;
> > +       #sound-dai-cells = <0>;
> > +
> > +       dai-link@6 {
> > +               link-name = "WCD Playback";
> > +               reg = <LPASS_CDC_DMA_RX0>;
> > +               cpu {
> > +                       sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
> > +               };
> > +
> > +               codec {
> > +                       sound-dai = <&wcd938x 0>, <&swr0 0>, <&rxmacro 0>;
> > +               };
> > +       };
> > +
> > +       dai-link@19 {
> > +               link-name = "WCD Capture";
> > +               reg = <LPASS_CDC_DMA_TX3>;
> > +               cpu {
> > +                       sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
> > +               };
> > +
> > +               codec {
> > +                       sound-dai = <&wcd938x 1>, <&swr1 0>, <&txmacro 0>;
> > +               };
> > +       };
> > +
> > +       dai-link@1 {
> > +               link-name = "Secondary MI2S Playback";
> > +               reg = <MI2S_SECONDARY>;
> > +               cpu {
> > +                       sound-dai = <&lpass_cpu MI2S_SECONDARY>;
> > +               };
> > +
> > +               codec {
> > +                       sound-dai = <&max98360a>;
> > +               };
> > +       };
> > +
> > +       dai-link@5 {
> > +               link-name = "DP Playback";
> > +               reg = <LPASS_DP_RX>;
> > +               cpu {
> > +                       sound-dai = <&lpass_cpu LPASS_DP_RX>;
> > +               };
> > +
> > +               codec {
> > +                               sound-dai = <&msm_dp>;
> Why double tabbed?
Okay. will remove it.
> > +               };
> > +       };
> > +
> > +       dai-link@25 {
> > +               link-name = "DMIC Capture";
> > +               reg = <LPASS_CDC_DMA_VA_TX0>;
> > +               cpu {
> > +                       sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
> > +               };
> > +
> > +               codec {
> > +                       sound-dai = <&vamacro 0>;
> > +               };
> > +       };
> The order of the nodes seems arbitrary. Is there any sort order that can
> be used to avoid conflicts in the future? Maybe the reg property because
> that's how we sort the SoC node.
Okay. Will change accordingly.
> > +};
> > +
> >   &swr0 {
> >          wcd_rx: wcd938x-rx{
> >                  compatible = "sdw20217010d00";
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 68c7755..57bc5ef 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -2786,6 +2786,9 @@
> >
> >                  };
> >
> > +               sound: sound {
> Is this really necessary? Certainly it shouldn't be in the SoC node as
> it doesn't have a reg property.
Okay. will remove it here and add in board specific files.
> > +               };
> > +
> >                  usb_1_hsphy: phy@88e3000 {
> >                          compatible = "qcom,sc7280-usb-hs-phy",
> >                                       "qcom,usb-snps-hs-7nm-phy";
> > --
> > 2.7.4