[1/4] serial: qcom-geni: fix console shutdown hang

Message ID 20230307164405.14218-2-johan+linaro@kernel.org
State New
Headers
Series serial: qcom-geni: fix console shutdown hang |

Commit Message

Johan Hovold March 7, 2023, 4:44 p.m. UTC
  A recent commit added back the calls top stop tx and rx to shutdown()
which had previously been removed by commit e83766334f96 ("tty: serial:
qcom_geni_serial: No need to stop tx/rx on UART shutdown") in order to
be able to use kgdb after stopping the getty.

Not only did this again break kgdb, but it also broke serial consoles
more generally by hanging TX when stopping the getty during reboot.

The underlying problem has been there since the driver was first merged
and fixing it is going to be a bit involved so simply stop calling the
broken stop functions during shutdown for consoles for now.

Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown")
Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/tty/serial/qcom_geni_serial.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Doug Anderson March 7, 2023, 6:34 p.m. UTC | #1
Hi,

On Tue, Mar 7, 2023 at 8:43 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> A recent commit added back the calls top stop tx and rx to shutdown()
> which had previously been removed by commit e83766334f96 ("tty: serial:
> qcom_geni_serial: No need to stop tx/rx on UART shutdown") in order to
> be able to use kgdb after stopping the getty.
>
> Not only did this again break kgdb, but it also broke serial consoles
> more generally by hanging TX when stopping the getty during reboot.
>
> The underlying problem has been there since the driver was first merged
> and fixing it is going to be a bit involved so simply stop calling the
> broken stop functions during shutdown for consoles for now.
>
> Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown")
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 4 ++++
>  1 file changed, 4 insertions(+)

I'm fine with either this change or my change [1] landing.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

[1] https://lore.kernel.org/r/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
  

Patch

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d69592e5e2ec..11da05d8f848 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1070,6 +1070,10 @@  static int setup_fifos(struct qcom_geni_serial_port *port)
 static void qcom_geni_serial_shutdown(struct uart_port *uport)
 {
 	disable_irq(uport->irq);
+
+	if (uart_console(uport))
+		return;
+
 	qcom_geni_serial_stop_tx(uport);
 	qcom_geni_serial_stop_rx(uport);
 }