* [PATCH 0/2] serial: core: fix console port-lock regression @ 2020-09-09 14:30 Johan Hovold 2020-09-09 14:31 ` [PATCH 1/2] serial: core: fix port-lock initialisation Johan Hovold 2020-09-09 14:31 ` [PATCH 2/2] serial: core: fix console port-lock regression Johan Hovold 0 siblings, 2 replies; 10+ messages in thread From: Johan Hovold @ 2020-09-09 14:30 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jiri Slaby, linux-serial, linux-kernel, Andy Shevchenko, Johan Hovold These patches fix up the regressions introduced by the console-detach changes after which the port lock was no longer initialised for not-yet registered consoles during port registration or during early console setup. Some of the reported driver regressions have been addressed by patching individual drivers and later by a work-around for the first issue in core but the early setup issue remained. Johan Johan Hovold (2): serial: core: fix port-lock initialisation serial: core: fix console port-lock regression drivers/tty/serial/serial_core.c | 44 +++++++++++++++----------------- include/linux/serial_core.h | 1 + 2 files changed, 21 insertions(+), 24 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] serial: core: fix port-lock initialisation 2020-09-09 14:30 [PATCH 0/2] serial: core: fix console port-lock regression Johan Hovold @ 2020-09-09 14:31 ` Johan Hovold [not found] ` <20200909152158.GC1891694@smile.fi.intel.com> 2020-09-09 14:31 ` [PATCH 2/2] serial: core: fix console port-lock regression Johan Hovold 1 sibling, 1 reply; 10+ messages in thread From: Johan Hovold @ 2020-09-09 14:31 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jiri Slaby, linux-serial, linux-kernel, Andy Shevchenko, Johan Hovold, stable 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(-) 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); -- 2.26.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20200909152158.GC1891694@smile.fi.intel.com>]
* Re: [PATCH 1/2] serial: core: fix port-lock initialisation [not found] ` <20200909152158.GC1891694@smile.fi.intel.com> @ 2020-09-10 7:08 ` Johan Hovold 0 siblings, 0 replies; 10+ messages in thread From: Johan Hovold @ 2020-09-10 7:08 UTC (permalink / raw) To: Andy Shevchenko Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] serial: core: fix console port-lock regression 2020-09-09 14:30 [PATCH 0/2] serial: core: fix console port-lock regression Johan Hovold 2020-09-09 14:31 ` [PATCH 1/2] serial: core: fix port-lock initialisation Johan Hovold @ 2020-09-09 14:31 ` Johan Hovold [not found] ` <20200909154815.GD1891694@smile.fi.intel.com> 1 sibling, 1 reply; 10+ messages in thread From: Johan Hovold @ 2020-09-09 14:31 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jiri Slaby, linux-serial, linux-kernel, Andy Shevchenko, Johan Hovold, stable Fix the port-lock initialisation regression introduced by commit a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") by making sure that the lock is again initialised during console setup. The console may be registered before the serial controller has been probed in which case the port lock needs to be initialised during console setup by a call to uart_set_options(). The console-detach changes introduced a regression in several drivers by effectively removing that initialisation by not initialising the lock when the port is used as a console (which is always the case during console setup). Add back the early lock initialisation and instead use a new console-reinit flag to handle the case where a console is being re-attached through sysfs. The question whether the console-detach interface should have been added in the first place is left for another discussion. Note that the console-enabled check in uart_set_options() is not redundant because of kgdboc, which can end up reinitialising an already enabled console (see commit 42b6a1baa3ec ("serial_core: Don't re-initialize a previously initialized spinlock.")). 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 | 32 +++++++++++++++----------------- include/linux/serial_core.h | 1 + 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 53b79e1fcbc8..124524ecfe26 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1916,24 +1916,12 @@ static inline bool uart_console_enabled(struct uart_port *port) return uart_console(port) && (port->cons->flags & CON_ENABLED); } -static void __uart_port_spin_lock_init(struct uart_port *port) +static void uart_port_spin_lock_init(struct uart_port *port) { spin_lock_init(&port->lock); lockdep_set_class(&port->lock, &port_lock_key); } -/* - * Ensure that the serial console lock is initialised early. - * If this port is a console, then the spinlock is already initialised. - */ -static inline void uart_port_spin_lock_init(struct uart_port *port) -{ - if (uart_console(port)) - return; - - __uart_port_spin_lock_init(port); -} - #if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(CONFIG_CONSOLE_POLL) /** * uart_console_write - write a console message to a serial port @@ -2086,7 +2074,15 @@ uart_set_options(struct uart_port *port, struct console *co, struct ktermios termios; static struct ktermios dummy; - uart_port_spin_lock_init(port); + /* + * Ensure that the serial-console lock is initialised early. + * + * Note that the console-enabled check is needed because of kgdboc, + * which can end up calling uart_set_options() for an already enabled + * console via tty_find_polling_driver() and uart_poll_init(). + */ + if (!uart_console_enabled(port) && !port->console_reinit) + uart_port_spin_lock_init(port); memset(&termios, 0, sizeof(struct ktermios)); @@ -2794,10 +2790,12 @@ static ssize_t console_store(struct device *dev, if (oldconsole && !newconsole) { ret = unregister_console(uport->cons); } else if (!oldconsole && newconsole) { - if (uart_console(uport)) + if (uart_console(uport)) { + uport->console_reinit = 1; register_console(uport->cons); - else + } else { ret = -ENOENT; + } } } else { ret = -ENXIO; @@ -2898,7 +2896,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) * initialised. */ if (!uart_console_enabled(uport)) - __uart_port_spin_lock_init(uport); + uart_port_spin_lock_init(uport); if (uport->cons && uport->dev) of_console_check(uport->dev->of_node, uport->cons->name, uport->line); diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 01fc4d9c9c54..8a99279a579b 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -248,6 +248,7 @@ struct uart_port { unsigned char hub6; /* this should be in the 8250 driver */ unsigned char suspended; + unsigned char console_reinit; const char *name; /* port name */ struct attribute_group *attr_group; /* port specific attributes */ const struct attribute_group **tty_groups; /* all attributes (serial core use only) */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <20200909154815.GD1891694@smile.fi.intel.com>]
* Re: [PATCH 2/2] serial: core: fix console port-lock regression [not found] ` <20200909154815.GD1891694@smile.fi.intel.com> @ 2020-09-10 7:35 ` Johan Hovold 2020-09-10 9:27 ` Andy Shevchenko 0 siblings, 1 reply; 10+ messages in thread From: Johan Hovold @ 2020-09-10 7:35 UTC (permalink / raw) To: Andy Shevchenko Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable On Wed, Sep 09, 2020 at 06:48:15PM +0300, Andy Shevchenko wrote: > On Wed, Sep 09, 2020 at 04:31:01PM +0200, Johan Hovold wrote: > > Fix the port-lock initialisation regression introduced by commit > > a3cb39d258ef ("serial: core: Allow detach and attach serial device for > > console") by making sure that the lock is again initialised during > > console setup. > > > > The console may be registered before the serial controller has been > > probed in which case the port lock needs to be initialised during > > console setup by a call to uart_set_options(). The console-detach > > changes introduced a regression in several drivers by effectively > > removing that initialisation by not initialising the lock when the port > > is used as a console (which is always the case during console setup). > > > > Add back the early lock initialisation and instead use a new > > console-reinit flag to handle the case where a console is being > > re-attached through sysfs. > > > > The question whether the console-detach interface should have been added > > in the first place is left for another discussion. > > It was discussed in [1]. TL;DR: OMAP would like to keep runtime PM available > for UART while at the same time we disable it for kernel consoles in > bedb404e91bb. > > [1]: https://lists.openwall.net/linux-kernel/2018/09/29/65 Yeah, I remember that. My fear is just that the new interface opens up a can of worms as it removes the earlier assumption that the console would essentially never be deregistered without really fixing all those drivers, and core functions, written under that assumption. Just to mention a few issues; we have drivers enabling clocks and other resources during console setup which can now be done repeatedly, and several drivers whose setup callbacks are marked __init and will oops the minute you reattach the console. And what about power management which was the reason for wanting this on OMAP in the first place; tty core never calls shutdown() for a console port, not even when it's been detached using the new interface. I know, the console setup is all a mess, but this still seems a little rushed to me. I'm even inclined to suggest a revert until the above and similar issues have been addressed properly rather keeping a known buggy interface. > > Note that the console-enabled check in uart_set_options() is not > > redundant because of kgdboc, which can end up reinitialising an already > > enabled console (see commit 42b6a1baa3ec ("serial_core: Don't > > re-initialize a previously initialized spinlock.")). > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Thank you! > > One question below, though. > > > 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 | 32 +++++++++++++++----------------- > > include/linux/serial_core.h | 1 + > > 2 files changed, 16 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > > index 53b79e1fcbc8..124524ecfe26 100644 > > --- a/drivers/tty/serial/serial_core.c > > +++ b/drivers/tty/serial/serial_core.c > > @@ -1916,24 +1916,12 @@ static inline bool uart_console_enabled(struct uart_port *port) > > return uart_console(port) && (port->cons->flags & CON_ENABLED); > > } > > > > -static void __uart_port_spin_lock_init(struct uart_port *port) > > +static void uart_port_spin_lock_init(struct uart_port *port) > > { > > spin_lock_init(&port->lock); > > lockdep_set_class(&port->lock, &port_lock_key); > > } > > > > -/* > > - * Ensure that the serial console lock is initialised early. > > - * If this port is a console, then the spinlock is already initialised. > > - */ > > -static inline void uart_port_spin_lock_init(struct uart_port *port) > > -{ > > > - if (uart_console(port)) > > I'm wondering if we may revert this line to be uart_console_enabled() and use a > helper below twice. I didn't do that on purpose as the rationale for why the uart_console_enabled() check is there is different in the two paths so merging the two comments, and moving it away from the call sites, wasn't really a good idea to begin with. > > - return; > > - > > - __uart_port_spin_lock_init(port); > > -} > > - > > #if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(CONFIG_CONSOLE_POLL) > > /** > > * uart_console_write - write a console message to a serial port > > @@ -2086,7 +2074,15 @@ uart_set_options(struct uart_port *port, struct console *co, > > struct ktermios termios; > > static struct ktermios dummy; > > > > - uart_port_spin_lock_init(port); > > + /* > > + * Ensure that the serial-console lock is initialised early. > > + * > > + * Note that the console-enabled check is needed because of kgdboc, > > + * which can end up calling uart_set_options() for an already enabled > > + * console via tty_find_polling_driver() and uart_poll_init(). > > + */ > > + if (!uart_console_enabled(port) && !port->console_reinit) > > + uart_port_spin_lock_init(port); > > > > memset(&termios, 0, sizeof(struct ktermios)); > > > > @@ -2794,10 +2790,12 @@ static ssize_t console_store(struct device *dev, > > if (oldconsole && !newconsole) { > > ret = unregister_console(uport->cons); > > } else if (!oldconsole && newconsole) { > > - if (uart_console(uport)) > > + if (uart_console(uport)) { > > + uport->console_reinit = 1; > > register_console(uport->cons); > > - else > > + } else { > > ret = -ENOENT; > > + } > > } > > } else { > > ret = -ENXIO; > > @@ -2898,7 +2896,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) > > * initialised. > > */ > > if (!uart_console_enabled(uport)) > > - __uart_port_spin_lock_init(uport); > > + uart_port_spin_lock_init(uport); > > > > if (uport->cons && uport->dev) > > of_console_check(uport->dev->of_node, uport->cons->name, uport->line); > > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h > > index 01fc4d9c9c54..8a99279a579b 100644 > > --- a/include/linux/serial_core.h > > +++ b/include/linux/serial_core.h > > @@ -248,6 +248,7 @@ struct uart_port { > > > > unsigned char hub6; /* this should be in the 8250 driver */ > > unsigned char suspended; > > + unsigned char console_reinit; > > const char *name; /* port name */ > > struct attribute_group *attr_group; /* port specific attributes */ > > const struct attribute_group **tty_groups; /* all attributes (serial core use only) */ > > -- > > 2.26.2 Johan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] serial: core: fix console port-lock regression 2020-09-10 7:35 ` Johan Hovold @ 2020-09-10 9:27 ` Andy Shevchenko 2020-09-10 11:10 ` Johan Hovold 2020-09-14 8:09 ` Tony Lindgren 0 siblings, 2 replies; 10+ messages in thread From: Andy Shevchenko @ 2020-09-10 9:27 UTC (permalink / raw) To: Johan Hovold, Tony Lindgren Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable +Cc: Tony, let me add Tony to the discussion. On Thu, Sep 10, 2020 at 09:35:27AM +0200, Johan Hovold wrote: > On Wed, Sep 09, 2020 at 06:48:15PM +0300, Andy Shevchenko wrote: > > On Wed, Sep 09, 2020 at 04:31:01PM +0200, Johan Hovold wrote: > > > Fix the port-lock initialisation regression introduced by commit > > > a3cb39d258ef ("serial: core: Allow detach and attach serial device for > > > console") by making sure that the lock is again initialised during > > > console setup. > > > > > > The console may be registered before the serial controller has been > > > probed in which case the port lock needs to be initialised during > > > console setup by a call to uart_set_options(). The console-detach > > > changes introduced a regression in several drivers by effectively > > > removing that initialisation by not initialising the lock when the port > > > is used as a console (which is always the case during console setup). > > > > > > Add back the early lock initialisation and instead use a new > > > console-reinit flag to handle the case where a console is being > > > re-attached through sysfs. > > > > > > The question whether the console-detach interface should have been added > > > in the first place is left for another discussion. > > > > It was discussed in [1]. TL;DR: OMAP would like to keep runtime PM available > > for UART while at the same time we disable it for kernel consoles in > > bedb404e91bb. > > > > [1]: https://lists.openwall.net/linux-kernel/2018/09/29/65 > > Yeah, I remember that. My fear is just that the new interface opens up a > can of worms as it removes the earlier assumption that the console would > essentially never be deregistered without really fixing all those > drivers, and core functions, written under that assumption. Just to > mention a few issues; we have drivers enabling clocks and other > resources during console setup which can now be done repeatedly, The series introduced the console ->exit() callback, so it should be easy to fix. > and > several drivers whose setup callbacks are marked __init and will oops > the minute you reattach the console. I believe this can be fixed relatively easy. As a last resort it can be a quirk that disables console detachment for problematic consoles. > And what about power management > which was the reason for wanting this on OMAP in the first place; tty > core never calls shutdown() for a console port, not even when it's been > detached using the new interface. That is interesting... Tony, do we have OMAP case working because of luck? > I know, the console setup is all a mess, but this still seems a little > rushed to me. I'm even inclined to suggest a revert until the above and > similar issues have been addressed properly rather keeping a known buggy > interface. You know that it will be a dead end. Any solution how to move forward? > > > Note that the console-enabled check in uart_set_options() is not > > > redundant because of kgdboc, which can end up reinitialising an already > > > enabled console (see commit 42b6a1baa3ec ("serial_core: Don't > > > re-initialize a previously initialized spinlock.")). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] serial: core: fix console port-lock regression 2020-09-10 9:27 ` Andy Shevchenko @ 2020-09-10 11:10 ` Johan Hovold 2020-09-14 8:09 ` Tony Lindgren 1 sibling, 0 replies; 10+ messages in thread From: Johan Hovold @ 2020-09-10 11:10 UTC (permalink / raw) To: Andy Shevchenko Cc: Johan Hovold, Tony Lindgren, Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable On Thu, Sep 10, 2020 at 12:27:15PM +0300, Andy Shevchenko wrote: > +Cc: Tony, let me add Tony to the discussion. > > On Thu, Sep 10, 2020 at 09:35:27AM +0200, Johan Hovold wrote: > > On Wed, Sep 09, 2020 at 06:48:15PM +0300, Andy Shevchenko wrote: > > > On Wed, Sep 09, 2020 at 04:31:01PM +0200, Johan Hovold wrote: > > > > Fix the port-lock initialisation regression introduced by commit > > > > a3cb39d258ef ("serial: core: Allow detach and attach serial device for > > > > console") by making sure that the lock is again initialised during > > > > console setup. > > > > > > > > The console may be registered before the serial controller has been > > > > probed in which case the port lock needs to be initialised during > > > > console setup by a call to uart_set_options(). The console-detach > > > > changes introduced a regression in several drivers by effectively > > > > removing that initialisation by not initialising the lock when the port > > > > is used as a console (which is always the case during console setup). > > > > > > > > Add back the early lock initialisation and instead use a new > > > > console-reinit flag to handle the case where a console is being > > > > re-attached through sysfs. > > > > > > > > The question whether the console-detach interface should have been added > > > > in the first place is left for another discussion. > > > > > > It was discussed in [1]. TL;DR: OMAP would like to keep runtime PM available > > > for UART while at the same time we disable it for kernel consoles in > > > bedb404e91bb. > > > > > > [1]: https://lists.openwall.net/linux-kernel/2018/09/29/65 > > > > Yeah, I remember that. My fear is just that the new interface opens up a > > can of worms as it removes the earlier assumption that the console would > > essentially never be deregistered without really fixing all those > > drivers, and core functions, written under that assumption. Just to > > mention a few issues; we have drivers enabling clocks and other > > resources during console setup which can now be done repeatedly, > > The series introduced the console ->exit() callback, so it should be > easy to fix. I'm not saying it's necessarily hard, I'm suggesting it should have been considered before merging. But there could still be complications as the console have always been considered a special case that's been hacked around. > > and > > several drivers whose setup callbacks are marked __init and will oops > > the minute you reattach the console. > > I believe this can be fixed relatively easy. As a last resort it can > be a quirk that disables console detachment for problematic consoles. Sure, but just removing the __init without vetting the drivers and testing the new interface may not be much better than letting them oops. > > And what about power management > > which was the reason for wanting this on OMAP in the first place; tty > > core never calls shutdown() for a console port, not even when it's been > > detached using the new interface. > > That is interesting... Tony, do we have OMAP case working because of luck? > > > I know, the console setup is all a mess, but this still seems a little > > rushed to me. I'm even inclined to suggest a revert until the above and > > similar issues have been addressed properly rather keeping a known buggy > > interface. > > You know that it will be a dead end. Any solution how to move forward? I guess that depends on how broken this is. I only gave a few examples of the top of my head of issues that needs to be considered. But if this is to stay then making the feature opt-in by only exposing the attribute for console drivers that implement the new exit() callback or some other serial-driver flag might work. Johan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] serial: core: fix console port-lock regression 2020-09-10 9:27 ` Andy Shevchenko 2020-09-10 11:10 ` Johan Hovold @ 2020-09-14 8:09 ` Tony Lindgren 2020-09-16 12:23 ` Andy Shevchenko 1 sibling, 1 reply; 10+ messages in thread From: Tony Lindgren @ 2020-09-14 8:09 UTC (permalink / raw) To: Andy Shevchenko Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable * Andy Shevchenko <andriy.shevchenko@linux.intel.com> [200910 09:27]: > +Cc: Tony, let me add Tony to the discussion. > > On Thu, Sep 10, 2020 at 09:35:27AM +0200, Johan Hovold wrote: > > And what about power management > > which was the reason for wanting this on OMAP in the first place; tty > > core never calls shutdown() for a console port, not even when it's been > > detached using the new interface. > > That is interesting... Tony, do we have OMAP case working because of luck? 8250_omap won't do anything unless autosuspend_timeout is configured for the uart(s). If configured, then the 8250_omap will idle when console is detached and the PM runtime usage count held by console is decremented, and the configured autosuspend_timeout expires. The console is still kept open by getty, so I don't see why shutdown() would be called for the console port. But maybe I don't follow what you're concerned about, let me know if you want me to check something :) Regards, Tony ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] serial: core: fix console port-lock regression 2020-09-14 8:09 ` Tony Lindgren @ 2020-09-16 12:23 ` Andy Shevchenko 2020-09-18 7:16 ` Tony Lindgren 0 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2020-09-16 12:23 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable On Mon, Sep 14, 2020 at 11:09:16AM +0300, Tony Lindgren wrote: > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> [200910 09:27]: > > +Cc: Tony, let me add Tony to the discussion. > > > > On Thu, Sep 10, 2020 at 09:35:27AM +0200, Johan Hovold wrote: > > > And what about power management > > > which was the reason for wanting this on OMAP in the first place; tty > > > core never calls shutdown() for a console port, not even when it's been > > > detached using the new interface. > > > > That is interesting... Tony, do we have OMAP case working because of luck? > > 8250_omap won't do anything unless autosuspend_timeout is configured for > the uart(s). If configured, then the 8250_omap will idle when console is > detached and the PM runtime usage count held by console is decremented, and > the configured autosuspend_timeout expires. > > The console is still kept open by getty, so I don't see why shutdown() would > be called for the console port. But maybe I don't follow what you're > concerned about, let me know if you want me to check something :) Is it possible to test configuration when you have kernel console enabled but no getty is run on it (perhaps something with ssh enabled access)? Then kernel console should call ->shutdown on detaching, right? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] serial: core: fix console port-lock regression 2020-09-16 12:23 ` Andy Shevchenko @ 2020-09-18 7:16 ` Tony Lindgren 0 siblings, 0 replies; 10+ messages in thread From: Tony Lindgren @ 2020-09-18 7:16 UTC (permalink / raw) To: Andy Shevchenko Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel, stable * Andy Shevchenko <andriy.shevchenko@linux.intel.com> [200916 12:34]: > On Mon, Sep 14, 2020 at 11:09:16AM +0300, Tony Lindgren wrote: > > * Andy Shevchenko <andriy.shevchenko@linux.intel.com> [200910 09:27]: > > > +Cc: Tony, let me add Tony to the discussion. > > > > > > On Thu, Sep 10, 2020 at 09:35:27AM +0200, Johan Hovold wrote: > > > > And what about power management > > > > which was the reason for wanting this on OMAP in the first place; tty > > > > core never calls shutdown() for a console port, not even when it's been > > > > detached using the new interface. > > > > > > That is interesting... Tony, do we have OMAP case working because of luck? > > > > 8250_omap won't do anything unless autosuspend_timeout is configured for > > the uart(s). If configured, then the 8250_omap will idle when console is > > detached and the PM runtime usage count held by console is decremented, and > > the configured autosuspend_timeout expires. > > > > The console is still kept open by getty, so I don't see why shutdown() would > > be called for the console port. But maybe I don't follow what you're > > concerned about, let me know if you want me to check something :) > > Is it possible to test configuration when you have kernel console enabled but > no getty is run on it (perhaps something with ssh enabled access)? > > Then kernel console should call ->shutdown on detaching, right? I gave this a quick try and Johan is right, shutdown is not called on console uart still even if detached and no getty running on that uart. I could not figure out easily where exactly this gets blocked.. So far I did try setting port->console = 0 on detach, then init it again on attach. Regards, Tony ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-18 7:20 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-09 14:30 [PATCH 0/2] serial: core: fix console port-lock regression Johan Hovold 2020-09-09 14:31 ` [PATCH 1/2] serial: core: fix port-lock initialisation Johan Hovold [not found] ` <20200909152158.GC1891694@smile.fi.intel.com> 2020-09-10 7:08 ` Johan Hovold 2020-09-09 14:31 ` [PATCH 2/2] serial: core: fix console port-lock regression Johan Hovold [not found] ` <20200909154815.GD1891694@smile.fi.intel.com> 2020-09-10 7:35 ` Johan Hovold 2020-09-10 9:27 ` Andy Shevchenko 2020-09-10 11:10 ` Johan Hovold 2020-09-14 8:09 ` Tony Lindgren 2020-09-16 12:23 ` Andy Shevchenko 2020-09-18 7:16 ` Tony Lindgren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).