Re: [RFC][PATCH] serial: amba-pl011: Make sure we initialize the port.lock spinlock

From: Andy Shevchenko
Date: Tue Apr 28 2020 - 06:03:38 EST


On Tue, Apr 28, 2020 at 11:54 AM Valentin Schneider
<valentin.schneider@xxxxxxx> 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