On Thu, Jun 18, 2020 at 1:52 AM Serge Semin
<sergey.se...@baikalelectronics.ru> wrote:
>
> Greg, Jiri. We've missed the last merge window. It would be pity to miss
> the next one. Please review/merge in the series.
>
> Regarding the patchset. It might be dangerous if an UART port reference
> clock rate is suddenly changed. In particular the 8250 port drivers
> (and AFAICS most of the tty drivers using common clock framework clocks)
> rely either on the exclusive reference clock utilization or on the ref
> clock rate being always constant. Needless to say that it turns out not
> true and if some other service suddenly changes the clock rate behind an
> UART port driver back no good can happen. So the port might not only end
> up with an invalid uartclk value saved, but may also experience a
> distorted output/input data since such action will effectively update the
> programmed baud-clock. We discovered such problem on Baikal-T1 SoC where
> two DW 8250 ports have got a shared reference clock. Allwinner SoC is
> equipped with an UART, which clock is derived from the CPU PLL clock
> source, so the CPU frequency change might be propagated down up to the
> serial port reference clock. This patchset provides a way to fix the
> problem to the 8250 serial port controllers and mostly fixes it for the
> DW 8250-compatible UART. I say mostly because due to not having a facility
> to pause/stop and resume/restart on-going transfers we implemented the
> UART clock rate update procedure executed post factum of the actual
> reference clock rate change.
>
> In addition the patchset includes a small optimization patch. It
> simplifies the DW APB UART ref clock rate setting procedure a bit.
>
> This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
> base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
> tag: v5.7-rc4
I'm wondering how this will collaborate with runtime PM.
> Changelog v3:
> - Refactor the original patch to adjust the UART port divisor instead of
> requesting an exclusive ref clock utilization.
>
> Changelog v4:
> - Discard commit b426bf0fb085 ("serial: 8250: Fix max baud limit in generic
> 8250 port") since Greg has already merged it into the tty-next branch.
> - Use EXPORT_SYMBOL_GPL() for the serial8250_update_uartclk() method.
>
> Changelog v5:
> - Refactor dw8250_clk_work_cb() function cheking the clk_get_rate()
> return value for being erroneous and exit if it is.
> - Don't update p->uartclk in the port startup. It will be updated later in
> the same procedure at the set_termios() function being invoked by the
> serial_core anyway.
>
> Changelog v6:
> - Resend
>
> Signed-off-by: Serge Semin <sergey.se...@baikalelectronics.ru>
> Cc: Alexey Malahov <alexey.mala...@baikalelectronics.ru>
> Cc: Maxim Kaurkin <maxim.kaur...@baikalelectronics.ru>
> Cc: Pavel Parkhomenko <pavel.parkhome...@baikalelectronics.ru>
> Cc: Alexey Kolotnikov <alexey.kolotni...@baikalelectronics.ru>
> Cc: Ramil Zaripov <ramil.zari...@baikalelectronics.ru>
> Cc: Ekaterina Skachko <ekaterina.skac...@baikalelectronics.ru>
> Cc: Vadim Vlasov <v.vla...@baikalelectronics.ru>
> Cc: Alexey Kolotnikov <alexey.kolotni...@baikalelectronics.ru>
> Cc: Arnd Bergmann <a...@arndb.de>
> Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> Cc: Maxime Ripard <mrip...@kernel.org>
> Cc: Will Deacon <w...@kernel.org>
> Cc: Russell King <li...@armlinux.org.uk>
> Cc: linux-m...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-ser...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> Serge Semin (3):
> serial: 8250: Add 8250 port clock update method
> serial: 8250_dw: Simplify the ref clock rate setting procedure
> serial: 8250_dw: Fix common clocks usage race condition
>
> drivers/tty/serial/8250/8250_dw.c | 116 +++++++++++++++++++++++++---
> drivers/tty/serial/8250/8250_port.c | 38 +++++++++
> include/linux/serial_8250.h | 2 +
> 3 files changed, 144 insertions(+), 12 deletions(-)
>
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
