[v8,0/4] serial: 8250_dw: Fix ref clock usage

Message

Serge Semin July 14, 2020, 12:48 p.m. UTC

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

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

Link: https://lore.kernel.org/linux-serial/[e-mail]
Changelog v7:
- Wake the device up on the serial port divider update.

Link: https://lore.kernel.org/linux-serial/[e-mail]
Changelog v8:
- Add a new patch:
  "serial: 8250_dw: Pass the same rate to the clk round and set rate methods"

Signed-off-by: Serge Semin <[e-mail]>
Cc: Alexey Malahov <[e-mail]>
Cc: Pavel Parkhomenko <[e-mail]>
Cc: Thomas Bogendoerfer <[e-mail]>
Cc: Andy Shevchenko <[e-mail]>
Cc: Maxime Ripard <[e-mail]>
Cc: Will Deacon <[e-mail]>
Cc: Russell King <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]

Serge Semin (4):
  serial: 8250: Add 8250 port clock update method
  serial: 8250_dw: Simplify the ref clock rate setting procedure
  serial: 8250_dw: Pass the same rate to the clk round and set rate
    methods
  serial: 8250_dw: Fix common clocks usage race condition

 drivers/tty/serial/8250/8250_dw.c   | 120 ++++++++++++++++++++++++----
 drivers/tty/serial/8250/8250_port.c |  40 ++++++++++
 include/linux/serial_8250.h         |   2 +
 3 files changed, 148 insertions(+), 14 deletions(-)