[PATCH v6 0/2] Add lpass pin control support for audio on sc7280 based targets

[PATCH v6 0/2] Add lpass pin control support for audio on sc7280 based targets

[Srinivasa Rao Mandadapu]

This patch set is to add lpass pin control support for Audio over I2S,
wcd codec and digital mics.
This patch set depends on:
	-- Lpass-lpi pinctrl patches [https://patchwork.kernel.org/project/alsa-devel/list/?series=623951&archive=both&state=*]

Changes Since V5:
    -- Remove redundant function property in amp_en node.
    -- Move board specific properties of lpass pin control node to board specific file.
    -- Remove redundant properties in pin control nodes.
    -- Move wcd938x codec reset and CTIA/OMTP pin control patches to other series.
Changes Since V4:
    -- Add primary and secondary I2S pinmux nodes for herobrine specific targets.
Changes Since V3:
    -- Add pinctrl nodes for wcd codec reset and CTIA/OMTP headset selection.
Changes Since V2:
    -- Move lpass pin control node to main dtsi file.
    -- Sort nodes alphabetically.
    -- Remove redundant wcd reset gpio nodes.
    -- Remove redundant input-enable field in dmic pin control nodes.
    -- Update amp_en node. 
    -- Fix typo errors.
    -- Modify node names.
    -- Create patches on latest kernel.    
Changes Since V1:
    -- Merge pinmux and pinconf properties in amp_en and wcd pin reset node.
    -- Split common i2s pin control nodes to functionality specific nodes.
    -- Move board specific properties to board specific dtsi file.
    -- Update dmic pin control node name.

Srinivasa Rao Mandadapu (2):
  arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  arm64: dts: qcom: sc7280: add lpass lpi pin controller node

 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  34 ++++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 118 ++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi           | 148 +++++++++++++++++++++++++
 3 files changed, 300 insertions(+)

-- 
2.7.4

[PATCH v6 1/2] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset

[Srinivasa Rao Mandadapu]

Add AMP enable node and pinmux for primary and secondary I2S
for 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-herobrine.dtsi | 34 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 20 +++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi           | 41 ++++++++++++++++++++++++++
 3 files changed, 95 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index dc17f20..de646d9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -530,6 +530,26 @@ ap_ec_spi: &spi10 {
 	drive-strength = <2>;
 };
 
+&pri_mi2s_data0 {
+	drive-strength = <6>;
+};
+
+&pri_mi2s_data1 {
+	drive-strength = <6>;
+};
+
+&pri_mi2s_mclk {
+	drive-strength = <6>;
+};
+
+&pri_mi2s_sclk {
+	drive-strength = <6>;
+};
+
+&pri_mi2s_ws {
+	drive-strength = <6>;
+};
+
 &qspi_cs0 {
 	bias-disable;
 	drive-strength = <8>;
@@ -610,6 +630,20 @@ ap_ec_spi: &spi10 {
 	drive-strength = <10>;
 };
 
+&sec_mi2s_data0 {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&sec_mi2s_sclk {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&sec_mi2s_ws {
+	drive-strength = <6>;
+};
+
 /* PINCTRL - board-specific pinctrl */
 
 &pm7325_gpios {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index ecbf2b8..2afbbe3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -462,7 +462,27 @@
 	drive-strength = <10>;
 };
 
+&sec_mi2s_data0 {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&sec_mi2s_sclk {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&sec_mi2s_ws {
+	drive-strength = <6>;
+};
+
 &tlmm {
+	amp_en: amp-en {
+		pins = "gpio63";
+		bias-pull-down;
+		drive-strength = <2>;
+	};
+
 	bt_en: bt-en {
 		pins = "gpio85";
 		function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index f0b64be..8d8cec5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3527,6 +3527,31 @@
 				function = "pcie1_clkreqn";
 			};
 
+			pri_mi2s_data0: pri-mi2s-data0 {
+				pins = "gpio98";
+				function = "mi2s0_data0";
+			};
+
+			pri_mi2s_data1: pri-mi2s-data1 {
+				pins = "gpio99";
+				function = "mi2s0_data1";
+			};
+
+			pri_mi2s_mclk: pri-mi2s-mclk {
+				pins = "gpio96";
+				function = "pri_mi2s";
+			};
+
+			pri_mi2s_sclk: pri-mi2s-sclk {
+				pins = "gpio97";
+				function = "mi2s0_sck";
+			};
+
+			pri_mi2s_ws: pri-mi2s-ws {
+				pins = "gpio100";
+				function = "mi2s0_ws";
+			};
+
 			qspi_clk: qspi-clk {
 				pins = "gpio14";
 				function = "qspi_clk";
@@ -4261,6 +4286,22 @@
 				drive-strength = <2>;
 				bias-bus-hold;
 			};
+
+			sec_mi2s_data0: sec-mi2s-data0 {
+				pins = "gpio107";
+				function = "mi2s1_data0";
+			};
+
+			sec_mi2s_sclk: sec-mi2s-sclk {
+				pins = "gpio106";
+				function = "mi2s1_sck";
+			};
+
+			sec_mi2s_ws: sec-mi2s-ws {
+				pins = "gpio108";
+				function = "mi2s1_ws";
+			};
+
 		};
 
 		imem@146a5000 {
-- 
2.7.4

[PATCH v6 2/2] arm64: dts: qcom: sc7280: add lpass lpi pin controller node

[Srinivasa Rao Mandadapu]

Add LPASS LPI pinctrl node required for Audio functionality 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 |  98 ++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi     | 107 +++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 2afbbe3..f912a89 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -238,6 +238,104 @@
 	modem-init;
 };
 
+&lpass_tlmm {
+	dmic01_active: dmic01-active {
+		clk {
+			drive-strength = <8>;
+		};
+
+		data {
+			drive-strength = <8>;
+		};
+	};
+
+	dmic01_sleep: dmic01-sleep {
+		clk {
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		data {
+			drive-strength = <2>;
+			pull-down;
+		};
+	};
+
+	dmic23_active: dmic02-active {
+		clk {
+			drive-strength = <8>;
+		};
+
+		data {
+			drive-strength = <8>;
+		};
+	};
+
+	dmic23_sleep: dmic02-sleep {
+		clk {
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		data {
+			drive-strength = <2>;
+			pull-down;
+		};
+	};
+
+	rx_swr_active: rx-swr-active {
+		clk {
+			drive-strength = <2>;
+			slew-rate = <1>;
+			bias-disable;
+		};
+
+		data {
+			drive-strength = <2>;
+			slew-rate = <1>;
+			bias-bus-hold;
+		};
+	};
+
+	rx_swr_sleep: rx-swr-sleep {
+		clk {
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+
+		data {
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+	};
+
+	tx_swr_active: tx-swr-active {
+		clk {
+			drive-strength = <2>;
+			slew-rate = <1>;
+			bias-disable;
+		};
+
+		data {
+			drive-strength = <2>;
+			slew-rate = <1>;
+			bias-bus-hold;
+		};
+	};
+
+	tx_swr_sleep: tx-swr-sleep {
+		clk {
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+
+		data {
+			drive-strength = <2>;
+			bias-bus-hold;
+		};
+	};
+};
+
 &pcie1 {
 	status = "okay";
 	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 8d8cec5..db74fc3 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1987,6 +1987,113 @@
 			qcom,bcm-voters = <&apps_bcm_voter>;
 		};
 
+		lpass_tlmm: pinctrl@33c0000 {
+			compatible = "qcom,sc7280-lpass-lpi-pinctrl";
+			reg = <0 0x33c0000 0x0 0x20000>,
+				<0 0x3550000 0x0 0x10000>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&lpass_tlmm 0 0 15>;
+
+			#clock-cells = <1>;
+
+			dmic01_active: dmic01-active {
+				clk {
+					pins = "gpio6";
+					function = "dmic1_clk";
+				};
+
+				data {
+					pins = "gpio7";
+					function = "dmic1_data";
+				};
+			};
+
+			dmic01_sleep: dmic01-sleep {
+				clk {
+					pins = "gpio6";
+					function = "dmic1_clk";
+				};
+
+				data {
+					pins = "gpio7";
+					function = "dmic1_data";
+				};
+			};
+
+			dmic23_active: dmic02-active {
+				clk {
+					pins = "gpio8";
+					function = "dmic2_clk";
+				};
+
+				data {
+					pins = "gpio9";
+					function = "dmic2_data";
+				};
+			};
+
+			dmic23_sleep: dmic02-sleep {
+				clk {
+					pins = "gpio8";
+					function = "dmic2_clk";
+				};
+
+				data {
+					pins = "gpio9";
+					function = "dmic2_data";
+				};
+			};
+
+			rx_swr_active: rx-swr-active {
+				clk {
+					pins = "gpio3";
+					function = "swr_rx_clk";
+				};
+
+				data {
+					pins = "gpio4", "gpio5";
+					function = "swr_rx_data";
+				};
+			};
+
+			rx_swr_sleep: rx-swr-sleep {
+				clk {
+					pins = "gpio3";
+					function = "swr_rx_clk";
+				};
+
+				data {
+					pins = "gpio4", "gpio5";
+					function = "swr_rx_data";
+				};
+			};
+
+			tx_swr_active: tx-swr-active {
+				clk {
+					pins = "gpio0";
+					function = "swr_tx_clk";
+				};
+
+				data {
+					pins = "gpio1", "gpio2", "gpio14";
+					function = "swr_tx_data";
+				};
+			};
+
+			tx_swr_sleep: tx-swr-sleep {
+				clk {
+					pins = "gpio0";
+					function = "swr_tx_clk";
+				};
+
+				data {
+					pins = "gpio1", "gpio2", "gpio14";
+					function = "swr_tx_data";
+				};
+			};
+		};
+
 		gpu: gpu@3d00000 {
 			compatible = "qcom,adreno-635.0", "qcom,adreno";
 			reg = <0 0x03d00000 0 0x40000>,
-- 
2.7.4

Re: [PATCH v6 2/2] arm64: dts: qcom: sc7280: add lpass lpi pin controller node

[Matthias Kaehlcke]

On Tue, Apr 05, 2022 at 04:42:47PM +0530, Srinivasa Rao Mandadapu wrote:
> Add LPASS LPI pinctrl node required for Audio functionality 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 |  98 ++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi     | 107 +++++++++++++++++++++++++++++++
>  2 files changed, 205 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 2afbbe3..f912a89 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -238,6 +238,104 @@
>  	modem-init;
>  };
>  
> +&lpass_tlmm {
> +	dmic01_active: dmic01-active {
> +		clk {
> +			drive-strength = <8>;
> +		};
> +
> +		data {
> +			drive-strength = <8>;

The DMIC data pins are input pins, right? Why does an input pin need a drive
strength? Same for other input pins.

> +		};
> +	};

There's no need to reference 'lpass_tlmm' nor to repeat $label: $node.
Instead just use phandles:

&dmic01_active {
	clk {
		drive-strength = <8>;
	};

	data {
		drive-strength = <8>;
	};
};

Rather than replicating the node hierarchy you could also just give each pin a
label, and then:

&dmic01_clk_active {
	drive-strength = <8>;
};

&dmic01_data_active {
	drive-strength = <8>;
};

I don't have a strong preference, but wonder if the grouping adds any value.

> +
> +	dmic01_sleep: dmic01-sleep {
> +		clk {
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +
> +		data {
> +			drive-strength = <2>;
> +			pull-down;
> +		};
> +	};
> +
> +	dmic23_active: dmic02-active {
> +		clk {
> +			drive-strength = <8>;
> +		};
> +
> +		data {
> +			drive-strength = <8>;
> +		};
> +	};
> +
> +	dmic23_sleep: dmic02-sleep {
> +		clk {
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +
> +		data {
> +			drive-strength = <2>;
> +			pull-down;
> +		};
> +	};
> +
> +	rx_swr_active: rx-swr-active {
> +		clk {
> +			drive-strength = <2>;
> +			slew-rate = <1>;
> +			bias-disable;
> +		};
> +
> +		data {
> +			drive-strength = <2>;
> +			slew-rate = <1>;
> +			bias-bus-hold;
> +		};
> +	};
> +
> +	rx_swr_sleep: rx-swr-sleep {
> +		clk {
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +
> +		data {
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +	};
> +
> +	tx_swr_active: tx-swr-active {
> +		clk {
> +			drive-strength = <2>;
> +			slew-rate = <1>;
> +			bias-disable;
> +		};
> +
> +		data {
> +			drive-strength = <2>;
> +			slew-rate = <1>;
> +			bias-bus-hold;
> +		};
> +	};
> +
> +	tx_swr_sleep: tx-swr-sleep {
> +		clk {
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +
> +		data {
> +			drive-strength = <2>;
> +			bias-bus-hold;
> +		};
> +	};
> +};
> +
>  &pcie1 {
>  	status = "okay";
>  	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 8d8cec5..db74fc3 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -1987,6 +1987,113 @@
>  			qcom,bcm-voters = <&apps_bcm_voter>;
>  		};
>  
> +		lpass_tlmm: pinctrl@33c0000 {
> +			compatible = "qcom,sc7280-lpass-lpi-pinctrl";
> +			reg = <0 0x33c0000 0x0 0x20000>,
> +				<0 0x3550000 0x0 0x10000>;

Pad addresses to 8 digits.

> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&lpass_tlmm 0 0 15>;
> +
> +			#clock-cells = <1>;
> +
> +			dmic01_active: dmic01-active {
> +				clk {
> +					pins = "gpio6";
> +					function = "dmic1_clk";
> +				};
> +
> +				data {
> +					pins = "gpio7";
> +					function = "dmic1_data";
> +				};
> +			};
> +
> +			dmic01_sleep: dmic01-sleep {
> +				clk {
> +					pins = "gpio6";
> +					function = "dmic1_clk";
> +				};
> +
> +				data {
> +					pins = "gpio7";
> +					function = "dmic1_data";
> +				};
> +			};
> +
> +			dmic23_active: dmic02-active {

is it intentional that the node name is 'dmic02*', but the label 'dmic23*'?

> +				clk {
> +					pins = "gpio8";
> +					function = "dmic2_clk";
> +				};
> +
> +				data {
> +					pins = "gpio9";
> +					function = "dmic2_data";
> +				};
> +			};
> +
> +			dmic23_sleep: dmic02-sleep {

ditto

> +				clk {
> +					pins = "gpio8";
> +					function = "dmic2_clk";
> +				};
> +
> +				data {
> +					pins = "gpio9";
> +					function = "dmic2_data";
> +				};
> +			};
> +
> +			rx_swr_active: rx-swr-active {
> +				clk {
> +					pins = "gpio3";
> +					function = "swr_rx_clk";
> +				};
> +
> +				data {
> +					pins = "gpio4", "gpio5";
> +					function = "swr_rx_data";
> +				};
> +			};
> +
> +			rx_swr_sleep: rx-swr-sleep {
> +				clk {
> +					pins = "gpio3";
> +					function = "swr_rx_clk";
> +				};
> +
> +				data {
> +					pins = "gpio4", "gpio5";
> +					function = "swr_rx_data";
> +				};
> +			};
> +
> +			tx_swr_active: tx-swr-active {
> +				clk {
> +					pins = "gpio0";
> +					function = "swr_tx_clk";
> +				};
> +
> +				data {
> +					pins = "gpio1", "gpio2", "gpio14";
> +					function = "swr_tx_data";
> +				};
> +			};
> +
> +			tx_swr_sleep: tx-swr-sleep {
> +				clk {
> +					pins = "gpio0";
> +					function = "swr_tx_clk";
> +				};
> +
> +				data {
> +					pins = "gpio1", "gpio2", "gpio14";
> +					function = "swr_tx_data";
> +				};
> +			};
> +		};
> +
>  		gpu: gpu@3d00000 {
>  			compatible = "qcom,adreno-635.0", "qcom,adreno";
>  			reg = <0 0x03d00000 0 0x40000>,

Re: [PATCH v6 1/2] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset

[Matthias Kaehlcke]

On Tue, Apr 05, 2022 at 04:42:46PM +0530, Srinivasa Rao Mandadapu wrote:
> Add AMP enable node and pinmux for primary and secondary I2S
> for 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-herobrine.dtsi | 34 +++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 20 +++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi           | 41 ++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index dc17f20..de646d9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -530,6 +530,26 @@ ap_ec_spi: &spi10 {
>  	drive-strength = <2>;
>  };
>  
> +&pri_mi2s_data0 {
> +	drive-strength = <6>;

Isn't this pin used as an input (HP_DIN)? Is specifying the drive strength
really needed?

> +};
> +
> +&pri_mi2s_data1 {
> +	drive-strength = <6>;
> +};
> +
> +&pri_mi2s_mclk {
> +	drive-strength = <6>;
> +};
> +
> +&pri_mi2s_sclk {
> +	drive-strength = <6>;
> +};
> +
> +&pri_mi2s_ws {
> +	drive-strength = <6>;
> +};
> +
>  &qspi_cs0 {
>  	bias-disable;
>  	drive-strength = <8>;
> @@ -610,6 +630,20 @@ ap_ec_spi: &spi10 {
>  	drive-strength = <10>;
>  };
>  
> +&sec_mi2s_data0 {
> +	drive-strength = <6>;
> +	bias-disable;
> +};
> +
> +&sec_mi2s_sclk {
> +	drive-strength = <6>;
> +	bias-disable;
> +};
> +
> +&sec_mi2s_ws {
> +	drive-strength = <6>;
> +};

Actually there are several sound configs for herobrine boards. For now I
think it's ok to specify the config for herobrine -rev1 (as this patch
does) and we can sort out later how to best support the different configs.

>  /* PINCTRL - board-specific pinctrl */
>  
>  &pm7325_gpios {
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index ecbf2b8..2afbbe3 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -462,7 +462,27 @@
>  	drive-strength = <10>;
>  };
>  
> +&sec_mi2s_data0 {
> +	drive-strength = <6>;
> +	bias-disable;
> +};
> +
> +&sec_mi2s_sclk {
> +	drive-strength = <6>;
> +	bias-disable;
> +};
> +
> +&sec_mi2s_ws {
> +	drive-strength = <6>;
> +};
> +
>  &tlmm {
> +	amp_en: amp-en {
> +		pins = "gpio63";
> +		bias-pull-down;
> +		drive-strength = <2>;
> +	};

nit: all the other pins are i2s related, it might make sense to add amp_en
in a separate patch.

> +
>  	bt_en: bt-en {
>  		pins = "gpio85";
>  		function = "gpio";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index f0b64be..8d8cec5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3527,6 +3527,31 @@
>  				function = "pcie1_clkreqn";
>  			};
>  
> +			pri_mi2s_data0: pri-mi2s-data0 {
> +				pins = "gpio98";
> +				function = "mi2s0_data0";
> +			};
> +
> +			pri_mi2s_data1: pri-mi2s-data1 {
> +				pins = "gpio99";
> +				function = "mi2s0_data1";
> +			};
> +
> +			pri_mi2s_mclk: pri-mi2s-mclk {
> +				pins = "gpio96";
> +				function = "pri_mi2s";
> +			};
> +
> +			pri_mi2s_sclk: pri-mi2s-sclk {
> +				pins = "gpio97";
> +				function = "mi2s0_sck";
> +			};
> +
> +			pri_mi2s_ws: pri-mi2s-ws {
> +				pins = "gpio100";
> +				function = "mi2s0_ws";
> +			};
> +
>  			qspi_clk: qspi-clk {
>  				pins = "gpio14";
>  				function = "qspi_clk";
> @@ -4261,6 +4286,22 @@
>  				drive-strength = <2>;
>  				bias-bus-hold;
>  			};
> +
> +			sec_mi2s_data0: sec-mi2s-data0 {
> +				pins = "gpio107";
> +				function = "mi2s1_data0";
> +			};
> +
> +			sec_mi2s_sclk: sec-mi2s-sclk {
> +				pins = "gpio106";
> +				function = "mi2s1_sck";
> +			};
> +
> +			sec_mi2s_ws: sec-mi2s-ws {
> +				pins = "gpio108";
> +				function = "mi2s1_ws";
> +			};

Is there a particular reason for the pri/sec nomenclature? The datasheet and
schematics call the pin mi2sN_xyz, it seems it would be clearer to follow
that naming. Primary/secondary seems to imply a 'master/slave' topology, but
these are independent controllers IIUC. The datasheet refers to pin 96 as
PRI_MI2S_MCLK and pin 105 SEC_MI2S_MCLK, I guess the naming was derived from
that.

My suggestion would be to follow the naming in the datasheet/schematic, i.e.
mi2sN_data0, mi2sN_data1, pri/sec_mi2s_mclk, mi2sN_sck, mi2sN_ws.

Re: [PATCH v6 2/2] arm64: dts: qcom: sc7280: add lpass lpi pin controller node

[Matthias Kaehlcke]

On Tue, Apr 05, 2022 at 10:14:20AM -0700, Matthias Kaehlcke wrote:
> On Tue, Apr 05, 2022 at 04:42:47PM +0530, Srinivasa Rao Mandadapu wrote:
> > Add LPASS LPI pinctrl node required for Audio functionality 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 |  98 ++++++++++++++++++++++++++++
> >  arch/arm64/boot/dts/qcom/sc7280.dtsi     | 107 +++++++++++++++++++++++++++++++
> >  2 files changed, 205 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > index 2afbbe3..f912a89 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > @@ -238,6 +238,104 @@
> >  	modem-init;
> >  };
> >  
> > +&lpass_tlmm {
> > +	dmic01_active: dmic01-active {
> > +		clk {
> > +			drive-strength = <8>;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <8>;
> 
> The DMIC data pins are input pins, right? Why does an input pin need a drive
> strength? Same for other input pins.
> 
> > +		};
> > +	};
> 
> There's no need to reference 'lpass_tlmm' nor to repeat $label: $node.
> Instead just use phandles:
> 
> &dmic01_active {
> 	clk {
> 		drive-strength = <8>;
> 	};
> 
> 	data {
> 		drive-strength = <8>;
> 	};
> };
> 
> Rather than replicating the node hierarchy you could also just give each pin a
> label, and then:
> 
> &dmic01_clk_active {
> 	drive-strength = <8>;
> };
> 
> &dmic01_data_active {
> 	drive-strength = <8>;
> };
> 
> I don't have a strong preference, but wonder if the grouping adds any value.

One more thing: please also drop the '_active' suffix, it is not needed. With
that I think it's clearer to get rid of the grouping.

> > +
> > +	dmic01_sleep: dmic01-sleep {
> > +		clk {
> > +			drive-strength = <2>;
> > +			bias-disable;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <2>;
> > +			pull-down;
> > +		};
> > +	};
> > +
> > +	dmic23_active: dmic02-active {
> > +		clk {
> > +			drive-strength = <8>;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <8>;
> > +		};
> > +	};
> > +
> > +	dmic23_sleep: dmic02-sleep {
> > +		clk {
> > +			drive-strength = <2>;
> > +			bias-disable;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <2>;
> > +			pull-down;
> > +		};
> > +	};
> > +
> > +	rx_swr_active: rx-swr-active {
> > +		clk {
> > +			drive-strength = <2>;
> > +			slew-rate = <1>;
> > +			bias-disable;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <2>;
> > +			slew-rate = <1>;
> > +			bias-bus-hold;
> > +		};
> > +	};
> > +
> > +	rx_swr_sleep: rx-swr-sleep {
> > +		clk {
> > +			drive-strength = <2>;
> > +			bias-pull-down;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <2>;
> > +			bias-pull-down;
> > +		};
> > +	};
> > +
> > +	tx_swr_active: tx-swr-active {
> > +		clk {
> > +			drive-strength = <2>;
> > +			slew-rate = <1>;
> > +			bias-disable;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <2>;
> > +			slew-rate = <1>;
> > +			bias-bus-hold;
> > +		};
> > +	};
> > +
> > +	tx_swr_sleep: tx-swr-sleep {
> > +		clk {
> > +			drive-strength = <2>;
> > +			bias-pull-down;
> > +		};
> > +
> > +		data {
> > +			drive-strength = <2>;
> > +			bias-bus-hold;
> > +		};
> > +	};
> > +};
> > +
> >  &pcie1 {
> >  	status = "okay";
> >  	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 8d8cec5..db74fc3 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -1987,6 +1987,113 @@
> >  			qcom,bcm-voters = <&apps_bcm_voter>;
> >  		};
> >  
> > +		lpass_tlmm: pinctrl@33c0000 {
> > +			compatible = "qcom,sc7280-lpass-lpi-pinctrl";
> > +			reg = <0 0x33c0000 0x0 0x20000>,
> > +				<0 0x3550000 0x0 0x10000>;
> 
> Pad addresses to 8 digits.
> 
> > +			gpio-controller;
> > +			#gpio-cells = <2>;
> > +			gpio-ranges = <&lpass_tlmm 0 0 15>;
> > +
> > +			#clock-cells = <1>;
> > +
> > +			dmic01_active: dmic01-active {
> > +				clk {
> > +					pins = "gpio6";
> > +					function = "dmic1_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio7";
> > +					function = "dmic1_data";
> > +				};
> > +			};
> > +
> > +			dmic01_sleep: dmic01-sleep {
> > +				clk {
> > +					pins = "gpio6";
> > +					function = "dmic1_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio7";
> > +					function = "dmic1_data";
> > +				};
> > +			};
> > +
> > +			dmic23_active: dmic02-active {
> 
> is it intentional that the node name is 'dmic02*', but the label 'dmic23*'?
> 
> > +				clk {
> > +					pins = "gpio8";
> > +					function = "dmic2_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio9";
> > +					function = "dmic2_data";
> > +				};
> > +			};
> > +
> > +			dmic23_sleep: dmic02-sleep {
> 
> ditto
> 
> > +				clk {
> > +					pins = "gpio8";
> > +					function = "dmic2_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio9";
> > +					function = "dmic2_data";
> > +				};
> > +			};
> > +
> > +			rx_swr_active: rx-swr-active {
> > +				clk {
> > +					pins = "gpio3";
> > +					function = "swr_rx_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio4", "gpio5";
> > +					function = "swr_rx_data";
> > +				};
> > +			};
> > +
> > +			rx_swr_sleep: rx-swr-sleep {
> > +				clk {
> > +					pins = "gpio3";
> > +					function = "swr_rx_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio4", "gpio5";
> > +					function = "swr_rx_data";
> > +				};
> > +			};
> > +
> > +			tx_swr_active: tx-swr-active {
> > +				clk {
> > +					pins = "gpio0";
> > +					function = "swr_tx_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio1", "gpio2", "gpio14";
> > +					function = "swr_tx_data";
> > +				};
> > +			};
> > +
> > +			tx_swr_sleep: tx-swr-sleep {
> > +				clk {
> > +					pins = "gpio0";
> > +					function = "swr_tx_clk";
> > +				};
> > +
> > +				data {
> > +					pins = "gpio1", "gpio2", "gpio14";
> > +					function = "swr_tx_data";
> > +				};
> > +			};
> > +		};
> > +
> >  		gpu: gpu@3d00000 {
> >  			compatible = "qcom,adreno-635.0", "qcom,adreno";
> >  			reg = <0 0x03d00000 0 0x40000>,

Re: [PATCH v6 2/2] arm64: dts: qcom: sc7280: add lpass lpi pin controller node

[Srinivasa Rao Mandadapu]

On 4/6/2022 7:45 AM, Matthias Kaehlcke wrote:
Thanks Matthias for your time and Valuable inputs!!!
> On Tue, Apr 05, 2022 at 10:14:20AM -0700, Matthias Kaehlcke wrote:
>> On Tue, Apr 05, 2022 at 04:42:47PM +0530, Srinivasa Rao Mandadapu wrote:
>>> Add LPASS LPI pinctrl node required for Audio functionality 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 |  98 ++++++++++++++++++++++++++++
>>>   arch/arm64/boot/dts/qcom/sc7280.dtsi     | 107 +++++++++++++++++++++++++++++++
>>>   2 files changed, 205 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>>> index 2afbbe3..f912a89 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>>> @@ -238,6 +238,104 @@
>>>   	modem-init;
>>>   };
>>>   
>>> +&lpass_tlmm {
>>> +	dmic01_active: dmic01-active {
>>> +		clk {
>>> +			drive-strength = <8>;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <8>;
>> The DMIC data pins are input pins, right? Why does an input pin need a drive
>> strength? Same for other input pins.
Okay. Will remove this.
>>
>>> +		};
>>> +	};
>> There's no need to reference 'lpass_tlmm' nor to repeat $label: $node.
>> Instead just use phandles:
Okay. Will change accordingly.
>>
>> &dmic01_active {
>> 	clk {
>> 		drive-strength = <8>;
>> 	};
>>
>> 	data {
>> 		drive-strength = <8>;
>> 	};
>> };
>>
>> Rather than replicating the node hierarchy you could also just give each pin a
>> label, and then:
>>
>> &dmic01_clk_active {
>> 	drive-strength = <8>;
>> };
>>
>> &dmic01_data_active {
>> 	drive-strength = <8>;
>> };
>>
>> I don't have a strong preference, but wonder if the grouping adds any value.
> One more thing: please also drop the '_active' suffix, it is not needed. With
> that I think it's clearer to get rid of the grouping.
For now will continue as it's.  Anyway we are removing dmic01_data_active.
>
>>> +
>>> +	dmic01_sleep: dmic01-sleep {
>>> +		clk {
>>> +			drive-strength = <2>;
>>> +			bias-disable;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <2>;
>>> +			pull-down;
>>> +		};
>>> +	};
>>> +
>>> +	dmic23_active: dmic02-active {
>>> +		clk {
>>> +			drive-strength = <8>;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <8>;
>>> +		};
>>> +	};
>>> +
>>> +	dmic23_sleep: dmic02-sleep {
>>> +		clk {
>>> +			drive-strength = <2>;
>>> +			bias-disable;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <2>;
>>> +			pull-down;
>>> +		};
>>> +	};
>>> +
>>> +	rx_swr_active: rx-swr-active {
>>> +		clk {
>>> +			drive-strength = <2>;
>>> +			slew-rate = <1>;
>>> +			bias-disable;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <2>;
>>> +			slew-rate = <1>;
>>> +			bias-bus-hold;
>>> +		};
>>> +	};
>>> +
>>> +	rx_swr_sleep: rx-swr-sleep {
>>> +		clk {
>>> +			drive-strength = <2>;
>>> +			bias-pull-down;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <2>;
>>> +			bias-pull-down;
>>> +		};
>>> +	};
>>> +
>>> +	tx_swr_active: tx-swr-active {
>>> +		clk {
>>> +			drive-strength = <2>;
>>> +			slew-rate = <1>;
>>> +			bias-disable;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <2>;
>>> +			slew-rate = <1>;
>>> +			bias-bus-hold;
>>> +		};
>>> +	};
>>> +
>>> +	tx_swr_sleep: tx-swr-sleep {
>>> +		clk {
>>> +			drive-strength = <2>;
>>> +			bias-pull-down;
>>> +		};
>>> +
>>> +		data {
>>> +			drive-strength = <2>;
>>> +			bias-bus-hold;
>>> +		};
>>> +	};
>>> +};
>>> +
>>>   &pcie1 {
>>>   	status = "okay";
>>>   	perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>;
>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>> index 8d8cec5..db74fc3 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>> @@ -1987,6 +1987,113 @@
>>>   			qcom,bcm-voters = <&apps_bcm_voter>;
>>>   		};
>>>   
>>> +		lpass_tlmm: pinctrl@33c0000 {
>>> +			compatible = "qcom,sc7280-lpass-lpi-pinctrl";
>>> +			reg = <0 0x33c0000 0x0 0x20000>,
>>> +				<0 0x3550000 0x0 0x10000>;
>> Pad addresses to 8 digits.
Okay.
>>
>>> +			gpio-controller;
>>> +			#gpio-cells = <2>;
>>> +			gpio-ranges = <&lpass_tlmm 0 0 15>;
>>> +
>>> +			#clock-cells = <1>;
>>> +
>>> +			dmic01_active: dmic01-active {
>>> +				clk {
>>> +					pins = "gpio6";
>>> +					function = "dmic1_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio7";
>>> +					function = "dmic1_data";
>>> +				};
>>> +			};
>>> +
>>> +			dmic01_sleep: dmic01-sleep {
>>> +				clk {
>>> +					pins = "gpio6";
>>> +					function = "dmic1_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio7";
>>> +					function = "dmic1_data";
>>> +				};
>>> +			};
>>> +
>>> +			dmic23_active: dmic02-active {
>> is it intentional that the node name is 'dmic02*', but the label 'dmic23*'?
typo error. will fix it.
>>
>>> +				clk {
>>> +					pins = "gpio8";
>>> +					function = "dmic2_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio9";
>>> +					function = "dmic2_data";
>>> +				};
>>> +			};
>>> +
>>> +			dmic23_sleep: dmic02-sleep {
>> ditto
Okay.
>>
>>> +				clk {
>>> +					pins = "gpio8";
>>> +					function = "dmic2_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio9";
>>> +					function = "dmic2_data";
>>> +				};
>>> +			};
>>> +
>>> +			rx_swr_active: rx-swr-active {
>>> +				clk {
>>> +					pins = "gpio3";
>>> +					function = "swr_rx_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio4", "gpio5";
>>> +					function = "swr_rx_data";
>>> +				};
>>> +			};
>>> +
>>> +			rx_swr_sleep: rx-swr-sleep {
>>> +				clk {
>>> +					pins = "gpio3";
>>> +					function = "swr_rx_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio4", "gpio5";
>>> +					function = "swr_rx_data";
>>> +				};
>>> +			};
>>> +
>>> +			tx_swr_active: tx-swr-active {
>>> +				clk {
>>> +					pins = "gpio0";
>>> +					function = "swr_tx_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio1", "gpio2", "gpio14";
>>> +					function = "swr_tx_data";
>>> +				};
>>> +			};
>>> +
>>> +			tx_swr_sleep: tx-swr-sleep {
>>> +				clk {
>>> +					pins = "gpio0";
>>> +					function = "swr_tx_clk";
>>> +				};
>>> +
>>> +				data {
>>> +					pins = "gpio1", "gpio2", "gpio14";
>>> +					function = "swr_tx_data";
>>> +				};
>>> +			};
>>> +		};
>>> +
>>>   		gpu: gpu@3d00000 {
>>>   			compatible = "qcom,adreno-635.0", "qcom,adreno";
>>>   			reg = <0 0x03d00000 0 0x40000>,

Re: [PATCH v6 1/2] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset

[Srinivasa Rao Mandadapu]

On 4/6/2022 12:04 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Tue, Apr 05, 2022 at 04:42:46PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add AMP enable node and pinmux for primary and secondary I2S
>> for 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-herobrine.dtsi | 34 +++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi       | 20 +++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi           | 41 ++++++++++++++++++++++++++
>>   3 files changed, 95 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> index dc17f20..de646d9 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>> @@ -530,6 +530,26 @@ ap_ec_spi: &spi10 {
>>   	drive-strength = <2>;
>>   };
>>   
>> +&pri_mi2s_data0 {
>> +	drive-strength = <6>;
> Isn't this pin used as an input (HP_DIN)? Is specifying the drive strength
> really needed?
Okay. will remove this.
>
>> +};
>> +
>> +&pri_mi2s_data1 {
>> +	drive-strength = <6>;
>> +};
>> +
>> +&pri_mi2s_mclk {
>> +	drive-strength = <6>;
>> +};
>> +
>> +&pri_mi2s_sclk {
>> +	drive-strength = <6>;
>> +};
>> +
>> +&pri_mi2s_ws {
>> +	drive-strength = <6>;
>> +};
>> +
>>   &qspi_cs0 {
>>   	bias-disable;
>>   	drive-strength = <8>;
>> @@ -610,6 +630,20 @@ ap_ec_spi: &spi10 {
>>   	drive-strength = <10>;
>>   };
>>   
>> +&sec_mi2s_data0 {
>> +	drive-strength = <6>;
>> +	bias-disable;
>> +};
>> +
>> +&sec_mi2s_sclk {
>> +	drive-strength = <6>;
>> +	bias-disable;
>> +};
>> +
>> +&sec_mi2s_ws {
>> +	drive-strength = <6>;
>> +};
> Actually there are several sound configs for herobrine boards. For now I
> think it's ok to specify the config for herobrine -rev1 (as this patch
> does) and we can sort out later how to best support the different configs.
Okay. Will skip this for now.
>
>>   /* PINCTRL - board-specific pinctrl */
>>   
>>   &pm7325_gpios {
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index ecbf2b8..2afbbe3 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -462,7 +462,27 @@
>>   	drive-strength = <10>;
>>   };
>>   
>> +&sec_mi2s_data0 {
>> +	drive-strength = <6>;
>> +	bias-disable;
>> +};
>> +
>> +&sec_mi2s_sclk {
>> +	drive-strength = <6>;
>> +	bias-disable;
>> +};
>> +
>> +&sec_mi2s_ws {
>> +	drive-strength = <6>;
>> +};
>> +
>>   &tlmm {
>> +	amp_en: amp-en {
>> +		pins = "gpio63";
>> +		bias-pull-down;
>> +		drive-strength = <2>;
>> +	};
> nit: all the other pins are i2s related, it might make sense to add amp_en
> in a separate patch.
Okay. will add it in corresponding consumer patch.
>
>> +
>>   	bt_en: bt-en {
>>   		pins = "gpio85";
>>   		function = "gpio";
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index f0b64be..8d8cec5 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -3527,6 +3527,31 @@
>>   				function = "pcie1_clkreqn";
>>   			};
>>   
>> +			pri_mi2s_data0: pri-mi2s-data0 {
>> +				pins = "gpio98";
>> +				function = "mi2s0_data0";
>> +			};
>> +
>> +			pri_mi2s_data1: pri-mi2s-data1 {
>> +				pins = "gpio99";
>> +				function = "mi2s0_data1";
>> +			};
>> +
>> +			pri_mi2s_mclk: pri-mi2s-mclk {
>> +				pins = "gpio96";
>> +				function = "pri_mi2s";
>> +			};
>> +
>> +			pri_mi2s_sclk: pri-mi2s-sclk {
>> +				pins = "gpio97";
>> +				function = "mi2s0_sck";
>> +			};
>> +
>> +			pri_mi2s_ws: pri-mi2s-ws {
>> +				pins = "gpio100";
>> +				function = "mi2s0_ws";
>> +			};
>> +
>>   			qspi_clk: qspi-clk {
>>   				pins = "gpio14";
>>   				function = "qspi_clk";
>> @@ -4261,6 +4286,22 @@
>>   				drive-strength = <2>;
>>   				bias-bus-hold;
>>   			};
>> +
>> +			sec_mi2s_data0: sec-mi2s-data0 {
>> +				pins = "gpio107";
>> +				function = "mi2s1_data0";
>> +			};
>> +
>> +			sec_mi2s_sclk: sec-mi2s-sclk {
>> +				pins = "gpio106";
>> +				function = "mi2s1_sck";
>> +			};
>> +
>> +			sec_mi2s_ws: sec-mi2s-ws {
>> +				pins = "gpio108";
>> +				function = "mi2s1_ws";
>> +			};
> Is there a particular reason for the pri/sec nomenclature? The datasheet and
> schematics call the pin mi2sN_xyz, it seems it would be clearer to follow
> that naming. Primary/secondary seems to imply a 'master/slave' topology, but
> these are independent controllers IIUC. The datasheet refers to pin 96 as
> PRI_MI2S_MCLK and pin 105 SEC_MI2S_MCLK, I guess the naming was derived from
> that.
>
> My suggestion would be to follow the naming in the datasheet/schematic, i.e.
> mi2sN_data0, mi2sN_data1, pri/sec_mi2s_mclk, mi2sN_sck, mi2sN_ws.
Okay. Actually we followed same in Rennel architecture. Will change 
accordingly.