Re: [PATCH v7 14/17] printk: Update Braille options when preferred more times

[Chris Down <chris@chrisdown.name>]

Hi Petr,

Petr Mladek writes:
>Make the behavior consistent and always update pc->brl_options when
>@brl_options are passed and they were not defined yet.

This changes what users get for something like "console=ttyS0
console=brl,ttyS0". Before, ttyS0 stayed a normal console with CON_CONSDEV
and the Braille request was dropped. Now ttyS0 becomes the Braille console
and, with 15/17, tty0 gets enabled as the default console instead. That
fits with Braille always winning in 13/17, but could you say so here like
you did there?

With only this patch applied, that command line also leaves the guest
without a console until 15/17. preferred_dev_console still points at the
ttyS0 entry after it turns into Braille, so tty0 is not enabled as the
default console. ttyS0 itself is only enabled as the Braille console.
That leaves only earlyser0 registered and "Warning: unable to open an
initial console". Going by the code, the base already ends up there for
the reverse order (console=brl,ttyS0 console=ttyS0), and both of these
seem to be fixed in 15/17. So maybe squashing the two is the simplest way
to keep bisect clean.

>+	/*
>+	 * Allow to override normal console with the Braille variant,
>+	 * for example when ttyS0 is preferred via SPCR brl,ttyS0
>+	 * via the command line.
>+	 */
>+	if (brl_options)
>+		braille_update_options(pc, brl_options);

When an existing entry turns into Braille here, it keeps pc->options from
whoever added it first, and braille_register_console() only falls back to
"57600o8" when those are NULL. So the serial options given with the Braille
request are dropped and the result depends on the order. The cflag of
braille_co ends up as:

     console=ttyS1,115200 console=brl,ttyS1,57600  -> B115200
     console=brl,ttyS1,57600 console=ttyS1,115200  -> B57600

Should the options from the Braille request win when the entry switches
over here? Or would you rather leave that for the separate series about
overriding options?

Also, SPCR never adds a "ttyS" entry, it uses "uart", "pl011" and friends.
For "uart", univ8250_console_match() succeeds when
try_enable_braille_console() tries it, so that one ends up as a normal
console anyway. Maybe use a second console= on the command line as the
example. There's also an "and" missing before "brl,ttyS0".

A few changelog typos while you're there: "stored only it is a new entry"
-> "only when it is", "preferred_console[]" -> "preferred_consoles[]" and
".brl->options" -> ".brl_options".

Once those are fixed, please feel free to add:

Reviewed-by: Chris Down <chris@chrisdown.name>

Chris