Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH] serial: amba-pl011: drop redundant BUSY wait from earlycon pl011_putc()
@ 2026-07-03 19:16 Eric Curtin
  2026-07-03 19:38 ` [PATCH v2] serial: amba-pl011: don't wait for BUSY after every earlycon character Eric Curtin
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Curtin @ 2026-07-03 19:16 UTC (permalink / raw)
  To: Russell King, Greg Kroah-Hartman, Jiri Slaby
  Cc: Eric Curtin, linux-serial, linux-kernel

pl011_putc(), used exclusively by the pl011 earlycon (pl011_early_write()
-> uart_console_write()), waits for UART01x_FR_TXFF to clear before
writing a character (correct: don't overrun the TX FIFO) and then *also*
busy-waits for UART01x_FR_BUSY to clear before returning, i.e. it waits
for the character to be fully shifted out on the wire before the next
character in the string can even be considered.

This second wait is not required for correctness and is inconsistent
with the rest of this same driver: both pl011_console_putchar() (the
regular, always-built console write path used by every printk() once
the full driver is up) and pl011_put_poll_char() (kgdb/kdb polling I/O)
only wait for TXFF before writing and never wait for BUSY afterwards.
pl011_putc() is the only one of the three that serializes on BUSY, and
it is the one exercised the most heavily, since it backs earlycon and
therefore every line printed during early boot before the real console
takes over.

Waiting for BUSY defeats the TX FIFO: instead of letting the UART
buffer several queued bytes and transmit them back to back, every
single character printed through earlycon is forced to wait for that
character's own complete transmission (a full UART bit-time at the
configured baud rate) before the driver will even look at writing the
next one. This is wasted time on real hardware, and it is much worse
under virtualization: each read of UARTFR and each write to UARTDR is
an MMIO access that traps to the hypervisor, so removing the extra
poll removes one entire VM-exit/entry round trip per character printed
through earlycon.

This is easy to see, and to prove, in a fast-booting VMM. Using a small
KVM-based VMM that boots Linux guests directly on arm64 (no firmware),
booting an otherwise-identical 7.2.0-rc1 kernel (single vCPU, 512MB
guest, direct PL011 MMIO with no interrupt-driven UART - i.e. earlycon
is the only console active) up to the (expected, rootfs-less)
"Unable to mount root fs" panic, with 20 boot samples per kernel per
configuration:

  cmdline: console=ttyAMA0 earlycon=pl011,0x09000000 ignore_loglevel
           initcall_debug printk.time=1     (~99KB printed over earlycon)
    before: min 466.3ms  avg 471.4ms  max 480.3ms  (n=20)
    after:  min 429.5ms  avg 433.3ms  max 442.6ms  (n=19)
    -> ~38ms / ~8% faster, non-overlapping distributions

  cmdline: <the VMM's actual default, quiet, earlycon=pl011,0x09000000>
    before: min  74.3ms  avg  76.5ms  max  82.2ms  (n=20)
    after:  min  53.6ms  avg  55.4ms  max  60.7ms  (n=20)
    -> ~21ms / ~28% faster, non-overlapping distributions

The second measurement uses the VMM's real default boot configuration
(not a synthetic debug cmdline), so this is representative of ordinary
boots, not just verbose-logging ones. Console content was diffed
(timestamps and per-initcall "returned 0 after N usecs" numbers
excluded) between before/after runs and is byte-for-byte identical
line-for-line: this change is a pure removal of unnecessary polling,
with no functional or ordering change.

Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
---
 drivers/tty/serial/amba-pl011.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8ed91e1da22be..2a25095e8d8c7 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2741,8 +2741,6 @@ static void pl011_putc(struct uart_port *port, unsigned char c)
 		writel(c, port->membase + UART01x_DR);
 	else
 		writeb(c, port->membase + UART01x_DR);
-	while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
-		cpu_relax();
 }
 
 static void pl011_early_write(struct console *con, const char *s, unsigned int n)
-- 
2.54.0


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

* [PATCH v2] serial: amba-pl011: don't wait for BUSY after every earlycon character
  2026-07-03 19:16 [PATCH] serial: amba-pl011: drop redundant BUSY wait from earlycon pl011_putc() Eric Curtin
@ 2026-07-03 19:38 ` Eric Curtin
  2026-07-10 12:38   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Curtin @ 2026-07-03 19:38 UTC (permalink / raw)
  To: Russell King, Greg Kroah-Hartman, Jiri Slaby
  Cc: Eric Curtin, linux-serial, linux-kernel

pl011_putc(), used exclusively by the pl011 earlycon (pl011_early_write()
-> uart_console_write()), waits for UART01x_FR_TXFF to clear before
writing a character (correct: don't overrun the TX FIFO) and then *also*
busy-waited for UART01x_FR_BUSY to clear before returning, i.e. it waited
for the character to be fully shifted out on the wire before the next
character in the string could even be considered.

Waiting for BUSY per character defeats the TX FIFO: instead of letting
the UART buffer several queued bytes and transmit them back to back,
every single character printed through earlycon was forced to wait for
that character's own complete transmission (a full UART bit-time at the
configured baud rate) before the driver would even look at writing the
next one. This is wasted time on real hardware, and it is much worse
under virtualization: each read of UARTFR and each write to UARTDR is
an MMIO access that traps to the hypervisor, so every extra poll is a
full VM-exit/entry round trip.

git blame on this function is unhelpful (this tree's history stops at a
shallow-clone boundary), but the equivalent history is available from
the upstream patch that added the QDF2400 erratum 44 workaround:

commit d8a4995bcea1 ("tty: pl011: Work around QDF2400 E44 stuck BUSY bit"):

That patch added a *separate* qdf2400_e44_putc() for hardware where
BUSY can get stuck at 1, rather than touching pl011_putc() itself, and
it left pl011_putc()'s per-character BUSY wait in place. So there is
precedent for "waiting on BUSY is fragile on some hardware", but no
indication that pl011_putc()'s per-character wait was ever intentional
for standard hardware as opposed to simply mirroring the already-present
final-drain wait used elsewhere in this same file.

Both pl011_console_putchar() and pl011_put_poll_char() (the regular,
always-built console write path and the kgdb/kdb polling path) only
wait for TXFF before writing, never for BUSY. The regular console path
still gets a full-drain guarantee, but correctly obtains it only *once*,
after the whole buffer has been written: pl011_console_write_atomic()
and pl011_console_write_thread() each call uart_console_write() with
pl011_console_putchar (no per-character BUSY wait), then separately
wait for BUSY exactly once, after the loop, before restoring the CR
register.

pl011_early_write() had no equivalent single final wait, so simply
deleting the wait from pl011_putc() would drop the "the UART has
actually finished transmitting by the time this function returns"
guarantee that earlycon currently provides, e.g. right before a panic
message is followed immediately by a reboot/poweroff. Instead, move the
same "wait for BUSY" out of the per-character pl011_putc() and place it
once in pl011_early_write(), after uart_console_write() completes,
mirroring the pattern already used by pl011_console_write_atomic()/
_thread(). This keeps the flush guarantee while turning an O(n) wait
(once per character) into an O(1) wait (once per earlycon write call).

This is easy to see, and to prove, in a fast-booting VMM. Using a small
KVM-based VMM that boots Linux guests directly on arm64 (no firmware),
booting an otherwise-identical 7.2.0-rc1 kernel (single vCPU, 512MB
guest, direct PL011 MMIO with no interrupt-driven UART - i.e. earlycon
is the only console active) up to the (expected, rootfs-less)
"Unable to mount root fs" panic, with 20 boot samples per kernel per
configuration:

  cmdline: console=ttyAMA0 earlycon=pl011,0x09000000 ignore_loglevel
           initcall_debug printk.time=1     (~99KB printed over earlycon)
    before: min 466.3ms  avg 471.4ms  max 480.3ms  (n=20)
    after:  min 429.4ms  avg 435.3ms  max 449.0ms  (n=20)
    -> ~36ms / ~7.7% faster, non-overlapping distributions

  cmdline: <the VMM's actual default, quiet, earlycon=pl011,0x09000000>
    before: min  74.3ms  avg  76.5ms  max  82.2ms  (n=20)
    after:  min  53.8ms  avg  56.8ms  max  74.9ms  (n=20)
    -> ~20ms / ~25.8% faster

The second measurement uses the VMM's real default boot configuration
(not a synthetic debug cmdline), so this is representative of ordinary
boots, not just verbose-logging ones. Console content was diffed
(timestamps and per-initcall "returned 0 after N usecs" numbers
excluded) between before/after runs and is identical line-for-line:
this change does not drop, reorder, or corrupt any output.

Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
---
v2: Rather than simply deleting the wait, move it out of the
    per-character pl011_putc() and into pl011_early_write(), done once
    after the whole buffer is written (mirroring the existing pattern in
    pl011_console_write_atomic()/_thread()), so earlycon keeps its
    "fully transmitted by the time this call returns" guarantee. Also
    added the QDF2400 erratum 44 history as context for why the BUSY
    wait existed, and re-measured with the revised patch (numbers
    updated accordingly, same conclusion).

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

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 8ed91e1da22be..05a783dda4c4a 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2741,8 +2741,6 @@ static void pl011_putc(struct uart_port *port, unsigned char c)
 		writel(c, port->membase + UART01x_DR);
 	else
 		writeb(c, port->membase + UART01x_DR);
-	while (readl(port->membase + UART01x_FR) & UART01x_FR_BUSY)
-		cpu_relax();
 }
 
 static void pl011_early_write(struct console *con, const char *s, unsigned int n)
@@ -2750,6 +2748,20 @@ static void pl011_early_write(struct console *con, const char *s, unsigned int n
 	struct earlycon_device *dev = con->data;
 
 	uart_console_write(&dev->port, s, n, pl011_putc);
+
+	/*
+	 * Wait for the last character to be fully transmitted before
+	 * returning, same as pl011_console_write_atomic()/_thread() do for
+	 * the non-early console. There is no need to do this after every
+	 * character in pl011_putc(): checking TXFF there already prevents
+	 * overrunning the FIFO, and waiting for BUSY per character forces
+	 * the UART to be drained serially instead of letting it buffer
+	 * queued bytes, which is needlessly slow, especially so under
+	 * virtualization where each poll of UARTFR/UARTDR is a trapped MMIO
+	 * access.
+	 */
+	while (readl(dev->port.membase + UART01x_FR) & UART01x_FR_BUSY)
+		cpu_relax();
 }
 
 #ifdef CONFIG_CONSOLE_POLL
-- 
2.54.0


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

* Re: [PATCH v2] serial: amba-pl011: don't wait for BUSY after every earlycon character
  2026-07-03 19:38 ` [PATCH v2] serial: amba-pl011: don't wait for BUSY after every earlycon character Eric Curtin
@ 2026-07-10 12:38   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2026-07-10 12:38 UTC (permalink / raw)
  To: Eric Curtin; +Cc: Russell King, Jiri Slaby, linux-serial, linux-kernel

On Fri, Jul 03, 2026 at 07:38:04PM +0000, Eric Curtin wrote:
> pl011_putc(), used exclusively by the pl011 earlycon (pl011_early_write()
> -> uart_console_write()), waits for UART01x_FR_TXFF to clear before
> writing a character (correct: don't overrun the TX FIFO) and then *also*
> busy-waited for UART01x_FR_BUSY to clear before returning, i.e. it waited
> for the character to be fully shifted out on the wire before the next
> character in the string could even be considered.
> 
> Waiting for BUSY per character defeats the TX FIFO: instead of letting
> the UART buffer several queued bytes and transmit them back to back,
> every single character printed through earlycon was forced to wait for
> that character's own complete transmission (a full UART bit-time at the
> configured baud rate) before the driver would even look at writing the
> next one. This is wasted time on real hardware, and it is much worse
> under virtualization: each read of UARTFR and each write to UARTDR is
> an MMIO access that traps to the hypervisor, so every extra poll is a
> full VM-exit/entry round trip.
> 
> git blame on this function is unhelpful (this tree's history stops at a
> shallow-clone boundary), but the equivalent history is available from
> the upstream patch that added the QDF2400 erratum 44 workaround:

So this was generated by a LLM, right?  Otherwise why can't you find
where this changed by NOT doing a shallow clone?

When using a LLM, you must document that.

thanks,

greg k-h

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

end of thread, other threads:[~2026-07-10 12:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03 19:16 [PATCH] serial: amba-pl011: drop redundant BUSY wait from earlycon pl011_putc() Eric Curtin
2026-07-03 19:38 ` [PATCH v2] serial: amba-pl011: don't wait for BUSY after every earlycon character Eric Curtin
2026-07-10 12:38   ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox