[PATCH v6 0/3] Add soundcard support for sc7280 based platforms.

[PATCH v6 0/3] Add soundcard support for sc7280 based platforms.

[Srinivasa Rao Mandadapu]

This patch set is to add bolero digital macros, WCD and maxim codecs nodes
for audio on sc7280 based platforms.

This patch set depends on:
    -- https://patchwork.kernel.org/project/linux-arm-msm/list/?series=625134
    -- https://patchwork.kernel.org/project/alsa-devel/list/?series=625078
    -- https://patchwork.kernel.org/project/linux-arm-msm/patch/20220202053207.14256-1-tdas@codeaurora.org/.
    -- https://patchwork.kernel.org/project/linux-arm-msm/list/?series=601249
    -- Clock reset control patches

Changes Since V5:
    -- Move soc specific bolero digital codec nodes to soc specific file.
    -- Bring wcd938x codec reset pin control and US/EURO HS selection nodes from other series.
    -- Change node name and remove redundant status property in sound node.
Changes Since V4:
    -- Update nodes in sorting order.
    -- Update DTS node names as per dt-bindings.
    -- Update Node properties in proper order.
    -- Update missing pinctrl properties like US/EURO HS selection, wcd reset control.
    -- Remove redundant labels.
    -- Remove unused size cells and address cells in tx macro node.
    -- Keep all same nodes at one place, which are defined in same file.
    -- Add max98360a codec node to herobrine board specific targets.
Changes Since V3:
    -- Move digital codec macro nodes to board specific dtsi file.
    -- Update pin controls in lpass cpu node.
    -- Update dependency patch list.
    -- Create patches on latest kernel.
Changes Since V2:
    -- Add power domains to digital codec macro nodes.
    -- Change clock node usage in lpass cpu node.
    -- Add codec mem clock to lpass cpu node.
    -- Modify the node names to be generic.
    -- Move sound and codec nodes to root node.
    -- sort dai links as per reg.
    -- Fix typo errors.
Changes Since V1:
    -- Update the commit message of cpu node patch.
    -- Add gpio control property to support Euro headset in wcd938x node.
    -- Fix clock properties in lpass cpu and digital codec macro node.

Srinivasa Rao Mandadapu (3):
  arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and
    external codecs
  arm64: dts: qcom: sc7280: Add lpass cpu node
  arm64: dts: qcom: sc7280: Add dt nodes for sound card

 arch/arm64/boot/dts/qcom/sc7280-crd.dts        |  14 ++
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   8 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 209 +++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi           | 184 ++++++++++++++++++++++
 4 files changed, 415 insertions(+)

-- 
2.7.4

[PATCH v6 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

[Srinivasa Rao Mandadapu]

SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
codecs like WCD938x, max98360a using soundwire masters and i2s bus.
Add these nodes for sc7280 based platforms audio use case.
Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280-crd.dts        |   6 ++
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   8 ++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       |  88 ++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi           | 121 +++++++++++++++++++++++++
 4 files changed, 223 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index e2efbdd..224a82d 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -84,6 +84,12 @@ ap_ts_pen_1v8: &i2c13 {
 	pins = "gpio51";
 };
 
+&wcd938x {
+	pinctrl-names = "default";
+	pinctrl-0 = <&us_euro_select>;
+	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
+};
+
 &tlmm {
 	tp_int_odl: tp-int-odl {
 		pins = "gpio7";
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index de646d9..c6a04c3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -20,6 +20,14 @@
 #include "sc7280-chrome-common.dtsi"
 
 / {
+	max98360a: audio-codec-0 {
+		compatible = "maxim,max98360a";
+		pinctrl-names = "default";
+		pinctrl-0 = <&amp_en>;
+		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
+		#sound-dai-cells = <0>;
+	};
+
 	chosen {
 		stdout-path = "serial0:115200n8";
 	};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index f912a89..83c76b2 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -20,6 +20,41 @@
 		serial1 = &uart7;
 	};
 
+	max98360a: audio-codec-0 {
+		compatible = "maxim,max98360a";
+		pinctrl-names = "default";
+		pinctrl-0 = <&amp_en>;
+		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
+		#sound-dai-cells = <0>;
+	};
+
+	wcd938x: audio-codec-1 {
+		compatible = "qcom,wcd9380-codec";
+		#sound-dai-cells = <1>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&wcd938x_reset_active>, <&wcd938x_reset_sleep>;
+		reset-gpios = <&tlmm 83 GPIO_ACTIVE_HIGH>;
+
+		qcom,rx-device = <&wcd_rx>;
+		qcom,tx-device = <&wcd_tx>;
+
+		vdd-rxtx-supply = <&vreg_l18b_1p8>;
+		vdd-io-supply = <&vreg_l18b_1p8>;
+		vdd-buck-supply = <&vreg_l17b_1p8>;
+		vdd-mic-bias-supply = <&vreg_bob>;
+
+		qcom,micbias1-microvolt = <1800000>;
+		qcom,micbias2-microvolt = <1800000>;
+		qcom,micbias3-microvolt = <1800000>;
+		qcom,micbias4-microvolt = <1800000>;
+
+		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000
+							  500000 500000 500000>;
+		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
+		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 		label = "gpio-keys";
@@ -373,6 +408,19 @@
 	status = "okay";
 };
 
+&rxmacro {
+	status = "okay";
+};
+
+&txmacro {
+	status = "okay";
+};
+
+&vamacro {
+	status = "okay";
+	vdd-micb-supply = <&vreg_bob>;
+};
+
 &sdhc_1 {
 	status = "okay";
 
@@ -574,6 +622,24 @@
 	drive-strength = <6>;
 };
 
+&swr0 {
+	wcd_rx: codec@0,4 {
+		compatible = "sdw20217010d00";
+		reg = <0 4>;
+		#sound-dai-cells = <1>;
+		qcom,rx-port-mapping = <1 2 3 4 5>;
+	};
+};
+
+&swr1 {
+	wcd_tx: codec@0,3 {
+		compatible = "sdw20217010d00";
+		reg = <0 3>;
+		#sound-dai-cells = <1>;
+		qcom,tx-port-mapping = <1 2 3 4>;
+	};
+};
+
 &tlmm {
 	amp_en: amp-en {
 		pins = "gpio63";
@@ -663,5 +729,27 @@
 		function = "gpio";
 		bias-pull-down;
 	};
+
+	us_euro_select: us-euro-select {
+		pins = "gpio81";
+		function = "gpio";
+		bias-pull-down;
+		drive-strength = <2>;
+	};
+
+	wcd938x_reset_active: wcd938x-reset-active {
+		pins = "gpio83";
+		function = "gpio";
+		drive-strength = <16>;
+		output-high;
+	};
+
+	wcd938x_reset_sleep: wcd938x-reset-sleep {
+		pins = "gpio83";
+		function = "gpio";
+		drive-strength = <16>;
+		bias-disable;
+		output-low;
+	};
 };
 
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index db74fc3..78ec84c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -822,6 +822,127 @@
 			#power-domain-cells = <1>;
 		};
 
+		rxmacro: codec@3200000 {
+			compatible = "qcom,sc7280-lpass-rx-macro";
+			reg = <0 0x03200000 0 0x1000>;
+
+			status = "disabled";
+			pinctrl-names = "default";
+			pinctrl-0 = <&rx_swr_active>;
+
+			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+				 <&vamacro>;
+			clock-names = "mclk", "npl", "fsgen";
+
+			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
+					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
+			power-domain-names ="macro", "dcodec";
+
+			#clock-cells = <0>;
+			#sound-dai-cells = <1>;
+		};
+
+		txmacro: codec@3220000 {
+			compatible = "qcom,sc7280-lpass-tx-macro";
+			reg = <0 0x03220000 0 0x1000>;
+
+			status = "disabled";
+			pinctrl-names = "default";
+			pinctrl-0 = <&tx_swr_active>;
+
+			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+				 <&vamacro>;
+			clock-names = "mclk", "npl", "fsgen";
+
+			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
+					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
+			power-domain-names ="macro", "dcodec";
+
+			#clock-cells = <0>;
+			#sound-dai-cells = <1>;
+		};
+
+		vamacro: codec@3370000 {
+			compatible = "qcom,sc7280-lpass-va-macro";
+			reg = <0 0x03370000 0 0x1000>;
+
+			status = "disabled";
+			pinctrl-0 = <&dmic01_active>;
+			pinctrl-names = "default";
+
+			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>;
+			clock-names = "mclk";
+
+			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
+					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
+			power-domain-names ="macro", "dcodec";
+
+			#clock-cells = <0>;
+			#sound-dai-cells = <1>;
+		};
+
+		swr0: soundwire@3210000 {
+			compatible = "qcom,soundwire-v1.6.0";
+			reg = <0 0x03210000 0 0x2000>;
+
+			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&rxmacro>;
+			clock-names = "iface";
+
+			qcom,din-ports = <0>;
+			qcom,dout-ports = <5>;
+
+			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
+			reset-names = "swr_audio_cgcr";
+
+			qcom,ports-word-length =	/bits/ 8 <0x01 0x07 0x04 0xff 0xff>;
+			qcom,ports-sinterval-low =	/bits/ 8 <0x03 0x3f 0x1f 0x03 0x03>;
+			qcom,ports-offset1 =		/bits/ 8 <0x00 0x00 0x0b 0x01 0x01>;
+			qcom,ports-offset2 =		/bits/ 8 <0x00 0x00 0x0b 0x00 0x00>;
+			qcom,ports-lane-control =	/bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
+			qcom,ports-block-pack-mode =	/bits/ 8 <0xff 0x00 0x01 0xff 0xff>;
+			qcom,ports-hstart =		/bits/ 8 <0xff 0x03 0xff 0xff 0xff>;
+			qcom,ports-hstop =		/bits/ 8 <0xff 0x06 0xff 0xff 0xff>;
+			qcom,ports-block-group-count =	/bits/ 8 <0xff 0xff 0xff 0xff 0x00>;
+
+			#sound-dai-cells = <1>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+		};
+
+		swr1: soundwire@3230000 {
+			compatible = "qcom,soundwire-v1.6.0";
+			reg = <0 0x03230000 0 0x2000>;
+
+			interrupts-extended = <&intc GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH>,
+					      <&pdc 130 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "swr_master_irq", "swr_wake_irq";
+			clocks = <&txmacro>;
+			clock-names = "iface";
+
+			qcom,din-ports = <3>;
+			qcom,dout-ports = <0>;
+
+			resets = <&lpass_audiocc LPASS_AUDIO_SWR_TX_CGCR>;
+			reset-names = "swr_audio_cgcr";
+
+			qcom,ports-sinterval-low =	/bits/ 8 <0x01 0x03 0x03>;
+			qcom,ports-offset1 =		/bits/ 8 <0x01 0x00 0x02>;
+			qcom,ports-offset2 =		/bits/ 8 <0x00 0x00 0x00>;
+			qcom,ports-hstart =		/bits/ 8 <0xff 0xff 0xff>;
+			qcom,ports-hstop =		/bits/ 8 <0xff 0xff 0xff>;
+			qcom,ports-word-length =	/bits/ 8 <0xff 0x0 0xff>;
+			qcom,ports-block-pack-mode =	/bits/ 8 <0xff 0xff 0xff>;
+			qcom,ports-block-group-count =	/bits/ 8 <0xff 0xff 0xff>;
+			qcom,ports-lane-control =	/bits/ 8 <0x00 0x01 0x00>;
+			qcom,port-offset = <1>;
+
+			#sound-dai-cells = <1>;
+			#address-cells = <2>;
+			#size-cells = <0>;
+		};
 		ipcc: mailbox@408000 {
 			compatible = "qcom,sc7280-ipcc", "qcom,ipcc";
 			reg = <0 0x00408000 0 0x1000>;
-- 
2.7.4

[PATCH v6 2/3] arm64: dts: qcom: sc7280: Add lpass cpu node

[Srinivasa Rao Mandadapu]

Add lpass cpu node for audio on sc7280 based platforms.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 28 ++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi     | 63 ++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 83c76b2..e3d8cbf 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -273,6 +273,34 @@
 	modem-init;
 };
 
+&lpass_cpu {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&sec_mi2s_data0>, <&sec_mi2s_sclk>, <&sec_mi2s_ws>;
+
+	mi2s-secondary@1 {
+		reg = <MI2S_SECONDARY>;
+		qcom,playback-sd-lines = <0>;
+	};
+
+	hdmi-primary@5 {
+		reg = <LPASS_DP_RX>;
+	};
+
+	wcd-rx@6 {
+		reg = <LPASS_CDC_DMA_RX0>;
+	};
+
+	wcd-tx@19 {
+		reg = <LPASS_CDC_DMA_TX3>;
+	};
+
+	va-tx@25 {
+		reg = <LPASS_CDC_DMA_VA_TX0>;
+	};
+};
+
 &lpass_tlmm {
 	dmic01_active: dmic01-active {
 		clk {
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 78ec84c..e808d53 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -19,6 +19,7 @@
 #include <dt-bindings/reset/qcom,sdm845-aoss.h>
 #include <dt-bindings/reset/qcom,sdm845-pdc.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
+#include <dt-bindings/sound/qcom,lpass.h>
 #include <dt-bindings/thermal/thermal.h>
 
 / {
@@ -2101,6 +2102,68 @@
 			#clock-cells = <1>;
 		};
 
+		lpass_cpu: audio@3987000 {
+			compatible = "qcom,sc7280-lpass-cpu";
+
+			reg = <0 0x03987000 0 0x68000>,
+			      <0 0x03b00000 0 0x29000>,
+			      <0 0x03260000 0 0xc000>,
+			      <0 0x03280000 0 0x29000>,
+			      <0 0x03340000 0 0x29000>,
+			      <0 0x0336c000 0 0x3000>;
+
+			reg-names = "lpass-hdmiif",
+				    "lpass-lpaif",
+				    "lpass-rxtx-cdc-dma-lpm",
+				    "lpass-rxtx-lpaif",
+				    "lpass-va-lpaif",
+				    "lpass-va-cdc-dma-lpm";
+
+			iommus = <&apps_smmu 0x1820 0>,
+				 <&apps_smmu 0x1821 0>,
+				 <&apps_smmu 0x1832 0>;
+			status = "disabled";
+
+			power-domains =	<&rpmhpd SC7280_LCX>;
+			power-domain-names = "lcx";
+			required-opps = <&rpmhpd_opp_nom>;
+
+			clocks = <&lpass_aon LPASS_AON_CC_AUDIO_HM_H_CLK>,
+				 <&lpass_audiocc LPASS_AUDIO_CC_EXT_MCLK0_CLK>,
+				 <&lpasscore LPASS_CORE_CC_SYSNOC_MPORT_CORE_CLK>,
+				 <&lpasscore LPASS_CORE_CC_EXT_IF0_IBIT_CLK>,
+				 <&lpasscore LPASS_CORE_CC_EXT_IF1_IBIT_CLK>,
+				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM_CLK>,
+				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM0_CLK>,
+				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
+				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM2_CLK>,
+				 <&lpass_aon LPASS_AON_CC_VA_MEM0_CLK>;
+			clock-names = "aon_cc_audio_hm_h",
+				      "audio_cc_ext_mclk0",
+				      "core_cc_sysnoc_mport_core",
+				      "core_cc_ext_if0_ibit",
+				      "core_cc_ext_if1_ibit",
+				      "audio_cc_codec_mem",
+				      "audio_cc_codec_mem0",
+				      "audio_cc_codec_mem1",
+				      "audio_cc_codec_mem2",
+				      "aon_cc_va_mem0";
+
+			#sound-dai-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
+
+			interrupt-names = "lpass-irq-lpaif",
+					  "lpass-irq-hdmi",
+					  "lpass-irq-vaif",
+					  "lpass-irq-rxtxif";
+		};
+
 		lpass_ag_noc: interconnect@3c40000 {
 			reg = <0 0x03c40000 0 0xf080>;
 			compatible = "qcom,sc7280-lpass-ag-noc";
-- 
2.7.4

[PATCH v6 3/3] arm64: dts: qcom: sc7280: Add dt nodes for sound card

[Srinivasa Rao Mandadapu]

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 <quic_srivasam@quicinc.com>
Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280-crd.dts  |  8 +++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index 224a82d..b1b968a 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -90,6 +90,14 @@ ap_ts_pen_1v8: &i2c13 {
 	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
 };
 
+&sound {
+	audio-routing =
+		"VA DMIC0", "MIC BIAS1",
+		"VA DMIC1", "MIC BIAS1",
+		"VA DMIC2", "MIC BIAS3",
+		"VA DMIC3", "MIC BIAS3";
+};
+
 &tlmm {
 	tp_int_odl: tp-int-odl {
 		pins = "gpio7";
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index e3d8cbf..45e1d82 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -84,6 +84,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 = "Secondary MI2S Playback";
+			reg = <MI2S_SECONDARY>;
+			cpu {
+				sound-dai = <&lpass_cpu MI2S_SECONDARY>;
+			};
+
+			codec {
+				sound-dai = <&max98360a>;
+			};
+		};
+
+		dai-link@5 {
+			link-name = "DP Playback";
+			reg = <LPASS_DP_RX>;
+			cpu {
+				sound-dai = <&lpass_cpu LPASS_DP_RX>;
+			};
+
+			codec {
+				sound-dai = <&mdss_dp>;
+			};
+		};
+
+		dai-link@6 {
+			link-name = "WCD Playback";
+			reg = <LPASS_CDC_DMA_RX0>;
+			cpu {
+				sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
+			};
+
+			codec {
+				sound-dai = <&wcd938x 0>, <&swr0 0>, <&rxmacro 0>;
+			};
+		};
+
+		dai-link@19 {
+			link-name = "WCD Capture";
+			reg = <LPASS_CDC_DMA_TX3>;
+			cpu {
+				sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
+			};
+
+			codec {
+				sound-dai = <&wcd938x 1>, <&swr1 0>, <&txmacro 0>;
+			};
+		};
+
+		dai-link@25 {
+			link-name = "DMIC Capture";
+			reg = <LPASS_CDC_DMA_VA_TX0>;
+			cpu {
+				sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
+			};
+
+			codec {
+				sound-dai = <&vamacro 0>;
+			};
+		};
+	};
 };
 
 &apps_rsc {
-- 
2.7.4

Re: [PATCH v6 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

[Matthias Kaehlcke]

On Tue, Apr 05, 2022 at 04:43:38PM +0530, Srinivasa Rao Mandadapu wrote:
> SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
> codecs like WCD938x, max98360a using soundwire masters and i2s bus.
> Add these nodes for sc7280 based platforms audio use case.
> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---

A change log would be helpful for reviewers

>  arch/arm64/boot/dts/qcom/sc7280-crd.dts        |   6 ++
>  arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   8 ++
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       |  88 ++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi           | 121 +++++++++++++++++++++++++
>  4 files changed, 223 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..224a82d 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts

You might want to consider splitting this patch into one that
adds the SoC specific bits and one for each board. It's cleaner
and might allow to land the SoC specific bits even when the board
specific part is still under discussion. It also might make the
life easier for folks who want to pick the SoC specific parts,
but aren't interested in the board bits (e.g. because they want
to support a different board). Just a thought, the split is not
strictly necessary.

> @@ -84,6 +84,12 @@ ap_ts_pen_1v8: &i2c13 {
>  	pins = "gpio51";
>  };
>  
> +&wcd938x {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&us_euro_select>;
> +	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
> +};
> +
>  &tlmm {
>  	tp_int_odl: tp-int-odl {
>  		pins = "gpio7";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index de646d9..c6a04c3 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -20,6 +20,14 @@
>  #include "sc7280-chrome-common.dtsi"
>  
>  / {
> +	max98360a: audio-codec-0 {
> +		compatible = "maxim,max98360a";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&amp_en>;
> +		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
> +		#sound-dai-cells = <0>;
> +	};
> +

This node shouldn't be at top but with the other device nodes, in
alphabetical order, i.e. before 'pwmleds'.

>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index f912a89..83c76b2 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -20,6 +20,41 @@
>  		serial1 = &uart7;
>  	};
>  
> +	max98360a: audio-codec-0 {
> +		compatible = "maxim,max98360a";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&amp_en>;
> +		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
> +		#sound-dai-cells = <0>;
> +	};
> +
> +	wcd938x: audio-codec-1 {
> +		compatible = "qcom,wcd9380-codec";
> +		#sound-dai-cells = <1>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wcd938x_reset_active>, <&wcd938x_reset_sleep>;
> +		reset-gpios = <&tlmm 83 GPIO_ACTIVE_HIGH>;
> +
> +		qcom,rx-device = <&wcd_rx>;
> +		qcom,tx-device = <&wcd_tx>;
> +
> +		vdd-rxtx-supply = <&vreg_l18b_1p8>;
> +		vdd-io-supply = <&vreg_l18b_1p8>;
> +		vdd-buck-supply = <&vreg_l17b_1p8>;
> +		vdd-mic-bias-supply = <&vreg_bob>;
> +
> +		qcom,micbias1-microvolt = <1800000>;
> +		qcom,micbias2-microvolt = <1800000>;
> +		qcom,micbias3-microvolt = <1800000>;
> +		qcom,micbias4-microvolt = <1800000>;
> +
> +		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000
> +							  500000 500000 500000>;
> +		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
> +		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
> +	};
> +
>  	gpio-keys {
>  		compatible = "gpio-keys";
>  		label = "gpio-keys";
> @@ -373,6 +408,19 @@
>  	status = "okay";
>  };
>  
> +&rxmacro {
> +	status = "okay";
> +};
> +
> +&txmacro {
> +	status = "okay";
> +};
> +
> +&vamacro {
> +	status = "okay";
> +	vdd-micb-supply = <&vreg_bob>;
> +};
> +
>  &sdhc_1 {
>  	status = "okay";
>  
> @@ -574,6 +622,24 @@
>  	drive-strength = <6>;
>  };
>  
> +&swr0 {
> +	wcd_rx: codec@0,4 {
> +		compatible = "sdw20217010d00";
> +		reg = <0 4>;
> +		#sound-dai-cells = <1>;
> +		qcom,rx-port-mapping = <1 2 3 4 5>;
> +	};
> +};
> +
> +&swr1 {
> +	wcd_tx: codec@0,3 {
> +		compatible = "sdw20217010d00";
> +		reg = <0 3>;
> +		#sound-dai-cells = <1>;
> +		qcom,tx-port-mapping = <1 2 3 4>;
> +	};
> +};
> +
>  &tlmm {
>  	amp_en: amp-en {
>  		pins = "gpio63";
> @@ -663,5 +729,27 @@
>  		function = "gpio";
>  		bias-pull-down;
>  	};
> +
> +	us_euro_select: us-euro-select {

nit: in the schematic this is called 'us_euro_hs_sel'. IMO it's preferable
to use the name from the schematic rather than inventing a new one, unless
there's a good reason for it.

> +		pins = "gpio81";
> +		function = "gpio";
> +		bias-pull-down;
> +		drive-strength = <2>;
> +	};
> +
> +	wcd938x_reset_active: wcd938x-reset-active {

same as above: the schematic calls this pin 'wcd_reset_n'

drop the -active suffix

> +		pins = "gpio83";
> +		function = "gpio";
> +		drive-strength = <16>;

A value of 16 seems pretty high. Is it really needed?

> +		output-high;
> +	};
> +
> +	wcd938x_reset_sleep: wcd938x-reset-sleep {
> +		pins = "gpio83";
> +		function = "gpio";
> +		drive-strength = <16>;

see above

> +		bias-disable;
> +		output-low;
> +	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index db74fc3..78ec84c 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -822,6 +822,127 @@
>  			#power-domain-cells = <1>;
>  		};
>  
> +		rxmacro: codec@3200000 {

These node are not at the correct position. They should be sorted by
address and hence be inserted between 'lpasscc@3000000' and
'interconnect@3c40000'.

> +			compatible = "qcom,sc7280-lpass-rx-macro";
> +			reg = <0 0x03200000 0 0x1000>;
> +
> +			status = "disabled";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&rx_swr_active>;
> +
> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
> +				 <&vamacro>;
> +			clock-names = "mclk", "npl", "fsgen";
> +
> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
> +			power-domain-names ="macro", "dcodec";
> +
> +			#clock-cells = <0>;
> +			#sound-dai-cells = <1>;
> +		};
> +
> +		txmacro: codec@3220000 {
> +			compatible = "qcom,sc7280-lpass-tx-macro";
> +			reg = <0 0x03220000 0 0x1000>;
> +
> +			status = "disabled";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&tx_swr_active>;
> +
> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
> +				 <&vamacro>;
> +			clock-names = "mclk", "npl", "fsgen";
> +
> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
> +			power-domain-names ="macro", "dcodec";
> +
> +			#clock-cells = <0>;
> +			#sound-dai-cells = <1>;
> +		};
> +
> +		vamacro: codec@3370000 {
> +			compatible = "qcom,sc7280-lpass-va-macro";
> +			reg = <0 0x03370000 0 0x1000>;
> +
> +			status = "disabled";
> +			pinctrl-0 = <&dmic01_active>;
> +			pinctrl-names = "default";
> +
> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>;
> +			clock-names = "mclk";
> +
> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
> +			power-domain-names ="macro", "dcodec";
> +
> +			#clock-cells = <0>;
> +			#sound-dai-cells = <1>;
> +		};
> +
> +		swr0: soundwire@3210000 {
> +			compatible = "qcom,soundwire-v1.6.0";
> +			reg = <0 0x03210000 0 0x2000>;
> +
> +			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&rxmacro>;
> +			clock-names = "iface";
> +
> +			qcom,din-ports = <0>;
> +			qcom,dout-ports = <5>;
> +
> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
> +			reset-names = "swr_audio_cgcr";
> +
> +			qcom,ports-word-length =	/bits/ 8 <0x01 0x07 0x04 0xff 0xff>;
> +			qcom,ports-sinterval-low =	/bits/ 8 <0x03 0x3f 0x1f 0x03 0x03>;
> +			qcom,ports-offset1 =		/bits/ 8 <0x00 0x00 0x0b 0x01 0x01>;
> +			qcom,ports-offset2 =		/bits/ 8 <0x00 0x00 0x0b 0x00 0x00>;
> +			qcom,ports-lane-control =	/bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
> +			qcom,ports-block-pack-mode =	/bits/ 8 <0xff 0x00 0x01 0xff 0xff>;
> +			qcom,ports-hstart =		/bits/ 8 <0xff 0x03 0xff 0xff 0xff>;
> +			qcom,ports-hstop =		/bits/ 8 <0xff 0x06 0xff 0xff 0xff>;
> +			qcom,ports-block-group-count =	/bits/ 8 <0xff 0xff 0xff 0xff 0x00>;
> +
> +			#sound-dai-cells = <1>;
> +			#address-cells = <2>;
> +			#size-cells = <0>;
> +		};
> +
> +		swr1: soundwire@3230000 {
> +			compatible = "qcom,soundwire-v1.6.0";
> +			reg = <0 0x03230000 0 0x2000>;
> +
> +			interrupts-extended = <&intc GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH>,
> +					      <&pdc 130 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "swr_master_irq", "swr_wake_irq";
> +			clocks = <&txmacro>;
> +			clock-names = "iface";
> +
> +			qcom,din-ports = <3>;
> +			qcom,dout-ports = <0>;
> +
> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_TX_CGCR>;
> +			reset-names = "swr_audio_cgcr";
> +
> +			qcom,ports-sinterval-low =	/bits/ 8 <0x01 0x03 0x03>;
> +			qcom,ports-offset1 =		/bits/ 8 <0x01 0x00 0x02>;
> +			qcom,ports-offset2 =		/bits/ 8 <0x00 0x00 0x00>;
> +			qcom,ports-hstart =		/bits/ 8 <0xff 0xff 0xff>;
> +			qcom,ports-hstop =		/bits/ 8 <0xff 0xff 0xff>;
> +			qcom,ports-word-length =	/bits/ 8 <0xff 0x0 0xff>;

s/0x0/0x00/

> +			qcom,ports-block-pack-mode =	/bits/ 8 <0xff 0xff 0xff>;
> +			qcom,ports-block-group-count =	/bits/ 8 <0xff 0xff 0xff>;
> +			qcom,ports-lane-control =	/bits/ 8 <0x00 0x01 0x00>;
> +			qcom,port-offset = <1>;
> +
> +			#sound-dai-cells = <1>;
> +			#address-cells = <2>;
> +			#size-cells = <0>;
> +		};

add empty line

>  		ipcc: mailbox@408000 {
>  			compatible = "qcom,sc7280-ipcc", "qcom,ipcc";
>  			reg = <0 0x00408000 0 0x1000>;

Re: [PATCH v6 2/3] arm64: dts: qcom: sc7280: Add lpass cpu node

[Matthias Kaehlcke]

On Tue, Apr 05, 2022 at 04:43:39PM +0530, Srinivasa Rao Mandadapu wrote:
> Add lpass cpu node for audio on sc7280 based platforms.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 28 ++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi     | 63 ++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 83c76b2..e3d8cbf 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -273,6 +273,34 @@
>  	modem-init;
>  };
>  
> +&lpass_cpu {
> +	status = "okay";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&sec_mi2s_data0>, <&sec_mi2s_sclk>, <&sec_mi2s_ws>;
> +
> +	mi2s-secondary@1 {

in line with an earlier comment, the primary/secondary terminology seem to
reflect how things are called in the datasheet (which only talks about a
primary/secondary master clock, otherwise about mi2sN). How about calling
these nodes 'dai-link@N' as in the binding (and a bunch of other
boards/SoCs)?

> +		reg = <MI2S_SECONDARY>;
> +		qcom,playback-sd-lines = <0>;
> +	};
> +
> +	hdmi-primary@5 {
> +		reg = <LPASS_DP_RX>;
> +	};
> +
> +	wcd-rx@6 {
> +		reg = <LPASS_CDC_DMA_RX0>;
> +	};
> +
> +	wcd-tx@19 {
> +		reg = <LPASS_CDC_DMA_TX3>;
> +	};
> +
> +	va-tx@25 {
> +		reg = <LPASS_CDC_DMA_VA_TX0>;
> +	};
> +};
> +
>  &lpass_tlmm {
>  	dmic01_active: dmic01-active {
>  		clk {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 78ec84c..e808d53 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -19,6 +19,7 @@
>  #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>  #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> +#include <dt-bindings/sound/qcom,lpass.h>
>  #include <dt-bindings/thermal/thermal.h>
>  
>  / {
> @@ -2101,6 +2102,68 @@
>  			#clock-cells = <1>;
>  		};
>  
> +		lpass_cpu: audio@3987000 {
> +			compatible = "qcom,sc7280-lpass-cpu";
> +
> +			reg = <0 0x03987000 0 0x68000>,
> +			      <0 0x03b00000 0 0x29000>,
> +			      <0 0x03260000 0 0xc000>,
> +			      <0 0x03280000 0 0x29000>,
> +			      <0 0x03340000 0 0x29000>,
> +			      <0 0x0336c000 0 0x3000>;
> +

Drop empty line, 'reg' and 'reg-names' belong together'.

> +			reg-names = "lpass-hdmiif",
> +				    "lpass-lpaif",
> +				    "lpass-rxtx-cdc-dma-lpm",
> +				    "lpass-rxtx-lpaif",
> +				    "lpass-va-lpaif",
> +				    "lpass-va-cdc-dma-lpm";
> +
> +			iommus = <&apps_smmu 0x1820 0>,
> +				 <&apps_smmu 0x1821 0>,
> +				 <&apps_smmu 0x1832 0>;
> +			status = "disabled";

Detter put this at the end as many other nodes do, rather than
'hiding' it inmidst of the hardware description.

> +
> +			power-domains =	<&rpmhpd SC7280_LCX>;
> +			power-domain-names = "lcx";
> +			required-opps = <&rpmhpd_opp_nom>;
> +
> +			clocks = <&lpass_aon LPASS_AON_CC_AUDIO_HM_H_CLK>,
> +				 <&lpass_audiocc LPASS_AUDIO_CC_EXT_MCLK0_CLK>,
> +				 <&lpasscore LPASS_CORE_CC_SYSNOC_MPORT_CORE_CLK>,
> +				 <&lpasscore LPASS_CORE_CC_EXT_IF0_IBIT_CLK>,
> +				 <&lpasscore LPASS_CORE_CC_EXT_IF1_IBIT_CLK>,
> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM_CLK>,
> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM0_CLK>,
> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM2_CLK>,
> +				 <&lpass_aon LPASS_AON_CC_VA_MEM0_CLK>;
> +			clock-names = "aon_cc_audio_hm_h",
> +				      "audio_cc_ext_mclk0",
> +				      "core_cc_sysnoc_mport_core",
> +				      "core_cc_ext_if0_ibit",
> +				      "core_cc_ext_if1_ibit",
> +				      "audio_cc_codec_mem",
> +				      "audio_cc_codec_mem0",
> +				      "audio_cc_codec_mem1",
> +				      "audio_cc_codec_mem2",
> +				      "aon_cc_va_mem0";
> +
> +			#sound-dai-cells = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
> +

drop empty line, 'interrupts' and 'interrupt-names' belong together.

> +			interrupt-names = "lpass-irq-lpaif",
> +					  "lpass-irq-hdmi",
> +					  "lpass-irq-vaif",
> +					  "lpass-irq-rxtxif";
> +		};
> +
>  		lpass_ag_noc: interconnect@3c40000 {
>  			reg = <0 0x03c40000 0 0xf080>;
>  			compatible = "qcom,sc7280-lpass-ag-noc";

Re: [PATCH v6 3/3] arm64: dts: qcom: sc7280: Add dt nodes for sound card

[Matthias Kaehlcke]

On Tue, Apr 05, 2022 at 04:43:40PM +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 <quic_srivasam@quicinc.com>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-crd.dts  |  8 +++
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index 224a82d..b1b968a 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -90,6 +90,14 @@ ap_ts_pen_1v8: &i2c13 {
>  	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>  };
>  
> +&sound {
> +	audio-routing =
> +		"VA DMIC0", "MIC BIAS1",
> +		"VA DMIC1", "MIC BIAS1",
> +		"VA DMIC2", "MIC BIAS3",
> +		"VA DMIC3", "MIC BIAS3";
> +};
> +
>  &tlmm {
>  	tp_int_odl: tp-int-odl {
>  		pins = "gpio7";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index e3d8cbf..45e1d82 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -84,6 +84,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 = "Secondary MI2S Playback";

The other link names provide information about the other end
of the link (DP, WCD, DMIC), while this one describes the SoC side.
Shouldn't this be "MAX98360A"? Not sure about the 'Playback' part,
it seems 'link-name' is used as stream name, judging from a few
samples of peeking into '/proc/asound/pcm' on different devices
it seems that 'Playback' or 'Capture' is ususally not part of the
stream name.

> +			reg = <MI2S_SECONDARY>;
> +			cpu {
> +				sound-dai = <&lpass_cpu MI2S_SECONDARY>;
> +			};
> +
> +			codec {
> +				sound-dai = <&max98360a>;
> +			};
> +		};
> +
> +		dai-link@5 {
> +			link-name = "DP Playback";

See comment above about 'Playback'. Just 'DP' is maybe a bit short, how
about 'DisplayPort'?

> +			reg = <LPASS_DP_RX>;
> +			cpu {
> +				sound-dai = <&lpass_cpu LPASS_DP_RX>;
> +			};
> +
> +			codec {
> +				sound-dai = <&mdss_dp>;
> +			};
> +		};
> +
> +		dai-link@6 {
> +			link-name = "WCD Playback";

Most instances I found spell out the codec name. It seems here we need
the 'Playback'/'Capture' info (or something else) to not end up with
duplicate link names. So my suggestion here would be "WCD9385 Playback".

> +			reg = <LPASS_CDC_DMA_RX0>;
> +			cpu {
> +				sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
> +			};
> +
> +			codec {
> +				sound-dai = <&wcd938x 0>, <&swr0 0>, <&rxmacro 0>;
> +			};
> +		};
> +
> +		dai-link@19 {
> +			link-name = "WCD Capture";

"WCD9385 Capture"?

> +			reg = <LPASS_CDC_DMA_TX3>;
> +			cpu {
> +				sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
> +			};
> +
> +			codec {
> +				sound-dai = <&wcd938x 1>, <&swr1 0>, <&txmacro 0>;
> +			};
> +		};
> +
> +		dai-link@25 {
> +			link-name = "DMIC Capture";


just "DMIC"?

Re: [PATCH v6 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

[Srinivasa Rao Mandadapu]

On 4/6/2022 3:05 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Tue, Apr 05, 2022 at 04:43:38PM +0530, Srinivasa Rao Mandadapu wrote:
>> SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
>> codecs like WCD938x, max98360a using soundwire masters and i2s bus.
>> Add these nodes for sc7280 based platforms audio use case.
>> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
> A change log would be helpful for reviewers
Change log added in cover letter.
>
>>   arch/arm64/boot/dts/qcom/sc7280-crd.dts        |   6 ++
>>   arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   8 ++
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       |  88 ++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi           | 121 +++++++++++++++++++++++++
>>   4 files changed, 223 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index e2efbdd..224a82d 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> You might want to consider splitting this patch into one that
> adds the SoC specific bits and one for each board. It's cleaner
> and might allow to land the SoC specific bits even when the board
> specific part is still under discussion. It also might make the
> life easier for folks who want to pick the SoC specific parts,
> but aren't interested in the board bits (e.g. because they want
> to support a different board). Just a thought, the split is not
> strictly necessary.
Okay. will split accordingly.
>
>> @@ -84,6 +84,12 @@ ap_ts_pen_1v8: &i2c13 {
>>   	pins = "gpio51";
>>   };
>>   
>> +&wcd938x {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&us_euro_select>;
>> +	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>> +};
>> +
>>   &tlmm {
>>   	tp_int_odl: tp-int-odl {
>>   		pins = "gpio7";
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> index de646d9..c6a04c3 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> @@ -20,6 +20,14 @@
>>   #include "sc7280-chrome-common.dtsi"
>>   
>>   / {
>> +	max98360a: audio-codec-0 {
>> +		compatible = "maxim,max98360a";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&amp_en>;
>> +		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
>> +		#sound-dai-cells = <0>;
>> +	};
>> +
> This node shouldn't be at top but with the other device nodes, in
> alphabetical order, i.e. before 'pwmleds'.

Actually we are sorting as per node name. Hence kept it here. As per 
previous reviewer comments, sorted accordingly.

Please suggest better approach.

>
>>   	chosen {
>>   		stdout-path = "serial0:115200n8";
>>   	};
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index f912a89..83c76b2 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -20,6 +20,41 @@
>>   		serial1 = &uart7;
>>   	};
>>   
>> +	max98360a: audio-codec-0 {
>> +		compatible = "maxim,max98360a";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&amp_en>;
>> +		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
>> +		#sound-dai-cells = <0>;
>> +	};
>> +
>> +	wcd938x: audio-codec-1 {
>> +		compatible = "qcom,wcd9380-codec";
>> +		#sound-dai-cells = <1>;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&wcd938x_reset_active>, <&wcd938x_reset_sleep>;
>> +		reset-gpios = <&tlmm 83 GPIO_ACTIVE_HIGH>;
>> +
>> +		qcom,rx-device = <&wcd_rx>;
>> +		qcom,tx-device = <&wcd_tx>;
>> +
>> +		vdd-rxtx-supply = <&vreg_l18b_1p8>;
>> +		vdd-io-supply = <&vreg_l18b_1p8>;
>> +		vdd-buck-supply = <&vreg_l17b_1p8>;
>> +		vdd-mic-bias-supply = <&vreg_bob>;
>> +
>> +		qcom,micbias1-microvolt = <1800000>;
>> +		qcom,micbias2-microvolt = <1800000>;
>> +		qcom,micbias3-microvolt = <1800000>;
>> +		qcom,micbias4-microvolt = <1800000>;
>> +
>> +		qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000
>> +							  500000 500000 500000>;
>> +		qcom,mbhc-headset-vthreshold-microvolt = <1700000>;
>> +		qcom,mbhc-headphone-vthreshold-microvolt = <50000>;
>> +	};
>> +
>>   	gpio-keys {
>>   		compatible = "gpio-keys";
>>   		label = "gpio-keys";
>> @@ -373,6 +408,19 @@
>>   	status = "okay";
>>   };
>>   
>> +&rxmacro {
>> +	status = "okay";
>> +};
>> +
>> +&txmacro {
>> +	status = "okay";
>> +};
>> +
>> +&vamacro {
>> +	status = "okay";
>> +	vdd-micb-supply = <&vreg_bob>;
>> +};
>> +
>>   &sdhc_1 {
>>   	status = "okay";
>>   
>> @@ -574,6 +622,24 @@
>>   	drive-strength = <6>;
>>   };
>>   
>> +&swr0 {
>> +	wcd_rx: codec@0,4 {
>> +		compatible = "sdw20217010d00";
>> +		reg = <0 4>;
>> +		#sound-dai-cells = <1>;
>> +		qcom,rx-port-mapping = <1 2 3 4 5>;
>> +	};
>> +};
>> +
>> +&swr1 {
>> +	wcd_tx: codec@0,3 {
>> +		compatible = "sdw20217010d00";
>> +		reg = <0 3>;
>> +		#sound-dai-cells = <1>;
>> +		qcom,tx-port-mapping = <1 2 3 4>;
>> +	};
>> +};
>> +
>>   &tlmm {
>>   	amp_en: amp-en {
>>   		pins = "gpio63";
>> @@ -663,5 +729,27 @@
>>   		function = "gpio";
>>   		bias-pull-down;
>>   	};
>> +
>> +	us_euro_select: us-euro-select {
> nit: in the schematic this is called 'us_euro_hs_sel'. IMO it's preferable
> to use the name from the schematic rather than inventing a new one, unless
> there's a good reason for it.
Okay. will change accordingly.
>
>> +		pins = "gpio81";
>> +		function = "gpio";
>> +		bias-pull-down;
>> +		drive-strength = <2>;
>> +	};
>> +
>> +	wcd938x_reset_active: wcd938x-reset-active {
> same as above: the schematic calls this pin 'wcd_reset_n'
>
> drop the -active suffix
Okay. will change accordingly.
>
>> +		pins = "gpio83";
>> +		function = "gpio";
>> +		drive-strength = <16>;
> A value of 16 seems pretty high. Is it really needed?
Okay. Verified with drive strength 8. will change accordingly.
>
>> +		output-high;
>> +	};
>> +
>> +	wcd938x_reset_sleep: wcd938x-reset-sleep {
>> +		pins = "gpio83";
>> +		function = "gpio";
>> +		drive-strength = <16>;
> see above
okay.
>
>> +		bias-disable;
>> +		output-low;
>> +	};
>>   };
>>   
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index db74fc3..78ec84c 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -822,6 +822,127 @@
>>   			#power-domain-cells = <1>;
>>   		};
>>   
>> +		rxmacro: codec@3200000 {
> These node are not at the correct position. They should be sorted by
> address and hence be inserted between 'lpasscc@3000000' and
> 'interconnect@3c40000'.

Actually we are sorting as per node(codec) name. Hence kept it here. As 
per previous reviewer comments, sorted accordingly.

Please suggest better approach.

>
>> +			compatible = "qcom,sc7280-lpass-rx-macro";
>> +			reg = <0 0x03200000 0 0x1000>;
>> +
>> +			status = "disabled";
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&rx_swr_active>;
>> +
>> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
>> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
>> +				 <&vamacro>;
>> +			clock-names = "mclk", "npl", "fsgen";
>> +
>> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
>> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
>> +			power-domain-names ="macro", "dcodec";
>> +
>> +			#clock-cells = <0>;
>> +			#sound-dai-cells = <1>;
>> +		};
>> +
>> +		txmacro: codec@3220000 {
>> +			compatible = "qcom,sc7280-lpass-tx-macro";
>> +			reg = <0 0x03220000 0 0x1000>;
>> +
>> +			status = "disabled";
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&tx_swr_active>;
>> +
>> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
>> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
>> +				 <&vamacro>;
>> +			clock-names = "mclk", "npl", "fsgen";
>> +
>> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
>> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
>> +			power-domain-names ="macro", "dcodec";
>> +
>> +			#clock-cells = <0>;
>> +			#sound-dai-cells = <1>;
>> +		};
>> +
>> +		vamacro: codec@3370000 {
>> +			compatible = "qcom,sc7280-lpass-va-macro";
>> +			reg = <0 0x03370000 0 0x1000>;
>> +
>> +			status = "disabled";
>> +			pinctrl-0 = <&dmic01_active>;
>> +			pinctrl-names = "default";
>> +
>> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>;
>> +			clock-names = "mclk";
>> +
>> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
>> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
>> +			power-domain-names ="macro", "dcodec";
>> +
>> +			#clock-cells = <0>;
>> +			#sound-dai-cells = <1>;
>> +		};
>> +
>> +		swr0: soundwire@3210000 {
>> +			compatible = "qcom,soundwire-v1.6.0";
>> +			reg = <0 0x03210000 0 0x2000>;
>> +
>> +			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&rxmacro>;
>> +			clock-names = "iface";
>> +
>> +			qcom,din-ports = <0>;
>> +			qcom,dout-ports = <5>;
>> +
>> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
>> +			reset-names = "swr_audio_cgcr";
>> +
>> +			qcom,ports-word-length =	/bits/ 8 <0x01 0x07 0x04 0xff 0xff>;
>> +			qcom,ports-sinterval-low =	/bits/ 8 <0x03 0x3f 0x1f 0x03 0x03>;
>> +			qcom,ports-offset1 =		/bits/ 8 <0x00 0x00 0x0b 0x01 0x01>;
>> +			qcom,ports-offset2 =		/bits/ 8 <0x00 0x00 0x0b 0x00 0x00>;
>> +			qcom,ports-lane-control =	/bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
>> +			qcom,ports-block-pack-mode =	/bits/ 8 <0xff 0x00 0x01 0xff 0xff>;
>> +			qcom,ports-hstart =		/bits/ 8 <0xff 0x03 0xff 0xff 0xff>;
>> +			qcom,ports-hstop =		/bits/ 8 <0xff 0x06 0xff 0xff 0xff>;
>> +			qcom,ports-block-group-count =	/bits/ 8 <0xff 0xff 0xff 0xff 0x00>;
>> +
>> +			#sound-dai-cells = <1>;
>> +			#address-cells = <2>;
>> +			#size-cells = <0>;
>> +		};
>> +
>> +		swr1: soundwire@3230000 {
>> +			compatible = "qcom,soundwire-v1.6.0";
>> +			reg = <0 0x03230000 0 0x2000>;
>> +
>> +			interrupts-extended = <&intc GIC_SPI 496 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&pdc 130 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "swr_master_irq", "swr_wake_irq";
>> +			clocks = <&txmacro>;
>> +			clock-names = "iface";
>> +
>> +			qcom,din-ports = <3>;
>> +			qcom,dout-ports = <0>;
>> +
>> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_TX_CGCR>;
>> +			reset-names = "swr_audio_cgcr";
>> +
>> +			qcom,ports-sinterval-low =	/bits/ 8 <0x01 0x03 0x03>;
>> +			qcom,ports-offset1 =		/bits/ 8 <0x01 0x00 0x02>;
>> +			qcom,ports-offset2 =		/bits/ 8 <0x00 0x00 0x00>;
>> +			qcom,ports-hstart =		/bits/ 8 <0xff 0xff 0xff>;
>> +			qcom,ports-hstop =		/bits/ 8 <0xff 0xff 0xff>;
>> +			qcom,ports-word-length =	/bits/ 8 <0xff 0x0 0xff>;
> s/0x0/0x00/
okay.
>
>> +			qcom,ports-block-pack-mode =	/bits/ 8 <0xff 0xff 0xff>;
>> +			qcom,ports-block-group-count =	/bits/ 8 <0xff 0xff 0xff>;
>> +			qcom,ports-lane-control =	/bits/ 8 <0x00 0x01 0x00>;
>> +			qcom,port-offset = <1>;
>> +
>> +			#sound-dai-cells = <1>;
>> +			#address-cells = <2>;
>> +			#size-cells = <0>;
>> +		};
> add empty line
Okay. will remove it.
>
>>   		ipcc: mailbox@408000 {
>>   			compatible = "qcom,sc7280-ipcc", "qcom,ipcc";
>>   			reg = <0 0x00408000 0 0x1000>;

Re: [PATCH v6 2/3] arm64: dts: qcom: sc7280: Add lpass cpu node

[Srinivasa Rao Mandadapu]

On 4/6/2022 4:55 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Tue, Apr 05, 2022 at 04:43:39PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add lpass cpu node for audio on sc7280 based platforms.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 28 ++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi     | 63 ++++++++++++++++++++++++++++++++
>>   2 files changed, 91 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index 83c76b2..e3d8cbf 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -273,6 +273,34 @@
>>   	modem-init;
>>   };
>>   
>> +&lpass_cpu {
>> +	status = "okay";
>> +
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&sec_mi2s_data0>, <&sec_mi2s_sclk>, <&sec_mi2s_ws>;
>> +
>> +	mi2s-secondary@1 {
> in line with an earlier comment, the primary/secondary terminology seem to
> reflect how things are called in the datasheet (which only talks about a
> primary/secondary master clock, otherwise about mi2sN). How about calling
> these nodes 'dai-link@N' as in the binding (and a bunch of other
> boards/SoCs)?
Okay. will change accordingly.
>
>> +		reg = <MI2S_SECONDARY>;
>> +		qcom,playback-sd-lines = <0>;
>> +	};
>> +
>> +	hdmi-primary@5 {
>> +		reg = <LPASS_DP_RX>;
>> +	};
>> +
>> +	wcd-rx@6 {
>> +		reg = <LPASS_CDC_DMA_RX0>;
>> +	};
>> +
>> +	wcd-tx@19 {
>> +		reg = <LPASS_CDC_DMA_TX3>;
>> +	};
>> +
>> +	va-tx@25 {
>> +		reg = <LPASS_CDC_DMA_VA_TX0>;
>> +	};
>> +};
>> +
>>   &lpass_tlmm {
>>   	dmic01_active: dmic01-active {
>>   		clk {
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 78ec84c..e808d53 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -19,6 +19,7 @@
>>   #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>>   #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>>   #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>> +#include <dt-bindings/sound/qcom,lpass.h>
>>   #include <dt-bindings/thermal/thermal.h>
>>   
>>   / {
>> @@ -2101,6 +2102,68 @@
>>   			#clock-cells = <1>;
>>   		};
>>   
>> +		lpass_cpu: audio@3987000 {
>> +			compatible = "qcom,sc7280-lpass-cpu";
>> +
>> +			reg = <0 0x03987000 0 0x68000>,
>> +			      <0 0x03b00000 0 0x29000>,
>> +			      <0 0x03260000 0 0xc000>,
>> +			      <0 0x03280000 0 0x29000>,
>> +			      <0 0x03340000 0 0x29000>,
>> +			      <0 0x0336c000 0 0x3000>;
>> +
> Drop empty line, 'reg' and 'reg-names' belong together'.
Okay.
>
>> +			reg-names = "lpass-hdmiif",
>> +				    "lpass-lpaif",
>> +				    "lpass-rxtx-cdc-dma-lpm",
>> +				    "lpass-rxtx-lpaif",
>> +				    "lpass-va-lpaif",
>> +				    "lpass-va-cdc-dma-lpm";
>> +
>> +			iommus = <&apps_smmu 0x1820 0>,
>> +				 <&apps_smmu 0x1821 0>,
>> +				 <&apps_smmu 0x1832 0>;
>> +			status = "disabled";
> Detter put this at the end as many other nodes do, rather than
> 'hiding' it inmidst of the hardware description.
Okay. will do accordingly.
>
>> +
>> +			power-domains =	<&rpmhpd SC7280_LCX>;
>> +			power-domain-names = "lcx";
>> +			required-opps = <&rpmhpd_opp_nom>;
>> +
>> +			clocks = <&lpass_aon LPASS_AON_CC_AUDIO_HM_H_CLK>,
>> +				 <&lpass_audiocc LPASS_AUDIO_CC_EXT_MCLK0_CLK>,
>> +				 <&lpasscore LPASS_CORE_CC_SYSNOC_MPORT_CORE_CLK>,
>> +				 <&lpasscore LPASS_CORE_CC_EXT_IF0_IBIT_CLK>,
>> +				 <&lpasscore LPASS_CORE_CC_EXT_IF1_IBIT_CLK>,
>> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM_CLK>,
>> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM0_CLK>,
>> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM1_CLK>,
>> +				 <&lpass_audiocc LPASS_AUDIO_CC_CODEC_MEM2_CLK>,
>> +				 <&lpass_aon LPASS_AON_CC_VA_MEM0_CLK>;
>> +			clock-names = "aon_cc_audio_hm_h",
>> +				      "audio_cc_ext_mclk0",
>> +				      "core_cc_sysnoc_mport_core",
>> +				      "core_cc_ext_if0_ibit",
>> +				      "core_cc_ext_if1_ibit",
>> +				      "audio_cc_codec_mem",
>> +				      "audio_cc_codec_mem0",
>> +				      "audio_cc_codec_mem1",
>> +				      "audio_cc_codec_mem2",
>> +				      "aon_cc_va_mem0";
>> +
>> +			#sound-dai-cells = <1>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			interrupts = <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
>> +
> drop empty line, 'interrupts' and 'interrupt-names' belong together.
Okay.
>
>> +			interrupt-names = "lpass-irq-lpaif",
>> +					  "lpass-irq-hdmi",
>> +					  "lpass-irq-vaif",
>> +					  "lpass-irq-rxtxif";
>> +		};
>> +
>>   		lpass_ag_noc: interconnect@3c40000 {
>>   			reg = <0 0x03c40000 0 0xf080>;
>>   			compatible = "qcom,sc7280-lpass-ag-noc";

Re: [PATCH v6 3/3] arm64: dts: qcom: sc7280: Add dt nodes for sound card

[Srinivasa Rao Mandadapu]

On 4/7/2022 2:15 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Tue, Apr 05, 2022 at 04:43:40PM +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 <quic_srivasam@quicinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-crd.dts  |  8 +++
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++++
>>   2 files changed, 101 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index 224a82d..b1b968a 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> @@ -90,6 +90,14 @@ ap_ts_pen_1v8: &i2c13 {
>>   	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>>   };
>>   
>> +&sound {
>> +	audio-routing =
>> +		"VA DMIC0", "MIC BIAS1",
>> +		"VA DMIC1", "MIC BIAS1",
>> +		"VA DMIC2", "MIC BIAS3",
>> +		"VA DMIC3", "MIC BIAS3";
>> +};
>> +
>>   &tlmm {
>>   	tp_int_odl: tp-int-odl {
>>   		pins = "gpio7";
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index e3d8cbf..45e1d82 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -84,6 +84,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 = "Secondary MI2S Playback";
> The other link names provide information about the other end
> of the link (DP, WCD, DMIC), while this one describes the SoC side.
> Shouldn't this be "MAX98360A"? Not sure about the 'Playback' part,
> it seems 'link-name' is used as stream name, judging from a few
> samples of peeking into '/proc/asound/pcm' on different devices
> it seems that 'Playback' or 'Capture' is ususally not part of the
> stream name.
Okay. will change accordingly.
>
>> +			reg = <MI2S_SECONDARY>;
>> +			cpu {
>> +				sound-dai = <&lpass_cpu MI2S_SECONDARY>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&max98360a>;
>> +			};
>> +		};
>> +
>> +		dai-link@5 {
>> +			link-name = "DP Playback";
> See comment above about 'Playback'. Just 'DP' is maybe a bit short, how
> about 'DisplayPort'?
Okay. Will change accordingly.
>
>> +			reg = <LPASS_DP_RX>;
>> +			cpu {
>> +				sound-dai = <&lpass_cpu LPASS_DP_RX>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&mdss_dp>;
>> +			};
>> +		};
>> +
>> +		dai-link@6 {
>> +			link-name = "WCD Playback";
> Most instances I found spell out the codec name. It seems here we need
> the 'Playback'/'Capture' info (or something else) to not end up with
> duplicate link names. So my suggestion here would be "WCD9385 Playback".
Okay. Will  update accordingly.
>
>> +			reg = <LPASS_CDC_DMA_RX0>;
>> +			cpu {
>> +				sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&wcd938x 0>, <&swr0 0>, <&rxmacro 0>;
>> +			};
>> +		};
>> +
>> +		dai-link@19 {
>> +			link-name = "WCD Capture";
> "WCD9385 Capture"?
Okay.
>
>> +			reg = <LPASS_CDC_DMA_TX3>;
>> +			cpu {
>> +				sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&wcd938x 1>, <&swr1 0>, <&txmacro 0>;
>> +			};
>> +		};
>> +
>> +		dai-link@25 {
>> +			link-name = "DMIC Capture";
>
> just "DMIC"?
Okay.

Re: [PATCH v6 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

[Matthias Kaehlcke]

On Mon, Apr 11, 2022 at 07:32:33PM +0530, Srinivasa Rao Mandadapu wrote:
> 
> On 4/6/2022 3:05 AM, Matthias Kaehlcke wrote:
> Thanks for your time Matthias!!!
> > On Tue, Apr 05, 2022 at 04:43:38PM +0530, Srinivasa Rao Mandadapu wrote:
> > > SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
> > > codecs like WCD938x, max98360a using soundwire masters and i2s bus.
> > > Add these nodes for sc7280 based platforms audio use case.
> > > Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
> > > 
> > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> > > Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> > > Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
> > > ---
> > A change log would be helpful for reviewers
> Change log added in cover letter.

From my perspective as a reviewer I find it more practical to have a
change log per patch.

> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> > > index de646d9..c6a04c3 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> > > @@ -20,6 +20,14 @@
> > >   #include "sc7280-chrome-common.dtsi"
> > >   / {
> > > +	max98360a: audio-codec-0 {
> > > +		compatible = "maxim,max98360a";
> > > +		pinctrl-names = "default";
> > > +		pinctrl-0 = <&amp_en>;
> > > +		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
> > > +		#sound-dai-cells = <0>;
> > > +	};
> > > +
> > This node shouldn't be at top but with the other device nodes, in
> > alphabetical order, i.e. before 'pwmleds'.
> 
> Actually we are sorting as per node name. Hence kept it here. As per
> previous reviewer comments, sorted accordingly.
> 
> Please suggest better approach.

True, I forgot the node names are used for sorting, not the labels.

> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > index db74fc3..78ec84c 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > > @@ -822,6 +822,127 @@
> > >   			#power-domain-cells = <1>;
> > >   		};
> > > +		rxmacro: codec@3200000 {
> > These node are not at the correct position. They should be sorted by
> > address and hence be inserted between 'lpasscc@3000000' and
> > 'interconnect@3c40000'.
> 
> Actually we are sorting as per node(codec) name. Hence kept it here. As per
> previous reviewer comments, sorted accordingly.

Could you provide a pointer to those comments?

My understanding is that we are sorting by node name when nodes don't have
addresses or for overrides in board files/snippets, however the nodes under
'soc@0' are sorted by address.

Another nit: 'rx_macro, tx_macro, va_macro' instead of the labels without
underscore? Not really important, but maybe slightly more readable and
would match the 'spelling' of the compatible strings.

Re: [PATCH v6 1/3] arm64: dts: qcom: sc7280: Add nodes for va tx and rx macros and external codecs

[Srinivasa Rao Mandadapu]

On 4/12/2022 1:23 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Mon, Apr 11, 2022 at 07:32:33PM +0530, Srinivasa Rao Mandadapu wrote:
>> On 4/6/2022 3:05 AM, Matthias Kaehlcke wrote:
>> Thanks for your time Matthias!!!
>>> On Tue, Apr 05, 2022 at 04:43:38PM +0530, Srinivasa Rao Mandadapu wrote:
>>>> SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
>>>> codecs like WCD938x, max98360a using soundwire masters and i2s bus.
>>>> Add these nodes for sc7280 based platforms audio use case.
>>>> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>>>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com>
>>>> ---
>>> A change log would be helpful for reviewers
>> Change log added in cover letter.
>  From my perspective as a reviewer I find it more practical to have a
> change log per patch.
Okay. will do accordingly from next time.
>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> index de646d9..c6a04c3 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> @@ -20,6 +20,14 @@
>>>>    #include "sc7280-chrome-common.dtsi"
>>>>    / {
>>>> +	max98360a: audio-codec-0 {
>>>> +		compatible = "maxim,max98360a";
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&amp_en>;
>>>> +		sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>;
>>>> +		#sound-dai-cells = <0>;
>>>> +	};
>>>> +
>>> This node shouldn't be at top but with the other device nodes, in
>>> alphabetical order, i.e. before 'pwmleds'.
>> Actually we are sorting as per node name. Hence kept it here. As per
>> previous reviewer comments, sorted accordingly.
>>
>> Please suggest better approach.
> True, I forgot the node names are used for sorting, not the labels.
Okay.
>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>> index db74fc3..78ec84c 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>>> @@ -822,6 +822,127 @@
>>>>    			#power-domain-cells = <1>;
>>>>    		};
>>>> +		rxmacro: codec@3200000 {
>>> These node are not at the correct position. They should be sorted by
>>> address and hence be inserted between 'lpasscc@3000000' and
>>> 'interconnect@3c40000'.
>> Actually we are sorting as per node(codec) name. Hence kept it here. As per
>> previous reviewer comments, sorted accordingly.
> Could you provide a pointer to those comments?
Sorry. it seems we misunderstood.
>
> My understanding is that we are sorting by node name when nodes don't have
> addresses or for overrides in board files/snippets, however the nodes under
> 'soc@0' are sorted by address.
>
> Another nit: 'rx_macro, tx_macro, va_macro' instead of the labels without
> underscore? Not really important, but maybe slightly more readable and
> would match the 'spelling' of the compatible strings.
Okay. Will change accordingly!!!