* [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* 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 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 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
* [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* 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 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 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 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
* [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 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 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 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