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@xxxxxxxxxxxxxxx>
Looks good and seems to work well:
Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
Tested-by: Petr Mladek <pmladek@xxxxxxxx>
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
