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

[Andy Shevchenko]

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