mbox series

[v3,0/7] clk: meson: introduce Amlogic A1 SoC Family CPU clock controller driver

Message ID 20240515185103.20256-1-ddrokosov@salutedevices.com (mailing list archive)
Headers show
Series clk: meson: introduce Amlogic A1 SoC Family CPU clock controller driver | expand

Message

Dmitry Rokosov May 15, 2024, 6:47 p.m. UTC
The CPU clock controller plays a general role in the Amlogic A1 SoC
family by generating CPU clocks. As an APB slave module, it offers the
capability to inherit the CPU clock from two sources: the internal fixed
clock known as 'cpu fixed clock' and the external input provided by the
A1 PLL clock controller, referred to as 'syspll'.

It is important for the driver to handle the cpu_clk rate switching
effectively by transitioning to the CPU fixed clock to avoid any
potential execution freezes.

Validation:
* to double-check all clk flags, run the below helper script:

```
pushd /sys/kernel/debug/clk
for f in *; do
    if [[ -f "$f/clk_flags" ]]; then
        flags="$(cat $f/clk_flags | awk '{$1=$1};1' | sed ':a;N;$!ba;s/\n/ | /g')"
        echo -e "$f: $flags"
    fi
done
popd
```

* to trace the current clks state, use the
  '/sys/kernel/debug/clk/clk_dump' node with jq post-processing:

```
$ cat /sys/kernel/debug/clk/clk_dump | jq '.' > clk_dump.json
```

* to see the CPU clock hierarchy, use the
'/sys/kernel/debug/clk/clk_summary' node with jq post-processing:

```
$ cat /sys/kernel/debug/clk/clk_summary | jq '.' > clk_dump.json
```

when cpu_clk is inherited from sys_pll, it should be:

```
syspll_in    1  1  0  24000000    0  0  50000  Y  deviceless                 no_connection_id
  sys_pll    2  2  0  1200000000  0  0  50000  Y  deviceless                 no_connection_id
    cpu_clk  1  1  0  1200000000  0  0  50000  Y  cpu0                       no_connection_id
                                                  cpu0                       no_connection_id
                                                  fd000000.clock-controller  dvfs
                                                  deviceless                 no_connection_id
```

and from cpu fixed clock:

```
fclk_div3_div           1  1  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
  fclk_div3             4  4  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
    cpu_fsource_sel0    1  1  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
      cpu_fsource_div0  1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
        cpu_fsel0       1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
          cpu_fclk      1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
            cpu_clk     1  1  0  128000000  0  0  50000  Y  cpu0                       no_connection_id
                                                            cpu0                       no_connection_id
                                                            fd000000.clock-controller  dvfs
                                                            deviceless                 no_connection_id
```

* to debug cpu clk rate propagation and proper parent switching, compile
  kernel with the following definition:
    $ sed -i "s/undef CLOCK_ALLOW_WRITE_DEBUGFS/define CLOCK_ALLOW_WRITE_DEBUGFS/g" drivers/clk/clk.c
  after that, clk_rate debug node for each clock will be available for
  write operation

Changes v3 since v2 at [2]:
    - rename CLK_MESON_PLL_INIT_ONCE to CLK_MESON_PLL_NOINIT_ENABLED to
      accurately describe the behavior when we don't run the
      initialization sequence for an already enabled PLL
    - provide accurate comment about CLK_MESON_PLL_NOINIT_ENABLED flag
      to meson_clk_pll_init() and A1 sys_pll clock definition
    - tag syspll_in and sys_pll input clocks as optional in the a1-pll
      and a1-peripherals clkc bindings per Conor and Rob suggestion
    - move sys_pll_div16 clock from a1-pll clkc to a1-peripherals clkc
      as Jerome suggested

Changes v2 since v1 at [1]:
    - introduce new 'INIT_ONCE' flag to eliminate init for already
      enabled PLL
    - explain why we need to break ABI for a1-pll driver by adding
      sys_pll connections
    - implement sys_pll init sequence, which is applicable when sys_pll
      is disabled
    - remove CLK_IS_CRITICAL from sys_pll
    - move sys_pll_div16 binding to the end per Rob's suggestion
    - add Rob's RvB
    - remove holes from the beginning of the cpu clock controller regmap
    - move a1-cpu.h registers offsets definition to a1-cpu.c
    - set CLK_SET_RATE_GATE for parallel cpu fixed clock source trees
      per Martin's and Jerome's suggestion
    - redesign clock notifier block from cpu_clk to sys_pll to keep
      cpu_clock working continuously (the same implementation is located
      in the g12a clock driver)

Links:
    [1] https://lore.kernel.org/all/20240329205904.25002-1-ddrokosov@salutedevices.com/
    [2] https://lore.kernel.org/all/20240510090933.19464-1-ddrokosov@salutedevices.com/

Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>

Dmitry Rokosov (7):
  clk: meson: add 'NOINIT_ENABLED' flag to eliminate init for enabled
    PLL
  dt-bindings: clock: meson: a1: pll: introduce new syspll bindings
  clk: meson: a1: pll: support 'syspll' general-purpose PLL for CPU
    clock
  dt-bindings: clock: meson: a1: peripherals: support sys_pll input
  clk: meson: a1: peripherals: support 'sys_pll_div16' clock as GEN
    input
  dt-bindings: clock: meson: add A1 CPU clock controller bindings
  clk: meson: a1: add Amlogic A1 CPU clock controller driver

 .../bindings/clock/amlogic,a1-cpu-clkc.yaml   |  64 ++++
 .../clock/amlogic,a1-peripherals-clkc.yaml    |   9 +-
 .../bindings/clock/amlogic,a1-pll-clkc.yaml   |   9 +-
 drivers/clk/meson/Kconfig                     |  10 +
 drivers/clk/meson/Makefile                    |   1 +
 drivers/clk/meson/a1-cpu.c                    | 331 ++++++++++++++++++
 drivers/clk/meson/a1-peripherals.c            |  18 +-
 drivers/clk/meson/a1-pll.c                    |  72 ++++
 drivers/clk/meson/a1-pll.h                    |   6 +
 drivers/clk/meson/clk-pll.c                   |  40 ++-
 drivers/clk/meson/clk-pll.h                   |   1 +
 .../dt-bindings/clock/amlogic,a1-cpu-clkc.h   |  19 +
 .../clock/amlogic,a1-peripherals-clkc.h       |   1 +
 .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |   1 +
 14 files changed, 560 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
 create mode 100644 drivers/clk/meson/a1-cpu.c
 create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h

Comments

Dmitry Rokosov May 28, 2024, 5:41 p.m. UTC | #1
Hello guys!

Kindly reminder :)

On Wed, May 15, 2024 at 09:47:23PM +0300, Dmitry Rokosov wrote:
> The CPU clock controller plays a general role in the Amlogic A1 SoC
> family by generating CPU clocks. As an APB slave module, it offers the
> capability to inherit the CPU clock from two sources: the internal fixed
> clock known as 'cpu fixed clock' and the external input provided by the
> A1 PLL clock controller, referred to as 'syspll'.
> 
> It is important for the driver to handle the cpu_clk rate switching
> effectively by transitioning to the CPU fixed clock to avoid any
> potential execution freezes.
> 
> Validation:
> * to double-check all clk flags, run the below helper script:
> 
> ```
> pushd /sys/kernel/debug/clk
> for f in *; do
>     if [[ -f "$f/clk_flags" ]]; then
>         flags="$(cat $f/clk_flags | awk '{$1=$1};1' | sed ':a;N;$!ba;s/\n/ | /g')"
>         echo -e "$f: $flags"
>     fi
> done
> popd
> ```
> 
> * to trace the current clks state, use the
>   '/sys/kernel/debug/clk/clk_dump' node with jq post-processing:
> 
> ```
> $ cat /sys/kernel/debug/clk/clk_dump | jq '.' > clk_dump.json
> ```
> 
> * to see the CPU clock hierarchy, use the
> '/sys/kernel/debug/clk/clk_summary' node with jq post-processing:
> 
> ```
> $ cat /sys/kernel/debug/clk/clk_summary | jq '.' > clk_dump.json
> ```
> 
> when cpu_clk is inherited from sys_pll, it should be:
> 
> ```
> syspll_in    1  1  0  24000000    0  0  50000  Y  deviceless                 no_connection_id
>   sys_pll    2  2  0  1200000000  0  0  50000  Y  deviceless                 no_connection_id
>     cpu_clk  1  1  0  1200000000  0  0  50000  Y  cpu0                       no_connection_id
>                                                   cpu0                       no_connection_id
>                                                   fd000000.clock-controller  dvfs
>                                                   deviceless                 no_connection_id
> ```
> 
> and from cpu fixed clock:
> 
> ```
> fclk_div3_div           1  1  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
>   fclk_div3             4  4  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
>     cpu_fsource_sel0    1  1  0  512000000  0  0  50000  Y  deviceless                 no_connection_id
>       cpu_fsource_div0  1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
>         cpu_fsel0       1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
>           cpu_fclk      1  1  0  128000000  0  0  50000  Y  deviceless                 no_connection_id
>             cpu_clk     1  1  0  128000000  0  0  50000  Y  cpu0                       no_connection_id
>                                                             cpu0                       no_connection_id
>                                                             fd000000.clock-controller  dvfs
>                                                             deviceless                 no_connection_id
> ```
> 
> * to debug cpu clk rate propagation and proper parent switching, compile
>   kernel with the following definition:
>     $ sed -i "s/undef CLOCK_ALLOW_WRITE_DEBUGFS/define CLOCK_ALLOW_WRITE_DEBUGFS/g" drivers/clk/clk.c
>   after that, clk_rate debug node for each clock will be available for
>   write operation
> 
> Changes v3 since v2 at [2]:
>     - rename CLK_MESON_PLL_INIT_ONCE to CLK_MESON_PLL_NOINIT_ENABLED to
>       accurately describe the behavior when we don't run the
>       initialization sequence for an already enabled PLL
>     - provide accurate comment about CLK_MESON_PLL_NOINIT_ENABLED flag
>       to meson_clk_pll_init() and A1 sys_pll clock definition
>     - tag syspll_in and sys_pll input clocks as optional in the a1-pll
>       and a1-peripherals clkc bindings per Conor and Rob suggestion
>     - move sys_pll_div16 clock from a1-pll clkc to a1-peripherals clkc
>       as Jerome suggested
> 
> Changes v2 since v1 at [1]:
>     - introduce new 'INIT_ONCE' flag to eliminate init for already
>       enabled PLL
>     - explain why we need to break ABI for a1-pll driver by adding
>       sys_pll connections
>     - implement sys_pll init sequence, which is applicable when sys_pll
>       is disabled
>     - remove CLK_IS_CRITICAL from sys_pll
>     - move sys_pll_div16 binding to the end per Rob's suggestion
>     - add Rob's RvB
>     - remove holes from the beginning of the cpu clock controller regmap
>     - move a1-cpu.h registers offsets definition to a1-cpu.c
>     - set CLK_SET_RATE_GATE for parallel cpu fixed clock source trees
>       per Martin's and Jerome's suggestion
>     - redesign clock notifier block from cpu_clk to sys_pll to keep
>       cpu_clock working continuously (the same implementation is located
>       in the g12a clock driver)
> 
> Links:
>     [1] https://lore.kernel.org/all/20240329205904.25002-1-ddrokosov@salutedevices.com/
>     [2] https://lore.kernel.org/all/20240510090933.19464-1-ddrokosov@salutedevices.com/
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> 
> Dmitry Rokosov (7):
>   clk: meson: add 'NOINIT_ENABLED' flag to eliminate init for enabled
>     PLL
>   dt-bindings: clock: meson: a1: pll: introduce new syspll bindings
>   clk: meson: a1: pll: support 'syspll' general-purpose PLL for CPU
>     clock
>   dt-bindings: clock: meson: a1: peripherals: support sys_pll input
>   clk: meson: a1: peripherals: support 'sys_pll_div16' clock as GEN
>     input
>   dt-bindings: clock: meson: add A1 CPU clock controller bindings
>   clk: meson: a1: add Amlogic A1 CPU clock controller driver
> 
>  .../bindings/clock/amlogic,a1-cpu-clkc.yaml   |  64 ++++
>  .../clock/amlogic,a1-peripherals-clkc.yaml    |   9 +-
>  .../bindings/clock/amlogic,a1-pll-clkc.yaml   |   9 +-
>  drivers/clk/meson/Kconfig                     |  10 +
>  drivers/clk/meson/Makefile                    |   1 +
>  drivers/clk/meson/a1-cpu.c                    | 331 ++++++++++++++++++
>  drivers/clk/meson/a1-peripherals.c            |  18 +-
>  drivers/clk/meson/a1-pll.c                    |  72 ++++
>  drivers/clk/meson/a1-pll.h                    |   6 +
>  drivers/clk/meson/clk-pll.c                   |  40 ++-
>  drivers/clk/meson/clk-pll.h                   |   1 +
>  .../dt-bindings/clock/amlogic,a1-cpu-clkc.h   |  19 +
>  .../clock/amlogic,a1-peripherals-clkc.h       |   1 +
>  .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |   1 +
>  14 files changed, 560 insertions(+), 22 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml
>  create mode 100644 drivers/clk/meson/a1-cpu.c
>  create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h
> 
> -- 
> 2.43.0
>
Jerome Brunet June 10, 2024, 10:13 a.m. UTC | #2
On Wed 15 May 2024 at 21:47, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:

> The CPU clock controller plays a general role in the Amlogic A1 SoC
> family by generating CPU clocks. As an APB slave module, it offers the
> capability to inherit the CPU clock from two sources: the internal fixed
> clock known as 'cpu fixed clock' and the external input provided by the
> A1 PLL clock controller, referred to as 'syspll'.
>
> It is important for the driver to handle the cpu_clk rate switching
> effectively by transitioning to the CPU fixed clock to avoid any
> potential execution freezes.
>

Please group your changes, fixes then bindings then driver.
Jerome Brunet June 10, 2024, 10:32 a.m. UTC | #3
Applied to clk-meson (v6.11/drivers), thanks!

[1/7] clk: meson: add 'NOINIT_ENABLED' flag to eliminate init for enabled PLL
      https://github.com/BayLibre/clk-meson/commit/d4c83ac16c65
[2/7] dt-bindings: clock: meson: a1: pll: introduce new syspll bindings
      https://github.com/BayLibre/clk-meson/commit/96f3b9787363
[4/7] dt-bindings: clock: meson: a1: peripherals: support sys_pll input
      https://github.com/BayLibre/clk-meson/commit/41056416ed53

Best regards,
--
Jerome
Dmitry Rokosov June 10, 2024, 11:21 a.m. UTC | #4
On Mon, Jun 10, 2024 at 12:13:41PM +0200, Jerome Brunet wrote:
> On Wed 15 May 2024 at 21:47, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
> 
> > The CPU clock controller plays a general role in the Amlogic A1 SoC
> > family by generating CPU clocks. As an APB slave module, it offers the
> > capability to inherit the CPU clock from two sources: the internal fixed
> > clock known as 'cpu fixed clock' and the external input provided by the
> > A1 PLL clock controller, referred to as 'syspll'.
> >
> > It is important for the driver to handle the cpu_clk rate switching
> > effectively by transitioning to the CPU fixed clock to avoid any
> > potential execution freezes.
> >
> 
> Please group your changes, fixes then bindings then driver.

Sure, thank you for the suggestion!