Re: [RFC PATCH] always probe UART HW when options are not specified

From: Sebastian Frias
Date: Fri Dec 18 2015 - 08:53:16 EST


Hi Peter,

On 12/17/2015 09:09 PM, Peter Hurley wrote:
It's confusing though, given there are multiple ways to express the same thing.
I also found parts of the doc confusing in that regard as well.
ie: there's also a "stdout-path" DT key.

Yep. Thing is, once it goes into the command line and someone uses it,
it's permanent.

That's why it's important to get the semantics right the first time
(which only looks easy from hindsight).


I totally understand, we have the same constraints with our SDK's APIs but with major versions we drop old APIs that have been superseded.
I would have thought that the switch to DT would have been a good opportunity to clean all that up, since it requires a change in the bootloader, right?

Anyway, do you know of a comprehensive list of options, console=ttyS0, earlycon=uart, console=uart, stdout-path=, etc. that are tested?
I would figure that if there's no list, then it is not easy to create the testcases, and thus some end up not being tested (see further below).


So

"console=ttyS0" w/o options always initializes the h/w to 9600n81

Ok, I see. So that's not the option we need then.

"earlycon=uart" w/o options starts a bootconsole w/o initializing the h/w
"console=uart" w/o options starts a bootconsole w/o initializing the h/w,
then replaces that bootconsole with a regular
console (whatever ttySn matched that port)
In this case, the port is probed to discover
the h/w settings. Those also become the initial
settings for the /dev/ttySn device.

Ok, sounds like that last one is the one we need, I will check that, thanks.

Ok, so that does not work.
Actually, the kernel crashes (by the way, the is a potential crash on probe_baud if quot is zero, I had dealt with that on my patch)
Indeed, "console=uart" will crash at a call to uart_parse_earlycon() on drivers/tty/serial/8250/8250_core.c:univ8250_console_match() due to options=NULL.
I see that a similar call to uart_parse_earlycon() in drivers/tty/serial/earlycon.c does check for options!=NULL.

If we add a simple:

if (!options)
return -ENODEV;

then the kernel does not crashes but it appears that the console is not properly brought up, and once that we reach:

[ 0.353378] bootconsole [earlycon0] disabled

we loose it (ie: there are no more logs)

I think the whole process is too involved and I'm not sure I understand it all.
univ8250_console_match() seems to be called twice, here's the calltrace:

console_init()
register_console()
univ8250_console_match()
...
kernel_init()
...
of_platform_serial_driver_init()
...
of_platform_serial_probe()
serial8250_register_8250_port()
uart_add_one_port()
register_console()
univ8250_console_match()

Since options=NULL both times, I think the console is never brought up properly.

I thus used a less obvious (at first) solution:

if (!options)
return univ8250_console_setup(co, options);

however, since univ8250_console_setup() does not forces a probe, and options=NULL, it overwrites the UART config with '9600n8r'.

So, I still think we need to change serial8250_console_setup() and the "rfc patch" I had proposed is still ok for this.
I can remove the probing of the parity, bits, etc. but it looks like it would end up in a half cooked patch, in the sense that sentences like:

"console=uart" w/o options starts a bootconsole w/o initializing the h/w,

would come with some undocumented limitations.

Let me know what you think.



earlyprintk is implemented by arch-specific code, whereas earlycon is implemented
by the serial driver code.

Since earlyprintk is implemented in the arch code, it can be tweaked for
earlier use than early param parsing. There were some patches earlier this
year for x86 to initialize earlyprintk very early; not sure if they were
ever upstreamed. On ARM, earlyprintk is debug_ll.

So there are 3 levels of console?
earlyprintk: before early param
earlycon: early param?
console: after early param?

What's the use case for earlycon if earlyprink is operational by then?

They serve different masters.

Earlyprintk can be crucial for debugging arch-dependent code. For example,
earlycon expects page tables to be setup, whereas earlyprintk on many
arches does not. Earlyprintk is not tied to the driver source at all.

Earlycon is arch-independent and lives with the serial drivers. This makes
it more suitable to support different flavors of serial h/w. Earlycon is
now the boot console for driver developers and post-early init.

Ok, thanks for the explanation.
Out of curiosity:
Do you know what is the difference between "earlycon" and "console"?
I mean, why would one need "earlycon" if there's already "earlyprintk"?
Why does it matter if support is in arch-dependent or arch-independent code?, as long as it works, it shouldn't matter, right?
Why couldn't driver developers use the "earlyprintk" facilities?

Sorry for all the questions, I'm just curious about all these facilities.
I mean, maintaining all of them requires work and is error prone (as the crash above shows), so I'd like to understand why are you guys keeping them all.


I've noticed an increasing tendency for shipping product to also use
earlyprintk/earlycon; I think this is a terrible idea. Boot consoles should
be for debugging only.



Feel free to submit regular patches; reading the divisor via the 8250 port
driver is definitely a good idea.

Not to sure about probing for other than baud though; do you really want
7 data bits and even parity? Or are you just trying to get enable h/w
flow?


Actually, I was doing that for completeness, I can remove that part
of the code if it is wrong or unnecessary, although I'd thought that
one always wanted correct code.

Well, it's just one more thing to have to maintain, so if you don't actually
need those features, I'd rather not add that.

Ok, what about posting that as a separate patch in case somebody else needs it, would that be ok with you?

Thanks, regards,


Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/