| Message ID | 20200909143101.15389-2-johan@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] serial: core: fix port-lock initialisation | expand |
On Wed, Sep 09, 2020 at 06:21:58PM +0300, Andy Shevchenko wrote: > On Wed, Sep 09, 2020 at 04:31:00PM +0200, Johan Hovold wrote: > > Commit f743061a85f5 ("serial: core: Initialise spin lock before use in > > uart_configure_port()") tried to work around a breakage introduced by > > commit a3cb39d258ef ("serial: core: Allow detach and attach serial > > device for console") by adding a second initialisation of the port lock > > when registering the port. > > > > As reported by the build robots [1], this doesn't really solve the > > regression introduced by the console-detach changes and also adds a > > second redundant initialisation of the lock for normal ports. > > I thought, though doubtfully, it was a regression made by > 679193b7baf8 ("serial: 8250: Let serial core initialise spin lock") > and then I completely forgot about [1]. Yes, that driver has had an explicit early initialisation of the lock and it was indeed the removal of that which triggered the robot's report. With the initialisation again done during console setup (patch 2/2), that should no longer be an issue (at least not for the console port...). > > Start cleaning up this mess by removing the redundant initialisation and > > making sure that the port lock is again initialised once-only for ports > > that aren't already in use as a console. > > Thanks for looking into this! > > I agree this is better place for lock initialization. > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks for reviewing. Johan
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index f797c971cd82..53b79e1fcbc8 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2378,13 +2378,6 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state, /* Power up port for set_mctrl() */ uart_change_pm(state, UART_PM_STATE_ON); - /* - * If this driver supports console, and it hasn't been - * successfully registered yet, initialise spin lock for it. - */ - if (port->cons && !(port->cons->flags & CON_ENABLED)) - __uart_port_spin_lock_init(port); - /* * Ensure that the modem control lines are de-activated. * keep the DTR setting that is set in uart_set_options() @@ -2900,7 +2893,12 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) goto out; } - uart_port_spin_lock_init(uport); + /* + * If this port is in use as a console then the spinlock is already + * initialised. + */ + if (!uart_console_enabled(uport)) + __uart_port_spin_lock_init(uport); if (uport->cons && uport->dev) of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
Commit f743061a85f5 ("serial: core: Initialise spin lock before use in uart_configure_port()") tried to work around a breakage introduced by commit a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") by adding a second initialisation of the port lock when registering the port. As reported by the build robots [1], this doesn't really solve the regression introduced by the console-detach changes and also adds a second redundant initialisation of the lock for normal ports. Start cleaning up this mess by removing the redundant initialisation and making sure that the port lock is again initialised once-only for ports that aren't already in use as a console. [1] https://lore.kernel.org/r/20200802054852.GR23458@shao2-debian Fixes: f743061a85f5 ("serial: core: Initialise spin lock before use in uart_configure_port()") Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") Cc: stable <stable@vger.kernel.org> # 5.7 Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/tty/serial/serial_core.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)