Re: [PATCH v6 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver

[Srinivasa Rao Mandadapu <srivasam@codeaurora.org>]

Hi Stephen,

Thanks for Your Time and Inputs!!!

On 3/14/2021 2:26 AM, Stephen Boyd wrote:
> Quoting Srinivasa Rao Mandadapu (2021-03-12 21:46:53)
>> From: Ajit Pandey <ajitp@codeaurora.org>
>>
>> Add dai link for supporting lpass I2S driver, which is used
>> for audio capture and playback.
>> Add lpass-cpu node with  pin controls and i2s primary
> Why two spaces before 'pin'?
>
>> and secondary dai-links
> Please end sentence with a period.
>
>> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
>> Signed-off-by: V Sujith Kumar Reddy <vsujithk@codeaurora.org>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 58 ++++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>> index 436582279dad..3a24383247db 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>> @@ -283,6 +284,42 @@ keyboard_backlight: keyboard-backlight {
>>                          max-brightness = <1023>;
>>                  };
>>          };
>> +
>> +       sound: sound {
>> +               compatible = "google,sc7180-trogdor";
>> +               model = "sc7180-rt5682-max98357a-1mic";
>> +
>> +               audio-routing =
>> +                       "Headphone Jack", "HPOL",
>> +                       "Headphone Jack", "HPOR";
>> +
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               dai-link@0 {
>> +                       link-name = "MultiMedia0";
>> +                       reg = <MI2S_PRIMARY>;
>> +                       cpu {
>> +                               sound-dai = <&lpass_cpu MI2S_PRIMARY>;
>> +                       };
>> +
>> +                       sound_multimedia0_codec: codec {
>> +                               sound-dai = <&alc5682 0 /*aif1*/>;
> Nitpick, add a space for comment
>
>                                 sound-dai = <&alc5682 0 /* aif1 */>;
Okay. Will re post with Fix.
>
>> +                       };
>> +               };
>> +
>> +               dai-link@1 {
>> +                       link-name = "MultiMedia1";
>> +                       reg = <MI2S_SECONDARY>;
>> +                       cpu {
>> +                               sound-dai = <&lpass_cpu MI2S_SECONDARY>;
>> +                       };
>> +
>> +                       sound_multimedia1_codec: codec {
>> +                               sound-dai = <&max98357a>;
>> +                       };
>> +               };
>> +       };
>>   };
>>   
>>   &qfprom {
>> @@ -720,6 +757,27 @@ &ipa {
>>          modem-init;
>>   };
>>   
>> +&lpass_cpu {
>> +       status = "okay";
>> +
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&sec_mi2s_active &pri_mi2s_active &pri_mi2s_mclk_active>;
> Super nitpick: I prefer this style
>
> 	pinctrl-0 = <&sec_mi2s_active>, <&pri_mi2s_active>, <&pri_mi2s_mclk_active>;
>
> It's effectively the same but the brackets help us see that these are
> the end of the phandle specifier instead of having to figure out that
> the first phandle isn't specifying the second phandle as an argument.
Okay. Will change accordingly.
>
>> +
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
>> +
>> +       mi2s-primary@0 {
> Should the node name just be mi2s instead of mi2s-primary? We have reg
> property so I think 'mi2s' should be sufficient to differentiate.
Okay. I will change it as mi2s@0 instead of mi2s-primary@0
>
>> +               reg = <MI2S_PRIMARY>;
>> +               qcom,playback-sd-lines = <1>;
>> +               qcom,capture-sd-lines = <0>;
>> +       };
>> +
>> +       mi2s-secondary@1 {
>> +               reg = <MI2S_SECONDARY>;
>> +               qcom,playback-sd-lines = <0>;
>> +       };
>> +};
>> +
>>   &mdp {
>>          status = "okay";
>>   };

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.