Re: [PATCH 5/8] printk: Try to register each console as Braille first

[Petr Mladek <pmladek@suse.com>]

On Fri 2026-02-20 12:52:09, Chris Down wrote:
> Petr Mladek writes:
> > > The try_only_braille and is_braille_console_preferred(pc) checks likely need
> > > to happen before or independently of the match() vs. default matching
> > > branch.
> > 
> > This should never happen because register_console() always tries to
> > register Braille consoles first.
> 
> I think maybe we're talking a bit past each other :-) It does seem I
> misunderstood the ->match() semantics a bit though, I don't think there's a
> bug here any more, but maybe worth a clarifying code comment.
> 
> My concern wasn't so much about the ordering between
> try_enable_braille_console() and try_enable_preferred_console(). It was more
> about what happens inside __try_enable_preferred_console() when it takes
> try_only_braille=true. _braille_register_console and
> is_braille_console_preferred are both inside the default match block, which
> is only entered when ->match() returns non-zero or is absent. So, if
> ->match() returns zero, both would be skipped and the console would fall
> through to CON_ENABLED without _braille_register_console() being called.

You are right. I have completely missed this variant.

> But looking more closely I don't think this can trigger in practice as the
> tree is right now. The only ->match() callbacks in the tree only match
> earlycon-style names, and always return -ENODEV for other kinds of inputs.
> So actually they return -ENODEV in this case and we enter the match block
> normally.
> 
> But I still feel like maybe the structure is a bit subtle here, this
> confused me. The correctness of the Braille path seems to depend on
> ->match() never returning 0 for these kinds of console names, which I'm not
> sure is documented or enforced anywhere else. Maybe a comment mentioning
> this would help future readers? I won't block on it though.

You are right that these are subtle checks.

Hmm ,we might call __try_enable_preferred_console() three times and
we are always interested in some particular entries:

   + Braille consoles
   + User specified
   + Platform specified

I agree that we should distinguish this in the main loop. I am going
to add the following into v2:

static int __try_enable_preferred_console(struct console *newcon,
					  bool user_specified,
					  bool try_only_braille)
{
[...]
	for (i = 0, pc = preferred_consoles;
	     i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc->devname[0]);
	     i++, pc++) {
		/* Console not yet initialized? */
		if (!pc->name[0])
			continue;
+ 
+ 		/*
+ 		 * @try_only_braille and @user_specifified define which
+ 		 * preferred console entries are handled in this round.
+ 		 */
+ 		if (try_only_braille) {
+ 			if (!is_braille_console_preferred(pc))
+ 				continue;
+ 		} else {
+ 			if (pc->user_specified != user_specified)
+ 				continue;
+ 		}
+
		if (!newcon->match ||
		    newcon->match(newcon, pc->name, pc->index, pc->options) != 0) {
			/* default matching */
[...]


But there is still the problem that there might be two entries with
the same pc->name. The 2nd entry might appear later when pc->devname
matches, see match_devname_and_update_preferred_console().

I am afraid that the same HW device might get registered as both
Braille and non-Braille console.

I though about catching this in
match_devname_and_update_preferred_console() and returning -EBUSY.
But there is similar problem that another entry might match
also via con->match() callback.

But maybe this is not a problem at all. The above proposed check
and calling try_enable_braille_console() first ensures that
the Braille entry is preferred.

And each HW device should get associated with a particular
"struct console". The subsystem should make sure that it does not
register the same "struct console" twice. Maybe we could check
console_is_registered_locked() at the beginning on register_console()
to be on the safe side.

Best Regards,
Petr

PS: I hope that the above makes sense. My head is spinning a bit now.
    And I am leaving early today for a week long vacation which
    affects my concetration a bit...

    I think that I need to make more tests after I am back.