From: Johan Hovold <johan@kernel.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>,
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>,
Daniel Thompson <daniel.thompson@linaro.org>,
linux-serial@vger.kernel.org, linux-arm-msm@vger.kernel.org,
kgdb-bugreport@lists.sourceforge.net,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Andy Gross <agross@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] tty: serial: qcom-geni-serial: Fix kdb/kgdb after port shutdown (again)
Date: Tue, 7 Mar 2023 17:01:16 +0100 [thread overview]
Message-ID: <ZAdfzHnRtBtcDbKK@hovoldconsulting.com> (raw)
In-Reply-To: <20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid>
On Tue, Mar 07, 2023 at 07:32:11AM -0800, Douglas Anderson wrote:
> Commit d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations
> in progress at shutdown") was basically a straight revert of the
> commit it claims to fix without any explanation of why the problems
> talked about in the original patch were no longer relevant. Indeed,
> commit d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations
> in progress at shutdown") re-introduces the same problem that commit
> e83766334f96 ("tty: serial: qcom_geni_serial: No need to stop tx/rx on
> UART shutdown") fixed.
>
> The problems are very easy to see by simply doing this on a
> sc7180-based Chromebook:
>
> 1. Boot in developer mode with serial console enabled and kdb setup on
> the serial console.
> 2. via ssh: stop console-ttyMSM0; echo g > /proc/sysrq-trigger
>
> When you do the above you'll see the "kdb" prompt printed on the
> serial console but be unable to interact with it.
>
> Let's fix the problem again by noting that the console port is never
> configured in DMA mode and thus we don't need to stop things for the
> console.
>
> Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown")
The offending commit broke serial console more generally by breaking TX
and thus hanging the system when stopping the getty on reboot.
The underlying bug has been there since this driver was first merged,
and as fixing it properly is going to be a bit involved, I was about to
post a patch equivalent to this one to fix the immediate regression and
get us back to status quo.
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>
> drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index d69592e5e2ec..74a0e074c2de 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1070,8 +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);
> - qcom_geni_serial_stop_tx(uport);
> - qcom_geni_serial_stop_rx(uport);
> + if (!uart_console(uport)) {
> + qcom_geni_serial_stop_tx(uport);
> + qcom_geni_serial_stop_rx(uport);
> + }
> }
>
> static int qcom_geni_serial_port_setup(struct uart_port *uport)
Johan
prev parent reply other threads:[~2023-03-07 16:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-07 15:32 [PATCH 1/3] tty: serial: qcom-geni-serial: Fix kdb/kgdb after port shutdown (again) Douglas Anderson
2023-03-07 15:32 ` [PATCH 2/3] serial: uart_poll_init() should power on the UART Douglas Anderson
2023-03-13 20:53 ` Doug Anderson
2023-03-16 11:59 ` Greg Kroah-Hartman
2023-03-07 15:32 ` [PATCH 3/3] tty: serial: qcom-geni-serial: Add a poll_init() function Douglas Anderson
2023-03-07 15:38 ` [PATCH 1/3] tty: serial: qcom-geni-serial: Fix kdb/kgdb after port shutdown (again) Bartosz Golaszewski
2023-03-07 15:46 ` Bartosz Golaszewski
2023-03-07 16:01 ` Johan Hovold [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZAdfzHnRtBtcDbKK@hovoldconsulting.com \
--to=johan@kernel.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=bartosz.golaszewski@linaro.org \
--cc=daniel.thompson@linaro.org \
--cc=dianders@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox