[PATCH 2/2] serial: amba-pl011: keep console clock enabled for atomic writes

Karl Mehltretter <[email protected]>
Newsgroups dev.linux.lists.linux-rt-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-serial
Message-ID <[email protected]>
pl011_console_write_atomic() runs from nbcon atomic context, where
sleeping is not allowed. It calls clk_enable(), which takes the
common-clk enable_lock. Under PREEMPT_RT that is a sleeping lock:
clk_enable_lock() first tries spin_trylock_irqsave(), but on contention
falls back to spin_lock_irqsave(), so an atomic-context printk on an RT
kernel with a clk-backed pl011 can trip:

  BUG: sleeping function called from invalid context at spinlock_rt.c:48
    __might_resched from rt_spin_lock
    rt_spin_lock from clk_enable_lock
    clk_enable_lock from clk_enable
    clk_enable from pl011_console_write_atomic
    ... from vprintk_emit

This was found and reproduced on 32-bit ARM with PREEMPT_RT. In
addition, write_atomic() may be invoked from NMI context and is
documented to avoid locking. Since clk_enable() acquires the
common-clock enable_lock, removing it from the callback also avoids a
potentially unsafe NMI lock acquisition.

An nbcon atomic-capable console must be printable from any context, so
the clock cannot be gated between writes. Enable the clock while the
console is registered: use clk_prepare_enable() in
pl011_console_setup(), release it via clk_disable_unprepare() in the
console .exit() callback, and drop the per-write
clk_enable()/clk_disable() pairs from write_atomic() and
write_thread().

Keeping UARTCLK enabled may increase idle power on platforms where it
would otherwise be gated between console writes.

Fixes: 2eb2608618ce ("serial: amba-pl011: Implement nbcon console")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <[email protected]>
---
The PREEMPT_RT splat above was reproduced on 32-bit ARM (versatilepb,
ARM926EJ-S / ARMv5TE), Linux 7.2.0-rc3 based, CONFIG_PREEMPT_RT=y
CONFIG_HZ=1000, by driving an atomic-context printk; with this patch the
clk_enable() -> rt_spin_lock() splat is gone.

Tested on QEMU raspi (bcm2835, gateable CPRMAN UART clock) with the full
series applied: recycling the console via /sys/class/tty/ttyAMA0/console
keeps both prepare_count and enable_count balanced (2/2 across six
cycles), and console output resumes after each re-registration.
Suspend/resume was not exercised (no platform suspend under the QEMU
model).

 drivers/tty/serial/amba-pl011.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 1aa43994a3cd..4facd1b350d5 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2523,7 +2523,11 @@ static int pl011_console_setup(struct console *co, char *options)
 	/* Allow pins to be muxed in and configured */
 	pinctrl_pm_select_default_state(uap->port.dev);
 
-	ret = clk_prepare(uap->clk);
+	/*
+	 * Keep the clock enabled while registered because write_atomic() may
+	 * run in NMI context and must not acquire the clock framework lock.
+	 */
+	ret = clk_prepare_enable(uap->clk);
 	if (ret)
 		return ret;
 
@@ -2556,7 +2560,7 @@ static int pl011_console_exit(struct console *co)
 {
 	struct uart_amba_port *uap = amba_ports[co->index];
 
-	clk_unprepare(uap->clk);
+	clk_disable_unprepare(uap->clk);
 
 	return 0;
 }
@@ -2630,8 +2634,6 @@ pl011_console_write_atomic(struct console *co, struct nbcon_write_context *wctxt
 	if (!nbcon_enter_unsafe(wctxt))
 		return;
 
-	clk_enable(uap->clk);
-
 	if (!uap->vendor->always_enabled) {
 		old_cr = pl011_read(uap, REG_CR);
 		pl011_write((old_cr & ~UART011_CR_CTSEN) | (UART01x_CR_UARTEN | UART011_CR_TXE),
@@ -2648,8 +2650,6 @@ pl011_console_write_atomic(struct console *co, struct nbcon_write_context *wctxt
 	if (!uap->vendor->always_enabled)
 		pl011_write(old_cr, uap, REG_CR);
 
-	clk_disable(uap->clk);
-
 	nbcon_exit_unsafe(wctxt);
 }
 
@@ -2662,8 +2662,6 @@ pl011_console_write_thread(struct console *co, struct nbcon_write_context *wctxt
 	if (!nbcon_enter_unsafe(wctxt))
 		return;
 
-	clk_enable(uap->clk);
-
 	if (!uap->vendor->always_enabled) {
 		old_cr = pl011_read(uap, REG_CR);
 		pl011_write((old_cr & ~UART011_CR_CTSEN) | (UART01x_CR_UARTEN | UART011_CR_TXE),
@@ -2692,8 +2690,6 @@ pl011_console_write_thread(struct console *co, struct nbcon_write_context *wctxt
 	if (!uap->vendor->always_enabled)
 		pl011_write(old_cr, uap, REG_CR);
 
-	clk_disable(uap->clk);
-
 	nbcon_exit_unsafe(wctxt);
 }
 
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.