All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Srinivasa Rao Mandadapu <srivasam@codeaurora.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>,
	Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
Subject: Re: [PATCH v6 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver
Date: Sat, 13 Mar 2021 12:56:35 -0800	[thread overview]
Message-ID: <161566899554.1478170.1265435102634351195@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <20210313054654.11693-2-srivasam@codeaurora.org>

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 */>;

> +                       };
> +               };
> +
> +               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.

> +
> +       #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.

> +               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";
>  };

  reply	other threads:[~2021-03-13 20:57 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 [this message]
2021-03-14  5:50     ` Srinivasa Rao Mandadapu
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=161566899554.1478170.1265435102634351195@swboyd.mtv.corp.google.com \
    --to=swboyd@chromium.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=srivasam@codeaurora.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.