1. Patchew
  2. linux
  3. [PATCH v4 0/4] Fixes for console command line ordering
  4. View patch

[PATCH v4 4/4] serial: core: Add serial_base_match_and_update_preferred_console()

Tony Lindgren posted 4 patches 1 year, 5 months ago
[PATCH v4 4/4] serial: core: Add serial_base_match_and_update_preferred_console()
Posted by Tony Lindgren 1 year, 5 months ago 
Let's add serial_base_match_and_update_preferred_console() for consoles
using DEVNAME:0.0 style naming.

The earlier approach to add it caused issues in the kernel command line
ordering as we were calling __add_preferred_console() again for the
deferred consoles.

Signed-off-by: Tony Lindgren <tony.lindgren@linux.intel.com>
---
 drivers/tty/serial/serial_base.h     | 16 ++++++++++++
 drivers/tty/serial/serial_base_bus.c | 37 ++++++++++++++++++++++++++++
 drivers/tty/serial/serial_core.c     |  4 +++
 3 files changed, 57 insertions(+)

diff --git a/drivers/tty/serial/serial_base.h b/drivers/tty/serial/serial_base.h
index b6c38d2edfd4..0d50db5b660b 100644
--- a/drivers/tty/serial/serial_base.h
+++ b/drivers/tty/serial/serial_base.h
@@ -49,3 +49,19 @@ void serial_ctrl_unregister_port(struct uart_driver *drv, struct uart_port *port
 
 int serial_core_register_port(struct uart_driver *drv, struct uart_port *port);
 void serial_core_unregister_port(struct uart_driver *drv, struct uart_port *port);
+
+#ifdef CONFIG_SERIAL_CORE_CONSOLE
+
+int serial_base_match_and_update_preferred_console(struct uart_driver *drv,
+						   struct uart_port *port);
+
+#else
+
+static inline
+int serial_base_match_and_update_preferred_console(struct uart_driver *drv,
+						   struct uart_port *port)
+{
+	return 0;
+}
+
+#endif
diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c
index 4df2a4b10445..d822499ba9d6 100644
--- a/drivers/tty/serial/serial_base_bus.c
+++ b/drivers/tty/serial/serial_base_bus.c
@@ -8,6 +8,7 @@
  * The serial core bus manages the serial core controller instances.
  */
 
+#include <linux/cleanup.h>
 #include <linux/container_of.h>
 #include <linux/device.h>
 #include <linux/idr.h>
@@ -204,6 +205,42 @@ void serial_base_port_device_remove(struct serial_port_device *port_dev)
 	put_device(&port_dev->dev);
 }
 
+#ifdef CONFIG_SERIAL_CORE_CONSOLE
+
+/**
+ * serial_base_match_and_update_preferred_console - Match and update a preferred console
+ * @drv: Serial port device driver
+ * @port: Serial port instance
+ *
+ * Tries to match and update the preferred console for a serial port for
+ * the kernel command line option console=DEVNAME:0.0.
+ *
+ * Cannot be called early for ISA ports, depends on struct device.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int serial_base_match_and_update_preferred_console(struct uart_driver *drv,
+						   struct uart_port *port)
+{
+	const char *port_match __free(kfree) = NULL;
+	int ret;
+
+	port_match = kasprintf(GFP_KERNEL, "%s:%d.%d", dev_name(port->dev),
+			       port->ctrl_id, port->port_id);
+	if (!port_match)
+		return -ENOMEM;
+
+	ret = match_devname_and_update_preferred_console(port_match,
+							 drv->dev_name,
+							 port->line);
+	if (ret == -ENOENT)
+		return 0;
+
+	return ret;
+}
+
+#endif
+
 static int serial_base_init(void)
 {
 	int ret;
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2a8006e3d687..9a18d0b95a41 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3422,6 +3422,10 @@ int serial_core_register_port(struct uart_driver *drv, struct uart_port *port)
 	if (ret)
 		goto err_unregister_ctrl_dev;
 
+	ret = serial_base_match_and_update_preferred_console(drv, port);
+	if (ret)
+		goto err_unregister_port_dev;
+
 	ret = serial_core_add_one_port(drv, port);
 	if (ret)
 		goto err_unregister_port_dev;
-- 
2.45.2
Re: [PATCH v4 4/4] serial: core: Add serial_base_match_and_update_preferred_console()
Posted by Petr Mladek 1 year, 5 months ago 
On Thu 2024-06-20 15:45:29, Tony Lindgren wrote:
> Let's add serial_base_match_and_update_preferred_console() for consoles
> using DEVNAME:0.0 style naming.
> 
> The earlier approach to add it caused issues in the kernel command line
> ordering as we were calling __add_preferred_console() again for the
> deferred consoles.
> 
> Signed-off-by: Tony Lindgren <tony.lindgren@linux.intel.com>

Looks good and seems to work well:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

See an idea below.

> --- a/drivers/tty/serial/serial_base_bus.c
> +++ b/drivers/tty/serial/serial_base_bus.c
> @@ -204,6 +205,42 @@ void serial_base_port_device_remove(struct serial_port_device *port_dev)
>  	put_device(&port_dev->dev);
>  }
>  
> +#ifdef CONFIG_SERIAL_CORE_CONSOLE
> +
> +/**
> + * serial_base_match_and_update_preferred_console - Match and update a preferred console
> + * @drv: Serial port device driver
> + * @port: Serial port instance
> + *
> + * Tries to match and update the preferred console for a serial port for
> + * the kernel command line option console=DEVNAME:0.0.
> + *
> + * Cannot be called early for ISA ports, depends on struct device.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int serial_base_match_and_update_preferred_console(struct uart_driver *drv,
> +						   struct uart_port *port)
> +{
> +	const char *port_match __free(kfree) = NULL;
> +	int ret;
> +
> +	port_match = kasprintf(GFP_KERNEL, "%s:%d.%d", dev_name(port->dev),
> +			       port->ctrl_id, port->port_id);
> +	if (!port_match)
> +		return -ENOMEM;

The name is going to be compared with:

struct console_cmdline
{
[...]
	char	devname[32];			/* DEVNAME:0.0 style device name */

It looks like an overkill to allocate such a small buffer. It would
be perfectly fine to use a buffer on stack.

Well, we would need to define somewhere (likely in include/linux/console.h):

#define CONSOLE_DEVNAME_LEN 32

and then do

	char port_match[CONSOLE_DEVNAME_LEN];
	int len;

	len = snprintf(port_match, ARRAY_SIZE(port_match), "%s:%d.%d",
		       dev_name(port->dev), port->ctrl_id, port->port_id);
	if (len >= ARRAY_SIZE(port_match)) {
		pr_warn("Console devname does not fit into the buffer: "%s:%d.%d\n",
			 dev_name(port->dev), port->ctrl_id, port->port_id);
		return -ENOMEM;
	}

The advantage is that it would warn when there are longer device names.
It would help to catch situations when CONSOLE_DEVNAME_LEN is not big enough.

It might be done in a separate patch.

> +
> +	ret = match_devname_and_update_preferred_console(port_match,
> +							 drv->dev_name,
> +							 port->line);
> +	if (ret == -ENOENT)
> +		return 0;
> +
> +	return ret;
> +}
> +
> +#endif
> +
>  static int serial_base_init(void)
>  {
>  	int ret;

Best Regards,
Petr
Re: [PATCH v4 4/4] serial: core: Add serial_base_match_and_update_preferred_console()
Posted by Tony Lindgren 1 year, 5 months ago 
On Fri, Jun 21, 2024 at 02:37:27PM +0200, Petr Mladek wrote:
> On Thu 2024-06-20 15:45:29, Tony Lindgren wrote:
> > Let's add serial_base_match_and_update_preferred_console() for consoles
> > using DEVNAME:0.0 style naming.
> > 
> > The earlier approach to add it caused issues in the kernel command line
> > ordering as we were calling __add_preferred_console() again for the
> > deferred consoles.
> > 
> > Signed-off-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> 
> Looks good and seems to work well:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>

OK thanks for testing again.

> See an idea below.
>
> > --- a/drivers/tty/serial/serial_base_bus.c
> > +++ b/drivers/tty/serial/serial_base_bus.c
...
> > +int serial_base_match_and_update_preferred_console(struct uart_driver *drv,
> > +						   struct uart_port *port)
> > +{
> > +	const char *port_match __free(kfree) = NULL;
> > +	int ret;
> > +
> > +	port_match = kasprintf(GFP_KERNEL, "%s:%d.%d", dev_name(port->dev),
> > +			       port->ctrl_id, port->port_id);
> > +	if (!port_match)
> > +		return -ENOMEM;
> 
> The name is going to be compared with:
> 
> struct console_cmdline
> {
> [...]
> 	char	devname[32];			/* DEVNAME:0.0 style device name */
> 
> It looks like an overkill to allocate such a small buffer. It would
> be perfectly fine to use a buffer on stack.
> 
> Well, we would need to define somewhere (likely in include/linux/console.h):
> 
> #define CONSOLE_DEVNAME_LEN 32
> 
> and then do
> 
> 	char port_match[CONSOLE_DEVNAME_LEN];
> 	int len;
> 
> 	len = snprintf(port_match, ARRAY_SIZE(port_match), "%s:%d.%d",
> 		       dev_name(port->dev), port->ctrl_id, port->port_id);
> 	if (len >= ARRAY_SIZE(port_match)) {
> 		pr_warn("Console devname does not fit into the buffer: "%s:%d.%d\n",
> 			 dev_name(port->dev), port->ctrl_id, port->port_id);
> 		return -ENOMEM;
> 	}
> 
> The advantage is that it would warn when there are longer device names.
> It would help to catch situations when CONSOLE_DEVNAME_LEN is not big enough.

Good idea.

> It might be done in a separate patch.

Sounds good to me.

Regards,

Tony

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.