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

[John Ogness]

On 2026-07-20, Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:
> On 2026-07-19 06:52:14 [+0000], sashiko-bot@xxxxxxxxxx wrote:
>> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> > index 1aa43994a3cdc..4facd1b350d5d 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);
>> 
>> [Severity: High]
>> Will this keep the UART clock unconditionally enabled during system suspend?
>> 
>> Because the persistent clock reference is acquired here during console
>> registration and only dropped on console exit in pl011_console_exit(), the
>> reference remains active.
>> 
>> During system suspend, pl011_suspend() delegates to uart_suspend_port() but
>> fails to drop this console's persistent clock enable reference. This might
>> prevent the SoC from turning off the clock tree or parent PLLs, preventing
>> deep system sleep.
>
> The comment somehow makes sense. What is the general advice here? If we
> drop the clock during suspend and acquire it again during resume we need
> to keep track and ignore all prints in between. Is the printk core
> taking care of this?

This is what the suspend/resume pm callbacks are for. In this case, it
is pl011_suspend() and pl011_resume().

> There is also this no_console_suspend argument…

Yes, the callbacks need to check @console_suspend_enabled. If false, the
clocks obviously must not be disabled.

John Ogness