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

Dmitry Rokosov
2024-05-28T17:49:10Z

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
>

--
Thank you,
Dmitry