mbox series

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

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

Message

Marek Behún Feb. 19, 2022, 3:28 p.m. UTC
Hello Greg,

this is v9 of series adding support for higher baudrates on A37xx UART.

I've fixed the two things discovered in v8 (commit message typo and
order of last two patches).

Marek

Changes since v8:
* fixed typo in commit message of patch 3
* switched patches 5 and 6 as requested by Gregory Clement

Changes since 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
  serial: mvebu-uart: implement support for baudrates higher than 230400
    Bd
  arm64: dts: marvell: armada-37xx: add device node for UART clock and
    use it

 .../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

Stephen Boyd Feb. 24, 2022, 11:02 p.m. UTC | #1
Quoting Marek Behún (2022-02-19 07:28:15)
> 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.
> 
> From now on both the old mechanism (xtal or TBG referenced as parent
> clock in `clocks` phandle) and the new one (UART clock referenced in the
> `clocks` phandle) are supported, to provide full backward compatibility
> with existing DTS files, full backward compatibility with existing boot
> loaders, and to provide new features (runtime clock configuration to
> allow higher baudrates than 230400 Bd). New features are available only
> with new DTS files.
> 
> There was also a discussion about how the UART node and the
> clock-controller node could be wrapped together in a new binding [1, 2].
> As explained there, this is not possible if we want to keep backwards
> compatibility with existing bootloaders, and thus we are doing this by
> putting the UART clock-controller node inside the UART1 node.
> 
> [1] https://lore.kernel.org/linux-serial/20220120000651.in7s6nazif5qjkme@pali/
> [2] https://lore.kernel.org/linux-serial/20220125204006.A6D09C340E0@smtp.kernel.org/
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <kabel@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Pali Rohár Feb. 25, 2022, 7:32 p.m. UTC | #2
On Saturday 19 February 2022 16:28:12 Marek Behún wrote:
> 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
>   serial: mvebu-uart: implement support for baudrates higher than 230400
>     Bd
>   arm64: dts: marvell: armada-37xx: add device node for UART clock and
>     use it
> 
>  .../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

Hello Greg! I think that this patch series has now all reviews.
Could you look at it if you can finally take it? Or is there still
something missing?
Marek Behún Feb. 25, 2022, 7:39 p.m. UTC | #3
On Fri, 25 Feb 2022 20:32:29 +0100
Pali Rohár <pali@kernel.org> wrote:

> On Saturday 19 February 2022 16:28:12 Marek Behún wrote:
> > 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
> >   serial: mvebu-uart: implement support for baudrates higher than 230400
> >     Bd
> >   arm64: dts: marvell: armada-37xx: add device node for UART clock and
> >     use it
> > 
> >  .../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
> 
> Hello Greg! I think that this patch series has now all reviews.
> Could you look at it if you can finally take it? Or is there still
> something missing?

It was taken into tty-testing today 10:29.

Marek