All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Qualcomm's lpass device tree changes for I2s dai
@ 2021-03-11 16:48 Srinivasa Rao Mandadapu
  2021-03-11 16:48 ` [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver Srinivasa Rao Mandadapu
  2021-03-11 16:48 ` [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz Srinivasa Rao Mandadapu
  0 siblings, 2 replies; 8+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-03-11 16:48 UTC (permalink / raw)
  To: gross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu

These patches are device tree changes to support audio over I2S.

Changes Since v3:
  -- Compatible name changed as per trogdor 
Changes Since v2:
  -- sound node variable is added in sc7180-trogdor-coachz.dtsi.
  -- Audio-jack property is removed, as it may create conflicts with the boards 
     having different codecs.
  -- Replaced reg values with common dt-bindings macros
Changes Since v1:
  -- Ajith's Certification is added.
  -- Alias name added for sound dai link node

Ajit Pandey (1):
  arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver

Srinivasa Rao Mandadapu (1):
  arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz

 .../boot/dts/qcom/sc7180-trogdor-coachz.dtsi  | 18 ++++++
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi  | 58 +++++++++++++++++++
 2 files changed, 76 insertions(+)

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver
  2021-03-11 16:48 [PATCH v4 0/2] Qualcomm's lpass device tree changes for I2s dai Srinivasa Rao Mandadapu
@ 2021-03-11 16:48 ` Srinivasa Rao Mandadapu
  2021-03-11 19:56   ` Doug Anderson
  2021-03-11 16:48 ` [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz Srinivasa Rao Mandadapu
  1 sibling, 1 reply; 8+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-03-11 16:48 UTC (permalink / raw)
  To: gross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: Ajit Pandey, V Sujith Kumar Reddy, Srinivasa Rao Mandadapu

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
and secondary dai-links

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..501e3d4c9097 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/input/gpio-keys.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include <dt-bindings/sound/qcom,lpass.h>
 
 /* PMICs depend on spmi_bus label and so must come after SoC */
 #include "pm6150.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>;
+			};
+
+			codec {
+				sound-dai = <&alc5682 MI2S_PRIMARY>;
+			};
+		};
+
+		dai-link@1 {
+			link-name = "MultiMedia1";
+			reg = <MI2S_SECONDARY>;
+			cpu {
+				sound-dai = <&lpass_cpu MI2S_SECONDARY>;
+			};
+
+			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>;
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	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.


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz
  2021-03-11 16:48 [PATCH v4 0/2] Qualcomm's lpass device tree changes for I2s dai Srinivasa Rao Mandadapu
  2021-03-11 16:48 ` [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver Srinivasa Rao Mandadapu
@ 2021-03-11 16:48 ` Srinivasa Rao Mandadapu
  2021-03-11 19:54   ` Doug Anderson
  1 sibling, 1 reply; 8+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-03-11 16:48 UTC (permalink / raw)
  To: gross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu

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";
+	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>;
+		};
+	};
+};
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz
  2021-03-11 16:48 ` [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz Srinivasa Rao Mandadapu
@ 2021-03-11 19:54   ` Doug Anderson
  2021-03-12 15:53     ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 8+ messages in thread
From: Doug Anderson @ 2021-03-11 19:54 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rohit kumar, Srinivas Kandagatla, Stephen Boyd, Judy Hsiao

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?


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver
  2021-03-11 16:48 ` [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver Srinivasa Rao Mandadapu
@ 2021-03-11 19:56   ` Doug Anderson
  2021-03-12 13:02     ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 8+ messages in thread
From: Doug Anderson @ 2021-03-11 19:56 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu
  Cc: gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rohit kumar, Srinivas Kandagatla, Stephen Boyd, Judy Hsiao,
	Ajit Pandey, V Sujith Kumar Reddy

Hi,

On Thu, Mar 11, 2021 at 8:49 AM Srinivasa Rao Mandadapu
<srivasam@codeaurora.org> wrote:
>
> 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
> and secondary dai-links
>
> 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..501e3d4c9097 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -9,6 +9,7 @@
>  #include <dt-bindings/input/gpio-keys.h>
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +#include <dt-bindings/sound/qcom,lpass.h>

It seems marginally better to include "sc7180-lpass.h" to get this? I
don't really know the difference between the two but since unless
we're planning to delete the sc7180 version it seems like you might as
well include that one?


>  /* PMICs depend on spmi_bus label and so must come after SoC */
>  #include "pm6150.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>;
> +                       };
> +
> +                       codec {
> +                               sound-dai = <&alc5682 MI2S_PRIMARY>;

I'm an audio noob but isn't "MI2S_PRIMARY" something to be used with
"lpass_cpu", not with "alc5682" ?

I have no idea what the IDs correspond to on "alc5682". Are you sure
we even need an extra ID there? The "alc5682" bindings upstream don't
talk anything about dai-cells, but maybe they're just wrong...

-Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver
  2021-03-11 19:56   ` Doug Anderson
@ 2021-03-12 13:02     ` Srinivasa Rao Mandadapu
  2021-03-12 15:55       ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 8+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-03-12 13:02 UTC (permalink / raw)
  To: Doug Anderson
  Cc: gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rohit kumar, Srinivas Kandagatla, Stephen Boyd, Judy Hsiao,
	Ajit Pandey, V Sujith Kumar Reddy

Hi Doug,

Thanks for your time!!!

On 3/12/2021 1:26 AM, Doug Anderson wrote:
> Hi,
>
> On Thu, Mar 11, 2021 at 8:49 AM Srinivasa Rao Mandadapu
> <srivasam@codeaurora.org> wrote:
>> 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
>> and secondary dai-links
>>
>> 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..501e3d4c9097 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>> @@ -9,6 +9,7 @@
>>   #include <dt-bindings/input/gpio-keys.h>
>>   #include <dt-bindings/input/input.h>
>>   #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> +#include <dt-bindings/sound/qcom,lpass.h>
> It seems marginally better to include "sc7180-lpass.h" to get this? I
> don't really know the difference between the two but since unless
> we're planning to delete the sc7180 version it seems like you might as
> well include that one?

Yes, I agree.

Recently, all Quallcomm variant headers are combined and created 
"qcom,lpass.h".

"sc7180-lpass.h" still includes "qcom,lpass.h". So I will change and 
repost the patch.


Here is the reference commit:

>
>
>>   /* PMICs depend on spmi_bus label and so must come after SoC */
>>   #include "pm6150.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>;
>> +                       };
>> +
>> +                       codec {
>> +                               sound-dai = <&alc5682 MI2S_PRIMARY>;
> I'm an audio noob but isn't "MI2S_PRIMARY" something to be used with
> "lpass_cpu", not with "alc5682" ?
>
> I have no idea what the IDs correspond to on "alc5682". Are you sure
> we even need an extra ID there? The "alc5682" bindings upstream don't
> talk anything about dai-cells, but maybe they're just wrong...
Yes. I will change and re-post.
>
> -Doug

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz
  2021-03-11 19:54   ` Doug Anderson
@ 2021-03-12 15:53     ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 8+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-03-12 15:53 UTC (permalink / raw)
  To: Doug Anderson
  Cc: gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rohit kumar, Srinivas Kandagatla, Stephen Boyd, Judy Hsiao

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.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver
  2021-03-12 13:02     ` Srinivasa Rao Mandadapu
@ 2021-03-12 15:55       ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 8+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-03-12 15:55 UTC (permalink / raw)
  To: Doug Anderson
  Cc: gross, Bjorn Andersson, Rob Herring, linux-arm-msm,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rohit kumar, Srinivas Kandagatla, Stephen Boyd, Judy Hsiao,
	Ajit Pandey, V Sujith Kumar Reddy

Hi Doug,

On 3/12/2021 6:32 PM, Srinivasa Rao Mandadapu wrote:
> Hi Doug,
>
> Thanks for your time!!!
>
> On 3/12/2021 1:26 AM, Doug Anderson wrote:
>> Hi,
>>
>> On Thu, Mar 11, 2021 at 8:49 AM Srinivasa Rao Mandadapu
>> <srivasam@codeaurora.org> wrote:
>>> 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
>>> and secondary dai-links
>>>
>>> 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..501e3d4c9097 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>> @@ -9,6 +9,7 @@
>>>   #include <dt-bindings/input/gpio-keys.h>
>>>   #include <dt-bindings/input/input.h>
>>>   #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>>> +#include <dt-bindings/sound/qcom,lpass.h>
>> It seems marginally better to include "sc7180-lpass.h" to get this? I
>> don't really know the difference between the two but since unless
>> we're planning to delete the sc7180 version it seems like you might as
>> well include that one?
>
> Yes, I agree.
>
> Recently, all Quallcomm variant headers are combined and created 
> "qcom,lpass.h".
>
> "sc7180-lpass.h" still includes "qcom,lpass.h". So I will change and 
> repost the patch.
>
>
> Here is the reference commit:
>
>>
>>
>>>   /* PMICs depend on spmi_bus label and so must come after SoC */
>>>   #include "pm6150.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>;
>>> +                       };
>>> +
>>> +                       codec {
>>> +                               sound-dai = <&alc5682 MI2S_PRIMARY>;
>> I'm an audio noob but isn't "MI2S_PRIMARY" something to be used with
>> "lpass_cpu", not with "alc5682" ?
>>
>> I have no idea what the IDs correspond to on "alc5682". Are you sure
>> we even need an extra ID there? The "alc5682" bindings upstream don't
>> talk anything about dai-cells, but maybe they're just wrong...
> Yes. I will change and re-post.
Actually, without dai-cells getting compilation issues. So I think it's 
necessary here.
>>
>> -Doug
>
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-12 15:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 16:48 [PATCH v4 0/2] Qualcomm's lpass device tree changes for I2s dai Srinivasa Rao Mandadapu
2021-03-11 16:48 ` [PATCH v4 1/2] arm64: dts: qcom: sc7180-trogdor: Add lpass dai link for I2S driver Srinivasa Rao Mandadapu
2021-03-11 19:56   ` Doug Anderson
2021-03-12 13:02     ` Srinivasa Rao Mandadapu
2021-03-12 15:55       ` Srinivasa Rao Mandadapu
2021-03-11 16:48 ` [PATCH v4 2/2] arm64: dts: qcom: Add sound node for sc7180-trogdor-coachz Srinivasa Rao Mandadapu
2021-03-11 19:54   ` Doug Anderson
2021-03-12 15:53     ` Srinivasa Rao Mandadapu

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.