Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Tue, 12 Sep 2023 18:17:21 +0300
From:   Andy Shevchenko <andriy.shevchenko@intel.com>
To:     Tony Lindgren <tony@atomide.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jirislaby@kernel.org>, Dhruva Gole <d-gole@ti.com>,
        Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        John Ogness <john.ogness@linutronix.de>,
        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
Subject: Re: [PATCH v2 3/3] serial: core: Add sysfs links for serial core
 port instances for ttys
Message-ID: <ZQCBAXcFwDqZyBZP@smile.fi.intel.com>
References: <20230912110350.14482-1-tony@atomide.com>
 <20230912110350.14482-4-tony@atomide.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230912110350.14482-4-tony@atomide.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

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