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