Messages in this thread | | | From | Andy Shevchenko <> | Date | Thu, 18 Jun 2020 11:17:47 +0300 | Subject | Re: [PATCH RESEND v6 0/3] serial: 8250_dw: Fix ref clock usage |
| |
On Thu, Jun 18, 2020 at 1:52 AM Serge Semin <Sergey.Semin@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.Semin@baikalelectronics.ru> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> > Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru> > Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> > Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru> > Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru> > Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru> > Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru> > Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Will Deacon <will@kernel.org> > Cc: Russell King <linux@armlinux.org.uk> > Cc: linux-mips@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-serial@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
| |