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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 13 Mar 2021 12:56:35 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Srinivasa Rao Mandadapu <srivasam@...eaurora.org>,
        agross@...nel.org, bjorn.andersson@...aro.org,
        devicetree@...r.kernel.org, dianders@...omium.org,
        judyhsiao@...omium.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        rohitkr@...eaurora.org, srinivas.kandagatla@...aro.org
Cc:     Ajit Pandey <ajitp@...eaurora.org>,
        V Sujith Kumar Reddy <vsujithk@...eaurora.org>,
        Srinivasa Rao Mandadapu <srivasam@...eaurora.org>
Subject: Re: [PATCH v6 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver

Quoting Srinivasa Rao Mandadapu (2021-03-12 21:46:53)
> From: Ajit Pandey <ajitp@...eaurora.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@...eaurora.org>
> Signed-off-by: V Sujith Kumar Reddy <vsujithk@...eaurora.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@...eaurora.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";
>  };