Re: [PATCH v7 1/7] printk: Save console options for add_preferred_console_match()

Tony Lindgren
2024-05-28T05:05:50Z

On Mon, May 27, 2024 at 03:45:55PM +0200, Petr Mladek wrote:
> On Mon 2024-05-27 14:13:19, Tony Lindgren wrote:
> > To me it seems we can fix this by keeping track of the console position
> > in the kernel command line. I'll send a fix for this to discuss.
>
> Honestly, I would prefer some alternative solution of the whole
> problem. From my POV, the current patchset is a kind of a hack.
>
> 1. It hides console=DEVNAME:X.Y options so that register_console()
> does not know about them.

OK let's make register_console() aware of the DEVNAME:X.Y options.
I like what you're suggesting towards the end of your message for
this.

> 2. But wait, register_console() might then enable any random console
> by default when there are not console= options. For this the 3rd patch
> added @console_set_on_cmdline variable which would tell
> register_console(): "Hey, I have hidden some user preferences.
> I'll tell you about them when the right time comes."

That's to allow setting up a console when the driver is ready. So that
we don't need to rely on the hardcoded device name deciphering at
console_setup() time. Maybe there's a better way to signal that though.

> 3. When port init matches the pattern, it adds the preferred console
> so that the register_console() would know about it.
>
> 4. But wait, the ordering of preferred consoles is important.
> Which would require more hacks to preserve the ordering.

Preserving the ordering part is probably the smallest issue to deal with
here :) I agree we should try to make things simpler though and there
certainly are already lots of magic switches setting up the console.

> 5. Also serial_base_add_prefcon() adds the preferred console
> with the generic name "ttyS" which is not specific
> for the matched device. It just hopes that the very next
> "register_console()" call will be the one related to
> the matching device. Is this really guaranteed on SMP system?

Hmm not sure I get this issue though, when serial_base_add_prefcon() gets
called we know the device name. The "ttyS" parts are needed to avoid
relying on the hardcoded device name deciphering at console_setup() time.

If you're thinking about the serial8250_isa_init_ports() related calls,
the serial port mapping uses SERIAL_PORT_DFNS. And then a hardware
specific 8250 may take over at some point :)

> IMHO, the only solution would be to add a function which would
> return "ttySX" for the fiven device name.

Yes agreed, this will simplify things.

> Honestly, I do not know the hiearachy of the structures in detail.
> But the documentation in the 7th patch says:
>
> + The mapping of the serial ports to the tty instances
> + can be viewed with:
> +
> + $ ls -d /sys/bus/serial-base/devices/*:*.*/tty/*
> + /sys/bus/serial-base/devices/00:04:0.0/tty/ttyS0
>
> BTW: I get on my test system:
>
> # ls -1 -d /sys/bus/serial-base/devices/*:*.*/tty/*
> /sys/bus/serial-base/devices/00:00:0.0/tty/ttyS0
> /sys/bus/serial-base/devices/serial8250:0.1/tty/ttyS1
> /sys/bus/serial-base/devices/serial8250:0.2/tty/ttyS2
> /sys/bus/serial-base/devices/serial8250:0.3/tty/ttyS3
> ...

OK

> It looks like it should be possible to provide a function which would
> return:
>
> "ttyS0" for "00:00:0.0"
> "ttyS1" for "serial8250:0.1"
> ...
>
>
> This function might then be used in "register_console()"
> to convert "console=DEVNAME:0.0" option to "ttyS" + "index".
>
> The advantage would be that the relation between "DEVNAME:0.0"
> and "ttyS0" will be clear. And the code would see the same hiearachy
> as the user in /sys/bus/serial-base/devices/.

OK makes sense to me.

> Of course, I might be too naive. Maybe, the sysfs hieararchy is
> created too late. Maybe, it is not easy to go throught the
> hiearachy...
>
> But still. I wonder if there is a straightforard way which would
> allow translation between "ttySX" and "DEVNAME:0.0" naming schemes.

We can do that on driver probe time no problem. The issues are mostly
related to setting up things early on.

Regards,

Tony