Re: [PATCH v4] serial: amba-pl011: don't wait for BUSY after every earlycon character
Greg KH <[email protected]> Thu, 30 Jul 2026 16:53:34 +0200
| Newsgroups | org.kernel.vger.linux-serial,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <2026073045-drove-underuse-fcde@gregkh> |
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 <[email protected]> > --- > 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