Re: [PATCH v3 4/4] serial: 8250_dw: Fix common clocks usage race condition

From: Andy Shevchenko
Date: Fri May 15 2020 - 10:10:51 EST


On Thu, May 07, 2020 at 02:31:35AM +0300, Serge Semin wrote:
> The race condition may happen if the UART reference clock is shared with
> some other device (on Baikal-T1 SoC it's another DW UART port). In this
> case if that device changes the clock rate while serial console is using
> it the DW 8250 UART port might not only end up with an invalid uartclk
> value saved, but may also experience a distorted output data since
> baud-clock could have been changed. In order to fix this lets at least
> try to adjust the 8250 port setting like UART clock rate in case if the
> reference clock rate change is discovered. The driver will call the new
> method to update 8250 UART port clock rate settings. It's done by means of
> the clock event notifier registered at the port startup and unregistered
> in the shutdown callback method.

I'm wondering if clock framework itself can provide such a notifier?

> Note 1. In order to avoid deadlocks we had to execute the UART port update
> method in a dedicated deferred work. This is due to (in my opinion
> redundant) the clock update implemented in the dw8250_set_termios()
> method.

So, and how you propose to update the clock when ->set_termios() is called?

> Note 2. Before the ref clock is manually changed by the custom
> set_termios() function we swap the port uartclk value with new rate
> adjusted to be suitable for the requested baud. It is necessary in
> order to effectively disable a functionality of the ref clock events
> handler for the current UART port, since uartclk update will be done
> a bit further in the generic serial8250_do_set_termios() function.

...

> + struct notifier_block clk_notifier;
> + struct work_struct clk_work;

Oh, this seems too much.
Perhaps, the compatible based quirk with your initial approach is much better for time being.

--
With Best Regards,
Andy Shevchenko