| Message ID | 1644334454-16719-4-git-send-email-quic_srivasam@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add lpass pin control support for audio on sc7280 based targets | expand |
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: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: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.
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 {