mbox series

[v2,0/3] Add support for DEVNAME:0.0 style hardware based addressing

Message ID 20230912110350.14482-1-tony@atomide.com
Headers show
Series Add support for DEVNAME:0.0 style hardware based addressing | expand

Message

Tony Lindgren Sept. 12, 2023, 11:03 a.m. UTC
Hi all,

With the recent serial core changes in v6.5, we can now add DEVNAME:0.0
style addressing for the serial ports. When using DEVNAME:0.0 naming, we
don't need to care which ttyS instance number is allocated depending on
HSUART settings or if the devicetree has added aliases for all the ports.

With these changes the port mapping is visible for usespace in sysfs with:

$ grep DEVNAME /sys/bus/serial-base/devices/*/tty/uevent

Regards,

Tony

Changes since v1:

- Constify printk add_preferred_console() as suggested by Jiri

- Use proper kernel command line helpers for parsing console as
  suggested by Jiri

- Update description for HSUART based on Andy's comments

- Standardize on DEVNAME:0.0 style naming as suggested by Andy

- Added missing put_device() calls paired with device_find_child()

Tony Lindgren (3):
  printk: Constify name for add_preferred_console()
  serial: core: Add support for DEVNAME:0.0 style naming for kernel
    console
  serial: core: Add sysfs links for serial core port instances for ttys

 drivers/tty/serial/Makefile          |   3 +
 drivers/tty/serial/serial_base.h     |  11 +++
 drivers/tty/serial/serial_base_con.c | 133 +++++++++++++++++++++++++++
 drivers/tty/serial/serial_core.c     |  26 ++++++
 include/linux/console.h              |   2 +-
 kernel/printk/printk.c               |   4 +-
 6 files changed, 176 insertions(+), 3 deletions(-)
 create mode 100644 drivers/tty/serial/serial_base_con.c


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c

Comments

Tony Lindgren Sept. 13, 2023, 12:06 p.m. UTC | #1
* Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> [230912 12:24]:
> On Tue, 12 Sep 2023, Tony Lindgren wrote:
> > +struct serial_base_console {
> > +	struct list_head node;
> > +	char *name;
> 
> Can't this be const char as too?

Yes thanks,

Tony
Tony Lindgren Sept. 13, 2023, 12:15 p.m. UTC | #2
* Andy Shevchenko <andriy.shevchenko@intel.com> [230912 15:07]:
> On Tue, Sep 12, 2023 at 02:03:44PM +0300, Tony Lindgren wrote:
> > +static LIST_HEAD(serial_base_consoles);
> 
> Don't you need a locking to access this list?
> If not, perhaps a comment why it's okay?

It's updated at arch_initcall() time only, I'll add a comment.

> > +	port_match = kasprintf(GFP_KERNEL, "%s:%i.%i", dev_name(port->dev),
> > +			       port->ctrl_id, port->port_id);
> 
> What about starting using cleanup.h?

OK seems to simplify things nicely :)

> > +EXPORT_SYMBOL_GPL(serial_base_add_preferred_console);
> 
> Can we use (start using) namespaced exports?

Sorry forgot about the namespace stuff already..

> ...
> 
> > +static int __init serial_base_add_con(char *name, char *opt)
> 
> const name
> const opt
> ?

For name yes, opt has issues as noted in the first patch in this
series.

> > +	opt = strchr(val, ',');
> > +	if (opt) {
> > +		opt[0] = '\0';
> > +		opt++;
> > +	}
> 
> strsep() ?
> 
> Actually param_array() uses strcspn() in similar situation.

OK I'll change to use strcspn().

> > +	if (!strlen(val))
> > +		return 0;
> 
> Btw, have you seen lib/cmdline.c? Can it be helpful here?

I don't think so as at this point we don't have param=value
pairs and param is the port name.

Will fix up the rest of the stuff you commented too thanks.

Regards,

Tony
Tony Lindgren Sept. 13, 2023, 12:30 p.m. UTC | #3
* 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