All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add lpass pin control support for audio on sc7280 based targets
@ 2022-02-08 15:34 Srinivasa Rao Mandadapu
  2022-02-08 15:34 ` [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset Srinivasa Rao Mandadapu
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-08 15:34 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: 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:
	-- https://patchwork.kernel.org/project/alsa-devel/patch/1638891339-21806-3-git-send-email-quic_srivasam@quicinc.com/

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 (3):
  arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  arm64: dts: qcom: sc7280: add lpass lpi pin controller node
  arm64: dts: qcom: sc7280: Add wcd9380 pinmux

 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 205 +++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi     |  40 ++++++
 2 files changed, 245 insertions(+)

-- 
2.7.4


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

* [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  2022-02-08 15:34 [PATCH v2 0/3] Add lpass pin control support for audio on sc7280 based targets Srinivasa Rao Mandadapu
@ 2022-02-08 15:34 ` Srinivasa Rao Mandadapu
  2022-02-08 21:08   ` Stephen Boyd
  2022-02-08 15:34 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node Srinivasa Rao Mandadapu
  2022-02-08 15:34 ` [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux Srinivasa Rao Mandadapu
  2 siblings, 1 reply; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-08 15:34 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

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-idp.dtsi | 40 ++++++++++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi     | 40 ++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index d623d71..c7d6c46 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -436,6 +436,39 @@
 		qcom,drive-strength = <3>;
 	};
 };
+&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>;
+};
+
+&sec_mi2s_data0 {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&sec_mi2s_sclk {
+	drive-strength = <6>;
+	bias-disable;
+};
+
+&sec_mi2s_ws {
+	drive-strength = <6>;
+};
 
 &qspi_cs0 {
 	bias-disable;
@@ -491,6 +524,13 @@
 };
 
 &tlmm {
+	amp_en: amp-en {
+		pins = "gpio63";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
 	nvme_pwren: nvme-pwren {
 		function = "gpio";
 	};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 937c2e0..76e73e9 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3461,6 +3461,46 @@
 				};
 			};
 
+			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";
+			};
+
+			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";
+			};
+
 			qup_uart8_cts: qup-uart8-cts {
 				pins = "gpio32";
 				function = "qup10";
-- 
2.7.4


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

* [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node
  2022-02-08 15:34 [PATCH v2 0/3] Add lpass pin control support for audio on sc7280 based targets Srinivasa Rao Mandadapu
  2022-02-08 15:34 ` [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset Srinivasa Rao Mandadapu
@ 2022-02-08 15:34 ` Srinivasa Rao Mandadapu
  2022-02-08 21:11   ` Stephen Boyd
  2022-02-08 15:34 ` [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux Srinivasa Rao Mandadapu
  2 siblings, 1 reply; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-08 15:34 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

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

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index c7d6c46..4704a93 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -638,3 +638,153 @@
 		bias-pull-up;
 	};
 };
+&soc {
+	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-pins {
+			clk {
+				pins = "gpio6";
+				function = "dmic1_clk";
+				drive-strength = <8>;
+				output-high;
+			};
+			data {
+				pins = "gpio7";
+				function = "dmic1_data";
+				drive-strength = <8>;
+				input-enable;
+			};
+		};
+
+		dmic01_sleep: dmic01-sleep-pins {
+			clk {
+				pins = "gpio6";
+				function = "dmic1_clk";
+				drive-strength = <2>;
+				bias-disable;
+				output-low;
+			};
+
+			data {
+				pins = "gpio7";
+				function = "dmic1_data";
+				drive-strength = <2>;
+				pull-down;
+				input-enable;
+			};
+		};
+
+		dmic23_active: dmic02-active-pins {
+			clk {
+				pins = "gpio8";
+				function = "dmic2_clk";
+				drive-strength = <8>;
+				output-high;
+			};
+			data {
+				pins = "gpio9";
+				function = "dmic2_data";
+				drive-strength = <8>;
+				input-enable;
+			};
+		};
+
+		dmic23_sleep: dmic02-sleep-pins {
+			clk {
+				pins = "gpio8";
+				function = "dmic2_clk";
+				drive-strength = <2>;
+				bias-disable;
+				output-low;
+			};
+
+			data {
+				pins = "gpio9";
+				function = "dmic2_data";
+				drive-strength = <2>;
+				pull-down;
+				input-enable;
+			};
+		};
+
+		rx_swr_active: rx_swr-active-pins {
+			clk {
+				pins = "gpio3";
+				function = "swr_rx_clk";
+				drive-strength = <2>;
+				slew-rate = <1>;
+				bias-disable;
+			};
+
+			data {
+				pins = "gpio4", "gpio5";
+				function = "swr_rx_data";
+				drive-strength = <2>;
+				slew-rate = <1>;
+				bias-bus-hold;
+			};
+		};
+
+		rx_swr_sleep: rx_swr-sleep-pins {
+			clk {
+				pins = "gpio3";
+				function = "swr_rx_clk";
+				drive-strength = <2>;
+				input-enable;
+				bias-pull-down;
+			};
+
+			data {
+				pins = "gpio4", "gpio5";
+				function = "swr_rx_data";
+				drive-strength = <2>;
+				input-enable;
+				bias-pull-down;
+			};
+		};
+
+		tx_swr_active: tx_swr-active-pins {
+			clk {
+				pins = "gpio0";
+				function = "swr_tx_clk";
+				drive-strength = <2>;
+				slew-rate = <1>;
+				bias-disable;
+			};
+
+			data {
+				pins = "gpio1", "gpio2", "gpio14";
+				function = "swr_tx_data";
+				drive-strength = <2>;
+				slew-rate = <1>;
+				bias-bus-hold;
+			};
+		};
+
+		tx_swr_sleep: tx_swr-sleep-pins {
+			clk {
+				pins = "gpio0";
+				function = "swr_tx_clk";
+				drive-strength = <2>;
+				input-enable;
+				bias-pull-down;
+			};
+
+			data {
+				pins = "gpio1", "gpio2", "gpio14";
+				function = "swr_tx_data";
+				drive-strength = <2>;
+				input-enable;
+				bias-bus-hold;
+			};
+		};
+	};
+};
-- 
2.7.4


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

* [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux
  2022-02-08 15:34 [PATCH v2 0/3] Add lpass pin control support for audio on sc7280 based targets Srinivasa Rao Mandadapu
  2022-02-08 15:34 ` [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset Srinivasa Rao Mandadapu
  2022-02-08 15:34 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node Srinivasa Rao Mandadapu
@ 2022-02-08 15:34 ` Srinivasa Rao Mandadapu
  2022-02-08 21:12   ` Stephen Boyd
  2 siblings, 1 reply; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-08 15:34 UTC (permalink / raw)
  To: agross, bjorn.andersson, robh+dt, linux-arm-msm, devicetree,
	linux-kernel, rohitkr, srinivas.kandagatla, dianders, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

Add pinmux to reset wcd codec, conneceted 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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 4704a93..6b38fa1 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -594,6 +594,21 @@
 		 */
 		bias-pull-up;
 	};
+
+	wcd938x_reset_active: wcd938x_reset_active {
+			pins = "gpio83";
+			function = "gpio";
+			drive-strength = <16>;
+			output-high;
+	};
+
+	wcd938x_reset_sleep: wcd938x_reset_sleep {
+			pins = "gpio83";
+			function = "gpio";
+			drive-strength = <16>;
+			bias-disable;
+			output-low;
+	};
 };
 
 &sdc1_on {
-- 
2.7.4


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

* Re: [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  2022-02-08 15:34 ` [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset Srinivasa Rao Mandadapu
@ 2022-02-08 21:08   ` Stephen Boyd
  2022-02-09 13:42     ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2022-02-08 21:08 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
	dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
	rohitkr, srinivas.kandagatla
  Cc: Venkata Prasad Potturu

Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:12)
> 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-idp.dtsi | 40 ++++++++++++++++++++++++++++++++
>  arch/arm64/boot/dts/qcom/sc7280.dtsi     | 40 ++++++++++++++++++++++++++++++++
>  2 files changed, 80 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index d623d71..c7d6c46 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -436,6 +436,39 @@
>                 qcom,drive-strength = <3>;
>         };
>  };

Newline here

> +&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>;
> +};
> +
> +&sec_mi2s_data0 {
> +       drive-strength = <6>;
> +       bias-disable;
> +};
> +
> +&sec_mi2s_sclk {
> +       drive-strength = <6>;
> +       bias-disable;
> +};
> +
> +&sec_mi2s_ws {
> +       drive-strength = <6>;
> +};

Please sort these nodes alphabetically on node name.

>
>  &qspi_cs0 {
>         bias-disable;
> @@ -491,6 +524,13 @@
>  };
>
>  &tlmm {
> +       amp_en: amp-en {
> +               pins = "gpio63";
> +               function = "gpio";
> +               bias-disable;

Is there an external pull?

> +               drive-strength = <2>;
> +       };
> +
>         nvme_pwren: nvme-pwren {
>                 function = "gpio";
>         };
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 937c2e0..76e73e9 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3461,6 +3461,46 @@
>                                 };
>                         };
>
> +                       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";
> +                       };
> +
> +                       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";
> +                       };

Please sort these nodes alphabetically on node name.

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

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node
  2022-02-08 15:34 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node Srinivasa Rao Mandadapu
@ 2022-02-08 21:11   ` Stephen Boyd
  2022-02-09 14:01     ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2022-02-08 21:11 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
	dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
	rohitkr, srinivas.kandagatla
  Cc: Venkata Prasad Potturu

Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:13)
> 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 | 150 +++++++++++++++++++++++++++++++
>  1 file changed, 150 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index c7d6c46..4704a93 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -638,3 +638,153 @@
>                 bias-pull-up;
>         };
>  };

Newline here.

> +&soc {
> +       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>;

Presumably this doesn't change so it should be moved to the sc7280.dtsi
file as part of the soc node. It can be marked status = "disabled" if
it's not commonly used, but I suspect it is commonly used on sc7280?

> +
> +               dmic01_active: dmic01-active-pins {

The '-pins' suffix is redundant. Please remove it.

> +                       clk {
> +                               pins = "gpio6";
> +                               function = "dmic1_clk";
> +                               drive-strength = <8>;
> +                               output-high;
> +                       };

Please be consistent and have a newline between nodes.

> +                       data {
> +                               pins = "gpio7";
> +                               function = "dmic1_data";
> +                               drive-strength = <8>;
> +                               input-enable;
> +                       };
> +               };
> +
> +               dmic01_sleep: dmic01-sleep-pins {
> +                       clk {
> +                               pins = "gpio6";
> +                               function = "dmic1_clk";
> +                               drive-strength = <2>;
> +                               bias-disable;
> +                               output-low;
> +                       };
> +
> +                       data {
> +                               pins = "gpio7";
> +                               function = "dmic1_data";
> +                               drive-strength = <2>;
> +                               pull-down;
> +                               input-enable;

Why does input-enable matter? It's not a gpio.

> +                       };
> +               };
> +
> +               dmic23_active: dmic02-active-pins {
> +                       clk {
> +                               pins = "gpio8";
> +                               function = "dmic2_clk";
> +                               drive-strength = <8>;
> +                               output-high;
> +                       };
> +                       data {
> +                               pins = "gpio9";
> +                               function = "dmic2_data";
> +                               drive-strength = <8>;
> +                               input-enable;
> +                       };
> +               };
> +
> +               dmic23_sleep: dmic02-sleep-pins {
> +                       clk {
> +                               pins = "gpio8";
> +                               function = "dmic2_clk";
> +                               drive-strength = <2>;
> +                               bias-disable;
> +                               output-low;
> +                       };
> +
> +                       data {
> +                               pins = "gpio9";
> +                               function = "dmic2_data";
> +                               drive-strength = <2>;
> +                               pull-down;
> +                               input-enable;
> +                       };
> +               };
> +
> +               rx_swr_active: rx_swr-active-pins {
> +                       clk {
> +                               pins = "gpio3";
> +                               function = "swr_rx_clk";
> +                               drive-strength = <2>;
> +                               slew-rate = <1>;
> +                               bias-disable;
> +                       };
> +
> +                       data {
> +                               pins = "gpio4", "gpio5";
> +                               function = "swr_rx_data";
> +                               drive-strength = <2>;
> +                               slew-rate = <1>;
> +                               bias-bus-hold;
> +                       };
> +               };
> +
> +               rx_swr_sleep: rx_swr-sleep-pins {
> +                       clk {
> +                               pins = "gpio3";
> +                               function = "swr_rx_clk";
> +                               drive-strength = <2>;
> +                               input-enable;
> +                               bias-pull-down;
> +                       };
> +
> +                       data {
> +                               pins = "gpio4", "gpio5";
> +                               function = "swr_rx_data";
> +                               drive-strength = <2>;
> +                               input-enable;
> +                               bias-pull-down;
> +                       };
> +               };
> +
> +               tx_swr_active: tx_swr-active-pins {
> +                       clk {
> +                               pins = "gpio0";
> +                               function = "swr_tx_clk";
> +                               drive-strength = <2>;
> +                               slew-rate = <1>;
> +                               bias-disable;
> +                       };
> +
> +                       data {
> +                               pins = "gpio1", "gpio2", "gpio14";
> +                               function = "swr_tx_data";
> +                               drive-strength = <2>;
> +                               slew-rate = <1>;
> +                               bias-bus-hold;
> +                       };
> +               };
> +
> +               tx_swr_sleep: tx_swr-sleep-pins {

No underscore in node names.

> +                       clk {
> +                               pins = "gpio0";
> +                               function = "swr_tx_clk";
> +                               drive-strength = <2>;
> +                               input-enable;
> +                               bias-pull-down;
> +                       };
> +
> +                       data {
> +                               pins = "gpio1", "gpio2", "gpio14";
> +                               function = "swr_tx_data";
> +                               drive-strength = <2>;
> +                               input-enable;
> +                               bias-bus-hold;
> +                       };
> +               };
> +       };
> +};
> --
> 2.7.4
>

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

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux
  2022-02-08 15:34 ` [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux Srinivasa Rao Mandadapu
@ 2022-02-08 21:12   ` Stephen Boyd
  2022-02-09 14:26     ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2022-02-08 21:12 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
	dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
	rohitkr, srinivas.kandagatla
  Cc: Venkata Prasad Potturu

Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:14)
> Add pinmux to reset wcd codec, conneceted 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 | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index 4704a93..6b38fa1 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -594,6 +594,21 @@
>                  */
>                 bias-pull-up;
>         };
> +
> +       wcd938x_reset_active: wcd938x_reset_active {

No underscore in node names.

> +                       pins = "gpio83";
> +                       function = "gpio";
> +                       drive-strength = <16>;
> +                       output-high;
> +       };
> +
> +       wcd938x_reset_sleep: wcd938x_reset_sleep {
> +                       pins = "gpio83";
> +                       function = "gpio";
> +                       drive-strength = <16>;
> +                       bias-disable;
> +                       output-low;

Why doesn't the device drive the reset gpio by requesting the gpio and
asserting and deasserting it? We shouldn't need to use pinctrl settings
to toggle reset gpios.

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

* Re: [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  2022-02-08 21:08   ` Stephen Boyd
@ 2022-02-09 13:42     ` Srinivasa Rao Mandadapu
  2022-02-10  0:07       ` Stephen Boyd
  0 siblings, 1 reply; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-09 13:42 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree, dianders,
	judyhsiao, linux-arm-msm, linux-kernel, robh+dt, rohitkr,
	srinivas.kandagatla
  Cc: Venkata Prasad Potturu


On 2/9/2022 2:38 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:12)
>> 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-idp.dtsi | 40 ++++++++++++++++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sc7280.dtsi     | 40 ++++++++++++++++++++++++++++++++
>>   2 files changed, 80 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index d623d71..c7d6c46 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -436,6 +436,39 @@
>>                  qcom,drive-strength = <3>;
>>          };
>>   };
> Newline here
Okay. will remove it.
>
>> +&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>;
>> +};
>> +
>> +&sec_mi2s_data0 {
>> +       drive-strength = <6>;
>> +       bias-disable;
>> +};
>> +
>> +&sec_mi2s_sclk {
>> +       drive-strength = <6>;
>> +       bias-disable;
>> +};
>> +
>> +&sec_mi2s_ws {
>> +       drive-strength = <6>;
>> +};
> Please sort these nodes alphabetically on node name.
Okay.
>
>>   &qspi_cs0 {
>>          bias-disable;
>> @@ -491,6 +524,13 @@
>>   };
>>
>>   &tlmm {
>> +       amp_en: amp-en {
>> +               pins = "gpio63";
>> +               function = "gpio";
>> +               bias-disable;
> Is there an external pull?
I think no external pull. In trogdor mentioned bias-pull-down but you 
suggested to remove it.
>
>> +               drive-strength = <2>;
>> +       };
>> +
>>          nvme_pwren: nvme-pwren {
>>                  function = "gpio";
>>          };
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 937c2e0..76e73e9 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -3461,6 +3461,46 @@
>>                                  };
>>                          };
>>
>> +                       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";
>> +                       };
>> +
>> +                       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";
>> +                       };
> Please sort these nodes alphabetically on node name.
Okay.

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

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node
  2022-02-08 21:11   ` Stephen Boyd
@ 2022-02-09 14:01     ` Srinivasa Rao Mandadapu
  2022-02-10  0:05       ` Stephen Boyd
  0 siblings, 1 reply; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-09 14:01 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree, dianders,
	judyhsiao, linux-arm-msm, linux-kernel, robh+dt, rohitkr,
	srinivas.kandagatla
  Cc: Venkata Prasad Potturu


On 2/9/2022 2:41 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:13)
>> 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 | 150 +++++++++++++++++++++++++++++++
>>   1 file changed, 150 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index c7d6c46..4704a93 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -638,3 +638,153 @@
>>                  bias-pull-up;
>>          };
>>   };
> Newline here.
Okay.
>
>> +&soc {
>> +       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>;
> Presumably this doesn't change so it should be moved to the sc7280.dtsi
> file as part of the soc node. It can be marked status = "disabled" if
> it's not commonly used, but I suspect it is commonly used on sc7280?
Okay. will move to common dtsi file.
>
>> +
>> +               dmic01_active: dmic01-active-pins {
> The '-pins' suffix is redundant. Please remove it.
Okay. will remove it.
>
>> +                       clk {
>> +                               pins = "gpio6";
>> +                               function = "dmic1_clk";
>> +                               drive-strength = <8>;
>> +                               output-high;
>> +                       };
> Please be consistent and have a newline between nodes.
Okay.
>
>> +                       data {
>> +                               pins = "gpio7";
>> +                               function = "dmic1_data";
>> +                               drive-strength = <8>;
>> +                               input-enable;
>> +                       };
>> +               };
>> +
>> +               dmic01_sleep: dmic01-sleep-pins {
>> +                       clk {
>> +                               pins = "gpio6";
>> +                               function = "dmic1_clk";
>> +                               drive-strength = <2>;
>> +                               bias-disable;
>> +                               output-low;
>> +                       };
>> +
>> +                       data {
>> +                               pins = "gpio7";
>> +                               function = "dmic1_data";
>> +                               drive-strength = <2>;
>> +                               pull-down;
>> +                               input-enable;
> Why does input-enable matter? It's not a gpio.
Actually the same is fallowed in sm8250.dtsi. Verified without it and 
working fine. Need take call on it.
>
>> +                       };
>> +               };
>> +
>> +               dmic23_active: dmic02-active-pins {
>> +                       clk {
>> +                               pins = "gpio8";
>> +                               function = "dmic2_clk";
>> +                               drive-strength = <8>;
>> +                               output-high;
>> +                       };
>> +                       data {
>> +                               pins = "gpio9";
>> +                               function = "dmic2_data";
>> +                               drive-strength = <8>;
>> +                               input-enable;
>> +                       };
>> +               };
>> +
>> +               dmic23_sleep: dmic02-sleep-pins {
>> +                       clk {
>> +                               pins = "gpio8";
>> +                               function = "dmic2_clk";
>> +                               drive-strength = <2>;
>> +                               bias-disable;
>> +                               output-low;
>> +                       };
>> +
>> +                       data {
>> +                               pins = "gpio9";
>> +                               function = "dmic2_data";
>> +                               drive-strength = <2>;
>> +                               pull-down;
>> +                               input-enable;
>> +                       };
>> +               };
>> +
>> +               rx_swr_active: rx_swr-active-pins {
>> +                       clk {
>> +                               pins = "gpio3";
>> +                               function = "swr_rx_clk";
>> +                               drive-strength = <2>;
>> +                               slew-rate = <1>;
>> +                               bias-disable;
>> +                       };
>> +
>> +                       data {
>> +                               pins = "gpio4", "gpio5";
>> +                               function = "swr_rx_data";
>> +                               drive-strength = <2>;
>> +                               slew-rate = <1>;
>> +                               bias-bus-hold;
>> +                       };
>> +               };
>> +
>> +               rx_swr_sleep: rx_swr-sleep-pins {
>> +                       clk {
>> +                               pins = "gpio3";
>> +                               function = "swr_rx_clk";
>> +                               drive-strength = <2>;
>> +                               input-enable;
>> +                               bias-pull-down;
>> +                       };
>> +
>> +                       data {
>> +                               pins = "gpio4", "gpio5";
>> +                               function = "swr_rx_data";
>> +                               drive-strength = <2>;
>> +                               input-enable;
>> +                               bias-pull-down;
>> +                       };
>> +               };
>> +
>> +               tx_swr_active: tx_swr-active-pins {
>> +                       clk {
>> +                               pins = "gpio0";
>> +                               function = "swr_tx_clk";
>> +                               drive-strength = <2>;
>> +                               slew-rate = <1>;
>> +                               bias-disable;
>> +                       };
>> +
>> +                       data {
>> +                               pins = "gpio1", "gpio2", "gpio14";
>> +                               function = "swr_tx_data";
>> +                               drive-strength = <2>;
>> +                               slew-rate = <1>;
>> +                               bias-bus-hold;
>> +                       };
>> +               };
>> +
>> +               tx_swr_sleep: tx_swr-sleep-pins {
> No underscore in node names.
Okay.
>
>> +                       clk {
>> +                               pins = "gpio0";
>> +                               function = "swr_tx_clk";
>> +                               drive-strength = <2>;
>> +                               input-enable;
>> +                               bias-pull-down;
>> +                       };
>> +
>> +                       data {
>> +                               pins = "gpio1", "gpio2", "gpio14";
>> +                               function = "swr_tx_data";
>> +                               drive-strength = <2>;
>> +                               input-enable;
>> +                               bias-bus-hold;
>> +                       };
>> +               };
>> +       };
>> +};
>> --
>> 2.7.4
>>

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

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux
  2022-02-08 21:12   ` Stephen Boyd
@ 2022-02-09 14:26     ` Srinivasa Rao Mandadapu
  2022-02-10  0:03       ` Stephen Boyd
  0 siblings, 1 reply; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-09 14:26 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree, dianders,
	judyhsiao, linux-arm-msm, linux-kernel, robh+dt, rohitkr,
	srinivas.kandagatla
  Cc: Venkata Prasad Potturu


On 2/9/2022 2:42 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:14)
>> Add pinmux to reset wcd codec, conneceted 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 | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index 4704a93..6b38fa1 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -594,6 +594,21 @@
>>                   */
>>                  bias-pull-up;
>>          };
>> +
>> +       wcd938x_reset_active: wcd938x_reset_active {
> No underscore in node names.
Okay. will remove it.
>
>> +                       pins = "gpio83";
>> +                       function = "gpio";
>> +                       drive-strength = <16>;
>> +                       output-high;
>> +       };
>> +
>> +       wcd938x_reset_sleep: wcd938x_reset_sleep {
>> +                       pins = "gpio83";
>> +                       function = "gpio";
>> +                       drive-strength = <16>;
>> +                       bias-disable;
>> +                       output-low;
> Why doesn't the device drive the reset gpio by requesting the gpio and
> asserting and deasserting it? We shouldn't need to use pinctrl settings
> to toggle reset gpios.
Okay. Verified without these nodes and didn't see any impact. But 
similar way it's mentioned in sm8250-mtp.dts. Could You please suggest 
on it how to go ahead on this?.

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

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux
  2022-02-09 14:26     ` Srinivasa Rao Mandadapu
@ 2022-02-10  0:03       ` Stephen Boyd
  2022-02-10 14:29         ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2022-02-10  0:03 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
	dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
	rohitkr, srinivas.kandagatla
  Cc: Venkata Prasad Potturu

Quoting Srinivasa Rao Mandadapu (2022-02-09 06:26:58)
>
> On 2/9/2022 2:42 AM, Stephen Boyd wrote:
> > Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:14)
> >
> >> +                       pins = "gpio83";
> >> +                       function = "gpio";
> >> +                       drive-strength = <16>;
> >> +                       output-high;
> >> +       };
> >> +
> >> +       wcd938x_reset_sleep: wcd938x_reset_sleep {
> >> +                       pins = "gpio83";
> >> +                       function = "gpio";
> >> +                       drive-strength = <16>;
> >> +                       bias-disable;
> >> +                       output-low;
> > Why doesn't the device drive the reset gpio by requesting the gpio and
> > asserting and deasserting it? We shouldn't need to use pinctrl settings
> > to toggle reset gpios.
> Okay. Verified without these nodes and didn't see any impact. But
> similar way it's mentioned in sm8250-mtp.dts. Could You please suggest
> on it how to go ahead on this?.

I'd expect the wcd938x codec device node to have a 'reset-gpios'
property like

	reset-gpios = <&tlmm 83 GPIO_ACTIVE_LOW>

and then the driver to request that gpio via

	reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);

so it gets the gpio during driver probe. Then the gpio can be deasserted
during suspend and reasserted on resume, if that's even important?

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

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node
  2022-02-09 14:01     ` Srinivasa Rao Mandadapu
@ 2022-02-10  0:05       ` Stephen Boyd
  2022-02-10 14:27         ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2022-02-10  0:05 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
	dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
	rohitkr, srinivas.kandagatla
  Cc: Venkata Prasad Potturu

Quoting Srinivasa Rao Mandadapu (2022-02-09 06:01:21)
>
> On 2/9/2022 2:41 AM, Stephen Boyd wrote:
> > Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:13)
> >> +                       data {
> >> +                               pins = "gpio7";
> >> +                               function = "dmic1_data";
> >> +                               drive-strength = <8>;
> >> +                               input-enable;
> >> +                       };
> >> +               };
> >> +
> >> +               dmic01_sleep: dmic01-sleep-pins {
> >> +                       clk {
> >> +                               pins = "gpio6";
> >> +                               function = "dmic1_clk";
> >> +                               drive-strength = <2>;
> >> +                               bias-disable;
> >> +                               output-low;
> >> +                       };
> >> +
> >> +                       data {
> >> +                               pins = "gpio7";
> >> +                               function = "dmic1_data";
> >> +                               drive-strength = <2>;
> >> +                               pull-down;
> >> +                               input-enable;
> > Why does input-enable matter? It's not a gpio.
> Actually the same is fallowed in sm8250.dtsi. Verified without it and
> working fine. Need take call on it.

Is that because the pin is already an input by default? What does gpio
debugfs say for this pin? Does it also work if you make it
output-low/output-high here? I thought that the gpio itself isn't muxed
out to the pad unless the function is "gpio" so I hope the input/output
settings have no effect here.

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

* Re: [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  2022-02-09 13:42     ` Srinivasa Rao Mandadapu
@ 2022-02-10  0:07       ` Stephen Boyd
  2022-02-10 14:01         ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2022-02-10  0:07 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, devicetree,
	dianders, judyhsiao, linux-arm-msm, linux-kernel, robh+dt,
	rohitkr, srinivas.kandagatla
  Cc: Venkata Prasad Potturu

Quoting Srinivasa Rao Mandadapu (2022-02-09 05:42:40)
>
> On 2/9/2022 2:38 AM, Stephen Boyd wrote:
> > Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:12)
> >>   &qspi_cs0 {
> >>          bias-disable;
> >> @@ -491,6 +524,13 @@
> >>   };
> >>
> >>   &tlmm {
> >> +       amp_en: amp-en {
> >> +               pins = "gpio63";
> >> +               function = "gpio";
> >> +               bias-disable;
> > Is there an external pull?
> I think no external pull. In trogdor mentioned bias-pull-down but you
> suggested to remove it.

Maybe on trogdor there was an external pull inside the amp that this pin
is connected to? Usually we have a comment like /* Has external
pull-{up,down} */ so please add that here depending on which way the
pull goes.

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

* Re: [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset
  2022-02-10  0:07       ` Stephen Boyd
@ 2022-02-10 14:01         ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-10 14:01 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree, dianders,
	judyhsiao, linux-arm-msm, linux-kernel, robh+dt, rohitkr,
	srinivas.kandagatla
  Cc: Venkata Prasad Potturu


On 2/10/2022 5:37 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-09 05:42:40)
>> On 2/9/2022 2:38 AM, Stephen Boyd wrote:
>>> Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:12)
>>>>    &qspi_cs0 {
>>>>           bias-disable;
>>>> @@ -491,6 +524,13 @@
>>>>    };
>>>>
>>>>    &tlmm {
>>>> +       amp_en: amp-en {
>>>> +               pins = "gpio63";
>>>> +               function = "gpio";
>>>> +               bias-disable;
>>> Is there an external pull?
>> I think no external pull. In trogdor mentioned bias-pull-down but you
>> suggested to remove it.
> Maybe on trogdor there was an external pull inside the amp that this pin
> is connected to? Usually we have a comment like /* Has external
> pull-{up,down} */ so please add that here depending on which way the
> pull goes.

As per Anderson suggestion we removed bias-pull-down. Actually, it's 
up-streamed for same platform in sc7280-herobrine.dtsi.

We will fallow the same. It contains bias-pull-down.


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

* Re: [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node
  2022-02-10  0:05       ` Stephen Boyd
@ 2022-02-10 14:27         ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-10 14:27 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree, dianders,
	judyhsiao, linux-arm-msm, linux-kernel, robh+dt, rohitkr,
	srinivas.kandagatla
  Cc: Venkata Prasad Potturu


On 2/10/2022 5:35 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-09 06:01:21)
>> On 2/9/2022 2:41 AM, Stephen Boyd wrote:
>>> Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:13)
>>>> +                       data {
>>>> +                               pins = "gpio7";
>>>> +                               function = "dmic1_data";
>>>> +                               drive-strength = <8>;
>>>> +                               input-enable;
>>>> +                       };
>>>> +               };
>>>> +
>>>> +               dmic01_sleep: dmic01-sleep-pins {
>>>> +                       clk {
>>>> +                               pins = "gpio6";
>>>> +                               function = "dmic1_clk";
>>>> +                               drive-strength = <2>;
>>>> +                               bias-disable;
>>>> +                               output-low;
>>>> +                       };
>>>> +
>>>> +                       data {
>>>> +                               pins = "gpio7";
>>>> +                               function = "dmic1_data";
>>>> +                               drive-strength = <2>;
>>>> +                               pull-down;
>>>> +                               input-enable;
>>> Why does input-enable matter? It's not a gpio.
>> Actually the same is fallowed in sm8250.dtsi. Verified without it and
>> working fine. Need take call on it.
> Is that because the pin is already an input by default? What does gpio
> debugfs say for this pin? Does it also work if you make it
> output-low/output-high here? I thought that the gpio itself isn't muxed
> out to the pad unless the function is "gpio" so I hope the input/output
> settings have no effect here.

Pin is in by default. debugfs says

gpio7 : in 1 8mA no pull

verified in downstream code also. Same is followed there also.


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

* Re: [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux
  2022-02-10  0:03       ` Stephen Boyd
@ 2022-02-10 14:29         ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 16+ messages in thread
From: Srinivasa Rao Mandadapu @ 2022-02-10 14:29 UTC (permalink / raw)
  To: Stephen Boyd, agross, bjorn.andersson, devicetree, dianders,
	judyhsiao, linux-arm-msm, linux-kernel, robh+dt, rohitkr,
	srinivas.kandagatla
  Cc: Venkata Prasad Potturu


On 2/10/2022 5:33 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-02-09 06:26:58)
>> On 2/9/2022 2:42 AM, Stephen Boyd wrote:
>>> Quoting Srinivasa Rao Mandadapu (2022-02-08 07:34:14)
>>>
>>>> +                       pins = "gpio83";
>>>> +                       function = "gpio";
>>>> +                       drive-strength = <16>;
>>>> +                       output-high;
>>>> +       };
>>>> +
>>>> +       wcd938x_reset_sleep: wcd938x_reset_sleep {
>>>> +                       pins = "gpio83";
>>>> +                       function = "gpio";
>>>> +                       drive-strength = <16>;
>>>> +                       bias-disable;
>>>> +                       output-low;
>>> Why doesn't the device drive the reset gpio by requesting the gpio and
>>> asserting and deasserting it? We shouldn't need to use pinctrl settings
>>> to toggle reset gpios.
>> Okay. Verified without these nodes and didn't see any impact. But
>> similar way it's mentioned in sm8250-mtp.dts. Could You please suggest
>> on it how to go ahead on this?.
> I'd expect the wcd938x codec device node to have a 'reset-gpios'
> property like
>
> 	reset-gpios = <&tlmm 83 GPIO_ACTIVE_LOW>
>
> and then the driver to request that gpio via
>
> 	reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>
> so it gets the gpio during driver probe. Then the gpio can be deasserted
> during suspend and reasserted on resume, if that's even important?
Okay will remove it. Already wcd938x node has reset gpio. It seems these 
are redundant.

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

end of thread, other threads:[~2022-02-10 14:29 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 15:34 [PATCH v2 0/3] Add lpass pin control support for audio on sc7280 based targets Srinivasa Rao Mandadapu
2022-02-08 15:34 ` [PATCH v2 1/3] arm64: dts: qcom: sc7280: Add pinmux for I2S speaker and Headset Srinivasa Rao Mandadapu
2022-02-08 21:08   ` Stephen Boyd
2022-02-09 13:42     ` Srinivasa Rao Mandadapu
2022-02-10  0:07       ` Stephen Boyd
2022-02-10 14:01         ` Srinivasa Rao Mandadapu
2022-02-08 15:34 ` [PATCH v2 2/3] arm64: dts: qcom: sc7280: add lpass lpi pin controller node Srinivasa Rao Mandadapu
2022-02-08 21:11   ` Stephen Boyd
2022-02-09 14:01     ` Srinivasa Rao Mandadapu
2022-02-10  0:05       ` Stephen Boyd
2022-02-10 14:27         ` Srinivasa Rao Mandadapu
2022-02-08 15:34 ` [PATCH v2 3/3] arm64: dts: qcom: sc7280: Add wcd9380 pinmux Srinivasa Rao Mandadapu
2022-02-08 21:12   ` Stephen Boyd
2022-02-09 14:26     ` Srinivasa Rao Mandadapu
2022-02-10  0:03       ` Stephen Boyd
2022-02-10 14:29         ` Srinivasa Rao Mandadapu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.