linux-kernel - Re: [PATCH V4 4/4] tty: serial: qcom_geni

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <01010174733dcefc-01bf923c-1fdd-4d80-b7a8-d360c2f6cbf5-000000@us-west-2.amazonses.com>
Date:   Wed, 9 Sep 2020 14:21:04 +0000
From:   skakit@...eaurora.org
To:     Matthias Kaehlcke <mka@...omium.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        gregkh@...uxfoundation.org, Andy Gross <agross@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, akashast@...eaurora.org,
        rojay@...eaurora.org, msavaliy@....qualcomm.com
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 <skakit@...eaurora.org>
>> Acked-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>> Reviewed-by: Akash Asthana <akashast@...eaurora.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
>>