From: Greg KH <gregkh@linuxfoundation.org>
To: Eric Curtin <ericcurtin17@gmail.com>
Cc: linux@armlinux.org.uk, jirislaby@kernel.org,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH v4] serial: amba-pl011: don't wait for BUSY after every earlycon character
Date: Thu, 30 Jul 2026 16:53:34 +0200 [thread overview]
Message-ID: <2026073045-drove-underuse-fcde@gregkh> (raw)
In-Reply-To: <20260720122646.16994-1-ericcurtin17@gmail.com>
On Mon, Jul 20, 2026 at 12:26:46PM +0000, Eric Curtin wrote:
> An AI coding tool (OpenCode CLI, using Claude as the backing model) was
> used in preparing this patch. It was given the observation that
> earlycon output was slower than expected under virtualization and
> asked to locate the cause and propose a fix; it identified the
> per-character BUSY wait in pl011_putc() and, after being pointed at
> pl011_console_write_atomic()/_thread() as the existing precedent for a
> single final drain, produced the code that moves the wait into
> pl011_early_write(). It also helped research the QDF2400 erratum 44
> history cited above and draft this changelog. All of the above was
> reviewed by hand against the driver's other console write paths to
> confirm correctness. The boot-timing measurements and the line-for-line
> console diff were run and captured by hand on the VMM described above;
> they were not produced by the tool.
This means an Assisted-by line should be added.
Also, your changelog should be a lot smaller, LLMs love to talk a lot...
>
> Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
> ---
> v4: v3 mistakenly replaced this patch's actual fix (moving the BUSY
> wait to run once in pl011_early_write(), preserving the "fully
> drained on return" guarantee) with a plain deletion of the wait,
> which would have been a functional regression (e.g. for panic
> output immediately followed by reboot/poweroff). This reverts to
> the v2 fix, keeps the real, measured boot-timing numbers, and adds
> the explicit disclosure of AI-tool assistance that was missing from
> v2, per Documentation/process/generated-content.rst.
> v3: (erroneous, superseded by the above; sent by mistake)
> 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 8ed91e1da22b..05a783dda4c4 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.
> + */
And this comment is also really not needed, right?
thanks,
greg k-h
prev parent reply other threads:[~2026-07-30 16:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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
2026-07-20 12:17 ` [PATCH v3] " Eric Curtin
2026-07-20 12:26 ` [PATCH v4] " Eric Curtin
2026-07-22 16:45 ` Michael Kelley
2026-07-30 14:53 ` Greg KH [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2026073045-drove-underuse-fcde@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=ericcurtin17@gmail.com \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux@armlinux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox