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

[Karl Mehltretter]

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 <kmehltretter@xxxxxxxxx>
---
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