diff mbox series

[v4,2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz

Message ID 20210311164815.14113-3-srivasam@codeaurora.org
State New
Headers show
Series Qualcomm's lpass device tree changes for I2s dai | expand

Commit Message

Srinivasa Rao Mandadapu March 11, 2021, 4:48 p.m. UTC
This is a trgodor variant, required to have sound node variable
for coachz specific platform.

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
 .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi   | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Srinivasa Rao Mandadapu March 12, 2021, 3:53 p.m. UTC | #1
Hi Doug,

Thanks for time and valuable inputs!!!

On 3/12/2021 1:24 AM, Doug Anderson wrote:
> Hi,

>

> On Thu, Mar 11, 2021 at 8:48 AM Srinivasa Rao Mandadapu

> <srivasam@codeaurora.org> wrote:

>> This is a trgodor variant, required to have sound node variable

>> for coachz specific platform.

>>

>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>

>> ---

>>   .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi   | 18 ++++++++++++++++++

>>   1 file changed, 18 insertions(+)

>>

>> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi

>> index 4ad520f00485..7623a30a64c7 100644

>> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi

>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi

>> @@ -238,3 +238,21 @@ &tlmm {

>>                            "DP_HOT_PLUG_DET",

>>                            "EC_IN_RW_ODL";

>>   };

>> +

>> +&sound {

>> +       compatible = "google,sc7180-coachz";

> You're placing this in the wrong place. Pay attention to the section

> headings. Your patch is putting this in the section "PINCTRL -

> board-specific pinctrl". That's not right.

>

>

>> +       model = "sc7180-adau7002-max98357a";

>> +       audio-routing = "PDM_DAT", "DMIC";

>> +

>> +       dai-link@0 {

>> +               link-name = "MultiMedia0";

>> +               reg = <0>;

>> +               cpu {

>> +                       sound-dai = <&lpass_cpu 0>;

> Shouldn't the 0 above be "MI2S_PRIMARY" ?  ...and the "reg" as well?

Yes, It's required. Will change, and re-post.
>

>

>> +               };

>> +

>> +               codec {

>> +                       sound-dai = <&adau7002>;

>> +               };

>> +       };

> Some overall notes, though:

>

> 1. You don't actually need to duplicate everything that you have

> above. Whether you realize it or not the way devicetree works is that

> it _merges_ the node in the "coachz" devicetree with the one from the

> trogdor one (it doesn't replace it). So in trogdor you have:

>

> dai-link@0 {

>    link-name = "MultiMedia0";

>    reg = <MI2S_PRIMARY>;

>    cpu {

>      sound-dai = <&lpass_cpu MI2S_PRIMARY>;

>    };

>

>    codec {

>      sound-dai = <&alc5682 MI2S_PRIMARY>;

>    };

> };

>

> ...and in coachz you have:

>

> dai-link@0 {

>    link-name = "MultiMedia0";

>    reg = <MI2S_PRIMARY>;

>    cpu {

>      sound-dai = <&lpass_cpu MI2S_PRIMARY>;

>    };

>

>    codec {

>      sound-dai = <&adau7002>;

>    };

> };

>

> Almost all of that is duplication. It's best not to duplicate. Thus,

> one step better than what you have would be to just have this in

> coachz to override what you need:

>

> dai-link@0 {

>    codec {

>      sound-dai = <&adau7002>;

>    };

> };

>

>

> 2. In general it's discouraged (and error prone) to try to replicate

> hierarchies from your parent. So the best would be to change trogdor's

> device tree to something like this:

>

> dai-link@0 {

>    link-name = "MultiMedia0";

>    reg = <MI2S_PRIMARY>;

>    cpu {

>      sound-dai = <&lpass_cpu MI2S_PRIMARY>;

>    };

>

>    multimedia0_codec: codec {

>      sound-dai = <&alc5682 MI2S_PRIMARY>;

>    };

> };

>

> ...and then in coachz you override like:

>

> &multimedia0_codec {

>     sound-dai = <&alc5682 MI2S_PRIMARY>;

> };

Okay. Will change accordingly and re-post.

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

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index 4ad520f00485..7623a30a64c7 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -238,3 +238,21 @@  &tlmm {
 			  "DP_HOT_PLUG_DET",
 			  "EC_IN_RW_ODL";
 };
+
+&sound {
+	compatible = "google,sc7180-coachz";
+	model = "sc7180-adau7002-max98357a";
+	audio-routing = "PDM_DAT", "DMIC";
+
+	dai-link@0 {
+		link-name = "MultiMedia0";
+		reg = <0>;
+		cpu {
+			sound-dai = <&lpass_cpu 0>;
+		};
+
+		codec {
+			sound-dai = <&adau7002>;
+		};
+	};
+};