* [PATCH V4 0/4] Add wakeup support over UART RX @ 2020-09-03 15:04 satya priya 2020-09-03 15:04 ` [PATCH V4 1/4] arm64: dts: sc7180: " satya priya ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: satya priya @ 2020-09-03 15:04 UTC (permalink / raw) To: Bjorn Andersson Cc: Matthias Kaehlcke, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy, satya priya Changes in V2: - As per Matthias's comment added wakeup support for all the UARTs of SC7180. - Added sleep state in sc7180-idp.dts file. - Modify the if check in set_mctrl API in serial driver to avoid making RFR high during suspend. Changes in V3: - As per Matthias's comments modify the idp dts pin config to keep only the required pin settings. - Remove the extra parentheses from serial driver patch. Changes in V4: - As per Matthias's comments, change the commit text to mention why GPIO function needs to be selected in sleep. - Add separate patch for improvements made in pin conf settings. satya priya (4): arm64: dts: sc7180: Add wakeup support over UART RX arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX arm64: dts: qcom: sc7180: Add sleep pin ctrl for BT UART tty: serial: qcom_geni_serial: Fix the UART wakeup issue arch/arm64/boot/dts/qcom/sc7180-idp.dts | 51 ++++++++++++++--- arch/arm64/boot/dts/qcom/sc7180.dtsi | 98 ++++++++++++++++++++++++++++----- drivers/tty/serial/qcom_geni_serial.c | 2 +- 3 files changed, 128 insertions(+), 23 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V4 1/4] arm64: dts: sc7180: Add wakeup support over UART RX 2020-09-03 15:04 [PATCH V4 0/4] Add wakeup support over UART RX satya priya @ 2020-09-03 15:04 ` satya priya 2020-09-03 16:05 ` Matthias Kaehlcke 2020-09-03 17:17 ` Matthias Kaehlcke 2020-09-03 15:04 ` [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX satya priya ` (2 subsequent siblings) 3 siblings, 2 replies; 17+ messages in thread From: satya priya @ 2020-09-03 15:04 UTC (permalink / raw) To: Bjorn Andersson Cc: Matthias Kaehlcke, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy, satya priya 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"; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V4 1/4] arm64: dts: sc7180: Add wakeup support over UART RX 2020-09-03 15:04 ` [PATCH V4 1/4] arm64: dts: sc7180: " satya priya @ 2020-09-03 16:05 ` Matthias Kaehlcke 2020-09-09 14:19 ` skakit 2020-09-03 17:17 ` Matthias Kaehlcke 1 sibling, 1 reply; 17+ messages in thread From: Matthias Kaehlcke @ 2020-09-03 16:05 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 1/4] arm64: dts: sc7180: Add wakeup support over UART RX 2020-09-03 16:05 ` Matthias Kaehlcke @ 2020-09-09 14:19 ` skakit 0 siblings, 0 replies; 17+ messages in thread From: skakit @ 2020-09-09 14:19 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 1/4] arm64: dts: sc7180: Add wakeup support over UART RX 2020-09-03 15:04 ` [PATCH V4 1/4] arm64: dts: sc7180: " satya priya 2020-09-03 16:05 ` Matthias Kaehlcke @ 2020-09-03 17:17 ` Matthias Kaehlcke 2020-09-09 14:20 ` skakit 1 sibling, 1 reply; 17+ messages in thread From: Matthias Kaehlcke @ 2020-09-03 17:17 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 1/4] arm64: dts: sc7180: Add wakeup support over UART RX 2020-09-03 17:17 ` Matthias Kaehlcke @ 2020-09-09 14:20 ` skakit 0 siblings, 0 replies; 17+ messages in thread From: skakit @ 2020-09-09 14:20 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX 2020-09-03 15:04 [PATCH V4 0/4] Add wakeup support over UART RX satya priya 2020-09-03 15:04 ` [PATCH V4 1/4] arm64: dts: sc7180: " satya priya @ 2020-09-03 15:04 ` satya priya 2020-09-03 16:14 ` Matthias Kaehlcke 2020-09-09 21:28 ` Doug Anderson 2020-09-03 15:04 ` [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART satya priya 2020-09-03 15:04 ` [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue satya priya 3 siblings, 2 replies; 17+ messages in thread From: satya priya @ 2020-09-03 15:04 UTC (permalink / raw) To: Bjorn Andersson Cc: Matthias Kaehlcke, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy, satya priya Configure no-pull for CTS, as this is driven by BT do not specify any pull in order to not conflict with BT pulls. Remove output-high from CTS and TX as this is not really required. During bringup to fix transfer failures this was added to match with console uart settings. Probably some boot loader config was missing then. As it is working fine now, remove it. Signed-off-by: satya priya <skakit@codeaurora.org> Reviewed-by: Akash Asthana <akashast@codeaurora.org> --- Changes in V4: - This is newly added in V4 to separate the improvements in pin settings and wakeup related changes. arch/arm64/boot/dts/qcom/sc7180-idp.dts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts index d8b5507..cecac3e 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts @@ -474,32 +474,30 @@ &qup_uart3_default { pinconf-cts { /* - * Configure a pull-down on 38 (CTS) to match the pull of - * the Bluetooth module. + * Configure no-pull on CTS. As this is driven by BT, do not + * specify any pull in order to not conflict with BT pulls. */ pins = "gpio38"; - bias-pull-down; - output-high; + bias-disable; }; pinconf-rts { - /* We'll drive 39 (RTS), so no pull */ + /* We'll drive RTS, so no pull */ pins = "gpio39"; drive-strength = <2>; bias-disable; }; pinconf-tx { - /* We'll drive 40 (TX), so no pull */ + /* We'll drive TX, so no pull */ pins = "gpio40"; drive-strength = <2>; bias-disable; - output-high; }; pinconf-rx { /* - * Configure a pull-up on 41 (RX). This is needed to avoid + * Configure a pull-up on RX. This is needed to avoid * garbage data when the TX pin of the Bluetooth module is * in tri-state (module powered off or not driving the * signal yet). -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX 2020-09-03 15:04 ` [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX satya priya @ 2020-09-03 16:14 ` Matthias Kaehlcke 2020-09-09 21:28 ` Doug Anderson 1 sibling, 0 replies; 17+ messages in thread From: Matthias Kaehlcke @ 2020-09-03 16:14 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy On Thu, Sep 03, 2020 at 08:34:56PM +0530, satya priya wrote: > Configure no-pull for CTS, as this is driven by BT do not specify any pull > in order to not conflict with BT pulls. > > Remove output-high from CTS and TX as this is not really required. During > bringup to fix transfer failures this was added to match with console uart > settings. Probably some boot loader config was missing then. As it is > working fine now, remove it. You might want to revisit the 'output-high' settings for the IDP console uart too. I still think this shouldn't do anything on an input pin that isn't configured as GPIO. Specifically this combination seems silly: bias-pull-down; output-high; > Signed-off-by: satya priya <skakit@codeaurora.org> > Reviewed-by: Akash Asthana <akashast@codeaurora.org> > --- > Changes in V4: > - This is newly added in V4 to separate the improvements in pin settings > and wakeup related changes. > > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > index d8b5507..cecac3e 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > @@ -474,32 +474,30 @@ > &qup_uart3_default { > pinconf-cts { > /* > - * Configure a pull-down on 38 (CTS) to match the pull of > - * the Bluetooth module. > + * Configure no-pull on CTS. As this is driven by BT, do not > + * specify any pull in order to not conflict with BT pulls. > */ > pins = "gpio38"; > - bias-pull-down; > - output-high; > + bias-disable; > }; > > pinconf-rts { > - /* We'll drive 39 (RTS), so no pull */ > + /* We'll drive RTS, so no pull */ > pins = "gpio39"; > drive-strength = <2>; > bias-disable; > }; > > pinconf-tx { > - /* We'll drive 40 (TX), so no pull */ > + /* We'll drive TX, so no pull */ > pins = "gpio40"; > drive-strength = <2>; > bias-disable; > - output-high; > }; > > pinconf-rx { > /* > - * Configure a pull-up on 41 (RX). This is needed to avoid > + * Configure a pull-up on RX. This is needed to avoid > * garbage data when the TX pin of the Bluetooth module is > * in tri-state (module powered off or not driving the > * signal yet). Reviewed-by: Matthias Kaehlcke <mka@chromium.org> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX 2020-09-03 15:04 ` [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX satya priya 2020-09-03 16:14 ` Matthias Kaehlcke @ 2020-09-09 21:28 ` Doug Anderson 2020-09-10 12:49 ` skakit 1 sibling, 1 reply; 17+ messages in thread From: Doug Anderson @ 2020-09-09 21:28 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, Matthias Kaehlcke, Greg Kroah-Hartman, Andy Gross, Rob Herring, linux-arm-msm, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Akash Asthana, Roja Rani Yarubandi, msavaliy Hi, On Thu, Sep 3, 2020 at 8:07 AM satya priya <skakit@codeaurora.org> wrote: > > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > @@ -474,32 +474,30 @@ > &qup_uart3_default { > pinconf-cts { > /* > - * Configure a pull-down on 38 (CTS) to match the pull of > - * the Bluetooth module. > + * Configure no-pull on CTS. As this is driven by BT, do not > + * specify any pull in order to not conflict with BT pulls. > */ > pins = "gpio38"; > - bias-pull-down; > - output-high; Weird, how did that output-high sneak in there? Glad it's going away. > + bias-disable; I'm not convinced that the removal of the pul is the correct thing here. Specifically for the rx line the comment makes the argument that if we power off the Bluetooth module then it will stop driving this pin. In that case if we remove the pull here then the line will be floating and that can cause some extra power consumption as the line floats between different logic levels. Do you really need to remove this pull? Same comment for the next patch where you add the sleep settings. -Doug ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX 2020-09-09 21:28 ` Doug Anderson @ 2020-09-10 12:49 ` skakit 0 siblings, 0 replies; 17+ messages in thread From: skakit @ 2020-09-10 12:49 UTC (permalink / raw) To: Doug Anderson Cc: Bjorn Andersson, Matthias Kaehlcke, Greg Kroah-Hartman, Andy Gross, Rob Herring, linux-arm-msm, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Akash Asthana, Roja Rani Yarubandi, msavaliy Hi Doug, On 2020-09-10 02:58, Doug Anderson wrote: > Hi, > > On Thu, Sep 3, 2020 at 8:07 AM satya priya <skakit@codeaurora.org> > wrote: >> >> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts >> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts >> @@ -474,32 +474,30 @@ >> &qup_uart3_default { >> pinconf-cts { >> /* >> - * Configure a pull-down on 38 (CTS) to match the pull >> of >> - * the Bluetooth module. >> + * Configure no-pull on CTS. As this is driven by BT, >> do not >> + * specify any pull in order to not conflict with BT >> pulls. >> */ >> pins = "gpio38"; >> - bias-pull-down; >> - output-high; > > Weird, how did that output-high sneak in there? Glad it's going away. > > >> + bias-disable; > > I'm not convinced that the removal of the pul is the correct thing > here. Specifically for the rx line the comment makes the argument > that if we power off the Bluetooth module then it will stop driving > this pin. In that case if we remove the pull here then the line will > be floating and that can cause some extra power consumption as the > line floats between different logic levels. Do you really need to > remove this pull? > Okay, will keep the pull-down back for CTS. > Same comment for the next patch where you add the sleep settings. > > > -Doug ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART 2020-09-03 15:04 [PATCH V4 0/4] Add wakeup support over UART RX satya priya 2020-09-03 15:04 ` [PATCH V4 1/4] arm64: dts: sc7180: " satya priya 2020-09-03 15:04 ` [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX satya priya @ 2020-09-03 15:04 ` satya priya 2020-09-03 16:23 ` Matthias Kaehlcke 2020-09-09 21:29 ` Doug Anderson 2020-09-03 15:04 ` [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue satya priya 3 siblings, 2 replies; 17+ messages in thread From: satya priya @ 2020-09-03 15:04 UTC (permalink / raw) To: Bjorn Andersson Cc: Matthias Kaehlcke, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy, satya priya Add sleep state for BT uart to support the wakeup feature. Signed-off-by: satya priya <skakit@codeaurora.org> Reviewed-by: Akash Asthana <akashast@codeaurora.org> --- Changes in V2: - This patch adds sleep state for BT UART. Newly added in V2. Changes in V3: - Remove "output-high" for TX from both sleep and default states as it is not required. Configure pull-up for TX in sleep state. Changes in V4: - As per Matthias's comment, removed drive-strength for sleep state and fixed nit-pick. arch/arm64/boot/dts/qcom/sc7180-idp.dts | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts index cecac3e..77e3523 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts @@ -507,6 +507,43 @@ }; }; +&qup_uart3_sleep { + pinconf-cts { + /* + * Configure no-pull on CTS. As this is driven by BT, do not + * specify any pull in order to not conflict with BT pulls. + */ + pins = "gpio38"; + bias-disable; + }; + + pinconf-rts { + /* + * Configure pull-down on RTS to make sure that the BT SoC can + * wake up the system by sending wakeup bytes during suspend. + */ + pins = "gpio39"; + bias-pull-down; + }; + + pinconf-tx { + /* Configure pull-up on TX when it isn't actively driven */ + pins = "gpio40"; + bias-pull-up; + }; + + pinconf-rx { + /* + * Configure a pull-up on RX. This is needed to avoid + * garbage data when the TX pin of the Bluetooth module is + * in tri-state (module powered off or not driving the + * signal yet). + */ + pins = "gpio41"; + bias-pull-up; + }; +}; + &qup_uart8_default { pinconf-tx { pins = "gpio44"; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART 2020-09-03 15:04 ` [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART satya priya @ 2020-09-03 16:23 ` Matthias Kaehlcke 2020-09-09 21:29 ` Doug Anderson 1 sibling, 0 replies; 17+ messages in thread From: Matthias Kaehlcke @ 2020-09-03 16:23 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy On Thu, Sep 03, 2020 at 08:34:57PM +0530, satya priya wrote: > Add sleep state for BT uart to support the wakeup feature. > > Signed-off-by: satya priya <skakit@codeaurora.org> > Reviewed-by: Akash Asthana <akashast@codeaurora.org> > --- > Changes in V2: > - This patch adds sleep state for BT UART. Newly added in V2. > > Changes in V3: > - Remove "output-high" for TX from both sleep and default states > as it is not required. Configure pull-up for TX in sleep state. > > Changes in V4: > - As per Matthias's comment, removed drive-strength for sleep state > and fixed nit-pick. > > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 37 +++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > index cecac3e..77e3523 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > @@ -507,6 +507,43 @@ > }; > }; > > +&qup_uart3_sleep { > + pinconf-cts { > + /* > + * Configure no-pull on CTS. As this is driven by BT, do not > + * specify any pull in order to not conflict with BT pulls. > + */ > + pins = "gpio38"; > + bias-disable; > + }; > + > + pinconf-rts { > + /* > + * Configure pull-down on RTS to make sure that the BT SoC can > + * wake up the system by sending wakeup bytes during suspend. > + */ > + pins = "gpio39"; > + bias-pull-down; > + }; > + > + pinconf-tx { > + /* Configure pull-up on TX when it isn't actively driven */ > + pins = "gpio40"; > + bias-pull-up; > + }; > + > + pinconf-rx { > + /* > + * Configure a pull-up on RX. This is needed to avoid > + * garbage data when the TX pin of the Bluetooth module is > + * in tri-state (module powered off or not driving the > + * signal yet). > + */ > + pins = "gpio41"; > + bias-pull-up; > + }; > +}; > + > &qup_uart8_default { > pinconf-tx { > pins = "gpio44"; Reviewed-by: Matthias Kaehlcke <mka@chromium.org> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART 2020-09-03 15:04 ` [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART satya priya 2020-09-03 16:23 ` Matthias Kaehlcke @ 2020-09-09 21:29 ` Doug Anderson 2020-09-10 12:50 ` skakit 1 sibling, 1 reply; 17+ messages in thread From: Doug Anderson @ 2020-09-09 21:29 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, Matthias Kaehlcke, Greg Kroah-Hartman, Andy Gross, Rob Herring, linux-arm-msm, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Akash Asthana, Roja Rani Yarubandi, msavaliy Hi, On Thu, Sep 3, 2020 at 8:08 AM satya priya <skakit@codeaurora.org> wrote: > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > index cecac3e..77e3523 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > @@ -507,6 +507,43 @@ > }; > }; > > +&qup_uart3_sleep { > + pinconf-cts { > + /* > + * Configure no-pull on CTS. As this is driven by BT, do not > + * specify any pull in order to not conflict with BT pulls. > + */ > + pins = "gpio38"; > + bias-disable; Same comment as in the previous patch that I'm not convinced removing the bias here is correct. > + }; > + > + pinconf-rts { > + /* > + * Configure pull-down on RTS to make sure that the BT SoC can > + * wake up the system by sending wakeup bytes during suspend. > + */ > + pins = "gpio39"; > + bias-pull-down; > + }; > + > + pinconf-tx { > + /* Configure pull-up on TX when it isn't actively driven */ > + pins = "gpio40"; > + bias-pull-up; > + }; > + > + pinconf-rx { > + /* > + * Configure a pull-up on RX. This is needed to avoid > + * garbage data when the TX pin of the Bluetooth module is > + * in tri-state (module powered off or not driving the > + * signal yet). > + */ > + pins = "gpio41"; > + bias-pull-up; > + }; > +}; > + > &qup_uart8_default { Slight nit that "default" starts with a "d" which sorts before "sleep" which starts with an "s". Thus "qup_uart8_default" should be above "qup_uart3_sleep", not below. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART 2020-09-09 21:29 ` Doug Anderson @ 2020-09-10 12:50 ` skakit 0 siblings, 0 replies; 17+ messages in thread From: skakit @ 2020-09-10 12:50 UTC (permalink / raw) To: Doug Anderson Cc: Bjorn Andersson, Matthias Kaehlcke, Greg Kroah-Hartman, Andy Gross, Rob Herring, linux-arm-msm, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Akash Asthana, Roja Rani Yarubandi, msavaliy On 2020-09-10 02:59, Doug Anderson wrote: > Hi, > > On Thu, Sep 3, 2020 at 8:08 AM satya priya <skakit@codeaurora.org> > wrote: >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts >> b/arch/arm64/boot/dts/qcom/sc7180-idp.dts >> index cecac3e..77e3523 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts >> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts >> @@ -507,6 +507,43 @@ >> }; >> }; >> >> +&qup_uart3_sleep { >> + pinconf-cts { >> + /* >> + * Configure no-pull on CTS. As this is driven by BT, >> do not >> + * specify any pull in order to not conflict with BT >> pulls. >> + */ >> + pins = "gpio38"; >> + bias-disable; > > Same comment as in the previous patch that I'm not convinced removing > the bias here is correct. > Okay. > >> + }; >> + >> + pinconf-rts { >> + /* >> + * Configure pull-down on RTS to make sure that the BT >> SoC can >> + * wake up the system by sending wakeup bytes during >> suspend. >> + */ >> + pins = "gpio39"; >> + bias-pull-down; >> + }; >> + >> + pinconf-tx { >> + /* Configure pull-up on TX when it isn't actively >> driven */ >> + pins = "gpio40"; >> + bias-pull-up; >> + }; >> + >> + pinconf-rx { >> + /* >> + * Configure a pull-up on RX. This is needed to avoid >> + * garbage data when the TX pin of the Bluetooth >> module is >> + * in tri-state (module powered off or not driving the >> + * signal yet). >> + */ >> + pins = "gpio41"; >> + bias-pull-up; >> + }; >> +}; >> + >> &qup_uart8_default { > > Slight nit that "default" starts with a "d" which sorts before "sleep" > which starts with an "s". Thus "qup_uart8_default" should be above > "qup_uart3_sleep", not below. Okay. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue 2020-09-03 15:04 [PATCH V4 0/4] Add wakeup support over UART RX satya priya ` (2 preceding siblings ...) 2020-09-03 15:04 ` [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART satya priya @ 2020-09-03 15:04 ` satya priya 2020-09-03 16:50 ` Matthias Kaehlcke 3 siblings, 1 reply; 17+ messages in thread From: satya priya @ 2020-09-03 15:04 UTC (permalink / raw) To: Bjorn Andersson Cc: Matthias Kaehlcke, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy, satya priya As a part of system suspend uart_port_suspend is called from the Serial driver, which calls set_mctrl passing mctrl as NULL. This makes RFR high(NOT_READY) during suspend. Due to this BT SoC is not able to send wakeup bytes to UART during suspend. Include if check for non-suspend case to keep RFR low during suspend. Signed-off-by: satya priya <skakit@codeaurora.org> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Akash Asthana <akashast@codeaurora.org> --- Changes in V2: - This patch fixes the UART flow control issue during suspend. Newly added in V2. Changes in V3: - As per Matthias's comment removed the extra parentheses. Changes in V4: - No change. drivers/tty/serial/qcom_geni_serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 07b7b6b..2aad9d7 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -242,7 +242,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport, if (mctrl & TIOCM_LOOP) port->loopback = RX_TX_CTS_RTS_SORTED; - if (!(mctrl & TIOCM_RTS)) + if (!(mctrl & TIOCM_RTS) && !uport->suspended) uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY; writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue 2020-09-03 15:04 ` [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue satya priya @ 2020-09-03 16:50 ` Matthias Kaehlcke 2020-09-09 14:21 ` skakit 0 siblings, 1 reply; 17+ messages in thread From: Matthias Kaehlcke @ 2020-09-03 16:50 UTC (permalink / raw) To: satya priya Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy On Thu, Sep 03, 2020 at 08:34:58PM +0530, satya priya wrote: > As a part of system suspend uart_port_suspend is called from the > Serial driver, which calls set_mctrl passing mctrl as NULL. This nit: s/NULL/0/ > makes RFR high(NOT_READY) during suspend. > > Due to this BT SoC is not able to send wakeup bytes to UART during > suspend. Include if check for non-suspend case to keep RFR low > during suspend. Is this patch actually needed? With the other patches in this series the UART doesn't control RFR on IDP, and I suppose corresponding pinconf changes should also be done on other devices that want to support wakeup. Effectively, I see Bluetooth wakeup working without this patch on a sc7180 device. > Signed-off-by: satya priya <skakit@codeaurora.org> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Reviewed-by: Akash Asthana <akashast@codeaurora.org> > --- > Changes in V2: > - This patch fixes the UART flow control issue during suspend. > Newly added in V2. > > Changes in V3: > - As per Matthias's comment removed the extra parentheses. > > Changes in V4: > - No change. > > drivers/tty/serial/qcom_geni_serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index 07b7b6b..2aad9d7 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -242,7 +242,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport, > if (mctrl & TIOCM_LOOP) > port->loopback = RX_TX_CTS_RTS_SORTED; > > - if (!(mctrl & TIOCM_RTS)) > + if (!(mctrl & TIOCM_RTS) && !uport->suspended) > uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY; > writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR); > } > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue 2020-09-03 16:50 ` Matthias Kaehlcke @ 2020-09-09 14:21 ` skakit 0 siblings, 0 replies; 17+ messages in thread From: skakit @ 2020-09-09 14:21 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Bjorn Andersson, gregkh, Andy Gross, Rob Herring, linux-arm-msm, devicetree, linux-kernel, akashast, rojay, msavaliy On 2020-09-03 22:20, Matthias Kaehlcke wrote: > On Thu, Sep 03, 2020 at 08:34:58PM +0530, satya priya wrote: >> As a part of system suspend uart_port_suspend is called from the >> Serial driver, which calls set_mctrl passing mctrl as NULL. This > > nit: s/NULL/0/ > Okay, will correct it. >> makes RFR high(NOT_READY) during suspend. >> >> Due to this BT SoC is not able to send wakeup bytes to UART during >> suspend. Include if check for non-suspend case to keep RFR low >> during suspend. > > Is this patch actually needed? > > With the other patches in this series the UART doesn't control RFR > on IDP, and I suppose corresponding pinconf changes should also be > done on other devices that want to support wakeup. Effectively, > I see Bluetooth wakeup working without this patch on a sc7180 > device. > I am also seeing the same observation now on the tip (checked on IDP), but previously if this patch is not present the RFR line would go high during suspend (even though GPIO mode is configured in sleep state), not sure how it is being low now. Theoretically, this fix is good to have, because in suspend UART_MANUAL_RFR is getting set to not ready state and if QUP gets power to drive this line, it may go high and wakeup from BT will fail. >> Signed-off-by: satya priya <skakit@codeaurora.org> >> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Reviewed-by: Akash Asthana <akashast@codeaurora.org> >> --- >> Changes in V2: >> - This patch fixes the UART flow control issue during suspend. >> Newly added in V2. >> >> Changes in V3: >> - As per Matthias's comment removed the extra parentheses. >> >> Changes in V4: >> - No change. >> >> drivers/tty/serial/qcom_geni_serial.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/tty/serial/qcom_geni_serial.c >> b/drivers/tty/serial/qcom_geni_serial.c >> index 07b7b6b..2aad9d7 100644 >> --- a/drivers/tty/serial/qcom_geni_serial.c >> +++ b/drivers/tty/serial/qcom_geni_serial.c >> @@ -242,7 +242,7 @@ static void qcom_geni_serial_set_mctrl(struct >> uart_port *uport, >> if (mctrl & TIOCM_LOOP) >> port->loopback = RX_TX_CTS_RTS_SORTED; >> >> - if (!(mctrl & TIOCM_RTS)) >> + if (!(mctrl & TIOCM_RTS) && !uport->suspended) >> uart_manual_rfr = UART_MANUAL_RFR_EN | UART_RFR_NOT_READY; >> writel(uart_manual_rfr, uport->membase + SE_UART_MANUAL_RFR); >> } >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a >> member >> of Code Aurora Forum, hosted by The Linux Foundation >> ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-09-10 13:11 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-03 15:04 [PATCH V4 0/4] Add wakeup support over UART RX satya priya 2020-09-03 15:04 ` [PATCH V4 1/4] arm64: dts: sc7180: " satya priya 2020-09-03 16:05 ` Matthias Kaehlcke 2020-09-09 14:19 ` skakit 2020-09-03 17:17 ` Matthias Kaehlcke 2020-09-09 14:20 ` skakit 2020-09-03 15:04 ` [PATCH V4 2/4] arm64: dts: qcom: sc7180: Improve the pin config settings for CTS and TX satya priya 2020-09-03 16:14 ` Matthias Kaehlcke 2020-09-09 21:28 ` Doug Anderson 2020-09-10 12:49 ` skakit 2020-09-03 15:04 ` [PATCH V4 3/4] arm64: dts: qcom: sc7180: Add sleep state for BT UART satya priya 2020-09-03 16:23 ` Matthias Kaehlcke 2020-09-09 21:29 ` Doug Anderson 2020-09-10 12:50 ` skakit 2020-09-03 15:04 ` [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue satya priya 2020-09-03 16:50 ` Matthias Kaehlcke 2020-09-09 14:21 ` skakit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).