2022-04-18 15:44:25

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH v8 4/4] arm64: dts: qcom: sc7280: Add dt nodes for sound card

Add dt nodes for sound card support, which is using WCD938x headset
playback, capture, I2S speaker playback and DMICs via VA macro.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Co-developed-by: Venkata Prasad Potturu <[email protected]>
Signed-off-by: Venkata Prasad Potturu <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts | 23 ++++++++
arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
index aa0bf6e2..bf15bbe 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
@@ -87,6 +87,29 @@ ap_ts_pen_1v8: &i2c13 {
pins = "gpio51";
};

+&sound {
+ audio-routing =
+ "IN1_HPHL", "HPHL_OUT",
+ "IN2_HPHR", "HPHR_OUT",
+ "AMIC1", "MIC BIAS1",
+ "AMIC2", "MIC BIAS2",
+ "VA DMIC0", "MIC BIAS1",
+ "VA DMIC1", "MIC BIAS1",
+ "VA DMIC2", "MIC BIAS3",
+ "VA DMIC3", "MIC BIAS3",
+ "TX SWR_ADC0", "ADC1_OUTPUT",
+ "TX SWR_ADC1", "ADC2_OUTPUT",
+ "TX SWR_ADC2", "ADC3_OUTPUT",
+ "TX SWR_DMIC0", "DMIC1_OUTPUT",
+ "TX SWR_DMIC1", "DMIC2_OUTPUT",
+ "TX SWR_DMIC2", "DMIC3_OUTPUT",
+ "TX SWR_DMIC3", "DMIC4_OUTPUT",
+ "TX SWR_DMIC4", "DMIC5_OUTPUT",
+ "TX SWR_DMIC5", "DMIC6_OUTPUT",
+ "TX SWR_DMIC6", "DMIC7_OUTPUT",
+ "TX SWR_DMIC7", "DMIC8_OUTPUT";
+};
+
&wcd938x {
pinctrl-names = "default";
pinctrl-0 = <&us_euro_hs_sel>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index e880837..640b1338 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -85,6 +85,99 @@
pinctrl-names = "default";
pinctrl-0 = <&nvme_pwren>;
};
+
+ sound: sound {
+ compatible = "google,sc7280-herobrine";
+ model = "sc7280-wcd938x-max98360a-1mic";
+
+ audio-routing =
+ "IN1_HPHL", "HPHL_OUT",
+ "IN2_HPHR", "HPHR_OUT",
+ "AMIC1", "MIC BIAS1",
+ "AMIC2", "MIC BIAS2",
+ "VA DMIC0", "MIC BIAS3",
+ "VA DMIC1", "MIC BIAS3",
+ "VA DMIC2", "MIC BIAS1",
+ "VA DMIC3", "MIC BIAS1",
+ "TX SWR_ADC0", "ADC1_OUTPUT",
+ "TX SWR_ADC1", "ADC2_OUTPUT",
+ "TX SWR_ADC2", "ADC3_OUTPUT",
+ "TX SWR_DMIC0", "DMIC1_OUTPUT",
+ "TX SWR_DMIC1", "DMIC2_OUTPUT",
+ "TX SWR_DMIC2", "DMIC3_OUTPUT",
+ "TX SWR_DMIC3", "DMIC4_OUTPUT",
+ "TX SWR_DMIC4", "DMIC5_OUTPUT",
+ "TX SWR_DMIC5", "DMIC6_OUTPUT",
+ "TX SWR_DMIC6", "DMIC7_OUTPUT",
+ "TX SWR_DMIC7", "DMIC8_OUTPUT";
+
+ qcom,msm-mbhc-hphl-swh = <1>;
+ qcom,msm-mbhc-gnd-swh = <1>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #sound-dai-cells = <0>;
+
+ dai-link@1 {
+ link-name = "MAX98360A";
+
+ cpu {
+ sound-dai = <&lpass_cpu MI2S_SECONDARY>;
+ };
+
+ codec {
+ sound-dai = <&max98360a>;
+ };
+ };
+
+ dai-link@5 {
+ link-name = "DisplayPort";
+
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_DP_RX>;
+ };
+
+ codec {
+ sound-dai = <&mdss_dp>;
+ };
+ };
+
+ dai-link@6 {
+ link-name = "WCD9385 Playback";
+
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
+ };
+
+ codec {
+ sound-dai = <&wcd938x 0>, <&swr0 0>, <&lpass_rx_macro 0>;
+ };
+ };
+
+ dai-link@19 {
+ link-name = "WCD9385 Capture";
+
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
+ };
+
+ codec {
+ sound-dai = <&wcd938x 1>, <&swr1 0>, <&lpass_tx_macro 0>;
+ };
+ };
+
+ dai-link@25 {
+ link-name = "DMIC";
+
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
+ };
+
+ codec {
+ sound-dai = <&lpass_va_macro 0>;
+ };
+ };
+ };
};

&apps_rsc {
--
2.7.4


2022-04-19 12:42:25

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v8 4/4] arm64: dts: qcom: sc7280: Add dt nodes for sound card

On Mon, Apr 18, 2022 at 07:44:12PM +0530, Srinivasa Rao Mandadapu wrote:
> Add dt nodes for sound card support, which is using WCD938x headset
> playback, capture, I2S speaker playback and DMICs via VA macro.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Co-developed-by: Venkata Prasad Potturu <[email protected]>
> Signed-off-by: Venkata Prasad Potturu <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts | 23 ++++++++
> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
> index aa0bf6e2..bf15bbe 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
> @@ -87,6 +87,29 @@ ap_ts_pen_1v8: &i2c13 {
> pins = "gpio51";
> };
>
> +&sound {
> + audio-routing =
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "AMIC1", "MIC BIAS1",
> + "AMIC2", "MIC BIAS2",
> + "VA DMIC0", "MIC BIAS1",
> + "VA DMIC1", "MIC BIAS1",
> + "VA DMIC2", "MIC BIAS3",
> + "VA DMIC3", "MIC BIAS3",
> + "TX SWR_ADC0", "ADC1_OUTPUT",
> + "TX SWR_ADC1", "ADC2_OUTPUT",
> + "TX SWR_ADC2", "ADC3_OUTPUT",
> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
> +};

Should this also be added to sc7280-herobrine-crd.dts?

> +
> &wcd938x {
> pinctrl-names = "default";
> pinctrl-0 = <&us_euro_hs_sel>;
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index e880837..640b1338 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -85,6 +85,99 @@
> pinctrl-names = "default";
> pinctrl-0 = <&nvme_pwren>;
> };
> +
> + sound: sound {
> + compatible = "google,sc7280-herobrine";
> + model = "sc7280-wcd938x-max98360a-1mic";
> +
> + audio-routing =
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "AMIC1", "MIC BIAS1",
> + "AMIC2", "MIC BIAS2",
> + "VA DMIC0", "MIC BIAS3",
> + "VA DMIC1", "MIC BIAS3",
> + "VA DMIC2", "MIC BIAS1",
> + "VA DMIC3", "MIC BIAS1",
> + "TX SWR_ADC0", "ADC1_OUTPUT",
> + "TX SWR_ADC1", "ADC2_OUTPUT",
> + "TX SWR_ADC2", "ADC3_OUTPUT",
> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
> +
> + qcom,msm-mbhc-hphl-swh = <1>;
> + qcom,msm-mbhc-gnd-swh = <1>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #sound-dai-cells = <0>;
> +
> + dai-link@1 {

The '@1' is the address of the lpass CPU DAI, which doesn't seem correct
here. As I brought up on v7 [1] I think this value isn't even necessarily
unique, a SoC could have multiple IP blocks with audio buses, each with
their own DAI address space. The binding (currently) requires an
'address'/id, rather than using the CPU DAI id I suggest to enumerate the
links linearly, starting with 0.

[1] https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/

> + link-name = "MAX98360A";

The binding requires a 'reg' property, even though it isn't used (also
discussed on v7). I think the 'reg' property should be removed from the
binding and the DTs that use it, but maybe that should be done in a
separate series. In the meantime the value should match that of the
node.

> +
> + cpu {
> + sound-dai = <&lpass_cpu MI2S_SECONDARY>;
> + };
> +
> + codec {
> + sound-dai = <&max98360a>;
> + };
> + };
> +
> + dai-link@5 {
> + link-name = "DisplayPort";
> +
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_DP_RX>;
> + };
> +
> + codec {
> + sound-dai = <&mdss_dp>;
> + };
> + };
> +
> + dai-link@6 {
> + link-name = "WCD9385 Playback";
> +
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
> + };
> +
> + codec {
> + sound-dai = <&wcd938x 0>, <&swr0 0>, <&lpass_rx_macro 0>;
> + };
> + };
> +
> + dai-link@19 {
> + link-name = "WCD9385 Capture";
> +
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
> + };
> +
> + codec {
> + sound-dai = <&wcd938x 1>, <&swr1 0>, <&lpass_tx_macro 0>;
> + };
> + };
> +
> + dai-link@25 {
> + link-name = "DMIC";
> +
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
> + };
> +
> + codec {
> + sound-dai = <&lpass_va_macro 0>;
> + };
> + };
> + };
> };
>
> &apps_rsc {
> --
> 2.7.4
>

2022-04-20 21:47:20

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: Re: [PATCH v8 4/4] arm64: dts: qcom: sc7280: Add dt nodes for sound card


On 4/18/2022 11:32 PM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Mon, Apr 18, 2022 at 07:44:12PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add dt nodes for sound card support, which is using WCD938x headset
>> playback, capture, I2S speaker playback and DMICs via VA macro.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Co-developed-by: Venkata Prasad Potturu <[email protected]>
>> Signed-off-by: Venkata Prasad Potturu <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts | 23 ++++++++
>> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++
>> 2 files changed, 116 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
>> index aa0bf6e2..bf15bbe 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts
>> @@ -87,6 +87,29 @@ ap_ts_pen_1v8: &i2c13 {
>> pins = "gpio51";
>> };
>>
>> +&sound {
>> + audio-routing =
>> + "IN1_HPHL", "HPHL_OUT",
>> + "IN2_HPHR", "HPHR_OUT",
>> + "AMIC1", "MIC BIAS1",
>> + "AMIC2", "MIC BIAS2",
>> + "VA DMIC0", "MIC BIAS1",
>> + "VA DMIC1", "MIC BIAS1",
>> + "VA DMIC2", "MIC BIAS3",
>> + "VA DMIC3", "MIC BIAS3",
>> + "TX SWR_ADC0", "ADC1_OUTPUT",
>> + "TX SWR_ADC1", "ADC2_OUTPUT",
>> + "TX SWR_ADC2", "ADC3_OUTPUT",
>> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
>> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
>> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
>> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
>> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
>> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
>> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
>> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
>> +};
> Should this also be added to sc7280-herobrine-crd.dts?
Okay. Will add it corresponding dts file.
>
>> +
>> &wcd938x {
>> pinctrl-names = "default";
>> pinctrl-0 = <&us_euro_hs_sel>;
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index e880837..640b1338 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -85,6 +85,99 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&nvme_pwren>;
>> };
>> +
>> + sound: sound {
>> + compatible = "google,sc7280-herobrine";
>> + model = "sc7280-wcd938x-max98360a-1mic";
>> +
>> + audio-routing =
>> + "IN1_HPHL", "HPHL_OUT",
>> + "IN2_HPHR", "HPHR_OUT",
>> + "AMIC1", "MIC BIAS1",
>> + "AMIC2", "MIC BIAS2",
>> + "VA DMIC0", "MIC BIAS3",
>> + "VA DMIC1", "MIC BIAS3",
>> + "VA DMIC2", "MIC BIAS1",
>> + "VA DMIC3", "MIC BIAS1",
>> + "TX SWR_ADC0", "ADC1_OUTPUT",
>> + "TX SWR_ADC1", "ADC2_OUTPUT",
>> + "TX SWR_ADC2", "ADC3_OUTPUT",
>> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
>> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
>> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
>> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
>> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
>> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
>> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
>> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
>> +
>> + qcom,msm-mbhc-hphl-swh = <1>;
>> + qcom,msm-mbhc-gnd-swh = <1>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + #sound-dai-cells = <0>;
>> +
>> + dai-link@1 {
> The '@1' is the address of the lpass CPU DAI, which doesn't seem correct
> here. As I brought up on v7 [1] I think this value isn't even necessarily
> unique, a SoC could have multiple IP blocks with audio buses, each with
> their own DAI address space. The binding (currently) requires an
> 'address'/id, rather than using the CPU DAI id I suggest to enumerate the
> links linearly, starting with 0.
Okay. Will update accordingly.
>
> [1] https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/
>
>> + link-name = "MAX98360A";
> The binding requires a 'reg' property, even though it isn't used (also
> discussed on v7). I think the 'reg' property should be removed from the
> binding and the DTs that use it, but maybe that should be done in a
> separate series. In the meantime the value should match that of the
> node.
Okay. for now will revert this change. and will take care your
suggestion in a separate patch set.
>
>> +
>> + cpu {
>> + sound-dai = <&lpass_cpu MI2S_SECONDARY>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&max98360a>;
>> + };
>> + };
>> +
>> + dai-link@5 {
>> + link-name = "DisplayPort";
>> +
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_DP_RX>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&mdss_dp>;
>> + };
>> + };
>> +
>> + dai-link@6 {
>> + link-name = "WCD9385 Playback";
>> +
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&wcd938x 0>, <&swr0 0>, <&lpass_rx_macro 0>;
>> + };
>> + };
>> +
>> + dai-link@19 {
>> + link-name = "WCD9385 Capture";
>> +
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&wcd938x 1>, <&swr1 0>, <&lpass_tx_macro 0>;
>> + };
>> + };
>> +
>> + dai-link@25 {
>> + link-name = "DMIC";
>> +
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&lpass_va_macro 0>;
>> + };
>> + };
>> + };
>> };
>>
>> &apps_rsc {
>> --
>> 2.7.4
>>