Linux Serial subsystem development
 help / color / mirror / Atom feed
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

      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