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.
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