ACPI firmware routinely calls add_preferred_console() via
acpi_parse_spcr() before we ever look at the kernel command line. After
that first registration we short-circuit on every duplicate name/index
match, so the subsequent console=ttyS0,... parameter never refreshes the
UART options that the firmware supplied.

Historically that just meant you couldn't tweak baud/flow control for a
firmware-provided serial console unless you picked a different device
name, but the per-console loglevel plumbing in this series relies on
those later console= entries being able to update the stored option
string. Without that, console=ttyS0,loglevel:5 simply never takes effect
on machines that get their console from SPCR/DT.

Teach __add_preferred_console() to update the existing slot when the
same console is mentioned again: we keep the original slot, but replace
its option string (and re-run braille option parsing) so that later
callers can override what firmware seeded. This keeps today's behaviour
unchanged for drivers, while allowing the cmdline UART parameters (and
soon the loglevel hints) to override the ACPI defaults.

Signed-off-by: Chris Down <chris@chrisdown.name>
---
 kernel/printk/printk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a3072ea39e5e..447d9c28f180 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2560,7 +2560,12 @@ static int __add_preferred_console(const char *name, const short idx,
 		    (devname && strcmp(c->devname, devname) == 0)) {
 			if (!brl_options)
 				preferred_console = i;
+
+			if (options)
+				c->options = options;
+
 			set_user_specified(c, user_specified);
+			braille_set_options(c, brl_options);
 			return 0;
 		}
 	}
-- 
2.51.2

On Fri 2025-11-28 03:43:21, Chris Down wrote:
> ACPI firmware routinely calls add_preferred_console() via
> acpi_parse_spcr() before we ever look at the kernel command line. After
> that first registration we short-circuit on every duplicate name/index
> match, so the subsequent console=ttyS0,... parameter never refreshes the
> UART options that the firmware supplied.
> 
> Historically that just meant you couldn't tweak baud/flow control for a
> firmware-provided serial console unless you picked a different device
> name, but the per-console loglevel plumbing in this series relies on
> those later console= entries being able to update the stored option
> string. Without that, console=ttyS0,loglevel:5 simply never takes effect
> on machines that get their console from SPCR/DT.
> 
> Teach __add_preferred_console() to update the existing slot when the
> same console is mentioned again: we keep the original slot, but replace
> its option string (and re-run braille option parsing) so that later
> callers can override what firmware seeded. This keeps today's behaviour
> unchanged for drivers, while allowing the cmdline UART parameters (and
> soon the loglevel hints) to override the ACPI defaults.

Another great catch!

> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2560,7 +2560,12 @@ static int __add_preferred_console(const char *name, const short idx,
>  		    (devname && strcmp(c->devname, devname) == 0)) {


__add_preferred_console() can be called also after processing the
command line, for example via the device tree:

  + serial8250_init()
    + serial8250_register_ports()
      + uart_add_one_port()
        + serial_ctrl_register_port()
	  + serial_core_register_port()
	    + serial_core_add_one_port()
	      + of_console_check()
	        + add_preferred_console

Or a platform default:

  + hvc_rtas_console_init()
    + add_preferred_console()

I would rather make sure that we update the values only when the
console is defined on the command line:

			/*
			 * Make sure that only command line is allowed
			 * to modify the default preference and options.
			 */
			if (!user_specified)
				return 0;

It probably won't have any effect. For example, of_console_check()
calls add_preferred_console() only when (!console_set_on_cmdline).
And hvc_rtas_console_init() does not pass any @options.

But I would rather be on the safe side and make the logic explicit here.

>  			if (!brl_options)
>  				preferred_console = i;
> +
> +			if (options)
> +				c->options = options;
> +
>  			set_user_specified(c, user_specified);
> +			braille_set_options(c, brl_options);
>  			return 0;
>  		}
>  	}

Otherwise, it looks good to me.

Best Regards,
Petr