Re: [PATCH printk v4 09/27] printk: nbcon: Implement processing in port->lock wrapper

[John Ogness <john.ogness@linutronix.de>]

On 2024-04-03, John Ogness <john.ogness@linutronix.de> wrote:
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index d6a58a9e072a..2652b4d5c944 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3146,7 +3146,7 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
>  	uport->state = state;
>  
>  	state->pm_state = UART_PM_STATE_UNDEFINED;
> -	uport->cons = drv->cons;
> +	uart_port_set_cons(uport, drv->cons);
>  	uport->minor = drv->tty_driver->minor_start + uport->line;
>  	uport->name = kasprintf(GFP_KERNEL, "%s%d", drv->dev_name,
>  				drv->tty_driver->name_base + uport->line);

Sebastian Siewior pointed out that the port lock is initialized shortly
after this code. Since uart_port_set_cons() uses the port lock, the
spinlock initialization must come first. The changes for serial_core.c
should be:

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index d6a58a9e072a..0c13ea6a3afa 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3145,8 +3145,15 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 	state->uart_port = uport;
 	uport->state = state;
 
+	/*
+	 * If this port is in use as a console then the spinlock is already
+	 * initialised.
+	 */
+	if (!uart_console_registered(uport))
+		uart_port_spin_lock_init(uport);
+
 	state->pm_state = UART_PM_STATE_UNDEFINED;
-	uport->cons = drv->cons;
+	uart_port_set_cons(uport, drv->cons);
 	uport->minor = drv->tty_driver->minor_start + uport->line;
 	uport->name = kasprintf(GFP_KERNEL, "%s%d", drv->dev_name,
 				drv->tty_driver->name_base + uport->line);
@@ -3155,13 +3162,6 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 		goto out;
 	}
 
-	/*
-	 * If this port is in use as a console then the spinlock is already
-	 * initialised.
-	 */
-	if (!uart_console_registered(uport))
-		uart_port_spin_lock_init(uport);
-
 	if (uport->cons && uport->dev)
 		of_console_check(uport->dev->of_node, uport->cons->name, uport->line);