Re: [PATCH v5 13/16] printk: Try to register each console as Braille first

[Petr Mladek <pmladek@suse.com>]

On Thu 2026-10-29 15:17:42, Chris Down wrote:
> Petr Mladek writes:
> >Braille consoles are unique: they can only be enabled via the command
> >line and use the preferred console framework for initialization, yet
> >they are never added to the console_list or associated with
> >/dev/console. Because of this, the preferred_console variable remains
> >unset when only a Braille console is requested.
> 
> This also changes what happens when the same driver is wanted both ways, which 
> might be worth a sentence in the commit message.
> 
> For example:
> 
>     console=ttyS1 console=brl,ttyS0
> 
> Here we used to end up with ttyS1 as a normal console and the Braille request 
> was silently ignored. Now ttyS0 becomes the Braille console, and every later 
> register_console() from serial core goes to the Braille round and gets -ENODEV 
> from braille_register_console(), so there is no serial console at all.

Great catch! I actually was not aware of this particular variant.
I am going to replace the "The refactoring even fixes two subtle bugs:"
section with the following:

<paste>
Behavior change:

1. The Braille console driver is newly always preferred even when different
   serial ports are preferred. For example, let's have the following
   command lines:

     a) console=ttyS1,115200     console=brl,ttyS0,115200
     b) console=00:00:0.0,115200 console=brl,ttyS0,115200
     c) console=brl,ttyS0,115200 console=ttyS1,115200

   They all create two entries in preferred_consoles[] because:

     a) and c) differ in @index (0 vs. 1).
     b) defines either @name or @devname.

   But the original code registers:

     a) ttyS1 as normal console because it is the 1st entry in
	preferred_consoles[].

     b) ttyS0 are Braille console because the generic "ttyS" driver
	is registered early in univ8250_console_init(). The @devname
	entry is resolved too late when serial8250_init()
	registers all found devices, it assigns "tty0" for
	the given "00:00:0.0" but it is already in use.

      c) ttyS0 as Braille console because it is the 1st entry in
	 preferred_consoles[].

    The new code would register ttyS0 as the Braille console in
    all three variants because try_enable_braille_console() is
    called 1st.

2. When only the Braille console is defined on the command line,
   the original code might attempt to enable the same console twice.
   One as a default and once as a Braille console. This results in calling
   the setup() callback twice and incorrectly setting the CON_CONSDEV flag.

The 1st change makes the behavior more consistent and understandable.
The 2nd change is an obvious bug fix.
</paste>

> I think this is probably fine, but just needs clarifying in the commit message.
> 
> >+		 * @try_only_braille and @user_specifified define which
> 
> -> @user_specified

Done.

> >@@ -4033,6 +4061,24 @@ static int try_enable_console(struct console *newcon)
> > {
> > 	int err;
> > 
> >+	/*
> >+	 * First, try to enable the console driver as a Braille console.
> >+	 * It would have metadata in the preferred_consoles[] array.
> >+	 * But it won't be counted as @preferred_console because
> 
> "@preferred_console" is preferred_dev_console now, and it is a variable rather 
> than a parameter, so no "@".
> 
> With those:
> 
> Acked-by: Chris Down <chris@chrisdown.name>

I am going to add this "Acked-by" even when the change in the commit
message is a bit bigger. But I believe that it is more correct now
and it is "just a commit message". I do not want to force you to
review the patch thoughtfully again.

Best Regards,
Petr