mbox series

[v8,0/6] serial: mvebu-uart: Support for higher baudrates

Message ID 20220211191238.2142-1-kabel@kernel.org
Headers show
Series serial: mvebu-uart: Support for higher baudrates | expand

Message

Marek Behún Feb. 11, 2022, 7:12 p.m. UTC
Hello Greg, Stephen, Gregory,

at Pali's request I have reviewed, updated and tested his series adding
support for higher baudrates on Marvell Armada A37xx boards.

I have updated commit messages, some comments and indentation at some
places. As per Stephen Boyd's request, commit message of patch 3 now
contains more information about why we need to have UART
clock-controller binding defined in such a way (due to backwards
compatibility).

Marek

Changes in v7:
* fixed lint errors in yaml binding file
* added Reviewed-by tags
* changed commit messages and comments a little
* fixed indentation at some places
* swapped patch 2 and 3 (dt-binding defining new binding should go
  before the driver adding usage of that new binding)

Changes in v6:
* fixed yaml binding file and dts files

Changes in v5:
* fixed yaml binding file

Changes in v4:
* converted armada3700-uart-clock documentation to YAML
* split documentation changes into two commits:
  - first which adds clock documentation
  - second which updates UART documentation

Changes in v3:
v3 is rebased on top of Linus master branch and all already applied patches
were dropped. There are no changes in patches itself since v2.

Pali Rohár (6):
  math64: New DIV_U64_ROUND_CLOSEST helper
  dt-bindings: mvebu-uart: document DT bindings for
    marvell,armada-3700-uart-clock
  serial: mvebu-uart: implement UART clock driver for configuring UART
    base clock
  dt-bindings: mvebu-uart: update information about UART clock
  arm64: dts: marvell: armada-37xx: add device node for UART clock and
    use it
  serial: mvebu-uart: implement support for baudrates higher than 230400
    Bd

 .../clock/marvell,armada-3700-uart-clock.yaml |  59 ++
 .../devicetree/bindings/serial/mvebu-uart.txt |   9 +-
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  14 +-
 drivers/tty/serial/Kconfig                    |   1 +
 drivers/tty/serial/mvebu-uart.c               | 596 +++++++++++++++++-
 include/linux/math64.h                        |  13 +
 6 files changed, 671 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/marvell,armada-3700-uart-clock.yaml

Comments

Marek Behún Feb. 11, 2022, 7:12 p.m. UTC | #1
From: Pali Rohár <pali@kernel.org>

Implement a new device driver for controlling UART clocks on Marvell
Armada 3700 SoC. This device driver is loaded for devices which match
the compatible string "marvell,armada-3700-uart-clock".

There are more pitfalls related to UART clocks:
- both UARTs use same parent clock source (which can be xtal or one of
  the TBG clocks),
- if a TBG clock is used as the parent clock, there are two additional
  divisors that can both be configured to divide the rate by 1, 2, ... 6,
  but these divisors are again shared between the two UART controllers
  on the SOC,
- the configuration of the parent clock source and divisors is done in
  the address space of the first UART controller, UART1. Clocks can be
  gated separately for UART1 and UART2, but this setting also lives in
  the address space of UART1,
- Marvell's Functional Specification for Armada 3720 document has the
  clock gating bits swapped, so the one described to gate UART1 clock
  actually gates UART2 and vice versa,
- each UART has it's own "special divisor", and this uses the parent
  clock described above. These divisors are configure in each UART's
  address space separately.

Thus the driver for UART2 controller needs to have access to UART1
address space, since UART1 address space contains some bits exclusive
for UART2 and also some bits which are shared between UART1 and UART2.

Also, during boot, when early console is active on one of the UARTs,
and we want to switch parent clock from xtal (default) to TBG (to be
more flexible with baudrates), the driver changing UART clocks also
needs to be able to change the "special divisor", so that the baudrate
of earlycon is not changed when swtiching to normal console. Thus the
clock driver also needs to be able to access UART2 register space,
for UART2's "special divisor".

For these reasons, this new UART clock driver does not use
ioremap_resource(), but only ioremap() to prevent resource conflicts
between UART clock driver and UART driver.
===
We need to share only two 32-bit registers between the UART driver and
the UART clock driver:
- UART Clock Control
- UART 2 Baud Rate Divisor
Access to these two registers are protected by one spinlock to prevent
any conflicts. Access is required only during probing, when changing
baudrate or during suspend/resume.

Hardware can be configured to use one of following clocks as UART parent
clock: TBG-A-P, TBG-B-P, TBG-A-S, TBG-B-S, xtal. Not every clock is
usable for higher buadrates. Any subset can be specified in the
device-tree and the driver will choose the best one which also still
supports the mandatory baudrate of 9600 Bd. For smooth boot log output
it is needed to specify clock used by early console, otherwise garbage
would be printed on UART during probe of UART clock driver and
transitioning from early console to normal console.

We are implementing this to be able to configure TBG clock as UART
parent clock, which is required to be able to achieve higher baudrates
than 230400 Bd. We achieve this by referencing this new UART clock
device node in UART's device node. UART clock device driver
automatically chooses the best clock source for UART driver.

Until now, UART's device-tree node needed to reference one of the static
clocks (xtal or one of the TBGs) as parent clock in the `clocks`
phandle - the parent clock which was configured before booting the
kernel. If bootloader changed UART's parent clock, it needed to change
the `clocks` phandle in DTB correspondingly before booting.
Pali Rohár Feb. 19, 2022, 3:26 p.m. UTC | #2
Hello Stephen!

On Friday 11 February 2022 20:12:32 Marek Behún wrote:
> Hello Greg, Stephen, Gregory,
> 
> at Pali's request I have reviewed, updated and tested his series adding
> support for higher baudrates on Marvell Armada A37xx boards.
> 
> I have updated commit messages, some comments and indentation at some
> places. As per Stephen Boyd's request, commit message of patch 3 now
> contains more information about why we need to have UART
> clock-controller binding defined in such a way (due to backwards
> compatibility).

Are updated commit messages better now?

> Marek
> 
> Changes in v7:
> * fixed lint errors in yaml binding file
> * added Reviewed-by tags
> * changed commit messages and comments a little
> * fixed indentation at some places
> * swapped patch 2 and 3 (dt-binding defining new binding should go
>   before the driver adding usage of that new binding)
> 
> Changes in v6:
> * fixed yaml binding file and dts files
> 
> Changes in v5:
> * fixed yaml binding file
> 
> Changes in v4:
> * converted armada3700-uart-clock documentation to YAML
> * split documentation changes into two commits:
>   - first which adds clock documentation
>   - second which updates UART documentation
> 
> Changes in v3:
> v3 is rebased on top of Linus master branch and all already applied patches
> were dropped. There are no changes in patches itself since v2.
> 
> Pali Rohár (6):
>   math64: New DIV_U64_ROUND_CLOSEST helper
>   dt-bindings: mvebu-uart: document DT bindings for
>     marvell,armada-3700-uart-clock
>   serial: mvebu-uart: implement UART clock driver for configuring UART
>     base clock
>   dt-bindings: mvebu-uart: update information about UART clock
>   arm64: dts: marvell: armada-37xx: add device node for UART clock and
>     use it
>   serial: mvebu-uart: implement support for baudrates higher than 230400
>     Bd
> 
>  .../clock/marvell,armada-3700-uart-clock.yaml |  59 ++
>  .../devicetree/bindings/serial/mvebu-uart.txt |   9 +-
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  14 +-
>  drivers/tty/serial/Kconfig                    |   1 +
>  drivers/tty/serial/mvebu-uart.c               | 596 +++++++++++++++++-
>  include/linux/math64.h                        |  13 +
>  6 files changed, 671 insertions(+), 21 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/marvell,armada-3700-uart-clock.yaml
> 
> -- 
> 2.34.1
>