| Message ID | 1644334454-16719-1-git-send-email-quic_srivasam@quicinc.com |
|---|---|
| Headers | show |
| Series | Add lpass pin control support for audio on sc7280 based targets | expand |
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.
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.
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.
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?.
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?
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.
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.
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(+)