2020-09-03 15:10:27

by Satya Priya

[permalink] [raw]
Subject: [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue

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 <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Akash Asthana <[email protected]>
---
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


2020-09-03 16:52:09

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue

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 <[email protected]>
> Acked-by: Greg Kroah-Hartman <[email protected]>
> Reviewed-by: Akash Asthana <[email protected]>
> ---
> 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
>

2020-09-09 15:47:40

by Satya Priya

[permalink] [raw]
Subject: Re: [PATCH V4 4/4] tty: serial: qcom_geni_serial: Fix the UART wakeup issue

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 <[email protected]>
>> Acked-by: Greg Kroah-Hartman <[email protected]>
>> Reviewed-by: Akash Asthana <[email protected]>
>> ---
>> 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
>>