+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.
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.")).