All of lore.kernel.org
 help / color / mirror / Atom feed
From: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
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: Ajit Pandey <ajitp@codeaurora.org>,
	V Sujith Kumar Reddy <vsujithk@codeaurora.org>
Subject: Re: [PATCH v6 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver
Date: Sun, 14 Mar 2021 11:20:46 +0530	[thread overview]
Message-ID: <0f4bc153-edca-ca9d-a5ab-df765e992d7f@codeaurora.org> (raw)
In-Reply-To: <161566899554.1478170.1265435102634351195@swboyd.mtv.corp.google.com>

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.


  reply	other threads:[~2021-03-14  6:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-13  5:46 [PATCH v6 0/2] Qualcomm's lpass device tree changes for I2s dai Srinivasa Rao Mandadapu
2021-03-13  5:46 ` [PATCH v6 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver Srinivasa Rao Mandadapu
2021-03-13 20:56   ` Stephen Boyd
2021-03-14  5:50     ` Srinivasa Rao Mandadapu [this message]
2021-03-13  5:46 ` [PATCH v6 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz Srinivasa Rao Mandadapu
2021-03-13 20:58   ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0f4bc153-edca-ca9d-a5ab-df765e992d7f@codeaurora.org \
    --to=srivasam@codeaurora.org \
    --cc=agross@kernel.org \
    --cc=ajitp@codeaurora.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=judyhsiao@chromium.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=rohitkr@codeaurora.org \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=swboyd@chromium.org \
    --cc=vsujithk@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.