linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH 0/2] tty: serial: Fix kgdb on qcom-geni-serial when no other UART users
@ 2023-03-16 20:20 Douglas Anderson
  2023-03-16 20:20 ` [RESEND PATCH 1/2] serial: uart_poll_init() should power on the UART Douglas Anderson
  2023-03-16 20:20 ` [RESEND PATCH 2/2] tty: serial: qcom-geni-serial: Add a poll_init() function Douglas Anderson
  0 siblings, 2 replies; 3+ messages in thread
From: Douglas Anderson @ 2023-03-16 20:20 UTC (permalink / raw)
  To: Bjorn Andersson, Greg Kroah-Hartman
  Cc: linux-arm-msm, Jiri Slaby, Bartosz Golaszewski, Daniel Thompson,
	kgdb-bugreport, Konrad Dybcio, linux-serial, Douglas Anderson,
	Andy Gross, linux-kernel

Today to get kgdb to work on qcom-geni-serial devices you need
_something_ to init/power on the UART. This could either the kernel
console output or an "agetty" running on the port. If nothing else
powers the port then you'll end up getting a silent hang when you try
to enter kgdb.

Let's fix this. The first patch here is for the tty layer to make sure
that we power on the port when we init it for polling. This would be
important for any drivers similar to qcom-geni-serial that actually
need to be powered on. The second patch here hooks up the poll_init()
function for qcom-geni-serial, leveraging an existing function in the
driver that does everything we need.

Originally these two patches were bundled together as pathes 2 and 3
of a 3-patch series.  We no longer need the first patch from the
orginal series since we landed a similar patch from Johan [1]
instead. The second two patches are still useful, though, so I've
reposted them alone and added this cover letter.

[1] https://lore.kernel.org/r/20230307164405.14218-1-johan+linaro@kernel.org


Douglas Anderson (2):
  serial: uart_poll_init() should power on the UART
  tty: serial: qcom-geni-serial: Add a poll_init() function

 drivers/tty/serial/qcom_geni_serial.c | 1 +
 drivers/tty/serial/serial_core.c      | 6 ++++++
 2 files changed, 7 insertions(+)

-- 
2.40.0.rc1.284.g88254d51c5-goog


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [RESEND PATCH 1/2] serial: uart_poll_init() should power on the UART
  2023-03-16 20:20 [RESEND PATCH 0/2] tty: serial: Fix kgdb on qcom-geni-serial when no other UART users Douglas Anderson
@ 2023-03-16 20:20 ` Douglas Anderson
  2023-03-16 20:20 ` [RESEND PATCH 2/2] tty: serial: qcom-geni-serial: Add a poll_init() function Douglas Anderson
  1 sibling, 0 replies; 3+ messages in thread
From: Douglas Anderson @ 2023-03-16 20:20 UTC (permalink / raw)
  To: Bjorn Andersson, Greg Kroah-Hartman
  Cc: linux-arm-msm, Jiri Slaby, Bartosz Golaszewski, Daniel Thompson,
	kgdb-bugreport, Konrad Dybcio, linux-serial, Douglas Anderson,
	linux-kernel

On Qualcomm devices which use the "geni" serial driver, kdb/kgdb won't
be very happy if you use it but the resources of the port haven't been
powered on. Today kdb/kgdb rely on someone else powering the port
on. This could be the normal kernel console or an agetty running.
Let's fix this to explicitly power things on when setting up a polling
driver.

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

 drivers/tty/serial/serial_core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2bd32c8ece39..b14b5ed6fff4 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2593,6 +2593,7 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 {
 	struct uart_driver *drv = driver->driver_state;
 	struct uart_state *state = drv->state + line;
+	enum uart_pm_state pm_state;
 	struct tty_port *tport;
 	struct uart_port *port;
 	int baud = 9600;
@@ -2610,6 +2611,9 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 		goto out;
 	}
 
+	pm_state = state->pm_state;
+	uart_change_pm(state, UART_PM_STATE_ON);
+
 	if (port->ops->poll_init) {
 		/*
 		 * We don't set initialized as we only initialized the hw,
@@ -2626,6 +2630,8 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 		console_list_unlock();
 	}
 out:
+	if (ret)
+		uart_change_pm(state, pm_state);
 	mutex_unlock(&tport->mutex);
 	return ret;
 }
-- 
2.40.0.rc1.284.g88254d51c5-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [RESEND PATCH 2/2] tty: serial: qcom-geni-serial: Add a poll_init() function
  2023-03-16 20:20 [RESEND PATCH 0/2] tty: serial: Fix kgdb on qcom-geni-serial when no other UART users Douglas Anderson
  2023-03-16 20:20 ` [RESEND PATCH 1/2] serial: uart_poll_init() should power on the UART Douglas Anderson
@ 2023-03-16 20:20 ` Douglas Anderson
  1 sibling, 0 replies; 3+ messages in thread
From: Douglas Anderson @ 2023-03-16 20:20 UTC (permalink / raw)
  To: Bjorn Andersson, Greg Kroah-Hartman
  Cc: linux-arm-msm, Jiri Slaby, Bartosz Golaszewski, Daniel Thompson,
	kgdb-bugreport, Konrad Dybcio, linux-serial, Douglas Anderson,
	Andy Gross, linux-kernel

On sc7180 Chromebooks, I did the following:
* Didn't enable earlycon in the kernel command line.
* Didn't enable serial console in the kernel command line.
* Didn't enable an agetty or any other client of "/dev/ttyMSM0".
* Added "kgdboc=ttyMSM0" to the kernel command line.

After I did that, I tried to enter kdb with this command over an ssh
session:
  echo g > /proc/sysrq-trigger

When I did that the system just hung.

Although I thought I'd tested this scenario before, I couldn't go back
and find a time when it was working. Previous testing must have relied
on either the UART acting as the kernel console or an agetty running.

It turns out to be pretty easy to fix: we can just use
qcom_geni_serial_port_setup() as the .poll_init() function. This,
together with the patch ("serial: uart_poll_init() should power on the
UART"), allows the debugger to work even if there are no other users
of the serial port.

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

 drivers/tty/serial/qcom_geni_serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d69592e5e2ec..7fdb3e12846d 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1532,6 +1532,7 @@ static const struct uart_ops qcom_geni_console_pops = {
 #ifdef CONFIG_CONSOLE_POLL
 	.poll_get_char	= qcom_geni_serial_get_char,
 	.poll_put_char	= qcom_geni_serial_poll_put_char,
+	.poll_init = qcom_geni_serial_port_setup,
 #endif
 	.pm = qcom_geni_serial_pm,
 };
-- 
2.40.0.rc1.284.g88254d51c5-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-16 20:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-16 20:20 [RESEND PATCH 0/2] tty: serial: Fix kgdb on qcom-geni-serial when no other UART users Douglas Anderson
2023-03-16 20:20 ` [RESEND PATCH 1/2] serial: uart_poll_init() should power on the UART Douglas Anderson
2023-03-16 20:20 ` [RESEND PATCH 2/2] tty: serial: qcom-geni-serial: Add a poll_init() function Douglas Anderson

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).