For the case of console_suspend disabled, if back to back suspend/resume
test is executed, at the end of test, sometimes console would appear to
be frozen not responding to input. This would happen because, for
console_suspend disabled, suspend/resume routines only turn resources
off/on but don't do a port close/open.
As a result, during resume, some rx transactions come in before system is
ready, malfunction of rx happens in turn resulting in console appearing
to be stuck.
Do a stop_rx in suspend sequence to prevent this. start_rx is already
present in resume sequence as part of call to set_termios which does a
stop_rx/start_rx.
Signed-off-by: Vijaya Krishna Nivarthi <[email protected]>
---
v3: swapped the order of conditions to be more human readable
v2: restricted patch to contain only stop_rx in suspend sequence
v1: intial patch contained 2 additional unrelated changes in vicinity
---
drivers/tty/serial/qcom_geni_serial.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 1543a60..53723d2 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1481,6 +1481,10 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
struct uart_port *uport = &port->uport;
struct qcom_geni_private_data *private_data = uport->private_data;
+ /* do a stop_rx here, start_rx is handled in uart_resume_port by call to setermios */
+ if (uart_console(uport) && !console_suspend_enabled)
+ uport->ops->stop_rx(uport);
+
/*
* This is done so we can hit the lowest possible state in suspend
* even with no_console_suspend
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.
On 07. 04. 22, 9:25, Vijaya Krishna Nivarthi wrote:
> For the case of console_suspend disabled, if back to back suspend/resume
> test is executed, at the end of test, sometimes console would appear to
> be frozen not responding to input. This would happen because, for
> console_suspend disabled, suspend/resume routines only turn resources
> off/on but don't do a port close/open.
> As a result, during resume, some rx transactions come in before system is
> ready, malfunction of rx happens in turn resulting in console appearing
> to be stuck.
>
> Do a stop_rx in suspend sequence to prevent this. start_rx is already
> present in resume sequence as part of call to set_termios which does a
> stop_rx/start_rx.
So why is it OK for every other driver? Should uart_suspend_port() be
fixed instead?
> Signed-off-by: Vijaya Krishna Nivarthi <[email protected]>
> ---
> v3: swapped the order of conditions to be more human readable
> v2: restricted patch to contain only stop_rx in suspend sequence
> v1: intial patch contained 2 additional unrelated changes in vicinity
> ---
> drivers/tty/serial/qcom_geni_serial.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 1543a60..53723d2 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1481,6 +1481,10 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
> struct uart_port *uport = &port->uport;
> struct qcom_geni_private_data *private_data = uport->private_data;
>
> + /* do a stop_rx here, start_rx is handled in uart_resume_port by call to setermios */
> + if (uart_console(uport) && !console_suspend_enabled)
> + uport->ops->stop_rx(uport);
> +
> /*
> * This is done so we can hit the lowest possible state in suspend
> * even with no_console_suspend
thanks,
--
js
suse labs
On 4/7/2022 1:21 PM, Jiri Slaby wrote:
> On 07. 04. 22, 9:25, Vijaya Krishna Nivarthi wrote:
>> For the case of console_suspend disabled, if back to back suspend/resume
>> test is executed, at the end of test, sometimes console would appear to
>> be frozen not responding to input. This would happen because, for
>> console_suspend disabled, suspend/resume routines only turn resources
>> off/on but don't do a port close/open.
>> As a result, during resume, some rx transactions come in before
>> system is
>> ready, malfunction of rx happens in turn resulting in console appearing
>> to be stuck.
>>
>> Do a stop_rx in suspend sequence to prevent this. start_rx is already
>> present in resume sequence as part of call to set_termios which does a
>> stop_rx/start_rx.
>
> So why is it OK for every other driver? Should uart_suspend_port() be
> fixed instead?
For qcom driver we know that set_termios() call in uart_suspend_port()
will recover with a call to start_rx.
However that may not be the case with other drivers.
We can move stop_rx to uart_suspend_port() and additionally have a
start_rx in uart_resume_port()
Please let know if such a change would be ok.
Thank you.
On Fri, Apr 08, 2022 at 11:45:11AM +0530, Vijaya Krishna Nivarthi wrote:
>
> On 4/7/2022 1:21 PM, Jiri Slaby wrote:
> > On 07. 04. 22, 9:25, Vijaya Krishna Nivarthi wrote:
> > > For the case of console_suspend disabled, if back to back suspend/resume
> > > test is executed, at the end of test, sometimes console would appear to
> > > be frozen not responding to input. This would happen because, for
> > > console_suspend disabled, suspend/resume routines only turn resources
> > > off/on but don't do a port close/open.
> > > As a result, during resume, some rx transactions come in before
> > > system is
> > > ready, malfunction of rx happens in turn resulting in console appearing
> > > to be stuck.
> > >
> > > Do a stop_rx in suspend sequence to prevent this. start_rx is already
> > > present in resume sequence as part of call to set_termios which does a
> > > stop_rx/start_rx.
> >
> > So why is it OK for every other driver? Should uart_suspend_port() be
> > fixed instead?
>
> For qcom driver we know that set_termios() call in uart_suspend_port() will
> recover with a call to start_rx.
> However that may not be the case with other drivers.
>
> We can move stop_rx to uart_suspend_port() and additionally have a start_rx
> in uart_resume_port()
> Please let know if such a change would be ok.
This should not be something that each individual driver has to do,
please fix it for everyone.
thanks,
greg k-h