diff mbox series

[v2,3/3] serial: core: Add sysfs links for serial core port instances for ttys

Message ID 20230912110350.14482-4-tony@atomide.com
State New
Headers
Series Add support for DEVNAME:0.0 style hardware based addressing |

Commit Message

Tony Lindgren Sept. 12, 2023, 11:03 a.m. UTC 
  Let's allow the userspace to find out the ttyS style name for a serial
core port device if a tty exists. This can be done with:

$ grep DEVNAME /sys/bus/serial-base/devices/*/tty/uevent
/sys/bus/serial-base/devices/00:04:0.0/tty/uevent:DEVNAME=ttyS0
/sys/bus/serial-base/devices/serial8250:0.1/tty/uevent:DEVNAME=ttyS1
/sys/bus/serial-base/devices/serial8250:0.2/tty/uevent:DEVNAME=ttyS2
/sys/bus/serial-base/devices/serial8250:0.3/tty/uevent:DEVNAME=ttyS3

With this change, we can add /dev/serial/by-id symlinks for the serial
core port device instances. This allows using hardware based port
addressing in addition to the legacy ttyS style naming.

The serial core port naming is DEVNAME:0.0, such as the 00:04:0.0 above.
The 0.0 above are serial core controller id and port id. The port id and
controller id are typically both zero unless the serial port hardware
controller has multiple controllers or ports.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/tty/serial/serial_core.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Andy Shevchenko Sept. 12, 2023, 3:17 p.m. UTC | #1 
On Tue, Sep 12, 2023 at 02:03:45PM +0300, Tony Lindgren wrote:
> Let's allow the userspace to find out the ttyS style name for a serial
> core port device if a tty exists. This can be done with:
> 
> $ grep DEVNAME /sys/bus/serial-base/devices/*/tty/uevent
> /sys/bus/serial-base/devices/00:04:0.0/tty/uevent:DEVNAME=ttyS0
> /sys/bus/serial-base/devices/serial8250:0.1/tty/uevent:DEVNAME=ttyS1
> /sys/bus/serial-base/devices/serial8250:0.2/tty/uevent:DEVNAME=ttyS2
> /sys/bus/serial-base/devices/serial8250:0.3/tty/uevent:DEVNAME=ttyS3
> 
> With this change, we can add /dev/serial/by-id symlinks for the serial
> core port device instances. This allows using hardware based port
> addressing in addition to the legacy ttyS style naming.
> 
> The serial core port naming is DEVNAME:0.0, such as the 00:04:0.0 above.
> The 0.0 above are serial core controller id and port id. The port id and
> controller id are typically both zero unless the serial port hardware
> controller has multiple controllers or ports.

...

> +	struct uart_match match = {port, drv};

A nit:

	struct uart_match match = { .port = port, .driver = drv };

...

> +	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");

I would do it on a single line (you already over 80 anyway).

> +		put_device(tty_dev);
> +		if (ret)
> +			goto err_remove_port;
> +	}

...

> +	struct uart_match match = {port, drv};

As per above.

...

> +	tty_dev = device_find_child(port->dev, &match, serial_match_port);

Can be written as

	tty_dev = device_find_child(phys_dev, &match, serial_match_port);

?

> +	if (tty_dev) {
> +		sysfs_remove_link(&port->port_dev->dev.kobj, "tty");

Can be written as

		sysfs_remove_link(&port_dev->dev.kobj, "tty");

can't be?

> +		put_device(tty_dev);
> +	}
Tony Lindgren Sept. 13, 2023, 12:30 p.m. UTC | #2 
* Andy Shevchenko <andriy.shevchenko@intel.com> [230912 15:17]:
> On Tue, Sep 12, 2023 at 02:03:45PM +0300, Tony Lindgren wrote:
> > +	tty_dev = device_find_child(port->dev, &match, serial_match_port);
> 
> Can be written as
> 
> 	tty_dev = device_find_child(phys_dev, &match, serial_match_port);
> 
> ?
> 
> > +	if (tty_dev) {
> > +		sysfs_remove_link(&port->port_dev->dev.kobj, "tty");
> 
> Can be written as
> 
> 		sysfs_remove_link(&port_dev->dev.kobj, "tty");
> 
> can't be?

Yes that's shorter.

Thanks,

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
@@ -3328,6 +3328,8 @@  static int serial_core_port_device_add(struct serial_ctrl_device *ctrl_dev,
 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);
@@ -3368,10 +3370,22 @@  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");
+		put_device(tty_dev);
+		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);
 
@@ -3393,12 +3407,20 @@  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");
+		put_device(tty_dev);
+	}
+
 	serial_core_remove_one_port(drv, port);
 
 	/* Note that struct uart_port *port is no longer valid at this point */