diff mbox series

[V4,4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue

Message ID 1599145498-20707-5-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
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(-)

Comments

Matthias Kaehlcke Sept. 3, 2020, 4:50 p.m. UTC | #1
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
>
Satya Priya Sept. 9, 2020, 2:21 p.m. UTC | #2
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
>>
diff mbox series

Patch

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);
 }