Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 28 Apr 2020 13:03:25 +0300 | Subject | Re: [RFC][PATCH] serial: amba-pl011: Make sure we initialize the port.lock spinlock |
| |
On Tue, Apr 28, 2020 at 11:54 AM Valentin Schneider <valentin.schneider@arm.com> wrote: > > > On 27/04/20 10:02, Andy Shevchenko wrote: > >> I did a tiny bit of git spelunking; I found a commit that changed > >> uart_console_enabled() into uart_console() within > >> uart_port_spin_lock_init(): > >> > >> a3cb39d258ef ("serial: core: Allow detach and attach serial device for console") > >> > >> Reverting just that one change in uart_port_spin_lock_init() seems to go > >> fine on both Juno & HiKey960, but I think that doesn't play well with the > >> rest of the aforementioned commit. I think that this initial (index, line) > >> tuple is to blame, though I've added Andy in Cc just in case. > > > > The above mentioned commit reveals the issue in the code which doesn't > > register console properly. > > > > See what I put in 0f87aa66e8c31 ("serial: sunhv: Initialize lock for > > non-registered console"). > > Thanks for the pointer. I'm still a puzzled as to why it goes fine on one > board and not on another, but at this point I don't have any better > suggestion than the unconditional init.
My patch relied on the behaviour of 8250 [1] and that comment (near to spin lock initialization routine). It seems AMBA UART drivers unconditionally assign consoles ([2], [3]) without registering it properly at console_initcall().
Least invasive fix is what John's patch does, but real fix is to do something like 8250 does.
So, the rule of thumb is simple: if we assign console to the port we must initialize the lock even if we are not registering console. I dunno the history of different behaviours among drivers and what change(s) brought us to the messy spin lock initialization code in them.
[1]: https://elixir.bootlin.com/linux/v5.7-rc3/source/drivers/tty/serial/8250/8250_core.c#L684 [2]: https://elixir.bootlin.com/linux/v5.7-rc3/source/drivers/tty/serial/amba-pl010.c#L691 [3]: https://elixir.bootlin.com/linux/v5.7-rc3/source/drivers/tty/serial/amba-pl011.c#L2496
-- With Best Regards, Andy Shevchenko
| |