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 <[email protected]>
> Co-developed-by: Venkata Prasad Potturu <[email protected]>
> Signed-off-by: Venkata Prasad Potturu <[email protected]>
> ---
> 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
>
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 <[email protected]>
>> Co-developed-by: Venkata Prasad Potturu <[email protected]>
>> Signed-off-by: Venkata Prasad Potturu <[email protected]>
>> ---
>> 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
>>
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.
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.