On Tue 2023-11-21 13:31:54, Tony Lindgren wrote:
> Hi all,
>
> With the recent serial core changes, we can now add DEVNAME:0.0 style
> addressing for the serial ports. When using DEVNAME:0.0 naming, we don't
> need to care which ttyS instance number is allocated depending on HSUART
> settings or if the devicetree has added aliases for all the ports.
>
> This also allows us to also drop the old console_setup() parsing for
> character device names.
>
> Tony Lindgren (3):
> printk: Save console options for add_preferred_console_match()
> serial: core: Add support for DEVNAME:0.0 style naming for kernel
> console
> serial: core: Move console character device handling from printk
First, I appreciate the effort to match aliases to the same console.
Well, my understanding is that it solves the problem only for the newly
added console=DEVICENAME:0.0 format. But it does not handle the
existing problems with matching console names passed via earlycon=
and console= parameters. Am I right?
Now, the bad news. This patchset causes regressions which are
not acceptable. I have found two so far but there might be more.
I used the following kernel command line:
earlycon=uart8250,io,0x3f8,115200 console=ttyS0,115200 console=tty0 ignore_loglevel log_buf_len=1M
1. The patchset caused that /dev/console became associated with
ttyS0 instead of tty0, see the "C" flag:
original # cat /proc/consoles
tty0 -WU (EC ) 4:1
ttyS0 -W- (E p a) 4:64
vs.
patched # cat /proc/consoles
ttyS0 -W- (EC p a) 4:64
tty0 -WU (E ) 4:1
This is most likely caused by the different ordering of
__add_preferred_console() calls.
The ordering is important because it defines which console
will get associated with /dev/console. It is a so called
preferred console defined by the last console= parameter.
Unfortunately also the ordering of the other parameters
is important when a console defined by the last console=
parameter is not registered at all. In this case,
/dev/console gets associated with the first console
with tty binding according to the order on the command line.
If you think that it is weird behavior then I agree.
But it is a historical mess. It is how people used it
when the various features were added. Many changes
in this code caused regressions and had to be reverted.
See the following to get the picture:
+ commit c6c7d83b9c9e6a8 ("Revert "console: don't
prefer first registered if DT specifies stdout-path")
+ commit dac8bbbae1d0ccb ("Revert "printk: fix double
printing with earlycon"").
2. The serial console gets registered much later with this
patchset:
original # dmesg | grep printk:
[ 0.000000] printk: legacy bootconsole [uart8250] enabled
[ 0.000000] printk: debug: ignoring loglevel setting.
[ 0.016859] printk: log_buf_len: 1048576 bytes
[ 0.017324] printk: early log buf free: 259624(99%)
[ 0.141859] printk: legacy console [tty0] enabled
[ 0.142399] printk: legacy bootconsole [uart8250] disabled
[ 0.143032] printk: legacy console [ttyS0] enabled
vs.
patched # dmesg | grep printk:
[ 0.000000] printk: legacy bootconsole [uart8250] enabled
[ 0.000000] printk: debug: ignoring loglevel setting.
[ 0.018142] printk: log_buf_len: 1048576 bytes
[ 0.018757] printk: early log buf free: 259624(99%)
[ 0.160706] printk: legacy console [tty0] enabled
[ 0.161213] printk: legacy bootconsole [uart8250] disabled
[ 1.592929] printk: legacy console [ttyS0] enabled
This is pretty bad because it would complicate or even prevent
debugging of the boot stage via serial console.
The graphical console is not usable when the system dies. Also
finding the right arguments for the earlycon= parameter is
tricky so that people enable it only when they have to debug
very early messages.
I am going to look at the patches more closely to see if I could
provide some hints.
Best Regards,
Petr
