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

[Chris Down]

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.

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.