2023-09-12 12:28:59

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH v2 3/3] serial: core: Add sysfs links for serial core port instances for ttys

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 <[email protected]>
---
drivers/tty/serial/serial_core.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

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 */
--
2.42.0


2023-09-12 17:20:15

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] serial: core: Add sysfs links for serial core port instances for ttys

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

--
With Best Regards,
Andy Shevchenko


2023-09-13 12:36:46

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] serial: core: Add sysfs links for serial core port instances for ttys

* Andy Shevchenko <[email protected]> [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