diff mbox series

serial: core: Add sysfs links for serial core port instances for ttys

Message ID 20230719051613.46569-1-tony@atomide.com
State New
Headers
Series serial: core: Add sysfs links for serial core port instances for ttys |

Commit Message

Tony Lindgren July 19, 2023, 5:16 a.m. UTC 
  Let's allow the userspace to find out the tty name for a serial core
controller id if a tty exists. This can be done with:

$ grep DEVNAME /sys/bus/serial-base/devices/port*/tty/uevent
/sys/bus/serial-base/devices/port.00:04.0/tty/uevent:DEVNAME=ttyS0
/sys/bus/serial-base/devices/port.serial8250.1/tty/uevent:DEVNAME=ttyS1
/sys/bus/serial-base/devices/port.serial8250.2/tty/uevent:DEVNAME=ttyS2
/sys/bus/serial-base/devices/port.serial8250.3/tty/uevent:DEVNAME=ttyS3

And with this, we can add /dev/serial/by-id symlinks to the serial port
device instances so we can start using serial core port addressing in
addition to the legacy ttyS naming.

The naming we can use is dev_name:0.0 where 0.0 are the serial core
controller id and port id, so for the ttyS0 example above the naming
would be 00:04.0:0.0.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---

Note that this depends on fix for serial core port ids patch
"[PATCH] serial: core: Fix serial core port id to not use port->line"

---
 drivers/tty/serial/serial_core.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Andy Shevchenko July 19, 2023, 5:33 a.m. UTC | #1 
On Wed, Jul 19, 2023 at 08:16:11AM +0300, Tony Lindgren wrote:
> Let's allow the userspace to find out the tty name for a serial core
> controller id if a tty exists. This can be done with:
> 
> $ grep DEVNAME /sys/bus/serial-base/devices/port*/tty/uevent
> /sys/bus/serial-base/devices/port.00:04.0/tty/uevent:DEVNAME=ttyS0
> /sys/bus/serial-base/devices/port.serial8250.1/tty/uevent:DEVNAME=ttyS1
> /sys/bus/serial-base/devices/port.serial8250.2/tty/uevent:DEVNAME=ttyS2
> /sys/bus/serial-base/devices/port.serial8250.3/tty/uevent:DEVNAME=ttyS3

What part is the controller ID here?

We also have something in procfs (I don't remember what info exactly is there).

> And with this, we can add /dev/serial/by-id symlinks to the serial port
> device instances so we can start using serial core port addressing in
> addition to the legacy ttyS naming.
> 
> The naming we can use is dev_name:0.0 where 0.0 are the serial core
> controller id and port id, so for the ttyS0 example above the naming
> would be 00:04.0:0.0.

This is interesting idea. But any hint why it can be useful?
Tony Lindgren July 19, 2023, 5:43 a.m. UTC | #2 
* Andy Shevchenko <andriy.shevchenko@intel.com> [230719 05:34]:
> On Wed, Jul 19, 2023 at 08:16:11AM +0300, Tony Lindgren wrote:
> > Let's allow the userspace to find out the tty name for a serial core
> > controller id if a tty exists. This can be done with:
> > 
> > $ grep DEVNAME /sys/bus/serial-base/devices/port*/tty/uevent
> > /sys/bus/serial-base/devices/port.00:04.0/tty/uevent:DEVNAME=ttyS0
> > /sys/bus/serial-base/devices/port.serial8250.1/tty/uevent:DEVNAME=ttyS1
> > /sys/bus/serial-base/devices/port.serial8250.2/tty/uevent:DEVNAME=ttyS2
> > /sys/bus/serial-base/devices/port.serial8250.3/tty/uevent:DEVNAME=ttyS3
> 
> What part is the controller ID here?

Oh looks like controller id it's missing in the name, I'll send a fix
for that.

> We also have something in procfs (I don't remember what info exactly is there).

Do you mean /proc/devices?

> > And with this, we can add /dev/serial/by-id symlinks to the serial port
> > device instances so we can start using serial core port addressing in
> > addition to the legacy ttyS naming.
> > 
> > The naming we can use is dev_name:0.0 where 0.0 are the serial core
> > controller id and port id, so for the ttyS0 example above the naming
> > would be 00:04.0:0.0.
> 
> This is interesting idea. But any hint why it can be useful?

If you have lots of serial ports and we are stuck with adding aliases
for the ports in the dts files where the ttyS naming and ordering does
not really help or may not necessarily make sense if the ports are on
different buses or domains. With CONFIG_SERIAL_8250_RUNTIME_UARTS=4,
the ttyS naming is only needed for the legacy ports really.

Regards,

Tony
Andy Shevchenko July 19, 2023, 1:59 p.m. UTC | #3 
On Wed, Jul 19, 2023 at 08:43:21AM +0300, Tony Lindgren wrote:
> * Andy Shevchenko <andriy.shevchenko@intel.com> [230719 05:34]:
> > On Wed, Jul 19, 2023 at 08:16:11AM +0300, Tony Lindgren wrote:
> > > Let's allow the userspace to find out the tty name for a serial core
> > > controller id if a tty exists. This can be done with:
> > > 
> > > $ grep DEVNAME /sys/bus/serial-base/devices/port*/tty/uevent
> > > /sys/bus/serial-base/devices/port.00:04.0/tty/uevent:DEVNAME=ttyS0
> > > /sys/bus/serial-base/devices/port.serial8250.1/tty/uevent:DEVNAME=ttyS1
> > > /sys/bus/serial-base/devices/port.serial8250.2/tty/uevent:DEVNAME=ttyS2
> > > /sys/bus/serial-base/devices/port.serial8250.3/tty/uevent:DEVNAME=ttyS3
> > 
> > What part is the controller ID here?
> 
> Oh looks like controller id it's missing in the name, I'll send a fix
> for that.
> 
> > We also have something in procfs (I don't remember what info exactly is there).
> 
> Do you mean /proc/devices?

Something tty specific, /proc/tty/, but I had a look and it seems for another
stuff.

> > > And with this, we can add /dev/serial/by-id symlinks to the serial port
> > > device instances so we can start using serial core port addressing in
> > > addition to the legacy ttyS naming.
> > > 
> > > The naming we can use is dev_name:0.0 where 0.0 are the serial core
> > > controller id and port id, so for the ttyS0 example above the naming
> > > would be 00:04.0:0.0.
> > 
> > This is interesting idea. But any hint why it can be useful?
> 
> If you have lots of serial ports and we are stuck with adding aliases
> for the ports in the dts files where the ttyS naming and ordering does
> not really help or may not necessarily make sense if the ports are on
> different buses or domains. With CONFIG_SERIAL_8250_RUNTIME_UARTS=4,
> the ttyS naming is only needed for the legacy ports really.

I see. Does it fix the long standing issue with ttyS enumeration (on x86
at least) when depending on the presence of the legacy ports the HSUART
(high speed) can preempt the legacy placeholders (ttyS0..ttyS3)?

To me sounds like it may very well do fix it and I would be glad to see that
in the commit message (as selling point) and in documentation.
Tony Lindgren July 20, 2023, 4:13 a.m. UTC | #4 
* Andy Shevchenko <andriy.shevchenko@intel.com> [230719 13:59]:
> On Wed, Jul 19, 2023 at 08:43:21AM +0300, Tony Lindgren wrote:
> > * Andy Shevchenko <andriy.shevchenko@intel.com> [230719 05:34]:
> > > On Wed, Jul 19, 2023 at 08:16:11AM +0300, Tony Lindgren wrote:
> > > > Let's allow the userspace to find out the tty name for a serial core
> > > > controller id if a tty exists. This can be done with:
> > > > 
> > > > $ grep DEVNAME /sys/bus/serial-base/devices/port*/tty/uevent
> > > > /sys/bus/serial-base/devices/port.00:04.0/tty/uevent:DEVNAME=ttyS0
> > > > /sys/bus/serial-base/devices/port.serial8250.1/tty/uevent:DEVNAME=ttyS1
> > > > /sys/bus/serial-base/devices/port.serial8250.2/tty/uevent:DEVNAME=ttyS2
> > > > /sys/bus/serial-base/devices/port.serial8250.3/tty/uevent:DEVNAME=ttyS3
> > > 
> > > What part is the controller ID here?
> > 
> > Oh looks like controller id it's missing in the name, I'll send a fix
> > for that.
> > 
> > > We also have something in procfs (I don't remember what info exactly is there).
> > 
> > Do you mean /proc/devices?
> 
> Something tty specific, /proc/tty/, but I had a look and it seems for another
> stuff.

OK

> > > > And with this, we can add /dev/serial/by-id symlinks to the serial port
> > > > device instances so we can start using serial core port addressing in
> > > > addition to the legacy ttyS naming.
> > > > 
> > > > The naming we can use is dev_name:0.0 where 0.0 are the serial core
> > > > controller id and port id, so for the ttyS0 example above the naming
> > > > would be 00:04.0:0.0.
> > > 
> > > This is interesting idea. But any hint why it can be useful?
> > 
> > If you have lots of serial ports and we are stuck with adding aliases
> > for the ports in the dts files where the ttyS naming and ordering does
> > not really help or may not necessarily make sense if the ports are on
> > different buses or domains. With CONFIG_SERIAL_8250_RUNTIME_UARTS=4,
> > the ttyS naming is only needed for the legacy ports really.
> 
> I see. Does it fix the long standing issue with ttyS enumeration (on x86
> at least) when depending on the presence of the legacy ports the HSUART
> (high speed) can preempt the legacy placeholders (ttyS0..ttyS3)?
>
> To me sounds like it may very well do fix it and I would be glad to see that
> in the commit message (as selling point) and in documentation.

It won't affect how ttyS0..ttyS3 get assigned, but it helps finding your
HSUART instance with DEVNAME:0.0 style addressing. So you don't need to
care what ttyS number the port has. If you have such a test case maybe give
it a try.

Regards,

Tony
Andy Shevchenko July 21, 2023, 10:05 a.m. UTC | #5 
On Thu, Jul 20, 2023 at 07:13:19AM +0300, Tony Lindgren wrote:
> * Andy Shevchenko <andriy.shevchenko@intel.com> [230719 13:59]:
> > On Wed, Jul 19, 2023 at 08:43:21AM +0300, Tony Lindgren wrote:
> > > * Andy Shevchenko <andriy.shevchenko@intel.com> [230719 05:34]:
> > > > On Wed, Jul 19, 2023 at 08:16:11AM +0300, Tony Lindgren wrote:

...

> > > > > And with this, we can add /dev/serial/by-id symlinks to the serial port
> > > > > device instances so we can start using serial core port addressing in
> > > > > addition to the legacy ttyS naming.
> > > > > 
> > > > > The naming we can use is dev_name:0.0 where 0.0 are the serial core
> > > > > controller id and port id, so for the ttyS0 example above the naming
> > > > > would be 00:04.0:0.0.
> > > > 
> > > > This is interesting idea. But any hint why it can be useful?
> > > 
> > > If you have lots of serial ports and we are stuck with adding aliases
> > > for the ports in the dts files where the ttyS naming and ordering does
> > > not really help or may not necessarily make sense if the ports are on
> > > different buses or domains. With CONFIG_SERIAL_8250_RUNTIME_UARTS=4,
> > > the ttyS naming is only needed for the legacy ports really.
> > 
> > I see. Does it fix the long standing issue with ttyS enumeration (on x86
> > at least) when depending on the presence of the legacy ports the HSUART
> > (high speed) can preempt the legacy placeholders (ttyS0..ttyS3)?
> >
> > To me sounds like it may very well do fix it and I would be glad to see that
> > in the commit message (as selling point) and in documentation.
> 
> It won't affect how ttyS0..ttyS3 get assigned, but it helps finding your
> HSUART instance with DEVNAME:0.0 style addressing. So you don't need to
> care what ttyS number the port has. If you have such a test case maybe give
> it a try.

Exactly, the problem (currently) is, that depending on the BIOS settings
the kernel can't use the same command line and this is quite a PITA for
our customers!

As far as we can specify hardware (path) to the console, it will be so
cool and hopefully solves this very long standing issue.
Tony Lindgren July 24, 2023, 5:26 a.m. UTC | #6 
* Andy Shevchenko <andriy.shevchenko@intel.com> [230721 10:06]:
> On Thu, Jul 20, 2023 at 07:13:19AM +0300, Tony Lindgren wrote:
> > * Andy Shevchenko <andriy.shevchenko@intel.com> [230719 13:59]:
> > > On Wed, Jul 19, 2023 at 08:43:21AM +0300, Tony Lindgren wrote:
> > > > * Andy Shevchenko <andriy.shevchenko@intel.com> [230719 05:34]:
> > > > > On Wed, Jul 19, 2023 at 08:16:11AM +0300, Tony Lindgren wrote:
> 
> ...
> 
> > > > > > And with this, we can add /dev/serial/by-id symlinks to the serial port
> > > > > > device instances so we can start using serial core port addressing in
> > > > > > addition to the legacy ttyS naming.
> > > > > > 
> > > > > > The naming we can use is dev_name:0.0 where 0.0 are the serial core
> > > > > > controller id and port id, so for the ttyS0 example above the naming
> > > > > > would be 00:04.0:0.0.
> > > > > 
> > > > > This is interesting idea. But any hint why it can be useful?
> > > > 
> > > > If you have lots of serial ports and we are stuck with adding aliases
> > > > for the ports in the dts files where the ttyS naming and ordering does
> > > > not really help or may not necessarily make sense if the ports are on
> > > > different buses or domains. With CONFIG_SERIAL_8250_RUNTIME_UARTS=4,
> > > > the ttyS naming is only needed for the legacy ports really.
> > > 
> > > I see. Does it fix the long standing issue with ttyS enumeration (on x86
> > > at least) when depending on the presence of the legacy ports the HSUART
> > > (high speed) can preempt the legacy placeholders (ttyS0..ttyS3)?
> > >
> > > To me sounds like it may very well do fix it and I would be glad to see that
> > > in the commit message (as selling point) and in documentation.
> > 
> > It won't affect how ttyS0..ttyS3 get assigned, but it helps finding your
> > HSUART instance with DEVNAME:0.0 style addressing. So you don't need to
> > care what ttyS number the port has. If you have such a test case maybe give
> > it a try.
> 
> Exactly, the problem (currently) is, that depending on the BIOS settings
> the kernel can't use the same command line and this is quite a PITA for
> our customers!
> 
> As far as we can specify hardware (path) to the console, it will be so
> cool and hopefully solves this very long standing issue.

OK great, I'll update the patch description for next revision.

Regards,

Tony
diff mbox series

Patch

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3371,6 +3371,8 @@  static int serial_core_add_preferred_console(struct uart_driver *drv,
 int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 {
 	struct serial_ctrl_device *ctrl_dev, *new_ctrl_dev = NULL;
+	struct uart_match match = {port, drv};
+	struct device *tty_dev;
 	int ret;
 
 	mutex_lock(&port_mutex);
@@ -3411,10 +3413,21 @@  int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 
 	port->flags &= ~UPF_DEAD;
 
+	tty_dev = device_find_child(port->dev, &match, serial_match_port);
+	if (tty_dev) {
+		ret = sysfs_create_link(&port->port_dev->dev.kobj, &tty_dev->kobj,
+					"tty");
+		if (ret)
+			goto err_remove_port;
+	}
+
 	mutex_unlock(&port_mutex);
 
 	return 0;
 
+err_remove_port:
+	serial_core_remove_one_port(drv, port);
+
 err_unregister_port_dev:
 	serial_base_port_device_remove(port->port_dev);
 
@@ -3436,12 +3449,18 @@  void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port
 	struct device *phys_dev = port->dev;
 	struct serial_port_device *port_dev = port->port_dev;
 	struct serial_ctrl_device *ctrl_dev = serial_core_get_ctrl_dev(port_dev);
+	struct uart_match match = {port, drv};
 	int ctrl_id = port->ctrl_id;
+	struct device *tty_dev;
 
 	mutex_lock(&port_mutex);
 
 	port->flags |= UPF_DEAD;
 
+	tty_dev = device_find_child(port->dev, &match, serial_match_port);
+	if (tty_dev)
+		sysfs_remove_link(&port->port_dev->dev.kobj, "tty");
+
 	serial_core_remove_one_port(drv, port);
 
 	/* Note that struct uart_port *port is no longer valid at this point */