Received-SPF: pass (google.com: domain of linux-kernel+bounces-191751-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 28 May 2024 08:05:37 +0300
From: Tony Lindgren <tony.lindgren@linux.intel.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Tony Lindgren <tony@atomide.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>, Jonathan Corbet <corbet@lwn.net>,
	Steven Rostedt <rostedt@goodmis.org>,
	John Ogness <john.ogness@linutronix.de>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	"David S . Miller" <davem@davemloft.net>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Dhruva Gole <d-gole@ti.com>,
	Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
	Johan Hovold <johan@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Vignesh Raghavendra <vigneshr@ti.com>, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org, Sebastian Reichel <sre@kernel.org>,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v7 1/7] printk: Save console options for
 add_preferred_console_match()
Message-ID: <ZlVmIcu2NQOXg9rx@tlindgre-MOBL1>
References: <20240327110021.59793-1-tony@atomide.com>
 <20240327110021.59793-2-tony@atomide.com>
 <ZlC6_Um4P4b-_WQE@pathway.suse.cz>
 <ZlRqz2b0ZrtkxScL@tlindgre-MOBL1>
 <ZlSOc5mtbf4DdI8O@pathway.suse.cz>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZlSOc5mtbf4DdI8O@pathway.suse.cz>

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