diff mbox series

[V4,1/4] arm64: dts: sc7180: Add wakeup support over UART RX

Message ID 1599145498-20707-2-git-send-email-skakit@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Add wakeup support over UART RX | expand

Commit Message

Satya Priya Sept. 3, 2020, 3:04 p.m. UTC
Add the necessary pinctrl and interrupts to make UART wakeup capable.

If QUP function is selected in sleep state, UART RTS/RFR is pulled high
during suspend and BT SoC not able to send wakeup bytes. So, configure
GPIO mode in sleep state to keep it low during suspend.

Signed-off-by: satya priya <skakit@codeaurora.org>
Reviewed-by: Akash Asthana <akashast@codeaurora.org>
---
Changes in V2:
 - As per Matthias's comment added wakeup support for all the UARTs
   of SC7180.

Changes in V3:
 - No change.

Changes in V4:
 - As per Matthias's comment, added the reason for configuring GPIO
   mode for sleep state in commit text.

 arch/arm64/boot/dts/qcom/sc7180.dtsi | 98 ++++++++++++++++++++++++++++++------
 1 file changed, 84 insertions(+), 14 deletions(-)

Comments

Matthias Kaehlcke Sept. 3, 2020, 4:05 p.m. UTC | #1
On Thu, Sep 03, 2020 at 08:34:55PM +0530, satya priya wrote:
> Add the necessary pinctrl and interrupts to make UART wakeup capable.
> 
> If QUP function is selected in sleep state, UART RTS/RFR is pulled high
> during suspend and BT SoC not able to send wakeup bytes. So, configure
> GPIO mode in sleep state to keep it low during suspend.
> 
> Signed-off-by: satya priya <skakit@codeaurora.org>
> Reviewed-by: Akash Asthana <akashast@codeaurora.org>
> ---
> Changes in V2:
>  - As per Matthias's comment added wakeup support for all the UARTs
>    of SC7180.
> 
> Changes in V3:
>  - No change.
> 
> Changes in V4:
>  - As per Matthias's comment, added the reason for configuring GPIO
>    mode for sleep state in commit text.
> 
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 98 ++++++++++++++++++++++++++++++------
>  1 file changed, 84 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index d46b383..855b13e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -793,9 +793,11 @@
>  				reg = <0 0x00880000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart0_default>;
> -				interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart0_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 37 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
> @@ -845,9 +847,11 @@
>  				reg = <0 0x00884000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart1_default>;
> -				interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart1_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 3 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
> @@ -931,9 +935,11 @@
>  				reg = <0 0x0088c000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP0_S3_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart3_default>;
> -				interrupts = <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart3_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 41 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
> @@ -1017,9 +1023,11 @@
>  				reg = <0 0x00894000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart5_default>;
> -				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart5_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 28 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
> @@ -1084,9 +1092,11 @@
>  				reg = <0 0x00a80000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP1_S0_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart6_default>;
> -				interrupts = <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart6_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 62 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt SLAVE_QUP_CORE_1>,
> @@ -1256,9 +1266,11 @@
>  				reg = <0 0x00a90000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP1_S4_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart10_default>;
> -				interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart10_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 89 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt SLAVE_QUP_CORE_1>,
> @@ -1308,9 +1320,11 @@
>  				reg = <0 0x00a94000 0 0x4000>;
>  				clock-names = "se";
>  				clocks = <&gcc GCC_QUPV3_WRAP1_S5_CLK>;
> -				pinctrl-names = "default";
> +				pinctrl-names = "default", "sleep";
>  				pinctrl-0 = <&qup_uart11_default>;
> -				interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
> +				pinctrl-1 = <&qup_uart11_sleep>;
> +				interrupts-extended = <&intc GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>,
> +							<&tlmm 56 IRQ_TYPE_EDGE_FALLING>;
>  				power-domains = <&rpmhpd SC7180_CX>;
>  				operating-points-v2 = <&qup_opp_table>;
>  				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt SLAVE_QUP_CORE_1>,
> @@ -1638,6 +1652,14 @@
>  				};
>  			};
>  
> +			qup_uart0_sleep: qup-uart0-sleep {
> +				pinmux {
> +					pins = "gpio34", "gpio35",
> +					       "gpio36", "gpio37";
> +					function = "gpio";
> +				};
> +			};
> +
>  			qup_uart1_default: qup-uart1-default {
>  				pinmux {
>  					pins = "gpio0", "gpio1",
> @@ -1646,6 +1668,14 @@
>  				};
>  			};
>  
> +			qup_uart1_sleep: qup-uart1-sleep {
> +				pinmux {
> +					pins = "gpio0", "gpio1",
> +					       "gpio2", "gpio3";
> +					function = "gpio";
> +				};
> +			};
> +
>  			qup_uart2_default: qup-uart2-default {
>  				pinmux {
>  					pins = "gpio15", "gpio16";
> @@ -1661,6 +1691,14 @@
>  				};
>  			};
>  
> +			qup_uart3_sleep: qup-uart3-sleep {
> +				pinmux {
> +					pins = "gpio38", "gpio39",
> +					       "gpio40", "gpio41";
> +					function = "gpio";
> +				};
> +			};
> +
>  			qup_uart4_default: qup-uart4-default {
>  				pinmux {
>  					pins = "gpio115", "gpio116";
> @@ -1676,6 +1714,14 @@
>  				};
>  			};
>  
> +			qup_uart5_sleep: qup-uart5-sleep {
> +				pinmux {
> +					pins = "gpio25", "gpio26",
> +					       "gpio27", "gpio28";
> +					function = "gpio";
> +				};
> +			};
> +
>  			qup_uart6_default: qup-uart6-default {
>  				pinmux {
>  					pins = "gpio59", "gpio60",
> @@ -1684,6 +1730,14 @@
>  				};
>  			};
>  
> +			qup_uart6_sleep: qup-uart6-sleep {
> +				pinmux {
> +					pins = "gpio59", "gpio60",
> +					       "gpio61", "gpio62";
> +					function = "gpio";
> +				};
> +			};
> +
>  			qup_uart7_default: qup-uart7-default {
>  				pinmux {
>  					pins = "gpio6", "gpio7";
> @@ -1713,6 +1767,14 @@
>  				};
>  			};
>  
> +			qup_uart10_sleep: qup-uart10-sleep {
> +				pinmux {
> +					pins = "gpio86", "gpio87",
> +					       "gpio88", "gpio89";
> +					function = "gpio";
> +				};
> +			};
> +
>  			qup_uart11_default: qup-uart11-default {
>  				pinmux {
>  					pins = "gpio53", "gpio54",
> @@ -1721,6 +1783,14 @@
>  				};
>  			};
>  
> +			qup_uart11_sleep: qup-uart11-sleep {
> +				pinmux {
> +					pins = "gpio53", "gpio54",
> +					       "gpio55", "gpio56";
> +					function = "gpio";
> +				};
> +			};
> +
>  			sdc1_on: sdc1-on {
>  				pinconf-clk {
>  					pins = "sdc1_clk";

It seems only the RTS pin actually requires a pinmux change. One could
argue that only the muxing of this pin should be changed in sleep mode.
But well, changing all pins to GPIO simplifies the config, so I guess
it's ok as long as there are no side effects.

I noticed you changed only the UARTs that have RTS/CTS signals. Do the
others not support wakeup? I understand that the pinmux change isn't
needed for these UARTs, since they have no RTS signal, however I'd expect
them to need the 'interrupts-extended' entry if they support wakeup.
Matthias Kaehlcke Sept. 3, 2020, 5:17 p.m. UTC | #2
On Thu, Sep 03, 2020 at 08:34:55PM +0530, satya priya wrote:
> Add the necessary pinctrl and interrupts to make UART wakeup capable.
> 
> If QUP function is selected in sleep state, UART RTS/RFR is pulled high
> during suspend and BT SoC not able to send wakeup bytes. So, configure
> GPIO mode in sleep state to keep it low during suspend.
> 
> Signed-off-by: satya priya <skakit@codeaurora.org>
> Reviewed-by: Akash Asthana <akashast@codeaurora.org>

One more doubt: does it actually make sense/is it safe to add the
sleep config for all UARTs in the SoC file? I wonder if there could
be undesired behavior (like noise on TX or RTS looking active to the
other side) without the corresponding pinconf in the board file. If
the pinconf is needed to avoid unexpected behavior then it is better
to change the muxing in the board file to have a sane default
configuration in the SoC .dtsi.

From a quick grep it seems that most SoCs don't specify a sleep config
for their UART pins and some boards add it in their DT.
Satya Priya Sept. 9, 2020, 2:19 p.m. UTC | #3
Hi Matthias,

On 2020-09-03 21:35, Matthias Kaehlcke wrote:
> On Thu, Sep 03, 2020 at 08:34:55PM +0530, satya priya wrote:
>> Add the necessary pinctrl and interrupts to make UART wakeup capable.
>> 
>> If QUP function is selected in sleep state, UART RTS/RFR is pulled 
>> high
>> during suspend and BT SoC not able to send wakeup bytes. So, configure
>> GPIO mode in sleep state to keep it low during suspend.
>> 
>> Signed-off-by: satya priya <skakit@codeaurora.org>
>> Reviewed-by: Akash Asthana <akashast@codeaurora.org>
>> ---
>> Changes in V2:
>>  - As per Matthias's comment added wakeup support for all the UARTs
>>    of SC7180.
>> 
>> Changes in V3:
>>  - No change.
>> 
>> Changes in V4:
>>  - As per Matthias's comment, added the reason for configuring GPIO
>>    mode for sleep state in commit text.
>> 
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 98 
>> ++++++++++++++++++++++++++++++------
>>  1 file changed, 84 insertions(+), 14 deletions(-)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index d46b383..855b13e 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -793,9 +793,11 @@
>>  				reg = <0 0x00880000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart0_default>;
>> -				interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart0_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 37 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt 
>> SLAVE_QUP_CORE_0>,
>> @@ -845,9 +847,11 @@
>>  				reg = <0 0x00884000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart1_default>;
>> -				interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart1_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 3 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt 
>> SLAVE_QUP_CORE_0>,
>> @@ -931,9 +935,11 @@
>>  				reg = <0 0x0088c000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP0_S3_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart3_default>;
>> -				interrupts = <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart3_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 41 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt 
>> SLAVE_QUP_CORE_0>,
>> @@ -1017,9 +1023,11 @@
>>  				reg = <0 0x00894000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart5_default>;
>> -				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart5_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 28 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt 
>> SLAVE_QUP_CORE_0>,
>> @@ -1084,9 +1092,11 @@
>>  				reg = <0 0x00a80000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP1_S0_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart6_default>;
>> -				interrupts = <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart6_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 62 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt 
>> SLAVE_QUP_CORE_1>,
>> @@ -1256,9 +1266,11 @@
>>  				reg = <0 0x00a90000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP1_S4_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart10_default>;
>> -				interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart10_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 89 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt 
>> SLAVE_QUP_CORE_1>,
>> @@ -1308,9 +1320,11 @@
>>  				reg = <0 0x00a94000 0 0x4000>;
>>  				clock-names = "se";
>>  				clocks = <&gcc GCC_QUPV3_WRAP1_S5_CLK>;
>> -				pinctrl-names = "default";
>> +				pinctrl-names = "default", "sleep";
>>  				pinctrl-0 = <&qup_uart11_default>;
>> -				interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
>> +				pinctrl-1 = <&qup_uart11_sleep>;
>> +				interrupts-extended = <&intc GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>,
>> +							<&tlmm 56 IRQ_TYPE_EDGE_FALLING>;
>>  				power-domains = <&rpmhpd SC7180_CX>;
>>  				operating-points-v2 = <&qup_opp_table>;
>>  				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt 
>> SLAVE_QUP_CORE_1>,
>> @@ -1638,6 +1652,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart0_sleep: qup-uart0-sleep {
>> +				pinmux {
>> +					pins = "gpio34", "gpio35",
>> +					       "gpio36", "gpio37";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			qup_uart1_default: qup-uart1-default {
>>  				pinmux {
>>  					pins = "gpio0", "gpio1",
>> @@ -1646,6 +1668,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart1_sleep: qup-uart1-sleep {
>> +				pinmux {
>> +					pins = "gpio0", "gpio1",
>> +					       "gpio2", "gpio3";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			qup_uart2_default: qup-uart2-default {
>>  				pinmux {
>>  					pins = "gpio15", "gpio16";
>> @@ -1661,6 +1691,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart3_sleep: qup-uart3-sleep {
>> +				pinmux {
>> +					pins = "gpio38", "gpio39",
>> +					       "gpio40", "gpio41";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			qup_uart4_default: qup-uart4-default {
>>  				pinmux {
>>  					pins = "gpio115", "gpio116";
>> @@ -1676,6 +1714,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart5_sleep: qup-uart5-sleep {
>> +				pinmux {
>> +					pins = "gpio25", "gpio26",
>> +					       "gpio27", "gpio28";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			qup_uart6_default: qup-uart6-default {
>>  				pinmux {
>>  					pins = "gpio59", "gpio60",
>> @@ -1684,6 +1730,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart6_sleep: qup-uart6-sleep {
>> +				pinmux {
>> +					pins = "gpio59", "gpio60",
>> +					       "gpio61", "gpio62";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			qup_uart7_default: qup-uart7-default {
>>  				pinmux {
>>  					pins = "gpio6", "gpio7";
>> @@ -1713,6 +1767,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart10_sleep: qup-uart10-sleep {
>> +				pinmux {
>> +					pins = "gpio86", "gpio87",
>> +					       "gpio88", "gpio89";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			qup_uart11_default: qup-uart11-default {
>>  				pinmux {
>>  					pins = "gpio53", "gpio54",
>> @@ -1721,6 +1783,14 @@
>>  				};
>>  			};
>> 
>> +			qup_uart11_sleep: qup-uart11-sleep {
>> +				pinmux {
>> +					pins = "gpio53", "gpio54",
>> +					       "gpio55", "gpio56";
>> +					function = "gpio";
>> +				};
>> +			};
>> +
>>  			sdc1_on: sdc1-on {
>>  				pinconf-clk {
>>  					pins = "sdc1_clk";
> 
> It seems only the RTS pin actually requires a pinmux change. One could
> argue that only the muxing of this pin should be changed in sleep mode.
> But well, changing all pins to GPIO simplifies the config, so I guess
> it's ok as long as there are no side effects.
> 
> I noticed you changed only the UARTs that have RTS/CTS signals. Do the
> others not support wakeup? I understand that the pinmux change isn't
> needed for these UARTs, since they have no RTS signal, however I'd 
> expect
> them to need the 'interrupts-extended' entry if they support wakeup.

We are planning to add the wakeup related changes(interrupts-extended, 
pin ctrls) to only uart3 in board specific file. As we understand, this 
wakeup is an optional feature and cannot be added to all the UARTs. So, 
now all the changes will be added in board specific files including the 
pinmux change for sleep state.

Thanks,
Satya Priya
Satya Priya Sept. 9, 2020, 2:20 p.m. UTC | #4
On 2020-09-03 22:47, Matthias Kaehlcke wrote:
> On Thu, Sep 03, 2020 at 08:34:55PM +0530, satya priya wrote:
>> Add the necessary pinctrl and interrupts to make UART wakeup capable.
>> 
>> If QUP function is selected in sleep state, UART RTS/RFR is pulled 
>> high
>> during suspend and BT SoC not able to send wakeup bytes. So, configure
>> GPIO mode in sleep state to keep it low during suspend.
>> 
>> Signed-off-by: satya priya <skakit@codeaurora.org>
>> Reviewed-by: Akash Asthana <akashast@codeaurora.org>
> 
> One more doubt: does it actually make sense/is it safe to add the
> sleep config for all UARTs in the SoC file?

yes, it is not good to have it this way. We are going to remove all this 
and add sleep config for only uart3 (BT uart) in board specific file.

> I wonder if there could
> be undesired behavior (like noise on TX or RTS looking active to the
> other side) without the corresponding pinconf in the board file. If
> the pinconf is needed to avoid unexpected behavior then it is better
> to change the muxing in the board file to have a sane default
> configuration in the SoC .dtsi.
> 

ok, will add pinmux for uart3 sleep state in board file.

> From a quick grep it seems that most SoCs don't specify a sleep config
> for their UART pins and some boards add it in their DT.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index d46b383..855b13e 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -793,9 +793,11 @@ 
 				reg = <0 0x00880000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart0_default>;
-				interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart0_sleep>;
+				interrupts-extended = <&intc GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 37 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
@@ -845,9 +847,11 @@ 
 				reg = <0 0x00884000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP0_S1_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart1_default>;
-				interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart1_sleep>;
+				interrupts-extended = <&intc GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 3 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
@@ -931,9 +935,11 @@ 
 				reg = <0 0x0088c000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP0_S3_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart3_default>;
-				interrupts = <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart3_sleep>;
+				interrupts-extended = <&intc GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 41 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
@@ -1017,9 +1023,11 @@ 
 				reg = <0 0x00894000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP0_S5_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart5_default>;
-				interrupts = <GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart5_sleep>;
+				interrupts-extended = <&intc GIC_SPI 606 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 28 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_0 &qup_virt SLAVE_QUP_CORE_0>,
@@ -1084,9 +1092,11 @@ 
 				reg = <0 0x00a80000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP1_S0_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart6_default>;
-				interrupts = <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart6_sleep>;
+				interrupts-extended = <&intc GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 62 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt SLAVE_QUP_CORE_1>,
@@ -1256,9 +1266,11 @@ 
 				reg = <0 0x00a90000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP1_S4_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart10_default>;
-				interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart10_sleep>;
+				interrupts-extended = <&intc GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 89 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt SLAVE_QUP_CORE_1>,
@@ -1308,9 +1320,11 @@ 
 				reg = <0 0x00a94000 0 0x4000>;
 				clock-names = "se";
 				clocks = <&gcc GCC_QUPV3_WRAP1_S5_CLK>;
-				pinctrl-names = "default";
+				pinctrl-names = "default", "sleep";
 				pinctrl-0 = <&qup_uart11_default>;
-				interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
+				pinctrl-1 = <&qup_uart11_sleep>;
+				interrupts-extended = <&intc GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>,
+							<&tlmm 56 IRQ_TYPE_EDGE_FALLING>;
 				power-domains = <&rpmhpd SC7180_CX>;
 				operating-points-v2 = <&qup_opp_table>;
 				interconnects = <&qup_virt MASTER_QUP_CORE_1 &qup_virt SLAVE_QUP_CORE_1>,
@@ -1638,6 +1652,14 @@ 
 				};
 			};
 
+			qup_uart0_sleep: qup-uart0-sleep {
+				pinmux {
+					pins = "gpio34", "gpio35",
+					       "gpio36", "gpio37";
+					function = "gpio";
+				};
+			};
+
 			qup_uart1_default: qup-uart1-default {
 				pinmux {
 					pins = "gpio0", "gpio1",
@@ -1646,6 +1668,14 @@ 
 				};
 			};
 
+			qup_uart1_sleep: qup-uart1-sleep {
+				pinmux {
+					pins = "gpio0", "gpio1",
+					       "gpio2", "gpio3";
+					function = "gpio";
+				};
+			};
+
 			qup_uart2_default: qup-uart2-default {
 				pinmux {
 					pins = "gpio15", "gpio16";
@@ -1661,6 +1691,14 @@ 
 				};
 			};
 
+			qup_uart3_sleep: qup-uart3-sleep {
+				pinmux {
+					pins = "gpio38", "gpio39",
+					       "gpio40", "gpio41";
+					function = "gpio";
+				};
+			};
+
 			qup_uart4_default: qup-uart4-default {
 				pinmux {
 					pins = "gpio115", "gpio116";
@@ -1676,6 +1714,14 @@ 
 				};
 			};
 
+			qup_uart5_sleep: qup-uart5-sleep {
+				pinmux {
+					pins = "gpio25", "gpio26",
+					       "gpio27", "gpio28";
+					function = "gpio";
+				};
+			};
+
 			qup_uart6_default: qup-uart6-default {
 				pinmux {
 					pins = "gpio59", "gpio60",
@@ -1684,6 +1730,14 @@ 
 				};
 			};
 
+			qup_uart6_sleep: qup-uart6-sleep {
+				pinmux {
+					pins = "gpio59", "gpio60",
+					       "gpio61", "gpio62";
+					function = "gpio";
+				};
+			};
+
 			qup_uart7_default: qup-uart7-default {
 				pinmux {
 					pins = "gpio6", "gpio7";
@@ -1713,6 +1767,14 @@ 
 				};
 			};
 
+			qup_uart10_sleep: qup-uart10-sleep {
+				pinmux {
+					pins = "gpio86", "gpio87",
+					       "gpio88", "gpio89";
+					function = "gpio";
+				};
+			};
+
 			qup_uart11_default: qup-uart11-default {
 				pinmux {
 					pins = "gpio53", "gpio54",
@@ -1721,6 +1783,14 @@ 
 				};
 			};
 
+			qup_uart11_sleep: qup-uart11-sleep {
+				pinmux {
+					pins = "gpio53", "gpio54",
+					       "gpio55", "gpio56";
+					function = "gpio";
+				};
+			};
+
 			sdc1_on: sdc1-on {
 				pinconf-clk {
 					pins = "sdc1_clk";