* [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown
@ 2020-03-13 20:46 Douglas Anderson
2020-03-13 20:46 ` [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console Douglas Anderson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Douglas Anderson @ 2020-03-13 20:46 UTC (permalink / raw)
To: gregkh
Cc: mka, swboyd, ryandcase, bjorn.andersson, akashast, skakit, rojay,
mgautam, Douglas Anderson, Andy Gross, Doug Anderson,
Girish Mahadevan, Jiri Slaby, Karthikeyan Ramasubramanian,
Sagar Dharia, linux-arm-msm, linux-kernel, linux-serial
On a board using qcom_geni_serial I found that I could no longer
interact with kdb if I got a crash after the "agetty" running on the
same serial port was killed. This meant that various classes of
crashes that happened at reboot time were undebuggable.
Reading through the code, I couldn't figure out why qcom_geni_serial
felt the need to run so much code at port shutdown time. All we need
to do is disable the interrupt.
After I make this change then a hardcoded kgdb_breakpoint in some late
shutdown code now allows me to interact with the debugger. I also
could freely close / re-open the port without problems.
Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/tty/serial/qcom_geni_serial.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 272bae0eebc7..09d8612517aa 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -827,17 +827,11 @@ static void get_tx_fifo_size(struct qcom_geni_serial_port *port)
static void qcom_geni_serial_shutdown(struct uart_port *uport)
{
- unsigned long flags;
-
/* Stop the console before stopping the current tx */
if (uart_console(uport))
console_stop(uport->cons);
disable_irq(uport->irq);
- spin_lock_irqsave(&uport->lock, flags);
- qcom_geni_serial_stop_tx(uport);
- qcom_geni_serial_stop_rx(uport);
- spin_unlock_irqrestore(&uport->lock, flags);
}
static int qcom_geni_serial_port_setup(struct uart_port *uport)
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console
2020-03-13 20:46 [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Douglas Anderson
@ 2020-03-13 20:46 ` Douglas Anderson
2020-03-16 17:57 ` Stephen Boyd
2020-03-17 13:07 ` Akash Asthana
2020-03-16 17:57 ` [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Stephen Boyd
2020-03-17 13:05 ` Akash Asthana
2 siblings, 2 replies; 6+ messages in thread
From: Douglas Anderson @ 2020-03-13 20:46 UTC (permalink / raw)
To: gregkh
Cc: mka, swboyd, ryandcase, bjorn.andersson, akashast, skakit, rojay,
mgautam, Douglas Anderson, Andy Gross, Doug Anderson,
Girish Mahadevan, Jiri Slaby, Karthikeyan Ramasubramanian,
Sagar Dharia, linux-arm-msm, linux-kernel, linux-serial
The geni serial driver's shutdown code had a special case to call
console_stop(). Grepping through the code, it was the only serial
driver doing something like this (the only other caller of
console_stop() was in serial_core.c).
As far as I can tell there's no reason to call console_stop() in the
geni code. ...and a good reason _not_ to call it. Specifically if
you have an agetty running on the same serial port as the console then
killing the agetty kills your console and if you start the agetty
again the console doesn't come back.
Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
drivers/tty/serial/qcom_geni_serial.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 09d8612517aa..69a0072e0c53 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -827,10 +827,6 @@ static void get_tx_fifo_size(struct qcom_geni_serial_port *port)
static void qcom_geni_serial_shutdown(struct uart_port *uport)
{
- /* Stop the console before stopping the current tx */
- if (uart_console(uport))
- console_stop(uport->cons);
-
disable_irq(uport->irq);
}
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console
2020-03-13 20:46 ` [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console Douglas Anderson
@ 2020-03-16 17:57 ` Stephen Boyd
2020-03-17 13:07 ` Akash Asthana
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2020-03-16 17:57 UTC (permalink / raw)
To: Douglas Anderson, gregkh
Cc: mka, ryandcase, bjorn.andersson, akashast, skakit, rojay,
mgautam, Douglas Anderson, Andy Gross, Doug Anderson,
Girish Mahadevan, Jiri Slaby, Karthikeyan Ramasubramanian,
Sagar Dharia, linux-arm-msm, linux-kernel, linux-serial
Quoting Douglas Anderson (2020-03-13 13:46:52)
> The geni serial driver's shutdown code had a special case to call
> console_stop(). Grepping through the code, it was the only serial
> driver doing something like this (the only other caller of
> console_stop() was in serial_core.c).
>
> As far as I can tell there's no reason to call console_stop() in the
> geni code. ...and a good reason _not_ to call it. Specifically if
> you have an agetty running on the same serial port as the console then
> killing the agetty kills your console and if you start the agetty
> again the console doesn't come back.
>
> Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console
2020-03-13 20:46 ` [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console Douglas Anderson
2020-03-16 17:57 ` Stephen Boyd
@ 2020-03-17 13:07 ` Akash Asthana
1 sibling, 0 replies; 6+ messages in thread
From: Akash Asthana @ 2020-03-17 13:07 UTC (permalink / raw)
To: Douglas Anderson, gregkh
Cc: mka, swboyd, ryandcase, bjorn.andersson, skakit, rojay, mgautam,
Andy Gross, Doug Anderson, Girish Mahadevan, Jiri Slaby,
Karthikeyan Ramasubramanian, Sagar Dharia, linux-arm-msm,
linux-kernel, linux-serial
On 3/14/2020 2:16 AM, Douglas Anderson wrote:
> The geni serial driver's shutdown code had a special case to call
> console_stop(). Grepping through the code, it was the only serial
> driver doing something like this (the only other caller of
> console_stop() was in serial_core.c).
>
> As far as I can tell there's no reason to call console_stop() in the
> geni code. ...and a good reason _not_ to call it. Specifically if
> you have an agetty running on the same serial port as the console then
> killing the agetty kills your console and if you start the agetty
> again the console doesn't come back.
>
> Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Akash Asthana <akashast@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown
2020-03-13 20:46 [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Douglas Anderson
2020-03-13 20:46 ` [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console Douglas Anderson
@ 2020-03-16 17:57 ` Stephen Boyd
2020-03-17 13:05 ` Akash Asthana
2 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2020-03-16 17:57 UTC (permalink / raw)
To: Douglas Anderson, gregkh
Cc: mka, ryandcase, bjorn.andersson, akashast, skakit, rojay,
mgautam, Douglas Anderson, Andy Gross, Doug Anderson,
Girish Mahadevan, Jiri Slaby, Karthikeyan Ramasubramanian,
Sagar Dharia, linux-arm-msm, linux-kernel, linux-serial
Quoting Douglas Anderson (2020-03-13 13:46:51)
> On a board using qcom_geni_serial I found that I could no longer
> interact with kdb if I got a crash after the "agetty" running on the
> same serial port was killed. This meant that various classes of
> crashes that happened at reboot time were undebuggable.
>
> Reading through the code, I couldn't figure out why qcom_geni_serial
> felt the need to run so much code at port shutdown time. All we need
> to do is disable the interrupt.
>
> After I make this change then a hardcoded kgdb_breakpoint in some late
> shutdown code now allows me to interact with the debugger. I also
> could freely close / re-open the port without problems.
>
> Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
Matches what is described in Documentation/driver-api/serial/driver.rst
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown
2020-03-13 20:46 [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Douglas Anderson
2020-03-13 20:46 ` [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console Douglas Anderson
2020-03-16 17:57 ` [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Stephen Boyd
@ 2020-03-17 13:05 ` Akash Asthana
2 siblings, 0 replies; 6+ messages in thread
From: Akash Asthana @ 2020-03-17 13:05 UTC (permalink / raw)
To: Douglas Anderson, gregkh
Cc: mka, swboyd, ryandcase, bjorn.andersson, skakit, rojay, mgautam,
Andy Gross, Doug Anderson, Girish Mahadevan, Jiri Slaby,
Karthikeyan Ramasubramanian, Sagar Dharia, linux-arm-msm,
linux-kernel, linux-serial
On 3/14/2020 2:16 AM, Douglas Anderson wrote:
> On a board using qcom_geni_serial I found that I could no longer
> interact with kdb if I got a crash after the "agetty" running on the
> same serial port was killed. This meant that various classes of
> crashes that happened at reboot time were undebuggable.
>
> Reading through the code, I couldn't figure out why qcom_geni_serial
> felt the need to run so much code at port shutdown time. All we need
> to do is disable the interrupt.
>
> After I make this change then a hardcoded kgdb_breakpoint in some late
> shutdown code now allows me to interact with the debugger. I also
> could freely close / re-open the port without problems.
>
> Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
Looks good to me.
Reviewed-by: Akash Asthana <akashast@codeaurora.org>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-17 13:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 20:46 [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Douglas Anderson
2020-03-13 20:46 ` [PATCH 2/2] tty: serial: qcom_geni_serial: Don't try to manually disable the console Douglas Anderson
2020-03-16 17:57 ` Stephen Boyd
2020-03-17 13:07 ` Akash Asthana
2020-03-16 17:57 ` [PATCH 1/2] tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown Stephen Boyd
2020-03-17 13:05 ` Akash Asthana
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).