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

[Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>]

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
>>