[PATCH v7 0/4] Add soundcard support for sc7280 based platforms.

[PATCH v7 0/4] 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=631506
    -- 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 V6:
    -- Modify link-names and audio routing in a sound node.
    -- Move amp_en pin control node to appropriate consumer patch.
    -- Split patches as per digital macro codecs and board specific codecs and sort it.
    -- Modify label and node names to lpass specific.
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 (4):
  arm64: dts: qcom: sc7280: Add nodes for soundwire and va tx rx digital
    macro codecs
  arm64: dts: qcom: sc7280: Add nodes for wcd9385 and max98360a codec
  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        |  29 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |   8 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 215 +++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi           | 184 +++++++++++++++++++++
 4 files changed, 436 insertions(+)

-- 
2.7.4

[PATCH v7 1/4] arm64: dts: qcom: sc7280: Add nodes for soundwire and va tx rx digital macro codecs

[Srinivasa Rao Mandadapu]

SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
external codecs using soundwire masters. Add these nodes for sc7280 based
platforms audio use case.

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.dtsi | 122 +++++++++++++++++++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 50fea0e..c0f127f 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1980,6 +1980,128 @@
 			#clock-cells = <1>;
 		};
 
+		lpass_rx_macro: codec@3200000 {
+			compatible = "qcom,sc7280-lpass-rx-macro";
+			reg = <0 0x03200000 0 0x1000>;
+
+			status = "disabled";
+			pinctrl-names = "default";
+			pinctrl-0 = <&lpass_rx_swr>;
+
+			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+				 <&lpass_va_macro>;
+			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>;
+		};
+
+		swr0: soundwire@3210000 {
+			compatible = "qcom,soundwire-v1.6.0";
+			reg = <0 0x03210000 0 0x2000>;
+
+			interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&lpass_rx_macro>;
+			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>;
+		};
+
+		lpass_tx_macro: codec@3220000 {
+			compatible = "qcom,sc7280-lpass-tx-macro";
+			reg = <0 0x03220000 0 0x1000>;
+
+			status = "disabled";
+			pinctrl-names = "default";
+			pinctrl-0 = <&lpass_tx_swr>;
+
+			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+				 <&lpass_va_macro>;
+			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>;
+		};
+
+		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 = <&lpass_tx_macro>;
+			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 0x00 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>;
+		};
+
+		lpass_va_macro: codec@3370000 {
+			compatible = "qcom,sc7280-lpass-va-macro";
+			reg = <0 0x03370000 0 0x1000>;
+
+			status = "disabled";
+			pinctrl-0 = <&lpass_dmic01>;
+			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>;
+		};
+
 		lpass_ag_noc: interconnect@3c40000 {
 			reg = <0 0x03c40000 0 0xf080>;
 			compatible = "qcom,sc7280-lpass-ag-noc";
-- 
2.7.4

Re: [PATCH v7 1/4] arm64: dts: qcom: sc7280: Add nodes for soundwire and va tx rx digital macro codecs

[Matthias Kaehlcke]

On Wed, Apr 13, 2022 at 08:51:14PM +0530, Srinivasa Rao Mandadapu wrote:
> SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
> external codecs using soundwire masters. Add these nodes for sc7280 based
> platforms audio use case.
> 
> 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.dtsi | 122 +++++++++++++++++++++++++++++++++++
>  1 file changed, 122 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 50fea0e..c0f127f 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1980,6 +1980,128 @@
>  			#clock-cells = <1>;
>  		};
>  
> +		lpass_rx_macro: codec@3200000 {
> +			compatible = "qcom,sc7280-lpass-rx-macro";
> +			reg = <0 0x03200000 0 0x1000>;
> +
> +			status = "disabled";

nit: this is easily missed here inmidst of the other stuff, best place
it at the end of the node as many other nodes do.

> +			pinctrl-names = "default";
> +			pinctrl-0 = <&lpass_rx_swr>;
> +
> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
> +				 <&lpass_va_macro>;
> +			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";

add space after '='.

> +
> +			#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 = <&lpass_rx_macro>;
> +			clock-names = "iface";
> +
> +			qcom,din-ports = <0>;
> +			qcom,dout-ports = <5>;
> +
> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
> +			reset-names = "swr_audio_cgcr";

The resets aren't mentioned in the binding, should they be added?

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

in difference to the other nodes the soundwire ones don't have
'status = "disabled"', should they be disabled by default?

> +		};
> +
> +		lpass_tx_macro: codec@3220000 {
> +			compatible = "qcom,sc7280-lpass-tx-macro";
> +			reg = <0 0x03220000 0 0x1000>;
> +
> +			status = "disabled";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&lpass_tx_swr>;
> +
> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
> +				 <&lpass_va_macro>;
> +			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";

add space after '='

> +
> +			#clock-cells = <0>;
> +			#sound-dai-cells = <1>;
> +		};
> +
> +		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";

From the binding

  - interrupt-names:
        Usage: Optional
        Value type: boolean
        Value type: <stringlist>
        Definition: should be "core" for core and "wakeup" for wake interrupt.

Does the binding need an update?

> +			clocks = <&lpass_tx_macro>;
> +			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 0x00 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>;
> +		};
> +
> +		lpass_va_macro: codec@3370000 {
> +			compatible = "qcom,sc7280-lpass-va-macro";
> +			reg = <0 0x03370000 0 0x1000>;
> +
> +			status = "disabled";
> +			pinctrl-0 = <&lpass_dmic01>;
> +			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";

add space after '='.

> +
> +			#clock-cells = <0>;
> +			#sound-dai-cells = <1>;
> +		};
> +
>  		lpass_ag_noc: interconnect@3c40000 {
>  			reg = <0 0x03c40000 0 0xf080>;
>  			compatible = "qcom,sc7280-lpass-ag-noc";
> -- 
> 2.7.4
> 

Re: [PATCH v7 1/4] arm64: dts: qcom: sc7280: Add nodes for soundwire and va tx rx digital macro codecs

[Srinivasa Rao Mandadapu]

On 4/14/2022 12:24 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Wed, Apr 13, 2022 at 08:51:14PM +0530, Srinivasa Rao Mandadapu wrote:
>> SC7280 has VA, TX and RX macros with SoundWire Controllers to attach with
>> external codecs using soundwire masters. Add these nodes for sc7280 based
>> platforms audio use case.
>>
>> 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.dtsi | 122 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 122 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 50fea0e..c0f127f 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -1980,6 +1980,128 @@
>>   			#clock-cells = <1>;
>>   		};
>>   
>> +		lpass_rx_macro: codec@3200000 {
>> +			compatible = "qcom,sc7280-lpass-rx-macro";
>> +			reg = <0 0x03200000 0 0x1000>;
>> +
>> +			status = "disabled";
> nit: this is easily missed here inmidst of the other stuff, best place
> it at the end of the node as many other nodes do.
Okay. will place accordingly.
>
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&lpass_rx_swr>;
>> +
>> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
>> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
>> +				 <&lpass_va_macro>;
>> +			clock-names = "mclk", "npl", "fsgen";
>> +
>> +			power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,p
>> +					<&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
>> +			power-domain-names ="macro", "dcodec";
> add space after '='.
Okay.
>
>> +
>> +			#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 = <&lpass_rx_macro>;
>> +			clock-names = "iface";
>> +
>> +			qcom,din-ports = <0>;
>> +			qcom,dout-ports = <5>;
>> +
>> +			resets = <&lpass_audiocc LPASS_AUDIO_SWR_RX_CGCR>;
>> +			reset-names = "swr_audio_cgcr";
> The resets aren't mentioned in the binding, should they be added?
Yes. They are to be added. Mentioned same in the cover letter dependencies.
>
>> +
>> +			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>;
> in difference to the other nodes the soundwire ones don't have
> 'status = "disabled"', should they be disabled by default?
Okay. will add accordingly.
>
>> +		};
>> +
>> +		lpass_tx_macro: codec@3220000 {
>> +			compatible = "qcom,sc7280-lpass-tx-macro";
>> +			reg = <0 0x03220000 0 0x1000>;
>> +
>> +			status = "disabled";
>> +			pinctrl-names = "default";
>> +			pinctrl-0 = <&lpass_tx_swr>;
>> +
>> +			clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
>> +				 <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
>> +				 <&lpass_va_macro>;
>> +			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";
> add space after '='
Okay.
>
>> +
>> +			#clock-cells = <0>;
>> +			#sound-dai-cells = <1>;
>> +		};
>> +
>> +		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";
>  From the binding
>
>    - interrupt-names:
>          Usage: Optional
>          Value type: boolean
>          Value type: <stringlist>
>          Definition: should be "core" for core and "wakeup" for wake interrupt.
>
> Does the binding need an update?
Here interrupt names are not being used. Will remove here.
>
>> +			clocks = <&lpass_tx_macro>;
>> +			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 0x00 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>;
>> +		};
>> +
>> +		lpass_va_macro: codec@3370000 {
>> +			compatible = "qcom,sc7280-lpass-va-macro";
>> +			reg = <0 0x03370000 0 0x1000>;
>> +
>> +			status = "disabled";
>> +			pinctrl-0 = <&lpass_dmic01>;
>> +			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";
> add space after '='.
Okay.
>
>> +
>> +			#clock-cells = <0>;
>> +			#sound-dai-cells = <1>;
>> +		};
>> +
>>   		lpass_ag_noc: interconnect@3c40000 {
>>   			reg = <0 0x03c40000 0 0xf080>;
>>   			compatible = "qcom,sc7280-lpass-ag-noc";
>> -- 
>> 2.7.4
>>

[PATCH v7 2/4] arm64: dts: qcom: sc7280: Add nodes for wcd9385 and max98360a codec

[Srinivasa Rao Mandadapu]

Add wcd938x and  max98360a codecs for audio use case on
sc7280 based platforms.
Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
Add amp_en node for  max98360a codec.

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       | 94 ++++++++++++++++++++++++++
 3 files changed, 108 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index e2efbdd..b944366 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_hs_sel>;
+	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 dc17f20..8e4f822 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 110d9e9..6004c08 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,wcd9385-codec";
+		#sound-dai-cells = <1>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&wcd_reset_n>, <&wcd_reset_n_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";
@@ -238,6 +273,19 @@
 	modem-init;
 };
 
+&lpass_rx_macro {
+	status = "okay";
+};
+
+&lpass_tx_macro {
+	status = "okay";
+};
+
+&lpass_va_macro {
+	status = "okay";
+	vdd-micb-supply = <&vreg_bob>;
+};
+
 &pcie1 {
 	status = "okay";
 	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
@@ -298,6 +346,24 @@
 	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
 };
 
+&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>;
+	};
+};
+
 &uart5 {
 	compatible = "qcom,geni-debug-uart";
 	status = "okay";
@@ -561,6 +627,12 @@
 };
 
 &tlmm {
+	amp_en: amp-en {
+		pins = "gpio63";
+		bias-pull-down;
+		drive-strength = <2>;
+	};
+
 	bt_en: bt-en {
 		pins = "gpio85";
 		function = "gpio";
@@ -643,5 +715,27 @@
 		function = "gpio";
 		bias-pull-down;
 	};
+
+	us_euro_hs_sel: us-euro-hs-sel {
+		pins = "gpio81";
+		function = "gpio";
+		bias-pull-down;
+		drive-strength = <2>;
+	};
+
+	wcd_reset_n: wcd-reset-n {
+		pins = "gpio83";
+		function = "gpio";
+		drive-strength = <8>;
+		output-high;
+	};
+
+	wcd_reset_n_sleep: wcd-reset-n-sleep {
+		pins = "gpio83";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-disable;
+		output-low;
+	};
 };
 
-- 
2.7.4

Re: [PATCH v7 2/4] arm64: dts: qcom: sc7280: Add nodes for wcd9385 and max98360a codec

[Matthias Kaehlcke]

On Wed, Apr 13, 2022 at 08:51:15PM +0530, Srinivasa Rao Mandadapu wrote:
> Add wcd938x and  max98360a codecs for audio use case on
> sc7280 based platforms.
> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
> Add amp_en node for  max98360a codec.
> 
> 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>
> ---

Applicable to all you patches: a per-patch change log would be really
nice for reviewers. It can help them to focus on the parts that have
actually changed with respect to earlier revisions they might have
already reviewed.

>  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       | 94 ++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..b944366 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts

Your branch needs a refresh, this file doesn't exist anymore.

You'll want to change sc7280-crd-r3.dts for the CRD <= 2.x and
sc7280-herobrine-crd.dts for the CRD >= 3.x

> @@ -84,6 +84,12 @@ ap_ts_pen_1v8: &i2c13 {
>  	pins = "gpio51";
>  };
>  
> +&wcd938x {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&us_euro_hs_sel>;
> +	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 dc17f20..8e4f822 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>;
> +	};
> +

I agreed earlier that sorting by node name is correct, but that wasn't
entirely true. The nodes should be sorted by name within the 'device'
section, i.e. after the comment "/* BOARD-SPECIFIC TOP LEVEL NODES */".

>  	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 110d9e9..6004c08 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,wcd9385-codec";
> +		#sound-dai-cells = <1>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wcd_reset_n>, <&wcd_reset_n_sleep>;

Shouldn't this be:

		pinctrl-names = "default", "sleep";
		pinctrl-0 = <&wcd_reset_n>;
		pinctrl-1 = <&wcd_reset_n_sleep>;

?

nit: make the ordering of properties (pinctrl, #sound-dai-cells) and
spacing within the node consistent for max98360a and wcd938x? Not
super-important, but since both are codecs and added by the same
patch, why not :)

> +		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";
> @@ -238,6 +273,19 @@
>  	modem-init;
>  };
>  
> +&lpass_rx_macro {
> +	status = "okay";
> +};
> +
> +&lpass_tx_macro {
> +	status = "okay";
> +};
> +
> +&lpass_va_macro {
> +	status = "okay";
> +	vdd-micb-supply = <&vreg_bob>;
> +};
> +
>  &pcie1 {
>  	status = "okay";
>  	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
> @@ -298,6 +346,24 @@
>  	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
>  };
>  
> +&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>;
> +	};
> +};
> +
>  &uart5 {
>  	compatible = "qcom,geni-debug-uart";
>  	status = "okay";
> @@ -561,6 +627,12 @@
>  };
>  
>  &tlmm {
> +	amp_en: amp-en {
> +		pins = "gpio63";
> +		bias-pull-down;
> +		drive-strength = <2>;
> +	};
> +
>  	bt_en: bt-en {
>  		pins = "gpio85";
>  		function = "gpio";
> @@ -643,5 +715,27 @@
>  		function = "gpio";
>  		bias-pull-down;
>  	};
> +
> +	us_euro_hs_sel: us-euro-hs-sel {
> +		pins = "gpio81";
> +		function = "gpio";
> +		bias-pull-down;
> +		drive-strength = <2>;
> +	};
> +
> +	wcd_reset_n: wcd-reset-n {
> +		pins = "gpio83";
> +		function = "gpio";
> +		drive-strength = <8>;
> +		output-high;
> +	};
> +
> +	wcd_reset_n_sleep: wcd-reset-n-sleep {
> +		pins = "gpio83";
> +		function = "gpio";
> +		drive-strength = <8>;
> +		bias-disable;
> +		output-low;
> +	};
>  };
>  
> -- 
> 2.7.4
> 

Re: [PATCH v7 2/4] arm64: dts: qcom: sc7280: Add nodes for wcd9385 and max98360a codec

[Srinivasa Rao Mandadapu]

On 4/14/2022 1:05 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Wed, Apr 13, 2022 at 08:51:15PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add wcd938x and  max98360a codecs for audio use case on
>> sc7280 based platforms.
>> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset.
>> Add amp_en node for  max98360a codec.
>>
>> 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>
>> ---
> Applicable to all you patches: a per-patch change log would be really
> nice for reviewers. It can help them to focus on the parts that have
> actually changed with respect to earlier revisions they might have
> already reviewed.

Okay. Understood. But we have been updating change log in cover letter, 
it would be difficult to segregate now.

Next time onward will take care.

>
>>   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       | 94 ++++++++++++++++++++++++++
>>   3 files changed, 108 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index e2efbdd..b944366 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> Your branch needs a refresh, this file doesn't exist anymore.
>
> You'll want to change sc7280-crd-r3.dts for the CRD <= 2.x and
> sc7280-herobrine-crd.dts for the CRD >= 3.x
Okay. Will do accordingly.
>
>> @@ -84,6 +84,12 @@ ap_ts_pen_1v8: &i2c13 {
>>   	pins = "gpio51";
>>   };
>>   
>> +&wcd938x {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&us_euro_hs_sel>;
>> +	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 dc17f20..8e4f822 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>;
>> +	};
>> +
> I agreed earlier that sorting by node name is correct, but that wasn't
> entirely true. The nodes should be sorted by name within the 'device'
> section, i.e. after the comment "/* BOARD-SPECIFIC TOP LEVEL NODES */".
Okay. will sort accordingly.
>
>>   	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 110d9e9..6004c08 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,wcd9385-codec";
>> +		#sound-dai-cells = <1>;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&wcd_reset_n>, <&wcd_reset_n_sleep>;
> Shouldn't this be:
>
> 		pinctrl-names = "default", "sleep";
> 		pinctrl-0 = <&wcd_reset_n>;
> 		pinctrl-1 = <&wcd_reset_n_sleep>;
>
> ?
Okay. Will update accordingly.
>
> nit: make the ordering of properties (pinctrl, #sound-dai-cells) and
> spacing within the node consistent for max98360a and wcd938x? Not
> super-important, but since both are codecs and added by the same
> patch, why not :)
Okay. will maintain accordingly.
>
>> +		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";
>> @@ -238,6 +273,19 @@
>>   	modem-init;
>>   };
>>   
>> +&lpass_rx_macro {
>> +	status = "okay";
>> +};
>> +
>> +&lpass_tx_macro {
>> +	status = "okay";
>> +};
>> +
>> +&lpass_va_macro {
>> +	status = "okay";
>> +	vdd-micb-supply = <&vreg_bob>;
>> +};
>> +
>>   &pcie1 {
>>   	status = "okay";
>>   	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
>> @@ -298,6 +346,24 @@
>>   	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
>>   };
>>   
>> +&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>;
>> +	};
>> +};
>> +
>>   &uart5 {
>>   	compatible = "qcom,geni-debug-uart";
>>   	status = "okay";
>> @@ -561,6 +627,12 @@
>>   };
>>   
>>   &tlmm {
>> +	amp_en: amp-en {
>> +		pins = "gpio63";
>> +		bias-pull-down;
>> +		drive-strength = <2>;
>> +	};
>> +
>>   	bt_en: bt-en {
>>   		pins = "gpio85";
>>   		function = "gpio";
>> @@ -643,5 +715,27 @@
>>   		function = "gpio";
>>   		bias-pull-down;
>>   	};
>> +
>> +	us_euro_hs_sel: us-euro-hs-sel {
>> +		pins = "gpio81";
>> +		function = "gpio";
>> +		bias-pull-down;
>> +		drive-strength = <2>;
>> +	};
>> +
>> +	wcd_reset_n: wcd-reset-n {
>> +		pins = "gpio83";
>> +		function = "gpio";
>> +		drive-strength = <8>;
>> +		output-high;
>> +	};
>> +
>> +	wcd_reset_n_sleep: wcd-reset-n-sleep {
>> +		pins = "gpio83";
>> +		function = "gpio";
>> +		drive-strength = <8>;
>> +		bias-disable;
>> +		output-low;
>> +	};
>>   };
>>   
>> -- 
>> 2.7.4
>>

[PATCH v7 3/4] 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     | 62 ++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 6004c08..cf62d06 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 = <&mi2s1_data0>, <&mi2s1_sclk>, <&mi2s1_ws>;
+
+	dai-link@1 {
+		reg = <MI2S_SECONDARY>;
+		qcom,playback-sd-lines = <0>;
+	};
+
+	dai-link@5 {
+		reg = <LPASS_DP_RX>;
+	};
+
+	dai-link@6 {
+		reg = <LPASS_CDC_DMA_RX0>;
+	};
+
+	dai-link@19 {
+		reg = <LPASS_CDC_DMA_TX3>;
+	};
+
+	dai-link@25 {
+		reg = <LPASS_CDC_DMA_VA_TX0>;
+	};
+};
+
 &lpass_rx_macro {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index c0f127f..69cb78f 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>
 
 / {
@@ -2102,6 +2103,67 @@
 			#sound-dai-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>;
+
+			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";
+
+			status = "disabled";
+		};
+
 		lpass_ag_noc: interconnect@3c40000 {
 			reg = <0 0x03c40000 0 0xf080>;
 			compatible = "qcom,sc7280-lpass-ag-noc";
-- 
2.7.4

Re: [PATCH v7 3/4] arm64: dts: qcom: sc7280: Add lpass cpu node

[Matthias Kaehlcke]

On Wed, Apr 13, 2022 at 08:51:16PM +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>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v7 3/4] arm64: dts: qcom: sc7280: Add lpass cpu node

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-04-13 08:21:16)
> 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>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

[PATCH v7 4/4] 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  | 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.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index b944366..1e16854 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
 	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
 };
 
+&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";
+};
+
 &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 cf62d06..a7c884a 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 = "MAX98360A";
+			reg = <MI2S_SECONDARY>;
+			cpu {
+				sound-dai = <&lpass_cpu MI2S_SECONDARY>;
+			};
+
+			codec {
+				sound-dai = <&max98360a>;
+			};
+		};
+
+		dai-link@5 {
+			link-name = "DisplayPort";
+			reg = <LPASS_DP_RX>;
+			cpu {
+				sound-dai = <&lpass_cpu LPASS_DP_RX>;
+			};
+
+			codec {
+				sound-dai = <&mdss_dp>;
+			};
+		};
+
+		dai-link@6 {
+			link-name = "WCD9385 Playback";
+			reg = <LPASS_CDC_DMA_RX0>;
+			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";
+			reg = <LPASS_CDC_DMA_TX3>;
+			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";
+			reg = <LPASS_CDC_DMA_VA_TX0>;
+			cpu {
+				sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
+			};
+
+			codec {
+				sound-dai = <&lpass_va_macro 0>;
+			};
+		};
+	};
 };
 
 &apps_rsc {
-- 
2.7.4

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

[Matthias Kaehlcke]

On Wed, Apr 13, 2022 at 08:51:17PM +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  | 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.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index b944366..1e16854 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts

You need to refresh your tree, this file has been renamed to
sc7280-crd-r3.dts. That DT is for the CRD <= 2.x, newer versions
use sc7280-herobrine-crd.dts.

> @@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
>  	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>  };
>  
> +&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";
> +};
> +
>  &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 cf62d06..a7c884a 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 = "MAX98360A";
> +			reg = <MI2S_SECONDARY>;

Dumb question: is this value actually used? A quick glance through
qcom_snd_parse_of() suggests it isn't. And the CPU DAI id is already
specified in the 'sound-dai' property below.

In a quick test I replaced the corresponding 'reg' values in
sc7180-trogdor.dtsi with 'random' values and audio playback on
my coachz (sc7180-trogdor-coachz-r3.dts) still works ...

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

nit: add an empty line (in all links) to separate the properties from the node

> +			cpu {
> +				sound-dai = <&lpass_cpu LPASS_DP_RX>;
> +			};
> +
> +			codec {
> +				sound-dai = <&mdss_dp>;
> +			};
> +		};
> +
> +		dai-link@6 {
> +			link-name = "WCD9385 Playback";
> +			reg = <LPASS_CDC_DMA_RX0>;
> +			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";
> +			reg = <LPASS_CDC_DMA_TX3>;
> +			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";
> +			reg = <LPASS_CDC_DMA_VA_TX0>;
> +			cpu {
> +				sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
> +			};
> +
> +			codec {
> +				sound-dai = <&lpass_va_macro 0>;
> +			};
> +		};
> +	};
>  };
>  
>  &apps_rsc {
> -- 
> 2.7.4
> 

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

[Srinivasa Rao Mandadapu]

On 4/14/2022 2:53 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Wed, Apr 13, 2022 at 08:51:17PM +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  | 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.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index b944366..1e16854 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> You need to refresh your tree, this file has been renamed to
> sc7280-crd-r3.dts. That DT is for the CRD <= 2.x, newer versions
> use sc7280-herobrine-crd.dts.
Okay. will update accordingly.
>
>> @@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
>>   	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>>   };
>>   
>> +&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";
>> +};
>> +
>>   &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 cf62d06..a7c884a 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 = "MAX98360A";
>> +			reg = <MI2S_SECONDARY>;
> Dumb question: is this value actually used? A quick glance through
> qcom_snd_parse_of() suggests it isn't. And the CPU DAI id is already
> specified in the 'sound-dai' property below.
>
> In a quick test I replaced the corresponding 'reg' values in
> sc7180-trogdor.dtsi with 'random' values and audio playback on
> my coachz (sc7180-trogdor-coachz-r3.dts) still works ...

Yes. agree that it's not being used. But i am not sure of general syntax 
followed.

for now  will delete it.

>
>> +			cpu {
>> +				sound-dai = <&lpass_cpu MI2S_SECONDARY>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&max98360a>;
>> +			};
>> +		};
>> +
>> +		dai-link@5 {
>> +			link-name = "DisplayPort";
>> +			reg = <LPASS_DP_RX>;
> nit: add an empty line (in all links) to separate the properties from the node
Okay.
>
>> +			cpu {
>> +				sound-dai = <&lpass_cpu LPASS_DP_RX>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&mdss_dp>;
>> +			};
>> +		};
>> +
>> +		dai-link@6 {
>> +			link-name = "WCD9385 Playback";
>> +			reg = <LPASS_CDC_DMA_RX0>;
>> +			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";
>> +			reg = <LPASS_CDC_DMA_TX3>;
>> +			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";
>> +			reg = <LPASS_CDC_DMA_VA_TX0>;
>> +			cpu {
>> +				sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&lpass_va_macro 0>;
>> +			};
>> +		};
>> +	};
>>   };
>>   
>>   &apps_rsc {
>> -- 
>> 2.7.4
>>

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

[Matthias Kaehlcke]

On Mon, Apr 18, 2022 at 07:39:44PM +0530, Srinivasa Rao Mandadapu wrote:
> 
> On 4/14/2022 2:53 AM, Matthias Kaehlcke wrote:
> Thanks for your time Matthias!!!
> > On Wed, Apr 13, 2022 at 08:51:17PM +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  | 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.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> > > index b944366..1e16854 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> > You need to refresh your tree, this file has been renamed to
> > sc7280-crd-r3.dts. That DT is for the CRD <= 2.x, newer versions
> > use sc7280-herobrine-crd.dts.
> Okay. will update accordingly.
> > 
> > > @@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
> > >   	us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
> > >   };
> > > +&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";
> > > +};
> > > +
> > >   &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 cf62d06..a7c884a 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 = "MAX98360A";
> > > +			reg = <MI2S_SECONDARY>;
> > Dumb question: is this value actually used? A quick glance through
> > qcom_snd_parse_of() suggests it isn't. And the CPU DAI id is already
> > specified in the 'sound-dai' property below.
> > 
> > In a quick test I replaced the corresponding 'reg' values in
> > sc7180-trogdor.dtsi with 'random' values and audio playback on
> > my coachz (sc7180-trogdor-coachz-r3.dts) still works ...
> 
> Yes. agree that it's not being used. But i am not sure of general syntax
> followed.
> 
> for now  will delete it.

The binding requires it though. I think the correct thing would be to remove it
from the binding and from all device trees currently using it. But that might be
beyond the scope of this series.

Maybe a two step approach would be the best: add 'reg' in this series, get it
landed and then send another series that changes the binding and deletes all
'reg' entries (and adjusts the 'address' in the node name).

I don't think the CPU DAI id should be in the node name either. It isn't even
necessarily a unique identifier if I understand correctly. A SoC could have
multiple IP blocks with audio buses, each with their own enumeration of DAIs.
I suggest to just enumerate the nodes (and 'reg' while it's there) linearly
starting with 0, or maybe even change the node name in the binding (in a
second step) to dai-link-N.