2022-08-05 09:01:47

by Yu Tu

[permalink] [raw]
Subject: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver

Add the S4 PLL clock controller found in the s4 SoC family.

Signed-off-by: Yu Tu <[email protected]>
---
drivers/clk/meson/Kconfig | 12 +
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/s4-pll.c | 891 +++++++++++++++++++++++++++++++++++++
drivers/clk/meson/s4-pll.h | 88 ++++
4 files changed, 992 insertions(+)
create mode 100644 drivers/clk/meson/s4-pll.c
create mode 100644 drivers/clk/meson/s4-pll.h

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index fc002c155bc3..f4244edc7b28 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -115,4 +115,16 @@ config COMMON_CLK_G12A
help
Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
devices, aka g12a. Say Y if you want peripherals to work.
+
+config COMMON_CLK_S4_PLL
+ tristate "S4 SoC PLL clock controllers support"
+ depends on ARM64
+ default y
+ select COMMON_CLK_MESON_MPLL
+ select COMMON_CLK_MESON_PLL
+ select COMMON_CLK_MESON_REGMAP
+ help
+ Support for the pll clock controller on Amlogic S805X2 and S905Y4 devices,
+ aka s4. Amlogic S805X2 and S905Y4 devices include AQ222 and AQ229.
+ Say Y if you want peripherals and CPU frequency scaling to work.
endmenu
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 6eca2a406ee3..376f49cc13f1 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
+obj-$(CONFIG_COMMON_CLK_S4_PLL) += s4-pll.o
diff --git a/drivers/clk/meson/s4-pll.c b/drivers/clk/meson/s4-pll.c
new file mode 100644
index 000000000000..478c78b5ed46
--- /dev/null
+++ b/drivers/clk/meson/s4-pll.c
@@ -0,0 +1,891 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Amlogic Meson-S4 Clock Controller Driver
+ *
+ * Copyright (c) 2021 Amlogic, inc.
+ * Author: Yu Tu <[email protected]>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include "clk-mpll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "s4-pll.h"
+
+static DEFINE_SPINLOCK(meson_clk_lock);
+
+static struct clk_regmap s4_fixed_pll_dco = {
+ .data = &(struct meson_clk_pll_data){
+ .en = {
+ .reg_off = ANACTRL_FIXPLL_CTRL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .m = {
+ .reg_off = ANACTRL_FIXPLL_CTRL0,
+ .shift = 0,
+ .width = 8,
+ },
+ .n = {
+ .reg_off = ANACTRL_FIXPLL_CTRL0,
+ .shift = 10,
+ .width = 5,
+ },
+ .frac = {
+ .reg_off = ANACTRL_FIXPLL_CTRL1,
+ .shift = 0,
+ .width = 17,
+ },
+ .l = {
+ .reg_off = ANACTRL_FIXPLL_CTRL0,
+ .shift = 31,
+ .width = 1,
+ },
+ .rst = {
+ .reg_off = ANACTRL_FIXPLL_CTRL0,
+ .shift = 29,
+ .width = 1,
+ },
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fixed_pll_dco",
+ .ops = &meson_clk_pll_ro_ops,
+ .parent_data = (const struct clk_parent_data []) {
+ { .fw_name = "xtal", }
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fixed_pll = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = ANACTRL_FIXPLL_CTRL0,
+ .shift = 16,
+ .width = 2,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fixed_pll",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fixed_pll_dco.hw
+ },
+ .num_parents = 1,
+ /*
+ * This clock won't ever change at runtime so
+ * CLK_SET_RATE_PARENT is not required
+ */
+ },
+};
+
+static struct clk_fixed_factor s4_fclk_div2_div = {
+ .mult = 1,
+ .div = 2,
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div2_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fclk_div2 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_FIXPLL_CTRL1,
+ .bit_idx = 24,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div2",
+ .ops = &clk_regmap_gate_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fclk_div2_div.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_fixed_factor s4_fclk_div3_div = {
+ .mult = 1,
+ .div = 3,
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div3_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fclk_div3 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_FIXPLL_CTRL1,
+ .bit_idx = 20,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div3",
+ .ops = &clk_regmap_gate_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fclk_div3_div.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_fixed_factor s4_fclk_div4_div = {
+ .mult = 1,
+ .div = 4,
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div4_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fclk_div4 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_FIXPLL_CTRL1,
+ .bit_idx = 21,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div4",
+ .ops = &clk_regmap_gate_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fclk_div4_div.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_fixed_factor s4_fclk_div5_div = {
+ .mult = 1,
+ .div = 5,
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div5_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fclk_div5 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_FIXPLL_CTRL1,
+ .bit_idx = 22,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div5",
+ .ops = &clk_regmap_gate_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fclk_div5_div.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_fixed_factor s4_fclk_div7_div = {
+ .mult = 1,
+ .div = 7,
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div7_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fclk_div7 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_FIXPLL_CTRL1,
+ .bit_idx = 23,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div7",
+ .ops = &clk_regmap_gate_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fclk_div7_div.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_fixed_factor s4_fclk_div2p5_div = {
+ .mult = 2,
+ .div = 5,
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div2p5_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fixed_pll.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_fclk_div2p5 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_FIXPLL_CTRL1,
+ .bit_idx = 25,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "fclk_div2p5",
+ .ops = &clk_regmap_gate_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fclk_div2p5_div.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static const struct pll_mult_range s4_gp0_pll_mult_range = {
+ .min = 125,
+ .max = 250,
+};
+
+/*
+ * Internal gp0 pll emulation configuration parameters
+ */
+static const struct reg_sequence s4_gp0_init_regs[] = {
+ { .reg = ANACTRL_GP0PLL_CTRL1, .def = 0x00000000 },
+ { .reg = ANACTRL_GP0PLL_CTRL2, .def = 0x00000000 },
+ { .reg = ANACTRL_GP0PLL_CTRL3, .def = 0x48681c00 },
+ { .reg = ANACTRL_GP0PLL_CTRL4, .def = 0x88770290 },
+ { .reg = ANACTRL_GP0PLL_CTRL5, .def = 0x39272000 },
+ { .reg = ANACTRL_GP0PLL_CTRL6, .def = 0x56540000 }
+};
+
+static struct clk_regmap s4_gp0_pll_dco = {
+ .data = &(struct meson_clk_pll_data){
+ .en = {
+ .reg_off = ANACTRL_GP0PLL_CTRL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .m = {
+ .reg_off = ANACTRL_GP0PLL_CTRL0,
+ .shift = 0,
+ .width = 8,
+ },
+ .n = {
+ .reg_off = ANACTRL_GP0PLL_CTRL0,
+ .shift = 10,
+ .width = 5,
+ },
+ .frac = {
+ .reg_off = ANACTRL_GP0PLL_CTRL1,
+ .shift = 0,
+ .width = 17,
+ },
+ .l = {
+ .reg_off = ANACTRL_GP0PLL_CTRL0,
+ .shift = 31,
+ .width = 1,
+ },
+ .rst = {
+ .reg_off = ANACTRL_GP0PLL_CTRL0,
+ .shift = 29,
+ .width = 1,
+ },
+ .range = &s4_gp0_pll_mult_range,
+ .init_regs = s4_gp0_init_regs,
+ .init_count = ARRAY_SIZE(s4_gp0_init_regs),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "gp0_pll_dco",
+ .ops = &meson_clk_pll_ops,
+ .parent_data = (const struct clk_parent_data []) {
+ { .fw_name = "xtal", }
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_gp0_pll = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = ANACTRL_GP0PLL_CTRL0,
+ .shift = 16,
+ .width = 3,
+ .flags = (CLK_DIVIDER_POWER_OF_TWO |
+ CLK_DIVIDER_ROUND_CLOSEST),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "gp0_pll",
+ .ops = &clk_regmap_divider_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_gp0_pll_dco.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+/*
+ * Internal hifi pll emulation configuration parameters
+ */
+static const struct reg_sequence s4_hifi_init_regs[] = {
+ { .reg = ANACTRL_HIFIPLL_CTRL1, .def = 0x00010e56 },
+ { .reg = ANACTRL_HIFIPLL_CTRL2, .def = 0x00000000 },
+ { .reg = ANACTRL_HIFIPLL_CTRL3, .def = 0x6a285c00 },
+ { .reg = ANACTRL_HIFIPLL_CTRL4, .def = 0x65771290 },
+ { .reg = ANACTRL_HIFIPLL_CTRL5, .def = 0x39272000 },
+ { .reg = ANACTRL_HIFIPLL_CTRL6, .def = 0x56540000 }
+};
+
+static struct clk_regmap s4_hifi_pll_dco = {
+ .data = &(struct meson_clk_pll_data){
+ .en = {
+ .reg_off = ANACTRL_HIFIPLL_CTRL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .m = {
+ .reg_off = ANACTRL_HIFIPLL_CTRL0,
+ .shift = 0,
+ .width = 8,
+ },
+ .n = {
+ .reg_off = ANACTRL_HIFIPLL_CTRL0,
+ .shift = 10,
+ .width = 5,
+ },
+ .frac = {
+ .reg_off = ANACTRL_HIFIPLL_CTRL1,
+ .shift = 0,
+ .width = 17,
+ },
+ .l = {
+ .reg_off = ANACTRL_HIFIPLL_CTRL0,
+ .shift = 31,
+ .width = 1,
+ },
+ .rst = {
+ .reg_off = ANACTRL_HIFIPLL_CTRL0,
+ .shift = 29,
+ .width = 1,
+ },
+ .range = &s4_gp0_pll_mult_range,
+ .init_regs = s4_hifi_init_regs,
+ .init_count = ARRAY_SIZE(s4_hifi_init_regs),
+ .flags = CLK_MESON_PLL_ROUND_CLOSEST,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "hifi_pll_dco",
+ .ops = &meson_clk_pll_ops,
+ .parent_data = (const struct clk_parent_data []) {
+ { .fw_name = "xtal", }
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_hifi_pll = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = ANACTRL_HIFIPLL_CTRL0,
+ .shift = 16,
+ .width = 2,
+ .flags = (CLK_DIVIDER_POWER_OF_TWO |
+ CLK_DIVIDER_ROUND_CLOSEST),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "hifi_pll",
+ .ops = &clk_regmap_divider_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_hifi_pll_dco.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap s4_hdmi_pll_dco = {
+ .data = &(struct meson_clk_pll_data){
+ .en = {
+ .reg_off = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .m = {
+ .reg_off = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 0,
+ .width = 8,
+ },
+ .n = {
+ .reg_off = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 10,
+ .width = 5,
+ },
+ .frac = {
+ .reg_off = ANACTRL_HDMIPLL_CTRL1,
+ .shift = 0,
+ .width = 17,
+ },
+ .l = {
+ .reg_off = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 31,
+ .width = 1,
+ },
+ .rst = {
+ .reg_off = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 29,
+ .width = 1,
+ },
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "hdmi_pll_dco",
+ .ops = &meson_clk_pll_ro_ops,
+ .parent_data = (const struct clk_parent_data []) {
+ { .fw_name = "xtal", }
+ },
+ .num_parents = 1,
+ /*
+ * Display directly handle hdmi pll registers ATM, we need
+ * NOCACHE to keep our view of the clock as accurate as
+ * possible
+ */
+ .flags = CLK_GET_RATE_NOCACHE,
+ },
+};
+
+static struct clk_regmap s4_hdmi_pll_od = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 16,
+ .width = 4,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "hdmi_pll_od",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_hdmi_pll_dco.hw
+ },
+ .num_parents = 1,
+ /*
+ * Display directly handle hdmi pll registers ATM, we need
+ * NOCACHE to keep our view of the clock as accurate as
+ * possible
+ */
+ .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap s4_hdmi_pll = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = ANACTRL_HDMIPLL_CTRL0,
+ .shift = 20,
+ .width = 2,
+ .flags = CLK_DIVIDER_POWER_OF_TWO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "hdmi_pll",
+ .ops = &clk_regmap_divider_ro_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_hdmi_pll_od.hw
+ },
+ .num_parents = 1,
+ /*
+ * Display directly handle hdmi pll registers ATM, we need
+ * NOCACHE to keep our view of the clock as accurate as
+ * possible
+ */
+ .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_fixed_factor s4_mpll_50m_div = {
+ .mult = 1,
+ .div = 80,
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll_50m_div",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fixed_pll_dco.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_mpll_50m = {
+ .data = &(struct clk_regmap_mux_data){
+ .offset = ANACTRL_FIXPLL_CTRL3,
+ .mask = 0x1,
+ .shift = 5,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll_50m",
+ .ops = &clk_regmap_mux_ro_ops,
+ .parent_data = (const struct clk_parent_data []) {
+ { .fw_name = "xtal", },
+ { .hw = &s4_mpll_50m_div.hw },
+ },
+ .num_parents = 2,
+ },
+};
+
+static struct clk_fixed_factor s4_mpll_prediv = {
+ .mult = 1,
+ .div = 2,
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll_prediv",
+ .ops = &clk_fixed_factor_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_fixed_pll_dco.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static const struct reg_sequence s4_mpll0_init_regs[] = {
+ { .reg = ANACTRL_MPLL_CTRL2, .def = 0x40000033 }
+};
+
+static struct clk_regmap s4_mpll0_div = {
+ .data = &(struct meson_clk_mpll_data){
+ .sdm = {
+ .reg_off = ANACTRL_MPLL_CTRL1,
+ .shift = 0,
+ .width = 14,
+ },
+ .sdm_en = {
+ .reg_off = ANACTRL_MPLL_CTRL1,
+ .shift = 30,
+ .width = 1,
+ },
+ .n2 = {
+ .reg_off = ANACTRL_MPLL_CTRL1,
+ .shift = 20,
+ .width = 9,
+ },
+ .ssen = {
+ .reg_off = ANACTRL_MPLL_CTRL1,
+ .shift = 29,
+ .width = 1,
+ },
+ .lock = &meson_clk_lock,
+ .init_regs = s4_mpll0_init_regs,
+ .init_count = ARRAY_SIZE(s4_mpll0_init_regs),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll0_div",
+ .ops = &meson_clk_mpll_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_mpll_prediv.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_mpll0 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_MPLL_CTRL1,
+ .bit_idx = 31,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll0",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_mpll0_div.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static const struct reg_sequence s4_mpll1_init_regs[] = {
+ { .reg = ANACTRL_MPLL_CTRL4, .def = 0x40000033 }
+};
+
+static struct clk_regmap s4_mpll1_div = {
+ .data = &(struct meson_clk_mpll_data){
+ .sdm = {
+ .reg_off = ANACTRL_MPLL_CTRL3,
+ .shift = 0,
+ .width = 14,
+ },
+ .sdm_en = {
+ .reg_off = ANACTRL_MPLL_CTRL3,
+ .shift = 30,
+ .width = 1,
+ },
+ .n2 = {
+ .reg_off = ANACTRL_MPLL_CTRL3,
+ .shift = 20,
+ .width = 9,
+ },
+ .ssen = {
+ .reg_off = ANACTRL_MPLL_CTRL3,
+ .shift = 29,
+ .width = 1,
+ },
+ .lock = &meson_clk_lock,
+ .init_regs = s4_mpll1_init_regs,
+ .init_count = ARRAY_SIZE(s4_mpll1_init_regs),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll1_div",
+ .ops = &meson_clk_mpll_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_mpll_prediv.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_mpll1 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_MPLL_CTRL3,
+ .bit_idx = 31,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll1",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_mpll1_div.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static const struct reg_sequence s4_mpll2_init_regs[] = {
+ { .reg = ANACTRL_MPLL_CTRL6, .def = 0x40000033 }
+};
+
+static struct clk_regmap s4_mpll2_div = {
+ .data = &(struct meson_clk_mpll_data){
+ .sdm = {
+ .reg_off = ANACTRL_MPLL_CTRL5,
+ .shift = 0,
+ .width = 14,
+ },
+ .sdm_en = {
+ .reg_off = ANACTRL_MPLL_CTRL5,
+ .shift = 30,
+ .width = 1,
+ },
+ .n2 = {
+ .reg_off = ANACTRL_MPLL_CTRL5,
+ .shift = 20,
+ .width = 9,
+ },
+ .ssen = {
+ .reg_off = ANACTRL_MPLL_CTRL5,
+ .shift = 29,
+ .width = 1,
+ },
+ .lock = &meson_clk_lock,
+ .init_regs = s4_mpll2_init_regs,
+ .init_count = ARRAY_SIZE(s4_mpll2_init_regs),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll2_div",
+ .ops = &meson_clk_mpll_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_mpll_prediv.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_mpll2 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_MPLL_CTRL5,
+ .bit_idx = 31,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll2",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_mpll2_div.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static const struct reg_sequence s4_mpll3_init_regs[] = {
+ { .reg = ANACTRL_MPLL_CTRL8, .def = 0x40000033 }
+};
+
+static struct clk_regmap s4_mpll3_div = {
+ .data = &(struct meson_clk_mpll_data){
+ .sdm = {
+ .reg_off = ANACTRL_MPLL_CTRL7,
+ .shift = 0,
+ .width = 14,
+ },
+ .sdm_en = {
+ .reg_off = ANACTRL_MPLL_CTRL7,
+ .shift = 30,
+ .width = 1,
+ },
+ .n2 = {
+ .reg_off = ANACTRL_MPLL_CTRL7,
+ .shift = 20,
+ .width = 9,
+ },
+ .ssen = {
+ .reg_off = ANACTRL_MPLL_CTRL7,
+ .shift = 29,
+ .width = 1,
+ },
+ .lock = &meson_clk_lock,
+ .init_regs = s4_mpll3_init_regs,
+ .init_count = ARRAY_SIZE(s4_mpll3_init_regs),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll3_div",
+ .ops = &meson_clk_mpll_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &s4_mpll_prediv.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap s4_mpll3 = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = ANACTRL_MPLL_CTRL7,
+ .bit_idx = 31,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "mpll3",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) { &s4_mpll3_div.hw },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+/* Array of all clocks provided by this provider */
+static struct clk_hw_onecell_data s4_pll_hw_onecell_data = {
+ .hws = {
+ [CLKID_FIXED_PLL_DCO] = &s4_fixed_pll_dco.hw,
+ [CLKID_FIXED_PLL] = &s4_fixed_pll.hw,
+ [CLKID_FCLK_DIV2_DIV] = &s4_fclk_div2_div.hw,
+ [CLKID_FCLK_DIV2] = &s4_fclk_div2.hw,
+ [CLKID_FCLK_DIV3_DIV] = &s4_fclk_div3_div.hw,
+ [CLKID_FCLK_DIV3] = &s4_fclk_div3.hw,
+ [CLKID_FCLK_DIV4_DIV] = &s4_fclk_div4_div.hw,
+ [CLKID_FCLK_DIV4] = &s4_fclk_div4.hw,
+ [CLKID_FCLK_DIV5_DIV] = &s4_fclk_div5_div.hw,
+ [CLKID_FCLK_DIV5] = &s4_fclk_div5.hw,
+ [CLKID_FCLK_DIV7_DIV] = &s4_fclk_div7_div.hw,
+ [CLKID_FCLK_DIV7] = &s4_fclk_div7.hw,
+ [CLKID_FCLK_DIV2P5_DIV] = &s4_fclk_div2p5_div.hw,
+ [CLKID_FCLK_DIV2P5] = &s4_fclk_div2p5.hw,
+ [CLKID_GP0_PLL_DCO] = &s4_gp0_pll_dco.hw,
+ [CLKID_GP0_PLL] = &s4_gp0_pll.hw,
+ [CLKID_HIFI_PLL_DCO] = &s4_hifi_pll_dco.hw,
+ [CLKID_HIFI_PLL] = &s4_hifi_pll.hw,
+ [CLKID_HDMI_PLL_DCO] = &s4_hdmi_pll_dco.hw,
+ [CLKID_HDMI_PLL_OD] = &s4_hdmi_pll_od.hw,
+ [CLKID_HDMI_PLL] = &s4_hdmi_pll.hw,
+ [CLKID_MPLL_50M_DIV] = &s4_mpll_50m_div.hw,
+ [CLKID_MPLL_50M] = &s4_mpll_50m.hw,
+ [CLKID_MPLL_PREDIV] = &s4_mpll_prediv.hw,
+ [CLKID_MPLL0_DIV] = &s4_mpll0_div.hw,
+ [CLKID_MPLL0] = &s4_mpll0.hw,
+ [CLKID_MPLL1_DIV] = &s4_mpll1_div.hw,
+ [CLKID_MPLL1] = &s4_mpll1.hw,
+ [CLKID_MPLL2_DIV] = &s4_mpll2_div.hw,
+ [CLKID_MPLL2] = &s4_mpll2.hw,
+ [CLKID_MPLL3_DIV] = &s4_mpll3_div.hw,
+ [CLKID_MPLL3] = &s4_mpll3.hw,
+
+ [NR_PLL_CLKS] = NULL
+ },
+ .num = NR_PLL_CLKS,
+};
+
+static struct clk_regmap *const s4_pll_clk_regmaps[] = {
+ &s4_fixed_pll_dco,
+ &s4_fixed_pll,
+ &s4_fclk_div2,
+ &s4_fclk_div3,
+ &s4_fclk_div4,
+ &s4_fclk_div5,
+ &s4_fclk_div7,
+ &s4_fclk_div2p5,
+ &s4_gp0_pll_dco,
+ &s4_gp0_pll,
+ &s4_hifi_pll_dco,
+ &s4_hifi_pll,
+ &s4_hdmi_pll_dco,
+ &s4_hdmi_pll_od,
+ &s4_hdmi_pll,
+ &s4_mpll_50m,
+ &s4_mpll0_div,
+ &s4_mpll0,
+ &s4_mpll1_div,
+ &s4_mpll1,
+ &s4_mpll2_div,
+ &s4_mpll2,
+ &s4_mpll3_div,
+ &s4_mpll3,
+};
+
+static const struct reg_sequence s4_init_regs[] = {
+ { .reg = ANACTRL_MPLL_CTRL0, .def = 0x00000543 },
+};
+
+static struct regmap_config clkc_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
+static int meson_s4_pll_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap;
+ void __iomem *base;
+ int ret, i;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ regmap = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ ret = regmap_multi_reg_write(regmap, s4_init_regs, ARRAY_SIZE(s4_init_regs));
+ if (ret) {
+ dev_err(dev, "Failed to init registers\n");
+ return ret;
+ }
+
+ /* Populate regmap for the regmap backed clocks */
+ for (i = 0; i < ARRAY_SIZE(s4_pll_clk_regmaps); i++)
+ s4_pll_clk_regmaps[i]->map = regmap;
+
+ for (i = 0; i < s4_pll_hw_onecell_data.num; i++) {
+ /* array might be sparse */
+ if (!s4_pll_hw_onecell_data.hws[i])
+ continue;
+
+ ret = devm_clk_hw_register(dev, s4_pll_hw_onecell_data.hws[i]);
+ if (ret) {
+ dev_err(dev, "Clock registration failed\n");
+ return ret;
+ }
+ }
+
+ return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+ &s4_pll_hw_onecell_data);
+}
+
+static const struct of_device_id clkc_match_table[] = {
+ {
+ .compatible = "amlogic,s4-pll-clkc",
+ },
+ {}
+};
+
+static struct platform_driver s4_driver = {
+ .probe = meson_s4_pll_probe,
+ .driver = {
+ .name = "s4-pll-clkc",
+ .of_match_table = clkc_match_table,
+ },
+};
+
+module_platform_driver(s4_driver);
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/s4-pll.h b/drivers/clk/meson/s4-pll.h
new file mode 100644
index 000000000000..41dc6de978c1
--- /dev/null
+++ b/drivers/clk/meson/s4-pll.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2021 Amlogic, inc.
+ * Author: Yu Tu <[email protected]>
+ */
+
+#ifndef __MESON_S4_PLL_H__
+#define __MESON_S4_PLL_H__
+
+/* ANA_CTRL - Registers
+ * REG_BASE: REGISTER_BASE_ADDR = 0xfe008000
+ */
+#define ANACTRL_FIXPLL_CTRL0 (0x0010 << 2)
+#define ANACTRL_FIXPLL_CTRL1 (0x0011 << 2)
+#define ANACTRL_FIXPLL_CTRL2 (0x0012 << 2)
+#define ANACTRL_FIXPLL_CTRL3 (0x0013 << 2)
+#define ANACTRL_FIXPLL_CTRL4 (0x0014 << 2)
+#define ANACTRL_FIXPLL_CTRL5 (0x0015 << 2)
+#define ANACTRL_FIXPLL_CTRL6 (0x0016 << 2)
+#define ANACTRL_FIXPLL_STS (0x0017 << 2)
+#define ANACTRL_GP0PLL_CTRL0 (0x0020 << 2)
+#define ANACTRL_GP0PLL_CTRL1 (0x0021 << 2)
+#define ANACTRL_GP0PLL_CTRL2 (0x0022 << 2)
+#define ANACTRL_GP0PLL_CTRL3 (0x0023 << 2)
+#define ANACTRL_GP0PLL_CTRL4 (0x0024 << 2)
+#define ANACTRL_GP0PLL_CTRL5 (0x0025 << 2)
+#define ANACTRL_GP0PLL_CTRL6 (0x0026 << 2)
+#define ANACTRL_GP0PLL_STS (0x0027 << 2)
+#define ANACTRL_HIFIPLL_CTRL0 (0x0040 << 2)
+#define ANACTRL_HIFIPLL_CTRL1 (0x0041 << 2)
+#define ANACTRL_HIFIPLL_CTRL2 (0x0042 << 2)
+#define ANACTRL_HIFIPLL_CTRL3 (0x0043 << 2)
+#define ANACTRL_HIFIPLL_CTRL4 (0x0044 << 2)
+#define ANACTRL_HIFIPLL_CTRL5 (0x0045 << 2)
+#define ANACTRL_HIFIPLL_CTRL6 (0x0046 << 2)
+#define ANACTRL_HIFIPLL_STS (0x0047 << 2)
+#define ANACTRL_MPLL_CTRL0 (0x0060 << 2)
+#define ANACTRL_MPLL_CTRL1 (0x0061 << 2)
+#define ANACTRL_MPLL_CTRL2 (0x0062 << 2)
+#define ANACTRL_MPLL_CTRL3 (0x0063 << 2)
+#define ANACTRL_MPLL_CTRL4 (0x0064 << 2)
+#define ANACTRL_MPLL_CTRL5 (0x0065 << 2)
+#define ANACTRL_MPLL_CTRL6 (0x0066 << 2)
+#define ANACTRL_MPLL_CTRL7 (0x0067 << 2)
+#define ANACTRL_MPLL_CTRL8 (0x0068 << 2)
+#define ANACTRL_MPLL_STS (0x0069 << 2)
+#define ANACTRL_HDMIPLL_CTRL0 (0x0070 << 2)
+#define ANACTRL_HDMIPLL_CTRL1 (0x0071 << 2)
+#define ANACTRL_HDMIPLL_CTRL2 (0x0072 << 2)
+#define ANACTRL_HDMIPLL_CTRL3 (0x0073 << 2)
+#define ANACTRL_HDMIPLL_CTRL4 (0x0074 << 2)
+#define ANACTRL_HDMIPLL_CTRL5 (0x0075 << 2)
+#define ANACTRL_HDMIPLL_CTRL6 (0x0076 << 2)
+#define ANACTRL_HDMIPLL_STS (0x0077 << 2)
+#define ANACTRL_HDMIPLL_VLOCK (0x0079 << 2)
+
+/*
+ * CLKID index values
+ *
+ * These indices are entirely contrived and do not map onto the hardware.
+ * It has now been decided to expose everything by default in the DT header:
+ * include/dt-bindings/clock/axg-clkc.h. Only the clocks ids we don't want
+ * to expose, such as the internal muxes and dividers of composite clocks,
+ * will remain defined here.
+ */
+#define CLKID_FIXED_PLL_DCO 0
+#define CLKID_FCLK_DIV2_DIV 2
+#define CLKID_FCLK_DIV3_DIV 4
+#define CLKID_FCLK_DIV4_DIV 6
+#define CLKID_FCLK_DIV5_DIV 8
+#define CLKID_FCLK_DIV7_DIV 10
+#define CLKID_FCLK_DIV2P5_DIV 12
+#define CLKID_GP0_PLL_DCO 14
+#define CLKID_HIFI_PLL_DCO 16
+#define CLKID_HDMI_PLL_DCO 18
+#define CLKID_HDMI_PLL_OD 19
+#define CLKID_MPLL_50M_DIV 21
+#define CLKID_MPLL_PREDIV 23
+#define CLKID_MPLL0_DIV 24
+#define CLKID_MPLL1_DIV 26
+#define CLKID_MPLL2_DIV 28
+#define CLKID_MPLL3_DIV 30
+
+#define NR_PLL_CLKS 32
+/* include the CLKIDs that have been made part of the DT binding */
+#include <dt-bindings/clock/amlogic,s4-pll-clkc.h>
+
+#endif /* __MESON_S4_PLL_H__ */
--
2.33.1



2022-08-10 14:18:46

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver


On Fri 05 Aug 2022 at 16:57, Yu Tu <[email protected]> wrote:

> Add the S4 PLL clock controller found in the s4 SoC family.
>
> Signed-off-by: Yu Tu <[email protected]>
> ---
> drivers/clk/meson/Kconfig | 12 +
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/s4-pll.c | 891 +++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/s4-pll.h | 88 ++++
> 4 files changed, 992 insertions(+)
> create mode 100644 drivers/clk/meson/s4-pll.c
> create mode 100644 drivers/clk/meson/s4-pll.h
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index fc002c155bc3..f4244edc7b28 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -115,4 +115,16 @@ config COMMON_CLK_G12A
> help
> Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
> devices, aka g12a. Say Y if you want peripherals to work.
> +
> +config COMMON_CLK_S4_PLL
> + tristate "S4 SoC PLL clock controllers support"
> + depends on ARM64
> + default y
> + select COMMON_CLK_MESON_MPLL
> + select COMMON_CLK_MESON_PLL
> + select COMMON_CLK_MESON_REGMAP
> + help
> + Support for the pll clock controller on Amlogic S805X2 and S905Y4 devices,
> + aka s4. Amlogic S805X2 and S905Y4 devices include AQ222 and AQ229.
> + Say Y if you want peripherals and CPU frequency scaling to work.

Looks like this doc is copy/paste of the peripheral controller.
This probably needs to be updated

> endmenu
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 6eca2a406ee3..376f49cc13f1 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
> +obj-$(CONFIG_COMMON_CLK_S4_PLL) += s4-pll.o
> diff --git a/drivers/clk/meson/s4-pll.c b/drivers/clk/meson/s4-pll.c
> new file mode 100644
> index 000000000000..478c78b5ed46
> --- /dev/null
> +++ b/drivers/clk/meson/s4-pll.c
> @@ -0,0 +1,891 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson-S4 Clock Controller Driver
> + *
> + * Copyright (c) 2021 Amlogic, inc.
> + * Author: Yu Tu <[email protected]>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +#include "clk-mpll.h"
> +#include "clk-pll.h"
> +#include "clk-regmap.h"
> +#include "s4-pll.h"
> +
> +static DEFINE_SPINLOCK(meson_clk_lock);
> +
> +static struct clk_regmap s4_fixed_pll_dco = {
> + .data = &(struct meson_clk_pll_data){
> + .en = {
> + .reg_off = ANACTRL_FIXPLL_CTRL0,
> + .shift = 28,
> + .width = 1,
> + },
> + .m = {
> + .reg_off = ANACTRL_FIXPLL_CTRL0,
> + .shift = 0,
> + .width = 8,
> + },
> + .n = {
> + .reg_off = ANACTRL_FIXPLL_CTRL0,
> + .shift = 10,
> + .width = 5,
> + },
> + .frac = {
> + .reg_off = ANACTRL_FIXPLL_CTRL1,
> + .shift = 0,
> + .width = 17,
> + },
> + .l = {
> + .reg_off = ANACTRL_FIXPLL_CTRL0,
> + .shift = 31,
> + .width = 1,
> + },
> + .rst = {
> + .reg_off = ANACTRL_FIXPLL_CTRL0,
> + .shift = 29,
> + .width = 1,
> + },
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fixed_pll_dco",
> + .ops = &meson_clk_pll_ro_ops,
> + .parent_data = (const struct clk_parent_data []) {
> + { .fw_name = "xtal", }
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fixed_pll = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = ANACTRL_FIXPLL_CTRL0,
> + .shift = 16,
> + .width = 2,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fixed_pll",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fixed_pll_dco.hw
> + },
> + .num_parents = 1,
> + /*
> + * This clock won't ever change at runtime so
> + * CLK_SET_RATE_PARENT is not required
> + */
> + },
> +};
> +
> +static struct clk_fixed_factor s4_fclk_div2_div = {
> + .mult = 1,
> + .div = 2,
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div2_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fclk_div2 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_FIXPLL_CTRL1,
> + .bit_idx = 24,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div2",
> + .ops = &clk_regmap_gate_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fclk_div2_div.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_fclk_div3_div = {
> + .mult = 1,
> + .div = 3,
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div3_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fclk_div3 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_FIXPLL_CTRL1,
> + .bit_idx = 20,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div3",
> + .ops = &clk_regmap_gate_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fclk_div3_div.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_fclk_div4_div = {
> + .mult = 1,
> + .div = 4,
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div4_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fclk_div4 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_FIXPLL_CTRL1,
> + .bit_idx = 21,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div4",
> + .ops = &clk_regmap_gate_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fclk_div4_div.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_fclk_div5_div = {
> + .mult = 1,
> + .div = 5,
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div5_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fclk_div5 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_FIXPLL_CTRL1,
> + .bit_idx = 22,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div5",
> + .ops = &clk_regmap_gate_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fclk_div5_div.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_fclk_div7_div = {
> + .mult = 1,
> + .div = 7,
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div7_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fclk_div7 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_FIXPLL_CTRL1,
> + .bit_idx = 23,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div7",
> + .ops = &clk_regmap_gate_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fclk_div7_div.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_fclk_div2p5_div = {
> + .mult = 2,
> + .div = 5,
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div2p5_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fixed_pll.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_fclk_div2p5 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_FIXPLL_CTRL1,
> + .bit_idx = 25,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "fclk_div2p5",
> + .ops = &clk_regmap_gate_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fclk_div2p5_div.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static const struct pll_mult_range s4_gp0_pll_mult_range = {
> + .min = 125,
> + .max = 250,
> +};
> +
> +/*
> + * Internal gp0 pll emulation configuration parameters
> + */
> +static const struct reg_sequence s4_gp0_init_regs[] = {
> + { .reg = ANACTRL_GP0PLL_CTRL1, .def = 0x00000000 },
> + { .reg = ANACTRL_GP0PLL_CTRL2, .def = 0x00000000 },
> + { .reg = ANACTRL_GP0PLL_CTRL3, .def = 0x48681c00 },
> + { .reg = ANACTRL_GP0PLL_CTRL4, .def = 0x88770290 },
> + { .reg = ANACTRL_GP0PLL_CTRL5, .def = 0x39272000 },
> + { .reg = ANACTRL_GP0PLL_CTRL6, .def = 0x56540000 }
> +};
> +
> +static struct clk_regmap s4_gp0_pll_dco = {
> + .data = &(struct meson_clk_pll_data){
> + .en = {
> + .reg_off = ANACTRL_GP0PLL_CTRL0,
> + .shift = 28,
> + .width = 1,
> + },
> + .m = {
> + .reg_off = ANACTRL_GP0PLL_CTRL0,
> + .shift = 0,
> + .width = 8,
> + },
> + .n = {
> + .reg_off = ANACTRL_GP0PLL_CTRL0,
> + .shift = 10,
> + .width = 5,
> + },
> + .frac = {
> + .reg_off = ANACTRL_GP0PLL_CTRL1,
> + .shift = 0,
> + .width = 17,
> + },
> + .l = {
> + .reg_off = ANACTRL_GP0PLL_CTRL0,
> + .shift = 31,
> + .width = 1,
> + },
> + .rst = {
> + .reg_off = ANACTRL_GP0PLL_CTRL0,
> + .shift = 29,
> + .width = 1,
> + },
> + .range = &s4_gp0_pll_mult_range,
> + .init_regs = s4_gp0_init_regs,
> + .init_count = ARRAY_SIZE(s4_gp0_init_regs),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "gp0_pll_dco",
> + .ops = &meson_clk_pll_ops,
> + .parent_data = (const struct clk_parent_data []) {
> + { .fw_name = "xtal", }
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_gp0_pll = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = ANACTRL_GP0PLL_CTRL0,
> + .shift = 16,
> + .width = 3,
> + .flags = (CLK_DIVIDER_POWER_OF_TWO |
> + CLK_DIVIDER_ROUND_CLOSEST),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "gp0_pll",
> + .ops = &clk_regmap_divider_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_gp0_pll_dco.hw
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +/*
> + * Internal hifi pll emulation configuration parameters
> + */
> +static const struct reg_sequence s4_hifi_init_regs[] = {
> + { .reg = ANACTRL_HIFIPLL_CTRL1, .def = 0x00010e56 },
> + { .reg = ANACTRL_HIFIPLL_CTRL2, .def = 0x00000000 },
> + { .reg = ANACTRL_HIFIPLL_CTRL3, .def = 0x6a285c00 },
> + { .reg = ANACTRL_HIFIPLL_CTRL4, .def = 0x65771290 },
> + { .reg = ANACTRL_HIFIPLL_CTRL5, .def = 0x39272000 },
> + { .reg = ANACTRL_HIFIPLL_CTRL6, .def = 0x56540000 }
> +};
> +
> +static struct clk_regmap s4_hifi_pll_dco = {
> + .data = &(struct meson_clk_pll_data){
> + .en = {
> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
> + .shift = 28,
> + .width = 1,
> + },
> + .m = {
> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
> + .shift = 0,
> + .width = 8,
> + },
> + .n = {
> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
> + .shift = 10,
> + .width = 5,
> + },
> + .frac = {
> + .reg_off = ANACTRL_HIFIPLL_CTRL1,
> + .shift = 0,
> + .width = 17,
> + },
> + .l = {
> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
> + .shift = 31,
> + .width = 1,
> + },
> + .rst = {
> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
> + .shift = 29,
> + .width = 1,
> + },
> + .range = &s4_gp0_pll_mult_range,
> + .init_regs = s4_hifi_init_regs,
> + .init_count = ARRAY_SIZE(s4_hifi_init_regs),
> + .flags = CLK_MESON_PLL_ROUND_CLOSEST,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "hifi_pll_dco",
> + .ops = &meson_clk_pll_ops,
> + .parent_data = (const struct clk_parent_data []) {
> + { .fw_name = "xtal", }
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_hifi_pll = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = ANACTRL_HIFIPLL_CTRL0,
> + .shift = 16,
> + .width = 2,
> + .flags = (CLK_DIVIDER_POWER_OF_TWO |
> + CLK_DIVIDER_ROUND_CLOSEST),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "hifi_pll",
> + .ops = &clk_regmap_divider_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_hifi_pll_dco.hw
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_regmap s4_hdmi_pll_dco = {
> + .data = &(struct meson_clk_pll_data){
> + .en = {
> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 28,
> + .width = 1,
> + },
> + .m = {
> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 0,
> + .width = 8,
> + },
> + .n = {
> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 10,
> + .width = 5,
> + },
> + .frac = {
> + .reg_off = ANACTRL_HDMIPLL_CTRL1,
> + .shift = 0,
> + .width = 17,
> + },
> + .l = {
> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 31,
> + .width = 1,
> + },
> + .rst = {
> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 29,
> + .width = 1,
> + },
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "hdmi_pll_dco",
> + .ops = &meson_clk_pll_ro_ops,
> + .parent_data = (const struct clk_parent_data []) {
> + { .fw_name = "xtal", }
> + },
> + .num_parents = 1,
> + /*
> + * Display directly handle hdmi pll registers ATM, we need
> + * NOCACHE to keep our view of the clock as accurate as
> + * possible
> + */

Is it really ?

Given that HDMI support for the s4 is there yet, the
addresses have changes and the region is no longer a syscon, it is time
for the HDMI driver to get fixed.

> + .flags = CLK_GET_RATE_NOCACHE,
> + },
> +};
> +
> +static struct clk_regmap s4_hdmi_pll_od = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 16,
> + .width = 4,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "hdmi_pll_od",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_hdmi_pll_dco.hw
> + },
> + .num_parents = 1,
> + /*
> + * Display directly handle hdmi pll registers ATM, we need
> + * NOCACHE to keep our view of the clock as accurate as
> + * possible
> + */
> + .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_regmap s4_hdmi_pll = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = ANACTRL_HDMIPLL_CTRL0,
> + .shift = 20,
> + .width = 2,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "hdmi_pll",
> + .ops = &clk_regmap_divider_ro_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_hdmi_pll_od.hw
> + },
> + .num_parents = 1,
> + /*
> + * Display directly handle hdmi pll registers ATM, we need
> + * NOCACHE to keep our view of the clock as accurate as
> + * possible
> + */
> + .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_mpll_50m_div = {
> + .mult = 1,
> + .div = 80,
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll_50m_div",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fixed_pll_dco.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_mpll_50m = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = ANACTRL_FIXPLL_CTRL3,
> + .mask = 0x1,
> + .shift = 5,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll_50m",
> + .ops = &clk_regmap_mux_ro_ops,
> + .parent_data = (const struct clk_parent_data []) {
> + { .fw_name = "xtal", },
> + { .hw = &s4_mpll_50m_div.hw },
> + },
> + .num_parents = 2,
> + },
> +};
> +
> +static struct clk_fixed_factor s4_mpll_prediv = {
> + .mult = 1,
> + .div = 2,
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll_prediv",
> + .ops = &clk_fixed_factor_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_fixed_pll_dco.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static const struct reg_sequence s4_mpll0_init_regs[] = {
> + { .reg = ANACTRL_MPLL_CTRL2, .def = 0x40000033 }
> +};
> +
> +static struct clk_regmap s4_mpll0_div = {
> + .data = &(struct meson_clk_mpll_data){
> + .sdm = {
> + .reg_off = ANACTRL_MPLL_CTRL1,
> + .shift = 0,
> + .width = 14,
> + },
> + .sdm_en = {
> + .reg_off = ANACTRL_MPLL_CTRL1,
> + .shift = 30,
> + .width = 1,
> + },
> + .n2 = {
> + .reg_off = ANACTRL_MPLL_CTRL1,
> + .shift = 20,
> + .width = 9,
> + },
> + .ssen = {
> + .reg_off = ANACTRL_MPLL_CTRL1,
> + .shift = 29,
> + .width = 1,
> + },
> + .lock = &meson_clk_lock,
> + .init_regs = s4_mpll0_init_regs,
> + .init_count = ARRAY_SIZE(s4_mpll0_init_regs),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll0_div",
> + .ops = &meson_clk_mpll_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_mpll_prediv.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_mpll0 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_MPLL_CTRL1,
> + .bit_idx = 31,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll0",
> + .ops = &clk_regmap_gate_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll0_div.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static const struct reg_sequence s4_mpll1_init_regs[] = {
> + { .reg = ANACTRL_MPLL_CTRL4, .def = 0x40000033 }
> +};
> +
> +static struct clk_regmap s4_mpll1_div = {
> + .data = &(struct meson_clk_mpll_data){
> + .sdm = {
> + .reg_off = ANACTRL_MPLL_CTRL3,
> + .shift = 0,
> + .width = 14,
> + },
> + .sdm_en = {
> + .reg_off = ANACTRL_MPLL_CTRL3,
> + .shift = 30,
> + .width = 1,
> + },
> + .n2 = {
> + .reg_off = ANACTRL_MPLL_CTRL3,
> + .shift = 20,
> + .width = 9,
> + },
> + .ssen = {
> + .reg_off = ANACTRL_MPLL_CTRL3,
> + .shift = 29,
> + .width = 1,
> + },
> + .lock = &meson_clk_lock,
> + .init_regs = s4_mpll1_init_regs,
> + .init_count = ARRAY_SIZE(s4_mpll1_init_regs),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll1_div",
> + .ops = &meson_clk_mpll_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_mpll_prediv.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_mpll1 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_MPLL_CTRL3,
> + .bit_idx = 31,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll1",
> + .ops = &clk_regmap_gate_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll1_div.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static const struct reg_sequence s4_mpll2_init_regs[] = {
> + { .reg = ANACTRL_MPLL_CTRL6, .def = 0x40000033 }
> +};
> +
> +static struct clk_regmap s4_mpll2_div = {
> + .data = &(struct meson_clk_mpll_data){
> + .sdm = {
> + .reg_off = ANACTRL_MPLL_CTRL5,
> + .shift = 0,
> + .width = 14,
> + },
> + .sdm_en = {
> + .reg_off = ANACTRL_MPLL_CTRL5,
> + .shift = 30,
> + .width = 1,
> + },
> + .n2 = {
> + .reg_off = ANACTRL_MPLL_CTRL5,
> + .shift = 20,
> + .width = 9,
> + },
> + .ssen = {
> + .reg_off = ANACTRL_MPLL_CTRL5,
> + .shift = 29,
> + .width = 1,
> + },
> + .lock = &meson_clk_lock,
> + .init_regs = s4_mpll2_init_regs,
> + .init_count = ARRAY_SIZE(s4_mpll2_init_regs),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll2_div",
> + .ops = &meson_clk_mpll_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_mpll_prediv.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_mpll2 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_MPLL_CTRL5,
> + .bit_idx = 31,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll2",
> + .ops = &clk_regmap_gate_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll2_div.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static const struct reg_sequence s4_mpll3_init_regs[] = {
> + { .reg = ANACTRL_MPLL_CTRL8, .def = 0x40000033 }
> +};
> +
> +static struct clk_regmap s4_mpll3_div = {
> + .data = &(struct meson_clk_mpll_data){
> + .sdm = {
> + .reg_off = ANACTRL_MPLL_CTRL7,
> + .shift = 0,
> + .width = 14,
> + },
> + .sdm_en = {
> + .reg_off = ANACTRL_MPLL_CTRL7,
> + .shift = 30,
> + .width = 1,
> + },
> + .n2 = {
> + .reg_off = ANACTRL_MPLL_CTRL7,
> + .shift = 20,
> + .width = 9,
> + },
> + .ssen = {
> + .reg_off = ANACTRL_MPLL_CTRL7,
> + .shift = 29,
> + .width = 1,
> + },
> + .lock = &meson_clk_lock,
> + .init_regs = s4_mpll3_init_regs,
> + .init_count = ARRAY_SIZE(s4_mpll3_init_regs),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll3_div",
> + .ops = &meson_clk_mpll_ops,
> + .parent_hws = (const struct clk_hw *[]) {
> + &s4_mpll_prediv.hw
> + },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_regmap s4_mpll3 = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = ANACTRL_MPLL_CTRL7,
> + .bit_idx = 31,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "mpll3",
> + .ops = &clk_regmap_gate_ops,
> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll3_div.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +/* Array of all clocks provided by this provider */
> +static struct clk_hw_onecell_data s4_pll_hw_onecell_data = {
> + .hws = {
> + [CLKID_FIXED_PLL_DCO] = &s4_fixed_pll_dco.hw,
> + [CLKID_FIXED_PLL] = &s4_fixed_pll.hw,
> + [CLKID_FCLK_DIV2_DIV] = &s4_fclk_div2_div.hw,
> + [CLKID_FCLK_DIV2] = &s4_fclk_div2.hw,
> + [CLKID_FCLK_DIV3_DIV] = &s4_fclk_div3_div.hw,
> + [CLKID_FCLK_DIV3] = &s4_fclk_div3.hw,
> + [CLKID_FCLK_DIV4_DIV] = &s4_fclk_div4_div.hw,
> + [CLKID_FCLK_DIV4] = &s4_fclk_div4.hw,
> + [CLKID_FCLK_DIV5_DIV] = &s4_fclk_div5_div.hw,
> + [CLKID_FCLK_DIV5] = &s4_fclk_div5.hw,
> + [CLKID_FCLK_DIV7_DIV] = &s4_fclk_div7_div.hw,
> + [CLKID_FCLK_DIV7] = &s4_fclk_div7.hw,
> + [CLKID_FCLK_DIV2P5_DIV] = &s4_fclk_div2p5_div.hw,
> + [CLKID_FCLK_DIV2P5] = &s4_fclk_div2p5.hw,
> + [CLKID_GP0_PLL_DCO] = &s4_gp0_pll_dco.hw,
> + [CLKID_GP0_PLL] = &s4_gp0_pll.hw,
> + [CLKID_HIFI_PLL_DCO] = &s4_hifi_pll_dco.hw,
> + [CLKID_HIFI_PLL] = &s4_hifi_pll.hw,
> + [CLKID_HDMI_PLL_DCO] = &s4_hdmi_pll_dco.hw,
> + [CLKID_HDMI_PLL_OD] = &s4_hdmi_pll_od.hw,
> + [CLKID_HDMI_PLL] = &s4_hdmi_pll.hw,
> + [CLKID_MPLL_50M_DIV] = &s4_mpll_50m_div.hw,
> + [CLKID_MPLL_50M] = &s4_mpll_50m.hw,
> + [CLKID_MPLL_PREDIV] = &s4_mpll_prediv.hw,
> + [CLKID_MPLL0_DIV] = &s4_mpll0_div.hw,
> + [CLKID_MPLL0] = &s4_mpll0.hw,
> + [CLKID_MPLL1_DIV] = &s4_mpll1_div.hw,
> + [CLKID_MPLL1] = &s4_mpll1.hw,
> + [CLKID_MPLL2_DIV] = &s4_mpll2_div.hw,
> + [CLKID_MPLL2] = &s4_mpll2.hw,
> + [CLKID_MPLL3_DIV] = &s4_mpll3_div.hw,
> + [CLKID_MPLL3] = &s4_mpll3.hw,
> +
> + [NR_PLL_CLKS] = NULL
> + },
> + .num = NR_PLL_CLKS,
> +};
> +
> +static struct clk_regmap *const s4_pll_clk_regmaps[] = {
> + &s4_fixed_pll_dco,
> + &s4_fixed_pll,
> + &s4_fclk_div2,
> + &s4_fclk_div3,
> + &s4_fclk_div4,
> + &s4_fclk_div5,
> + &s4_fclk_div7,
> + &s4_fclk_div2p5,
> + &s4_gp0_pll_dco,
> + &s4_gp0_pll,
> + &s4_hifi_pll_dco,
> + &s4_hifi_pll,
> + &s4_hdmi_pll_dco,
> + &s4_hdmi_pll_od,
> + &s4_hdmi_pll,
> + &s4_mpll_50m,
> + &s4_mpll0_div,
> + &s4_mpll0,
> + &s4_mpll1_div,
> + &s4_mpll1,
> + &s4_mpll2_div,
> + &s4_mpll2,
> + &s4_mpll3_div,
> + &s4_mpll3,
> +};
> +
> +static const struct reg_sequence s4_init_regs[] = {
> + { .reg = ANACTRL_MPLL_CTRL0, .def = 0x00000543 },
> +};
> +
> +static struct regmap_config clkc_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> +static int meson_s4_pll_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + void __iomem *base;
> + int ret, i;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + regmap = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + ret = regmap_multi_reg_write(regmap, s4_init_regs, ARRAY_SIZE(s4_init_regs));
> + if (ret) {
> + dev_err(dev, "Failed to init registers\n");
> + return ret;
> + }
> +
> + /* Populate regmap for the regmap backed clocks */
> + for (i = 0; i < ARRAY_SIZE(s4_pll_clk_regmaps); i++)
> + s4_pll_clk_regmaps[i]->map = regmap;
> +
> + for (i = 0; i < s4_pll_hw_onecell_data.num; i++) {
> + /* array might be sparse */
> + if (!s4_pll_hw_onecell_data.hws[i])
> + continue;
> +
> + ret = devm_clk_hw_register(dev, s4_pll_hw_onecell_data.hws[i]);
> + if (ret) {
> + dev_err(dev, "Clock registration failed\n");
> + return ret;
> + }
> + }
> +
> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> + &s4_pll_hw_onecell_data);
> +}
> +
> +static const struct of_device_id clkc_match_table[] = {
> + {
> + .compatible = "amlogic,s4-pll-clkc",
> + },
> + {}
> +};
> +
> +static struct platform_driver s4_driver = {
> + .probe = meson_s4_pll_probe,
> + .driver = {
> + .name = "s4-pll-clkc",
> + .of_match_table = clkc_match_table,
> + },
> +};
> +
> +module_platform_driver(s4_driver);
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/clk/meson/s4-pll.h b/drivers/clk/meson/s4-pll.h
> new file mode 100644
> index 000000000000..41dc6de978c1
> --- /dev/null
> +++ b/drivers/clk/meson/s4-pll.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2021 Amlogic, inc.
> + * Author: Yu Tu <[email protected]>
> + */
> +
> +#ifndef __MESON_S4_PLL_H__
> +#define __MESON_S4_PLL_H__
> +
> +/* ANA_CTRL - Registers
> + * REG_BASE: REGISTER_BASE_ADDR = 0xfe008000
> + */
> +#define ANACTRL_FIXPLL_CTRL0 (0x0010 << 2)

I already commented on the "<< 2" . Remove them please.

> +#define ANACTRL_FIXPLL_CTRL1 (0x0011 << 2)
> +#define ANACTRL_FIXPLL_CTRL2 (0x0012 << 2)
> +#define ANACTRL_FIXPLL_CTRL3 (0x0013 << 2)
> +#define ANACTRL_FIXPLL_CTRL4 (0x0014 << 2)
> +#define ANACTRL_FIXPLL_CTRL5 (0x0015 << 2)
> +#define ANACTRL_FIXPLL_CTRL6 (0x0016 << 2)
> +#define ANACTRL_FIXPLL_STS (0x0017 << 2)
> +#define ANACTRL_GP0PLL_CTRL0 (0x0020 << 2)
> +#define ANACTRL_GP0PLL_CTRL1 (0x0021 << 2)
> +#define ANACTRL_GP0PLL_CTRL2 (0x0022 << 2)
> +#define ANACTRL_GP0PLL_CTRL3 (0x0023 << 2)
> +#define ANACTRL_GP0PLL_CTRL4 (0x0024 << 2)
> +#define ANACTRL_GP0PLL_CTRL5 (0x0025 << 2)
> +#define ANACTRL_GP0PLL_CTRL6 (0x0026 << 2)
> +#define ANACTRL_GP0PLL_STS (0x0027 << 2)
> +#define ANACTRL_HIFIPLL_CTRL0 (0x0040 << 2)
> +#define ANACTRL_HIFIPLL_CTRL1 (0x0041 << 2)
> +#define ANACTRL_HIFIPLL_CTRL2 (0x0042 << 2)
> +#define ANACTRL_HIFIPLL_CTRL3 (0x0043 << 2)
> +#define ANACTRL_HIFIPLL_CTRL4 (0x0044 << 2)
> +#define ANACTRL_HIFIPLL_CTRL5 (0x0045 << 2)
> +#define ANACTRL_HIFIPLL_CTRL6 (0x0046 << 2)
> +#define ANACTRL_HIFIPLL_STS (0x0047 << 2)
> +#define ANACTRL_MPLL_CTRL0 (0x0060 << 2)
> +#define ANACTRL_MPLL_CTRL1 (0x0061 << 2)
> +#define ANACTRL_MPLL_CTRL2 (0x0062 << 2)
> +#define ANACTRL_MPLL_CTRL3 (0x0063 << 2)
> +#define ANACTRL_MPLL_CTRL4 (0x0064 << 2)
> +#define ANACTRL_MPLL_CTRL5 (0x0065 << 2)
> +#define ANACTRL_MPLL_CTRL6 (0x0066 << 2)
> +#define ANACTRL_MPLL_CTRL7 (0x0067 << 2)
> +#define ANACTRL_MPLL_CTRL8 (0x0068 << 2)
> +#define ANACTRL_MPLL_STS (0x0069 << 2)
> +#define ANACTRL_HDMIPLL_CTRL0 (0x0070 << 2)
> +#define ANACTRL_HDMIPLL_CTRL1 (0x0071 << 2)
> +#define ANACTRL_HDMIPLL_CTRL2 (0x0072 << 2)
> +#define ANACTRL_HDMIPLL_CTRL3 (0x0073 << 2)
> +#define ANACTRL_HDMIPLL_CTRL4 (0x0074 << 2)
> +#define ANACTRL_HDMIPLL_CTRL5 (0x0075 << 2)
> +#define ANACTRL_HDMIPLL_CTRL6 (0x0076 << 2)
> +#define ANACTRL_HDMIPLL_STS (0x0077 << 2)
> +#define ANACTRL_HDMIPLL_VLOCK (0x0079 << 2)
> +
> +/*
> + * CLKID index values
> + *
> + * These indices are entirely contrived and do not map onto the hardware.
> + * It has now been decided to expose everything by default in the DT header:
> + * include/dt-bindings/clock/axg-clkc.h. Only the clocks ids we don't want
> + * to expose, such as the internal muxes and dividers of composite clocks,
> + * will remain defined here.
> + */
> +#define CLKID_FIXED_PLL_DCO 0
> +#define CLKID_FCLK_DIV2_DIV 2
> +#define CLKID_FCLK_DIV3_DIV 4
> +#define CLKID_FCLK_DIV4_DIV 6
> +#define CLKID_FCLK_DIV5_DIV 8
> +#define CLKID_FCLK_DIV7_DIV 10
> +#define CLKID_FCLK_DIV2P5_DIV 12
> +#define CLKID_GP0_PLL_DCO 14
> +#define CLKID_HIFI_PLL_DCO 16
> +#define CLKID_HDMI_PLL_DCO 18
> +#define CLKID_HDMI_PLL_OD 19
> +#define CLKID_MPLL_50M_DIV 21
> +#define CLKID_MPLL_PREDIV 23
> +#define CLKID_MPLL0_DIV 24
> +#define CLKID_MPLL1_DIV 26
> +#define CLKID_MPLL2_DIV 28
> +#define CLKID_MPLL3_DIV 30
> +
> +#define NR_PLL_CLKS 32
> +/* include the CLKIDs that have been made part of the DT binding */
> +#include <dt-bindings/clock/amlogic,s4-pll-clkc.h>
> +
> +#endif /* __MESON_S4_PLL_H__ */

2022-08-15 06:46:04

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver

Hi Jerome,

On 2022/8/10 21:47, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Fri 05 Aug 2022 at 16:57, Yu Tu <[email protected]> wrote:
>
>> Add the S4 PLL clock controller found in the s4 SoC family.
>>
>> Signed-off-by: Yu Tu <[email protected]>
>> ---
>> drivers/clk/meson/Kconfig | 12 +
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/s4-pll.c | 891 +++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/s4-pll.h | 88 ++++
>> 4 files changed, 992 insertions(+)
>> create mode 100644 drivers/clk/meson/s4-pll.c
>> create mode 100644 drivers/clk/meson/s4-pll.h
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index fc002c155bc3..f4244edc7b28 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -115,4 +115,16 @@ config COMMON_CLK_G12A
>> help
>> Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
>> devices, aka g12a. Say Y if you want peripherals to work.
>> +
>> +config COMMON_CLK_S4_PLL
>> + tristate "S4 SoC PLL clock controllers support"
>> + depends on ARM64
>> + default y
>> + select COMMON_CLK_MESON_MPLL
>> + select COMMON_CLK_MESON_PLL
>> + select COMMON_CLK_MESON_REGMAP
>> + help
>> + Support for the pll clock controller on Amlogic S805X2 and S905Y4 devices,
>> + aka s4. Amlogic S805X2 and S905Y4 devices include AQ222 and AQ229.
>> + Say Y if you want peripherals and CPU frequency scaling to work.
>
> Looks like this doc is copy/paste of the peripheral controller.
> This probably needs to be updated
Okay.
>
>> endmenu
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index 6eca2a406ee3..376f49cc13f1 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>> +obj-$(CONFIG_COMMON_CLK_S4_PLL) += s4-pll.o
>> diff --git a/drivers/clk/meson/s4-pll.c b/drivers/clk/meson/s4-pll.c
>> new file mode 100644
>> index 000000000000..478c78b5ed46
>> --- /dev/null
>> +++ b/drivers/clk/meson/s4-pll.c
>> @@ -0,0 +1,891 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Amlogic Meson-S4 Clock Controller Driver
>> + *
>> + * Copyright (c) 2021 Amlogic, inc.
>> + * Author: Yu Tu <[email protected]>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "clk-mpll.h"
>> +#include "clk-pll.h"
>> +#include "clk-regmap.h"
>> +#include "s4-pll.h"
>> +
>> +static DEFINE_SPINLOCK(meson_clk_lock);
>> +
>> +static struct clk_regmap s4_fixed_pll_dco = {
>> + .data = &(struct meson_clk_pll_data){
>> + .en = {
>> + .reg_off = ANACTRL_FIXPLL_CTRL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = ANACTRL_FIXPLL_CTRL0,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .n = {
>> + .reg_off = ANACTRL_FIXPLL_CTRL0,
>> + .shift = 10,
>> + .width = 5,
>> + },
>> + .frac = {
>> + .reg_off = ANACTRL_FIXPLL_CTRL1,
>> + .shift = 0,
>> + .width = 17,
>> + },
>> + .l = {
>> + .reg_off = ANACTRL_FIXPLL_CTRL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = ANACTRL_FIXPLL_CTRL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fixed_pll_dco",
>> + .ops = &meson_clk_pll_ro_ops,
>> + .parent_data = (const struct clk_parent_data []) {
>> + { .fw_name = "xtal", }
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fixed_pll = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = ANACTRL_FIXPLL_CTRL0,
>> + .shift = 16,
>> + .width = 2,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fixed_pll",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fixed_pll_dco.hw
>> + },
>> + .num_parents = 1,
>> + /*
>> + * This clock won't ever change at runtime so
>> + * CLK_SET_RATE_PARENT is not required
>> + */
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_fclk_div2_div = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div2_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fclk_div2 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_FIXPLL_CTRL1,
>> + .bit_idx = 24,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div2",
>> + .ops = &clk_regmap_gate_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fclk_div2_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_fclk_div3_div = {
>> + .mult = 1,
>> + .div = 3,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div3_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fclk_div3 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_FIXPLL_CTRL1,
>> + .bit_idx = 20,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div3",
>> + .ops = &clk_regmap_gate_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fclk_div3_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_fclk_div4_div = {
>> + .mult = 1,
>> + .div = 4,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div4_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fclk_div4 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_FIXPLL_CTRL1,
>> + .bit_idx = 21,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div4",
>> + .ops = &clk_regmap_gate_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fclk_div4_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_fclk_div5_div = {
>> + .mult = 1,
>> + .div = 5,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div5_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fclk_div5 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_FIXPLL_CTRL1,
>> + .bit_idx = 22,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div5",
>> + .ops = &clk_regmap_gate_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fclk_div5_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_fclk_div7_div = {
>> + .mult = 1,
>> + .div = 7,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div7_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fclk_div7 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_FIXPLL_CTRL1,
>> + .bit_idx = 23,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div7",
>> + .ops = &clk_regmap_gate_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fclk_div7_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_fclk_div2p5_div = {
>> + .mult = 2,
>> + .div = 5,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div2p5_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fixed_pll.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_fclk_div2p5 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_FIXPLL_CTRL1,
>> + .bit_idx = 25,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "fclk_div2p5",
>> + .ops = &clk_regmap_gate_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fclk_div2p5_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static const struct pll_mult_range s4_gp0_pll_mult_range = {
>> + .min = 125,
>> + .max = 250,
>> +};
>> +
>> +/*
>> + * Internal gp0 pll emulation configuration parameters
>> + */
>> +static const struct reg_sequence s4_gp0_init_regs[] = {
>> + { .reg = ANACTRL_GP0PLL_CTRL1, .def = 0x00000000 },
>> + { .reg = ANACTRL_GP0PLL_CTRL2, .def = 0x00000000 },
>> + { .reg = ANACTRL_GP0PLL_CTRL3, .def = 0x48681c00 },
>> + { .reg = ANACTRL_GP0PLL_CTRL4, .def = 0x88770290 },
>> + { .reg = ANACTRL_GP0PLL_CTRL5, .def = 0x39272000 },
>> + { .reg = ANACTRL_GP0PLL_CTRL6, .def = 0x56540000 }
>> +};
>> +
>> +static struct clk_regmap s4_gp0_pll_dco = {
>> + .data = &(struct meson_clk_pll_data){
>> + .en = {
>> + .reg_off = ANACTRL_GP0PLL_CTRL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = ANACTRL_GP0PLL_CTRL0,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .n = {
>> + .reg_off = ANACTRL_GP0PLL_CTRL0,
>> + .shift = 10,
>> + .width = 5,
>> + },
>> + .frac = {
>> + .reg_off = ANACTRL_GP0PLL_CTRL1,
>> + .shift = 0,
>> + .width = 17,
>> + },
>> + .l = {
>> + .reg_off = ANACTRL_GP0PLL_CTRL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = ANACTRL_GP0PLL_CTRL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .range = &s4_gp0_pll_mult_range,
>> + .init_regs = s4_gp0_init_regs,
>> + .init_count = ARRAY_SIZE(s4_gp0_init_regs),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "gp0_pll_dco",
>> + .ops = &meson_clk_pll_ops,
>> + .parent_data = (const struct clk_parent_data []) {
>> + { .fw_name = "xtal", }
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_gp0_pll = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = ANACTRL_GP0PLL_CTRL0,
>> + .shift = 16,
>> + .width = 3,
>> + .flags = (CLK_DIVIDER_POWER_OF_TWO |
>> + CLK_DIVIDER_ROUND_CLOSEST),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "gp0_pll",
>> + .ops = &clk_regmap_divider_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_gp0_pll_dco.hw
>> + },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +/*
>> + * Internal hifi pll emulation configuration parameters
>> + */
>> +static const struct reg_sequence s4_hifi_init_regs[] = {
>> + { .reg = ANACTRL_HIFIPLL_CTRL1, .def = 0x00010e56 },
>> + { .reg = ANACTRL_HIFIPLL_CTRL2, .def = 0x00000000 },
>> + { .reg = ANACTRL_HIFIPLL_CTRL3, .def = 0x6a285c00 },
>> + { .reg = ANACTRL_HIFIPLL_CTRL4, .def = 0x65771290 },
>> + { .reg = ANACTRL_HIFIPLL_CTRL5, .def = 0x39272000 },
>> + { .reg = ANACTRL_HIFIPLL_CTRL6, .def = 0x56540000 }
>> +};
>> +
>> +static struct clk_regmap s4_hifi_pll_dco = {
>> + .data = &(struct meson_clk_pll_data){
>> + .en = {
>> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .n = {
>> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
>> + .shift = 10,
>> + .width = 5,
>> + },
>> + .frac = {
>> + .reg_off = ANACTRL_HIFIPLL_CTRL1,
>> + .shift = 0,
>> + .width = 17,
>> + },
>> + .l = {
>> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = ANACTRL_HIFIPLL_CTRL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .range = &s4_gp0_pll_mult_range,
>> + .init_regs = s4_hifi_init_regs,
>> + .init_count = ARRAY_SIZE(s4_hifi_init_regs),
>> + .flags = CLK_MESON_PLL_ROUND_CLOSEST,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "hifi_pll_dco",
>> + .ops = &meson_clk_pll_ops,
>> + .parent_data = (const struct clk_parent_data []) {
>> + { .fw_name = "xtal", }
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_hifi_pll = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = ANACTRL_HIFIPLL_CTRL0,
>> + .shift = 16,
>> + .width = 2,
>> + .flags = (CLK_DIVIDER_POWER_OF_TWO |
>> + CLK_DIVIDER_ROUND_CLOSEST),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "hifi_pll",
>> + .ops = &clk_regmap_divider_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_hifi_pll_dco.hw
>> + },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_hdmi_pll_dco = {
>> + .data = &(struct meson_clk_pll_data){
>> + .en = {
>> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .n = {
>> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 10,
>> + .width = 5,
>> + },
>> + .frac = {
>> + .reg_off = ANACTRL_HDMIPLL_CTRL1,
>> + .shift = 0,
>> + .width = 17,
>> + },
>> + .l = {
>> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "hdmi_pll_dco",
>> + .ops = &meson_clk_pll_ro_ops,
>> + .parent_data = (const struct clk_parent_data []) {
>> + { .fw_name = "xtal", }
>> + },
>> + .num_parents = 1,
>> + /*
>> + * Display directly handle hdmi pll registers ATM, we need
>> + * NOCACHE to keep our view of the clock as accurate as
>> + * possible
>> + */
>
> Is it really ?
>
> Given that HDMI support for the s4 is there yet, the
> addresses have changes and the region is no longer a syscon, it is time
> for the HDMI driver to get fixed.
>
>> + .flags = CLK_GET_RATE_NOCACHE,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_hdmi_pll_od = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 16,
>> + .width = 4,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "hdmi_pll_od",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_hdmi_pll_dco.hw
>> + },
>> + .num_parents = 1,
>> + /*
>> + * Display directly handle hdmi pll registers ATM, we need
>> + * NOCACHE to keep our view of the clock as accurate as
>> + * possible
>> + */
>> + .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_hdmi_pll = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = ANACTRL_HDMIPLL_CTRL0,
>> + .shift = 20,
>> + .width = 2,
>> + .flags = CLK_DIVIDER_POWER_OF_TWO,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "hdmi_pll",
>> + .ops = &clk_regmap_divider_ro_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_hdmi_pll_od.hw
>> + },
>> + .num_parents = 1,
>> + /*
>> + * Display directly handle hdmi pll registers ATM, we need
>> + * NOCACHE to keep our view of the clock as accurate as
>> + * possible
>> + */
>> + .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_mpll_50m_div = {
>> + .mult = 1,
>> + .div = 80,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll_50m_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fixed_pll_dco.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_mpll_50m = {
>> + .data = &(struct clk_regmap_mux_data){
>> + .offset = ANACTRL_FIXPLL_CTRL3,
>> + .mask = 0x1,
>> + .shift = 5,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll_50m",
>> + .ops = &clk_regmap_mux_ro_ops,
>> + .parent_data = (const struct clk_parent_data []) {
>> + { .fw_name = "xtal", },
>> + { .hw = &s4_mpll_50m_div.hw },
>> + },
>> + .num_parents = 2,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor s4_mpll_prediv = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll_prediv",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_fixed_pll_dco.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static const struct reg_sequence s4_mpll0_init_regs[] = {
>> + { .reg = ANACTRL_MPLL_CTRL2, .def = 0x40000033 }
>> +};
>> +
>> +static struct clk_regmap s4_mpll0_div = {
>> + .data = &(struct meson_clk_mpll_data){
>> + .sdm = {
>> + .reg_off = ANACTRL_MPLL_CTRL1,
>> + .shift = 0,
>> + .width = 14,
>> + },
>> + .sdm_en = {
>> + .reg_off = ANACTRL_MPLL_CTRL1,
>> + .shift = 30,
>> + .width = 1,
>> + },
>> + .n2 = {
>> + .reg_off = ANACTRL_MPLL_CTRL1,
>> + .shift = 20,
>> + .width = 9,
>> + },
>> + .ssen = {
>> + .reg_off = ANACTRL_MPLL_CTRL1,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .lock = &meson_clk_lock,
>> + .init_regs = s4_mpll0_init_regs,
>> + .init_count = ARRAY_SIZE(s4_mpll0_init_regs),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll0_div",
>> + .ops = &meson_clk_mpll_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_mpll_prediv.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_mpll0 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_MPLL_CTRL1,
>> + .bit_idx = 31,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll0",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll0_div.hw },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static const struct reg_sequence s4_mpll1_init_regs[] = {
>> + { .reg = ANACTRL_MPLL_CTRL4, .def = 0x40000033 }
>> +};
>> +
>> +static struct clk_regmap s4_mpll1_div = {
>> + .data = &(struct meson_clk_mpll_data){
>> + .sdm = {
>> + .reg_off = ANACTRL_MPLL_CTRL3,
>> + .shift = 0,
>> + .width = 14,
>> + },
>> + .sdm_en = {
>> + .reg_off = ANACTRL_MPLL_CTRL3,
>> + .shift = 30,
>> + .width = 1,
>> + },
>> + .n2 = {
>> + .reg_off = ANACTRL_MPLL_CTRL3,
>> + .shift = 20,
>> + .width = 9,
>> + },
>> + .ssen = {
>> + .reg_off = ANACTRL_MPLL_CTRL3,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .lock = &meson_clk_lock,
>> + .init_regs = s4_mpll1_init_regs,
>> + .init_count = ARRAY_SIZE(s4_mpll1_init_regs),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll1_div",
>> + .ops = &meson_clk_mpll_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_mpll_prediv.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_mpll1 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_MPLL_CTRL3,
>> + .bit_idx = 31,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll1",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll1_div.hw },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static const struct reg_sequence s4_mpll2_init_regs[] = {
>> + { .reg = ANACTRL_MPLL_CTRL6, .def = 0x40000033 }
>> +};
>> +
>> +static struct clk_regmap s4_mpll2_div = {
>> + .data = &(struct meson_clk_mpll_data){
>> + .sdm = {
>> + .reg_off = ANACTRL_MPLL_CTRL5,
>> + .shift = 0,
>> + .width = 14,
>> + },
>> + .sdm_en = {
>> + .reg_off = ANACTRL_MPLL_CTRL5,
>> + .shift = 30,
>> + .width = 1,
>> + },
>> + .n2 = {
>> + .reg_off = ANACTRL_MPLL_CTRL5,
>> + .shift = 20,
>> + .width = 9,
>> + },
>> + .ssen = {
>> + .reg_off = ANACTRL_MPLL_CTRL5,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .lock = &meson_clk_lock,
>> + .init_regs = s4_mpll2_init_regs,
>> + .init_count = ARRAY_SIZE(s4_mpll2_init_regs),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll2_div",
>> + .ops = &meson_clk_mpll_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_mpll_prediv.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_mpll2 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_MPLL_CTRL5,
>> + .bit_idx = 31,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll2",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll2_div.hw },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static const struct reg_sequence s4_mpll3_init_regs[] = {
>> + { .reg = ANACTRL_MPLL_CTRL8, .def = 0x40000033 }
>> +};
>> +
>> +static struct clk_regmap s4_mpll3_div = {
>> + .data = &(struct meson_clk_mpll_data){
>> + .sdm = {
>> + .reg_off = ANACTRL_MPLL_CTRL7,
>> + .shift = 0,
>> + .width = 14,
>> + },
>> + .sdm_en = {
>> + .reg_off = ANACTRL_MPLL_CTRL7,
>> + .shift = 30,
>> + .width = 1,
>> + },
>> + .n2 = {
>> + .reg_off = ANACTRL_MPLL_CTRL7,
>> + .shift = 20,
>> + .width = 9,
>> + },
>> + .ssen = {
>> + .reg_off = ANACTRL_MPLL_CTRL7,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .lock = &meson_clk_lock,
>> + .init_regs = s4_mpll3_init_regs,
>> + .init_count = ARRAY_SIZE(s4_mpll3_init_regs),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll3_div",
>> + .ops = &meson_clk_mpll_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &s4_mpll_prediv.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_regmap s4_mpll3 = {
>> + .data = &(struct clk_regmap_gate_data){
>> + .offset = ANACTRL_MPLL_CTRL7,
>> + .bit_idx = 31,
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "mpll3",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_hws = (const struct clk_hw *[]) { &s4_mpll3_div.hw },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +/* Array of all clocks provided by this provider */
>> +static struct clk_hw_onecell_data s4_pll_hw_onecell_data = {
>> + .hws = {
>> + [CLKID_FIXED_PLL_DCO] = &s4_fixed_pll_dco.hw,
>> + [CLKID_FIXED_PLL] = &s4_fixed_pll.hw,
>> + [CLKID_FCLK_DIV2_DIV] = &s4_fclk_div2_div.hw,
>> + [CLKID_FCLK_DIV2] = &s4_fclk_div2.hw,
>> + [CLKID_FCLK_DIV3_DIV] = &s4_fclk_div3_div.hw,
>> + [CLKID_FCLK_DIV3] = &s4_fclk_div3.hw,
>> + [CLKID_FCLK_DIV4_DIV] = &s4_fclk_div4_div.hw,
>> + [CLKID_FCLK_DIV4] = &s4_fclk_div4.hw,
>> + [CLKID_FCLK_DIV5_DIV] = &s4_fclk_div5_div.hw,
>> + [CLKID_FCLK_DIV5] = &s4_fclk_div5.hw,
>> + [CLKID_FCLK_DIV7_DIV] = &s4_fclk_div7_div.hw,
>> + [CLKID_FCLK_DIV7] = &s4_fclk_div7.hw,
>> + [CLKID_FCLK_DIV2P5_DIV] = &s4_fclk_div2p5_div.hw,
>> + [CLKID_FCLK_DIV2P5] = &s4_fclk_div2p5.hw,
>> + [CLKID_GP0_PLL_DCO] = &s4_gp0_pll_dco.hw,
>> + [CLKID_GP0_PLL] = &s4_gp0_pll.hw,
>> + [CLKID_HIFI_PLL_DCO] = &s4_hifi_pll_dco.hw,
>> + [CLKID_HIFI_PLL] = &s4_hifi_pll.hw,
>> + [CLKID_HDMI_PLL_DCO] = &s4_hdmi_pll_dco.hw,
>> + [CLKID_HDMI_PLL_OD] = &s4_hdmi_pll_od.hw,
>> + [CLKID_HDMI_PLL] = &s4_hdmi_pll.hw,
>> + [CLKID_MPLL_50M_DIV] = &s4_mpll_50m_div.hw,
>> + [CLKID_MPLL_50M] = &s4_mpll_50m.hw,
>> + [CLKID_MPLL_PREDIV] = &s4_mpll_prediv.hw,
>> + [CLKID_MPLL0_DIV] = &s4_mpll0_div.hw,
>> + [CLKID_MPLL0] = &s4_mpll0.hw,
>> + [CLKID_MPLL1_DIV] = &s4_mpll1_div.hw,
>> + [CLKID_MPLL1] = &s4_mpll1.hw,
>> + [CLKID_MPLL2_DIV] = &s4_mpll2_div.hw,
>> + [CLKID_MPLL2] = &s4_mpll2.hw,
>> + [CLKID_MPLL3_DIV] = &s4_mpll3_div.hw,
>> + [CLKID_MPLL3] = &s4_mpll3.hw,
>> +
>> + [NR_PLL_CLKS] = NULL
>> + },
>> + .num = NR_PLL_CLKS,
>> +};
>> +
>> +static struct clk_regmap *const s4_pll_clk_regmaps[] = {
>> + &s4_fixed_pll_dco,
>> + &s4_fixed_pll,
>> + &s4_fclk_div2,
>> + &s4_fclk_div3,
>> + &s4_fclk_div4,
>> + &s4_fclk_div5,
>> + &s4_fclk_div7,
>> + &s4_fclk_div2p5,
>> + &s4_gp0_pll_dco,
>> + &s4_gp0_pll,
>> + &s4_hifi_pll_dco,
>> + &s4_hifi_pll,
>> + &s4_hdmi_pll_dco,
>> + &s4_hdmi_pll_od,
>> + &s4_hdmi_pll,
>> + &s4_mpll_50m,
>> + &s4_mpll0_div,
>> + &s4_mpll0,
>> + &s4_mpll1_div,
>> + &s4_mpll1,
>> + &s4_mpll2_div,
>> + &s4_mpll2,
>> + &s4_mpll3_div,
>> + &s4_mpll3,
>> +};
>> +
>> +static const struct reg_sequence s4_init_regs[] = {
>> + { .reg = ANACTRL_MPLL_CTRL0, .def = 0x00000543 },
>> +};
>> +
>> +static struct regmap_config clkc_regmap_config = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> +};
>> +
>> +static int meson_s4_pll_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct regmap *regmap;
>> + void __iomem *base;
>> + int ret, i;
>> +
>> + base = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + regmap = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + ret = regmap_multi_reg_write(regmap, s4_init_regs, ARRAY_SIZE(s4_init_regs));
>> + if (ret) {
>> + dev_err(dev, "Failed to init registers\n");
>> + return ret;
>> + }
>> +
>> + /* Populate regmap for the regmap backed clocks */
>> + for (i = 0; i < ARRAY_SIZE(s4_pll_clk_regmaps); i++)
>> + s4_pll_clk_regmaps[i]->map = regmap;
>> +
>> + for (i = 0; i < s4_pll_hw_onecell_data.num; i++) {
>> + /* array might be sparse */
>> + if (!s4_pll_hw_onecell_data.hws[i])
>> + continue;
>> +
>> + ret = devm_clk_hw_register(dev, s4_pll_hw_onecell_data.hws[i]);
>> + if (ret) {
>> + dev_err(dev, "Clock registration failed\n");
>> + return ret;
>> + }
>> + }
>> +
>> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
>> + &s4_pll_hw_onecell_data);
>> +}
>> +
>> +static const struct of_device_id clkc_match_table[] = {
>> + {
>> + .compatible = "amlogic,s4-pll-clkc",
>> + },
>> + {}
>> +};
>> +
>> +static struct platform_driver s4_driver = {
>> + .probe = meson_s4_pll_probe,
>> + .driver = {
>> + .name = "s4-pll-clkc",
>> + .of_match_table = clkc_match_table,
>> + },
>> +};
>> +
>> +module_platform_driver(s4_driver);
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/clk/meson/s4-pll.h b/drivers/clk/meson/s4-pll.h
>> new file mode 100644
>> index 000000000000..41dc6de978c1
>> --- /dev/null
>> +++ b/drivers/clk/meson/s4-pll.h
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2021 Amlogic, inc.
>> + * Author: Yu Tu <[email protected]>
>> + */
>> +
>> +#ifndef __MESON_S4_PLL_H__
>> +#define __MESON_S4_PLL_H__
>> +
>> +/* ANA_CTRL - Registers
>> + * REG_BASE: REGISTER_BASE_ADDR = 0xfe008000
>> + */
>> +#define ANACTRL_FIXPLL_CTRL0 (0x0010 << 2)
>
> I already commented on the "<< 2" . Remove them please.
Sorry, maybe I didn't pay attention to this comment earlier. A little
bit of a question why is this not okay? I understand isn't it better for
the compiler to help with this calculation itself?
>
>> +#define ANACTRL_FIXPLL_CTRL1 (0x0011 << 2)
>> +#define ANACTRL_FIXPLL_CTRL2 (0x0012 << 2)
>> +#define ANACTRL_FIXPLL_CTRL3 (0x0013 << 2)
>> +#define ANACTRL_FIXPLL_CTRL4 (0x0014 << 2)
>> +#define ANACTRL_FIXPLL_CTRL5 (0x0015 << 2)
>> +#define ANACTRL_FIXPLL_CTRL6 (0x0016 << 2)
>> +#define ANACTRL_FIXPLL_STS (0x0017 << 2)
>> +#define ANACTRL_GP0PLL_CTRL0 (0x0020 << 2)
>> +#define ANACTRL_GP0PLL_CTRL1 (0x0021 << 2)
>> +#define ANACTRL_GP0PLL_CTRL2 (0x0022 << 2)
>> +#define ANACTRL_GP0PLL_CTRL3 (0x0023 << 2)
>> +#define ANACTRL_GP0PLL_CTRL4 (0x0024 << 2)
>> +#define ANACTRL_GP0PLL_CTRL5 (0x0025 << 2)
>> +#define ANACTRL_GP0PLL_CTRL6 (0x0026 << 2)
>> +#define ANACTRL_GP0PLL_STS (0x0027 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL0 (0x0040 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL1 (0x0041 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL2 (0x0042 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL3 (0x0043 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL4 (0x0044 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL5 (0x0045 << 2)
>> +#define ANACTRL_HIFIPLL_CTRL6 (0x0046 << 2)
>> +#define ANACTRL_HIFIPLL_STS (0x0047 << 2)
>> +#define ANACTRL_MPLL_CTRL0 (0x0060 << 2)
>> +#define ANACTRL_MPLL_CTRL1 (0x0061 << 2)
>> +#define ANACTRL_MPLL_CTRL2 (0x0062 << 2)
>> +#define ANACTRL_MPLL_CTRL3 (0x0063 << 2)
>> +#define ANACTRL_MPLL_CTRL4 (0x0064 << 2)
>> +#define ANACTRL_MPLL_CTRL5 (0x0065 << 2)
>> +#define ANACTRL_MPLL_CTRL6 (0x0066 << 2)
>> +#define ANACTRL_MPLL_CTRL7 (0x0067 << 2)
>> +#define ANACTRL_MPLL_CTRL8 (0x0068 << 2)
>> +#define ANACTRL_MPLL_STS (0x0069 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL0 (0x0070 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL1 (0x0071 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL2 (0x0072 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL3 (0x0073 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL4 (0x0074 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL5 (0x0075 << 2)
>> +#define ANACTRL_HDMIPLL_CTRL6 (0x0076 << 2)
>> +#define ANACTRL_HDMIPLL_STS (0x0077 << 2)
>> +#define ANACTRL_HDMIPLL_VLOCK (0x0079 << 2)
>> +
>> +/*
>> + * CLKID index values
>> + *
>> + * These indices are entirely contrived and do not map onto the hardware.
>> + * It has now been decided to expose everything by default in the DT header:
>> + * include/dt-bindings/clock/axg-clkc.h. Only the clocks ids we don't want
>> + * to expose, such as the internal muxes and dividers of composite clocks,
>> + * will remain defined here.
>> + */
>> +#define CLKID_FIXED_PLL_DCO 0
>> +#define CLKID_FCLK_DIV2_DIV 2
>> +#define CLKID_FCLK_DIV3_DIV 4
>> +#define CLKID_FCLK_DIV4_DIV 6
>> +#define CLKID_FCLK_DIV5_DIV 8
>> +#define CLKID_FCLK_DIV7_DIV 10
>> +#define CLKID_FCLK_DIV2P5_DIV 12
>> +#define CLKID_GP0_PLL_DCO 14
>> +#define CLKID_HIFI_PLL_DCO 16
>> +#define CLKID_HDMI_PLL_DCO 18
>> +#define CLKID_HDMI_PLL_OD 19
>> +#define CLKID_MPLL_50M_DIV 21
>> +#define CLKID_MPLL_PREDIV 23
>> +#define CLKID_MPLL0_DIV 24
>> +#define CLKID_MPLL1_DIV 26
>> +#define CLKID_MPLL2_DIV 28
>> +#define CLKID_MPLL3_DIV 30
>> +
>> +#define NR_PLL_CLKS 32
>> +/* include the CLKIDs that have been made part of the DT binding */
>> +#include <dt-bindings/clock/amlogic,s4-pll-clkc.h>
>> +
>> +#endif /* __MESON_S4_PLL_H__ */
>
> .

2022-08-15 13:28:41

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver



On 2022/8/15 14:34, Yu Tu wrote:
> Hi Jerome,
>
> On 2022/8/10 21:47, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>>
>> On Fri 05 Aug 2022 at 16:57, Yu Tu <[email protected]> wrote:
>>
>>> Add the S4 PLL clock controller found in the s4 SoC family.
>>>
>>> Signed-off-by: Yu Tu <[email protected]>
>>> ---
>>>   drivers/clk/meson/Kconfig  |  12 +
>>>   drivers/clk/meson/Makefile |   1 +
>>>   drivers/clk/meson/s4-pll.c | 891 +++++++++++++++++++++++++++++++++++++
>>>   drivers/clk/meson/s4-pll.h |  88 ++++
>>>   4 files changed, 992 insertions(+)
>>>   create mode 100644 drivers/clk/meson/s4-pll.c
>>>   create mode 100644 drivers/clk/meson/s4-pll.h
>>>
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index fc002c155bc3..f4244edc7b28 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -115,4 +115,16 @@ config COMMON_CLK_G12A
>>>       help
>>>         Support for the clock controller on Amlogic S905D2, S905X2
>>> and S905Y2
>>>         devices, aka g12a. Say Y if you want peripherals to work.
>>> +
>>> +config COMMON_CLK_S4_PLL
>>> +    tristate "S4 SoC PLL clock controllers support"
>>> +    depends on ARM64
>>> +    default y
>>> +    select COMMON_CLK_MESON_MPLL
>>> +    select COMMON_CLK_MESON_PLL
>>> +    select COMMON_CLK_MESON_REGMAP
>>> +    help
>>> +      Support for the pll clock controller on Amlogic S805X2 and
>>> S905Y4 devices,
>>> +      aka s4. Amlogic S805X2 and S905Y4 devices include AQ222 and
>>> AQ229.
>>> +      Say Y if you want peripherals and CPU frequency scaling to work.
>>
>> Looks like this doc is copy/paste of the peripheral controller.
>> This probably needs to be updated
> Okay.
>>
>>>   endmenu
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index 6eca2a406ee3..376f49cc13f1 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>   obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>   obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>>> +obj-$(CONFIG_COMMON_CLK_S4_PLL) += s4-pll.o
>>> diff --git a/drivers/clk/meson/s4-pll.c b/drivers/clk/meson/s4-pll.c
>>> new file mode 100644
>>> index 000000000000..478c78b5ed46
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/s4-pll.c
>>> @@ -0,0 +1,891 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Amlogic Meson-S4 Clock Controller Driver
>>> + *
>>> + * Copyright (c) 2021 Amlogic, inc.
>>> + * Author: Yu Tu <[email protected]>
>>> + */
>>> +
>>> +#include <linux/clk-provider.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#include "clk-mpll.h"
>>> +#include "clk-pll.h"
>>> +#include "clk-regmap.h"
>>> +#include "s4-pll.h"
>>> +
>>> +static DEFINE_SPINLOCK(meson_clk_lock);
>>> +
>>> +static struct clk_regmap s4_fixed_pll_dco = {
>>> +    .data = &(struct meson_clk_pll_data){
>>> +        .en = {
>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>> +            .shift   = 28,
>>> +            .width   = 1,
>>> +        },
>>> +        .m = {
>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>> +            .shift   = 0,
>>> +            .width   = 8,
>>> +        },
>>> +        .n = {
>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>> +            .shift   = 10,
>>> +            .width   = 5,
>>> +        },
>>> +        .frac = {
>>> +            .reg_off = ANACTRL_FIXPLL_CTRL1,
>>> +            .shift   = 0,
>>> +            .width   = 17,
>>> +        },
>>> +        .l = {
>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>> +            .shift   = 31,
>>> +            .width   = 1,
>>> +        },
>>> +        .rst = {
>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>> +            .shift   = 29,
>>> +            .width   = 1,
>>> +        },
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fixed_pll_dco",
>>> +        .ops = &meson_clk_pll_ro_ops,
>>> +        .parent_data = (const struct clk_parent_data []) {
>>> +            { .fw_name = "xtal", }
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fixed_pll = {
>>> +    .data = &(struct clk_regmap_div_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL0,
>>> +        .shift = 16,
>>> +        .width = 2,
>>> +        .flags = CLK_DIVIDER_POWER_OF_TWO,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fixed_pll",
>>> +        .ops = &clk_regmap_divider_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fixed_pll_dco.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +        /*
>>> +         * This clock won't ever change at runtime so
>>> +         * CLK_SET_RATE_PARENT is not required
>>> +         */
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_fclk_div2_div = {
>>> +    .mult = 1,
>>> +    .div = 2,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div2_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fclk_div2 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL1,
>>> +        .bit_idx = 24,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div2",
>>> +        .ops = &clk_regmap_gate_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fclk_div2_div.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_fclk_div3_div = {
>>> +    .mult = 1,
>>> +    .div = 3,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div3_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fclk_div3 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL1,
>>> +        .bit_idx = 20,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div3",
>>> +        .ops = &clk_regmap_gate_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fclk_div3_div.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_fclk_div4_div = {
>>> +    .mult = 1,
>>> +    .div = 4,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div4_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fclk_div4 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL1,
>>> +        .bit_idx = 21,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div4",
>>> +        .ops = &clk_regmap_gate_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fclk_div4_div.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_fclk_div5_div = {
>>> +    .mult = 1,
>>> +    .div = 5,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div5_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fclk_div5 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL1,
>>> +        .bit_idx = 22,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div5",
>>> +        .ops = &clk_regmap_gate_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fclk_div5_div.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_fclk_div7_div = {
>>> +    .mult = 1,
>>> +    .div = 7,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div7_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_fixed_pll.hw },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fclk_div7 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL1,
>>> +        .bit_idx = 23,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div7",
>>> +        .ops = &clk_regmap_gate_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fclk_div7_div.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_fclk_div2p5_div = {
>>> +    .mult = 2,
>>> +    .div = 5,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div2p5_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fixed_pll.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_fclk_div2p5 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL1,
>>> +        .bit_idx = 25,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "fclk_div2p5",
>>> +        .ops = &clk_regmap_gate_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fclk_div2p5_div.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static const struct pll_mult_range s4_gp0_pll_mult_range = {
>>> +    .min = 125,
>>> +    .max = 250,
>>> +};
>>> +
>>> +/*
>>> + * Internal gp0 pll emulation configuration parameters
>>> + */
>>> +static const struct reg_sequence s4_gp0_init_regs[] = {
>>> +    { .reg = ANACTRL_GP0PLL_CTRL1,    .def = 0x00000000 },
>>> +    { .reg = ANACTRL_GP0PLL_CTRL2,    .def = 0x00000000 },
>>> +    { .reg = ANACTRL_GP0PLL_CTRL3,    .def = 0x48681c00 },
>>> +    { .reg = ANACTRL_GP0PLL_CTRL4,    .def = 0x88770290 },
>>> +    { .reg = ANACTRL_GP0PLL_CTRL5,    .def = 0x39272000 },
>>> +    { .reg = ANACTRL_GP0PLL_CTRL6,    .def = 0x56540000 }
>>> +};
>>> +
>>> +static struct clk_regmap s4_gp0_pll_dco = {
>>> +    .data = &(struct meson_clk_pll_data){
>>> +        .en = {
>>> +            .reg_off = ANACTRL_GP0PLL_CTRL0,
>>> +            .shift   = 28,
>>> +            .width   = 1,
>>> +        },
>>> +        .m = {
>>> +            .reg_off = ANACTRL_GP0PLL_CTRL0,
>>> +            .shift   = 0,
>>> +            .width   = 8,
>>> +        },
>>> +        .n = {
>>> +            .reg_off = ANACTRL_GP0PLL_CTRL0,
>>> +            .shift   = 10,
>>> +            .width   = 5,
>>> +        },
>>> +        .frac = {
>>> +            .reg_off = ANACTRL_GP0PLL_CTRL1,
>>> +            .shift   = 0,
>>> +            .width   = 17,
>>> +        },
>>> +        .l = {
>>> +            .reg_off = ANACTRL_GP0PLL_CTRL0,
>>> +            .shift   = 31,
>>> +            .width   = 1,
>>> +        },
>>> +        .rst = {
>>> +            .reg_off = ANACTRL_GP0PLL_CTRL0,
>>> +            .shift   = 29,
>>> +            .width   = 1,
>>> +        },
>>> +        .range = &s4_gp0_pll_mult_range,
>>> +        .init_regs = s4_gp0_init_regs,
>>> +        .init_count = ARRAY_SIZE(s4_gp0_init_regs),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "gp0_pll_dco",
>>> +        .ops = &meson_clk_pll_ops,
>>> +        .parent_data = (const struct clk_parent_data []) {
>>> +            { .fw_name = "xtal", }
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_gp0_pll = {
>>> +    .data = &(struct clk_regmap_div_data){
>>> +        .offset = ANACTRL_GP0PLL_CTRL0,
>>> +        .shift = 16,
>>> +        .width = 3,
>>> +        .flags = (CLK_DIVIDER_POWER_OF_TWO |
>>> +              CLK_DIVIDER_ROUND_CLOSEST),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "gp0_pll",
>>> +        .ops = &clk_regmap_divider_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_gp0_pll_dco.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +        .flags = CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +/*
>>> + * Internal hifi pll emulation configuration parameters
>>> + */
>>> +static const struct reg_sequence s4_hifi_init_regs[] = {
>>> +    { .reg = ANACTRL_HIFIPLL_CTRL1,    .def = 0x00010e56 },
>>> +    { .reg = ANACTRL_HIFIPLL_CTRL2,    .def = 0x00000000 },
>>> +    { .reg = ANACTRL_HIFIPLL_CTRL3,    .def = 0x6a285c00 },
>>> +    { .reg = ANACTRL_HIFIPLL_CTRL4,    .def = 0x65771290 },
>>> +    { .reg = ANACTRL_HIFIPLL_CTRL5,    .def = 0x39272000 },
>>> +    { .reg = ANACTRL_HIFIPLL_CTRL6,    .def = 0x56540000 }
>>> +};
>>> +
>>> +static struct clk_regmap s4_hifi_pll_dco = {
>>> +    .data = &(struct meson_clk_pll_data){
>>> +        .en = {
>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +            .shift   = 28,
>>> +            .width   = 1,
>>> +        },
>>> +        .m = {
>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +            .shift   = 0,
>>> +            .width   = 8,
>>> +        },
>>> +        .n = {
>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +            .shift   = 10,
>>> +            .width   = 5,
>>> +        },
>>> +        .frac = {
>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL1,
>>> +            .shift   = 0,
>>> +            .width   = 17,
>>> +        },
>>> +        .l = {
>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +            .shift   = 31,
>>> +            .width   = 1,
>>> +        },
>>> +        .rst = {
>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>> +            .shift   = 29,
>>> +            .width   = 1,
>>> +        },
>>> +        .range = &s4_gp0_pll_mult_range,
>>> +        .init_regs = s4_hifi_init_regs,
>>> +        .init_count = ARRAY_SIZE(s4_hifi_init_regs),
>>> +        .flags = CLK_MESON_PLL_ROUND_CLOSEST,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "hifi_pll_dco",
>>> +        .ops = &meson_clk_pll_ops,
>>> +        .parent_data = (const struct clk_parent_data []) {
>>> +            { .fw_name = "xtal", }
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_hifi_pll = {
>>> +    .data = &(struct clk_regmap_div_data){
>>> +        .offset = ANACTRL_HIFIPLL_CTRL0,
>>> +        .shift = 16,
>>> +        .width = 2,
>>> +        .flags = (CLK_DIVIDER_POWER_OF_TWO |
>>> +              CLK_DIVIDER_ROUND_CLOSEST),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "hifi_pll",
>>> +        .ops = &clk_regmap_divider_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_hifi_pll_dco.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +        .flags = CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>> +    .data = &(struct meson_clk_pll_data){
>>> +        .en = {
>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>> +            .shift   = 28,
>>> +            .width   = 1,
>>> +        },
>>> +        .m = {
>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>> +            .shift   = 0,
>>> +            .width   = 8,
>>> +        },
>>> +        .n = {
>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>> +            .shift   = 10,
>>> +            .width   = 5,
>>> +        },
>>> +        .frac = {
>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>> +            .shift   = 0,
>>> +            .width   = 17,
>>> +        },
>>> +        .l = {
>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>> +            .shift   = 31,
>>> +            .width   = 1,
>>> +        },
>>> +        .rst = {
>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>> +            .shift   = 29,
>>> +            .width   = 1,
>>> +        },
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "hdmi_pll_dco",
>>> +        .ops = &meson_clk_pll_ro_ops,
>>> +        .parent_data = (const struct clk_parent_data []) {
>>> +            { .fw_name = "xtal", }
>>> +        },
>>> +        .num_parents = 1,
>>> +        /*
>>> +         * Display directly handle hdmi pll registers ATM, we need
>>> +         * NOCACHE to keep our view of the clock as accurate as
>>> +         * possible
>>> +         */
>>
>> Is it really ?
>>
>> Given that HDMI support for the s4 is there yet, the
>> addresses have changes and the region is no longer a syscon, it is time
>> for the HDMI driver to get fixed.
The HDMI PLL is configured in the Uboot phase and does not change the
frequency in the kernel phase. So we use the NOCACHE flag and "ro_ops".
>>
>>> +        .flags = CLK_GET_RATE_NOCACHE,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_hdmi_pll_od = {
>>> +    .data = &(struct clk_regmap_div_data){
>>> +        .offset = ANACTRL_HDMIPLL_CTRL0,
>>> +        .shift = 16,
>>> +        .width = 4,
>>> +        .flags = CLK_DIVIDER_POWER_OF_TWO,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "hdmi_pll_od",
>>> +        .ops = &clk_regmap_divider_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_hdmi_pll_dco.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +        /*
>>> +         * Display directly handle hdmi pll registers ATM, we need
>>> +         * NOCACHE to keep our view of the clock as accurate as
>>> +         * possible
>>> +         */
>>> +        .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_hdmi_pll = {
>>> +    .data = &(struct clk_regmap_div_data){
>>> +        .offset = ANACTRL_HDMIPLL_CTRL0,
>>> +        .shift = 20,
>>> +        .width = 2,
>>> +        .flags = CLK_DIVIDER_POWER_OF_TWO,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "hdmi_pll",
>>> +        .ops = &clk_regmap_divider_ro_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_hdmi_pll_od.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +        /*
>>> +         * Display directly handle hdmi pll registers ATM, we need
>>> +         * NOCACHE to keep our view of the clock as accurate as
>>> +         * possible
>>> +         */
>>> +        .flags = CLK_GET_RATE_NOCACHE | CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_mpll_50m_div = {
>>> +    .mult = 1,
>>> +    .div = 80,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll_50m_div",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fixed_pll_dco.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll_50m = {
>>> +    .data = &(struct clk_regmap_mux_data){
>>> +        .offset = ANACTRL_FIXPLL_CTRL3,
>>> +        .mask = 0x1,
>>> +        .shift = 5,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll_50m",
>>> +        .ops = &clk_regmap_mux_ro_ops,
>>> +        .parent_data = (const struct clk_parent_data []) {
>>> +            { .fw_name = "xtal", },
>>> +            { .hw = &s4_mpll_50m_div.hw },
>>> +        },
>>> +        .num_parents = 2,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_fixed_factor s4_mpll_prediv = {
>>> +    .mult = 1,
>>> +    .div = 2,
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll_prediv",
>>> +        .ops = &clk_fixed_factor_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_fixed_pll_dco.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static const struct reg_sequence s4_mpll0_init_regs[] = {
>>> +    { .reg = ANACTRL_MPLL_CTRL2, .def = 0x40000033 }
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll0_div = {
>>> +    .data = &(struct meson_clk_mpll_data){
>>> +        .sdm = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL1,
>>> +            .shift   = 0,
>>> +            .width   = 14,
>>> +        },
>>> +        .sdm_en = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL1,
>>> +            .shift   = 30,
>>> +            .width     = 1,
>>> +        },
>>> +        .n2 = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL1,
>>> +            .shift   = 20,
>>> +            .width   = 9,
>>> +        },
>>> +        .ssen = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL1,
>>> +            .shift   = 29,
>>> +            .width     = 1,
>>> +        },
>>> +        .lock = &meson_clk_lock,
>>> +        .init_regs = s4_mpll0_init_regs,
>>> +        .init_count = ARRAY_SIZE(s4_mpll0_init_regs),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll0_div",
>>> +        .ops = &meson_clk_mpll_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_mpll_prediv.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll0 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_MPLL_CTRL1,
>>> +        .bit_idx = 31,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll0",
>>> +        .ops = &clk_regmap_gate_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_mpll0_div.hw },
>>> +        .num_parents = 1,
>>> +        .flags = CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +static const struct reg_sequence s4_mpll1_init_regs[] = {
>>> +    { .reg = ANACTRL_MPLL_CTRL4,    .def = 0x40000033 }
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll1_div = {
>>> +    .data = &(struct meson_clk_mpll_data){
>>> +        .sdm = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL3,
>>> +            .shift   = 0,
>>> +            .width   = 14,
>>> +        },
>>> +        .sdm_en = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL3,
>>> +            .shift   = 30,
>>> +            .width     = 1,
>>> +        },
>>> +        .n2 = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL3,
>>> +            .shift   = 20,
>>> +            .width   = 9,
>>> +        },
>>> +        .ssen = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL3,
>>> +            .shift   = 29,
>>> +            .width     = 1,
>>> +        },
>>> +        .lock = &meson_clk_lock,
>>> +        .init_regs = s4_mpll1_init_regs,
>>> +        .init_count = ARRAY_SIZE(s4_mpll1_init_regs),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll1_div",
>>> +        .ops = &meson_clk_mpll_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_mpll_prediv.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll1 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_MPLL_CTRL3,
>>> +        .bit_idx = 31,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll1",
>>> +        .ops = &clk_regmap_gate_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_mpll1_div.hw },
>>> +        .num_parents = 1,
>>> +        .flags = CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +static const struct reg_sequence s4_mpll2_init_regs[] = {
>>> +    { .reg = ANACTRL_MPLL_CTRL6, .def = 0x40000033 }
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll2_div = {
>>> +    .data = &(struct meson_clk_mpll_data){
>>> +        .sdm = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL5,
>>> +            .shift   = 0,
>>> +            .width   = 14,
>>> +        },
>>> +        .sdm_en = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL5,
>>> +            .shift   = 30,
>>> +            .width     = 1,
>>> +        },
>>> +        .n2 = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL5,
>>> +            .shift   = 20,
>>> +            .width   = 9,
>>> +        },
>>> +        .ssen = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL5,
>>> +            .shift   = 29,
>>> +            .width     = 1,
>>> +        },
>>> +        .lock = &meson_clk_lock,
>>> +        .init_regs = s4_mpll2_init_regs,
>>> +        .init_count = ARRAY_SIZE(s4_mpll2_init_regs),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll2_div",
>>> +        .ops = &meson_clk_mpll_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_mpll_prediv.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll2 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_MPLL_CTRL5,
>>> +        .bit_idx = 31,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll2",
>>> +        .ops = &clk_regmap_gate_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_mpll2_div.hw },
>>> +        .num_parents = 1,
>>> +        .flags = CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +static const struct reg_sequence s4_mpll3_init_regs[] = {
>>> +    { .reg = ANACTRL_MPLL_CTRL8, .def = 0x40000033 }
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll3_div = {
>>> +    .data = &(struct meson_clk_mpll_data){
>>> +        .sdm = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL7,
>>> +            .shift   = 0,
>>> +            .width   = 14,
>>> +        },
>>> +        .sdm_en = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL7,
>>> +            .shift   = 30,
>>> +            .width     = 1,
>>> +        },
>>> +        .n2 = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL7,
>>> +            .shift   = 20,
>>> +            .width   = 9,
>>> +        },
>>> +        .ssen = {
>>> +            .reg_off = ANACTRL_MPLL_CTRL7,
>>> +            .shift   = 29,
>>> +            .width     = 1,
>>> +        },
>>> +        .lock = &meson_clk_lock,
>>> +        .init_regs = s4_mpll3_init_regs,
>>> +        .init_count = ARRAY_SIZE(s4_mpll3_init_regs),
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll3_div",
>>> +        .ops = &meson_clk_mpll_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) {
>>> +            &s4_mpll_prediv.hw
>>> +        },
>>> +        .num_parents = 1,
>>> +    },
>>> +};
>>> +
>>> +static struct clk_regmap s4_mpll3 = {
>>> +    .data = &(struct clk_regmap_gate_data){
>>> +        .offset = ANACTRL_MPLL_CTRL7,
>>> +        .bit_idx = 31,
>>> +    },
>>> +    .hw.init = &(struct clk_init_data){
>>> +        .name = "mpll3",
>>> +        .ops = &clk_regmap_gate_ops,
>>> +        .parent_hws = (const struct clk_hw *[]) { &s4_mpll3_div.hw },
>>> +        .num_parents = 1,
>>> +        .flags = CLK_SET_RATE_PARENT,
>>> +    },
>>> +};
>>> +
>>> +/* Array of all clocks provided by this provider */
>>> +static struct clk_hw_onecell_data s4_pll_hw_onecell_data = {
>>> +    .hws = {
>>> +        [CLKID_FIXED_PLL_DCO]        = &s4_fixed_pll_dco.hw,
>>> +        [CLKID_FIXED_PLL]        = &s4_fixed_pll.hw,
>>> +        [CLKID_FCLK_DIV2_DIV]        = &s4_fclk_div2_div.hw,
>>> +        [CLKID_FCLK_DIV2]        = &s4_fclk_div2.hw,
>>> +        [CLKID_FCLK_DIV3_DIV]        = &s4_fclk_div3_div.hw,
>>> +        [CLKID_FCLK_DIV3]        = &s4_fclk_div3.hw,
>>> +        [CLKID_FCLK_DIV4_DIV]        = &s4_fclk_div4_div.hw,
>>> +        [CLKID_FCLK_DIV4]        = &s4_fclk_div4.hw,
>>> +        [CLKID_FCLK_DIV5_DIV]        = &s4_fclk_div5_div.hw,
>>> +        [CLKID_FCLK_DIV5]        = &s4_fclk_div5.hw,
>>> +        [CLKID_FCLK_DIV7_DIV]        = &s4_fclk_div7_div.hw,
>>> +        [CLKID_FCLK_DIV7]        = &s4_fclk_div7.hw,
>>> +        [CLKID_FCLK_DIV2P5_DIV]        = &s4_fclk_div2p5_div.hw,
>>> +        [CLKID_FCLK_DIV2P5]        = &s4_fclk_div2p5.hw,
>>> +        [CLKID_GP0_PLL_DCO]        = &s4_gp0_pll_dco.hw,
>>> +        [CLKID_GP0_PLL]            = &s4_gp0_pll.hw,
>>> +        [CLKID_HIFI_PLL_DCO]        = &s4_hifi_pll_dco.hw,
>>> +        [CLKID_HIFI_PLL]        = &s4_hifi_pll.hw,
>>> +        [CLKID_HDMI_PLL_DCO]        = &s4_hdmi_pll_dco.hw,
>>> +        [CLKID_HDMI_PLL_OD]        = &s4_hdmi_pll_od.hw,
>>> +        [CLKID_HDMI_PLL]        = &s4_hdmi_pll.hw,
>>> +        [CLKID_MPLL_50M_DIV]        = &s4_mpll_50m_div.hw,
>>> +        [CLKID_MPLL_50M]        = &s4_mpll_50m.hw,
>>> +        [CLKID_MPLL_PREDIV]        = &s4_mpll_prediv.hw,
>>> +        [CLKID_MPLL0_DIV]        = &s4_mpll0_div.hw,
>>> +        [CLKID_MPLL0]            = &s4_mpll0.hw,
>>> +        [CLKID_MPLL1_DIV]        = &s4_mpll1_div.hw,
>>> +        [CLKID_MPLL1]            = &s4_mpll1.hw,
>>> +        [CLKID_MPLL2_DIV]        = &s4_mpll2_div.hw,
>>> +        [CLKID_MPLL2]            = &s4_mpll2.hw,
>>> +        [CLKID_MPLL3_DIV]        = &s4_mpll3_div.hw,
>>> +        [CLKID_MPLL3]            = &s4_mpll3.hw,
>>> +
>>> +        [NR_PLL_CLKS]            = NULL
>>> +    },
>>> +    .num = NR_PLL_CLKS,
>>> +};
>>> +
>>> +static struct clk_regmap *const s4_pll_clk_regmaps[] = {
>>> +    &s4_fixed_pll_dco,
>>> +    &s4_fixed_pll,
>>> +    &s4_fclk_div2,
>>> +    &s4_fclk_div3,
>>> +    &s4_fclk_div4,
>>> +    &s4_fclk_div5,
>>> +    &s4_fclk_div7,
>>> +    &s4_fclk_div2p5,
>>> +    &s4_gp0_pll_dco,
>>> +    &s4_gp0_pll,
>>> +    &s4_hifi_pll_dco,
>>> +    &s4_hifi_pll,
>>> +    &s4_hdmi_pll_dco,
>>> +    &s4_hdmi_pll_od,
>>> +    &s4_hdmi_pll,
>>> +    &s4_mpll_50m,
>>> +    &s4_mpll0_div,
>>> +    &s4_mpll0,
>>> +    &s4_mpll1_div,
>>> +    &s4_mpll1,
>>> +    &s4_mpll2_div,
>>> +    &s4_mpll2,
>>> +    &s4_mpll3_div,
>>> +    &s4_mpll3,
>>> +};
>>> +
>>> +static const struct reg_sequence s4_init_regs[] = {
>>> +    { .reg = ANACTRL_MPLL_CTRL0,    .def = 0x00000543 },
>>> +};
>>> +
>>> +static struct regmap_config clkc_regmap_config = {
>>> +    .reg_bits       = 32,
>>> +    .val_bits       = 32,
>>> +    .reg_stride     = 4,
>>> +};
>>> +
>>> +static int meson_s4_pll_probe(struct platform_device *pdev)
>>> +{
>>> +    struct device *dev = &pdev->dev;
>>> +    struct regmap *regmap;
>>> +    void __iomem *base;
>>> +    int ret, i;
>>> +
>>> +    base = devm_platform_ioremap_resource(pdev, 0);
>>> +    if (IS_ERR(base))
>>> +        return PTR_ERR(base);
>>> +
>>> +    regmap = devm_regmap_init_mmio(dev, base, &clkc_regmap_config);
>>> +    if (IS_ERR(regmap))
>>> +        return PTR_ERR(regmap);
>>> +
>>> +    ret = regmap_multi_reg_write(regmap, s4_init_regs,
>>> ARRAY_SIZE(s4_init_regs));
>>> +    if (ret) {
>>> +        dev_err(dev, "Failed to init registers\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    /* Populate regmap for the regmap backed clocks */
>>> +    for (i = 0; i < ARRAY_SIZE(s4_pll_clk_regmaps); i++)
>>> +        s4_pll_clk_regmaps[i]->map = regmap;
>>> +
>>> +    for (i = 0; i < s4_pll_hw_onecell_data.num; i++) {
>>> +        /* array might be sparse */
>>> +        if (!s4_pll_hw_onecell_data.hws[i])
>>> +            continue;
>>> +
>>> +        ret = devm_clk_hw_register(dev, s4_pll_hw_onecell_data.hws[i]);
>>> +        if (ret) {
>>> +            dev_err(dev, "Clock registration failed\n");
>>> +            return ret;
>>> +        }
>>> +    }
>>> +
>>> +    return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
>>> +                       &s4_pll_hw_onecell_data);
>>> +}
>>> +
>>> +static const struct of_device_id clkc_match_table[] = {
>>> +    {
>>> +        .compatible = "amlogic,s4-pll-clkc",
>>> +    },
>>> +    {}
>>> +};
>>> +
>>> +static struct platform_driver s4_driver = {
>>> +    .probe        = meson_s4_pll_probe,
>>> +    .driver        = {
>>> +        .name    = "s4-pll-clkc",
>>> +        .of_match_table = clkc_match_table,
>>> +    },
>>> +};
>>> +
>>> +module_platform_driver(s4_driver);
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/clk/meson/s4-pll.h b/drivers/clk/meson/s4-pll.h
>>> new file mode 100644
>>> index 000000000000..41dc6de978c1
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/s4-pll.h
>>> @@ -0,0 +1,88 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>>> +/*
>>> + * Copyright (c) 2021 Amlogic, inc.
>>> + * Author: Yu Tu <[email protected]>
>>> + */
>>> +
>>> +#ifndef __MESON_S4_PLL_H__
>>> +#define __MESON_S4_PLL_H__
>>> +
>>> +/* ANA_CTRL - Registers
>>> + * REG_BASE:  REGISTER_BASE_ADDR = 0xfe008000
>>> + */
>>> +#define ANACTRL_FIXPLL_CTRL0                       (0x0010 << 2)
>>
>> I already commented on the "<< 2" . Remove them please.
> Sorry, maybe I didn't pay attention to this comment earlier. A little
> bit of a question why is this not okay? I understand isn't it better for
> the compiler to help with this calculation itself?
>>
>>> +#define ANACTRL_FIXPLL_CTRL1                       (0x0011 << 2)
>>> +#define ANACTRL_FIXPLL_CTRL2                       (0x0012 << 2)
>>> +#define ANACTRL_FIXPLL_CTRL3                       (0x0013 << 2)
>>> +#define ANACTRL_FIXPLL_CTRL4                       (0x0014 << 2)
>>> +#define ANACTRL_FIXPLL_CTRL5                       (0x0015 << 2)
>>> +#define ANACTRL_FIXPLL_CTRL6                       (0x0016 << 2)
>>> +#define ANACTRL_FIXPLL_STS                         (0x0017 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL0                       (0x0020 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL1                       (0x0021 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL2                       (0x0022 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL3                       (0x0023 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL4                       (0x0024 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL5                       (0x0025 << 2)
>>> +#define ANACTRL_GP0PLL_CTRL6                       (0x0026 << 2)
>>> +#define ANACTRL_GP0PLL_STS                         (0x0027 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL0                      (0x0040 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL1                      (0x0041 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL2                      (0x0042 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL3                      (0x0043 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL4                      (0x0044 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL5                      (0x0045 << 2)
>>> +#define ANACTRL_HIFIPLL_CTRL6                      (0x0046 << 2)
>>> +#define ANACTRL_HIFIPLL_STS                        (0x0047 << 2)
>>> +#define ANACTRL_MPLL_CTRL0                         (0x0060 << 2)
>>> +#define ANACTRL_MPLL_CTRL1                         (0x0061 << 2)
>>> +#define ANACTRL_MPLL_CTRL2                         (0x0062 << 2)
>>> +#define ANACTRL_MPLL_CTRL3                         (0x0063 << 2)
>>> +#define ANACTRL_MPLL_CTRL4                         (0x0064 << 2)
>>> +#define ANACTRL_MPLL_CTRL5                         (0x0065 << 2)
>>> +#define ANACTRL_MPLL_CTRL6                         (0x0066 << 2)
>>> +#define ANACTRL_MPLL_CTRL7                         (0x0067 << 2)
>>> +#define ANACTRL_MPLL_CTRL8                         (0x0068 << 2)
>>> +#define ANACTRL_MPLL_STS                           (0x0069 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL0                      (0x0070 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL1                      (0x0071 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL2                      (0x0072 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL3                      (0x0073 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL4                      (0x0074 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL5                      (0x0075 << 2)
>>> +#define ANACTRL_HDMIPLL_CTRL6                      (0x0076 << 2)
>>> +#define ANACTRL_HDMIPLL_STS                        (0x0077 << 2)
>>> +#define ANACTRL_HDMIPLL_VLOCK                      (0x0079 << 2)
>>> +
>>> +/*
>>> + * CLKID index values
>>> + *
>>> + * These indices are entirely contrived and do not map onto the
>>> hardware.
>>> + * It has now been decided to expose everything by default in the DT
>>> header:
>>> + * include/dt-bindings/clock/axg-clkc.h. Only the clocks ids we
>>> don't want
>>> + * to expose, such as the internal muxes and dividers of composite
>>> clocks,
>>> + * will remain defined here.
>>> + */
>>> +#define CLKID_FIXED_PLL_DCO        0
>>> +#define CLKID_FCLK_DIV2_DIV        2
>>> +#define CLKID_FCLK_DIV3_DIV        4
>>> +#define CLKID_FCLK_DIV4_DIV        6
>>> +#define CLKID_FCLK_DIV5_DIV        8
>>> +#define CLKID_FCLK_DIV7_DIV        10
>>> +#define CLKID_FCLK_DIV2P5_DIV        12
>>> +#define CLKID_GP0_PLL_DCO        14
>>> +#define CLKID_HIFI_PLL_DCO        16
>>> +#define CLKID_HDMI_PLL_DCO        18
>>> +#define CLKID_HDMI_PLL_OD        19
>>> +#define CLKID_MPLL_50M_DIV        21
>>> +#define CLKID_MPLL_PREDIV        23
>>> +#define CLKID_MPLL0_DIV            24
>>> +#define CLKID_MPLL1_DIV            26
>>> +#define CLKID_MPLL2_DIV            28
>>> +#define CLKID_MPLL3_DIV            30
>>> +
>>> +#define NR_PLL_CLKS            32
>>> +/* include the CLKIDs that have been made part of the DT binding */
>>> +#include <dt-bindings/clock/amlogic,s4-pll-clkc.h>
>>> +
>>> +#endif /* __MESON_S4_PLL_H__ */
>>
>> .

2022-08-29 09:55:04

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver


On Mon 15 Aug 2022 at 14:34, Yu Tu <[email protected]> wrote:

> Hi Jerome,
>
> On 2022/8/10 21:47, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>> On Fri 05 Aug 2022 at 16:57, Yu Tu <[email protected]> wrote:
>> */

[... ]

>>> +#define ANACTRL_FIXPLL_CTRL0 (0x0010 << 2)
>> I already commented on the "<< 2" . Remove them please.
> Sorry, maybe I didn't pay attention to this comment earlier. A little bit
> of a question why is this not okay? I understand isn't it better for the
> compiler to help with this calculation itself?

Because it is aweful to read

Also please trim your replies.
It is a bit annoying to search for your comments in the replies



2022-08-29 10:18:42

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver


On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:

>>>> +
>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>> +    .data = &(struct meson_clk_pll_data){
>>>> +        .en = {
>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>> +            .shift   = 28,
>>>> +            .width   = 1,
>>>> +        },
>>>> +        .m = {
>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>> +            .shift   = 0,
>>>> +            .width   = 8,
>>>> +        },
>>>> +        .n = {
>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>> +            .shift   = 10,
>>>> +            .width   = 5,
>>>> +        },
>>>> +        .frac = {
>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>> +            .shift   = 0,
>>>> +            .width   = 17,
>>>> +        },
>>>> +        .l = {
>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>> +            .shift   = 31,
>>>> +            .width   = 1,
>>>> +        },
>>>> +        .rst = {
>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>> +            .shift   = 29,
>>>> +            .width   = 1,
>>>> +        },
>>>> +    },
>>>> +    .hw.init = &(struct clk_init_data){
>>>> +        .name = "hdmi_pll_dco",
>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>> +            { .fw_name = "xtal", }
>>>> +        },
>>>> +        .num_parents = 1,
>>>> +        /*
>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>> +         * possible
>>>> +         */
>>>
>>> Is it really ?
>>>
>>> Given that HDMI support for the s4 is there yet, the
>>> addresses have changes and the region is no longer a syscon, it is time
>>> for the HDMI driver to get fixed.
> The HDMI PLL is configured in the Uboot phase and does not change the
> frequency in the kernel phase. So we use the NOCACHE flag and
> "ro_ops".

That's no reason to put NOCACHE or ro-ops

If you want the frequencies to be statically assinged, the correct way
would be through assigned-rate in DT I guess.

2022-08-30 06:10:20

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver



On 2022/8/29 17:46, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Mon 15 Aug 2022 at 14:34, Yu Tu <[email protected]> wrote:
>
>> Hi Jerome,
>>
>> On 2022/8/10 21:47, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Fri 05 Aug 2022 at 16:57, Yu Tu <[email protected]> wrote:
>>> */
>
> [... ]
>
>>>> +#define ANACTRL_FIXPLL_CTRL0 (0x0010 << 2)
>>> I already commented on the "<< 2" . Remove them please.
>> Sorry, maybe I didn't pay attention to this comment earlier. A little bit
>> of a question why is this not okay? I understand isn't it better for the
>> compiler to help with this calculation itself?
>
> Because it is aweful to read
>
> Also please trim your replies.
> It is a bit annoying to search for your comments in the replies
>

Okay. Like this?

>
>
> .

2022-08-30 06:24:30

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver



On 2022/8/29 17:48, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>
>>>>> +
>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>> +        .en = {
>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>> +            .shift   = 28,
>>>>> +            .width   = 1,
>>>>> +        },
>>>>> +        .m = {
>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>> +            .shift   = 0,
>>>>> +            .width   = 8,
>>>>> +        },
>>>>> +        .n = {
>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>> +            .shift   = 10,
>>>>> +            .width   = 5,
>>>>> +        },
>>>>> +        .frac = {
>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>> +            .shift   = 0,
>>>>> +            .width   = 17,
>>>>> +        },
>>>>> +        .l = {
>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>> +            .shift   = 31,
>>>>> +            .width   = 1,
>>>>> +        },
>>>>> +        .rst = {
>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>> +            .shift   = 29,
>>>>> +            .width   = 1,
>>>>> +        },
>>>>> +    },
>>>>> +    .hw.init = &(struct clk_init_data){
>>>>> +        .name = "hdmi_pll_dco",
>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>> +            { .fw_name = "xtal", }
>>>>> +        },
>>>>> +        .num_parents = 1,
>>>>> +        /*
>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>> +         * possible
>>>>> +         */
>>>>
>>>> Is it really ?
>>>>
>>>> Given that HDMI support for the s4 is there yet, the
>>>> addresses have changes and the region is no longer a syscon, it is time
>>>> for the HDMI driver to get fixed.
>> The HDMI PLL is configured in the Uboot phase and does not change the
>> frequency in the kernel phase. So we use the NOCACHE flag and
>> "ro_ops".
>
> That's no reason to put NOCACHE or ro-ops
>
> If you want the frequencies to be statically assinged, the correct way
> would be through assigned-rate in DT I guess.

Okay. You're right. However, when registering with OPS, HDMI PLL will be
reset. It takes time for PLL to stabilize the output frequency, which
will lead to the startup screen flashing.

I would like to know how to solve this problem if not using ro_ops.

>

2022-08-30 07:57:31

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver


On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:

> On 2022/8/29 17:48, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>
>>>>>> +
>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>> +        .en = {
>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>> +            .shift   = 28,
>>>>>> +            .width   = 1,
>>>>>> +        },
>>>>>> +        .m = {
>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>> +            .shift   = 0,
>>>>>> +            .width   = 8,
>>>>>> +        },
>>>>>> +        .n = {
>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>> +            .shift   = 10,
>>>>>> +            .width   = 5,
>>>>>> +        },
>>>>>> +        .frac = {
>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>> +            .shift   = 0,
>>>>>> +            .width   = 17,
>>>>>> +        },
>>>>>> +        .l = {
>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>> +            .shift   = 31,
>>>>>> +            .width   = 1,
>>>>>> +        },
>>>>>> +        .rst = {
>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>> +            .shift   = 29,
>>>>>> +            .width   = 1,
>>>>>> +        },
>>>>>> +    },
>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>> +        .name = "hdmi_pll_dco",
>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>> +            { .fw_name = "xtal", }
>>>>>> +        },
>>>>>> +        .num_parents = 1,
>>>>>> +        /*
>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>> +         * possible
>>>>>> +         */
>>>>>
>>>>> Is it really ?
>>>>>
>>>>> Given that HDMI support for the s4 is there yet, the
>>>>> addresses have changes and the region is no longer a syscon, it is time
>>>>> for the HDMI driver to get fixed.
>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>> "ro_ops".
>> That's no reason to put NOCACHE or ro-ops
>> If you want the frequencies to be statically assinged, the correct way
>> would be through assigned-rate in DT I guess.
>
> Okay. You're right. However, when registering with OPS, HDMI PLL will be
> reset. It takes time for PLL to stabilize the output frequency, which will
> lead to the startup screen flashing.
>
> I would like to know how to solve this problem if not using ro_ops.
>
>>

You can add new ops or tweak the current init function.

Safest would be to do the following :
* Check if the PLLs is already on.
* Check if the 'pll->init_regs' matches what is already set
- if so, you can skip the reset
- if not, you need to reset as usual

2022-08-30 08:09:44

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver



On 2022/8/30 14:44, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:
>
>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>>
>>>>>>> +
>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>> +        .en = {
>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>> +            .shift   = 28,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +        .m = {
>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>> +            .shift   = 0,
>>>>>>> +            .width   = 8,
>>>>>>> +        },
>>>>>>> +        .n = {
>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>> +            .shift   = 10,
>>>>>>> +            .width   = 5,
>>>>>>> +        },
>>>>>>> +        .frac = {
>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>>> +            .shift   = 0,
>>>>>>> +            .width   = 17,
>>>>>>> +        },
>>>>>>> +        .l = {
>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>> +            .shift   = 31,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +        .rst = {
>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>> +            .shift   = 29,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "hdmi_pll_dco",
>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>>> +            { .fw_name = "xtal", }
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        /*
>>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>>> +         * possible
>>>>>>> +         */
>>>>>>
>>>>>> Is it really ?
>>>>>>
>>>>>> Given that HDMI support for the s4 is there yet, the
>>>>>> addresses have changes and the region is no longer a syscon, it is time
>>>>>> for the HDMI driver to get fixed.
>>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>>> "ro_ops".
>>> That's no reason to put NOCACHE or ro-ops
>>> If you want the frequencies to be statically assinged, the correct way
>>> would be through assigned-rate in DT I guess.
>>
>> Okay. You're right. However, when registering with OPS, HDMI PLL will be
>> reset. It takes time for PLL to stabilize the output frequency, which will
>> lead to the startup screen flashing.
>>
>> I would like to know how to solve this problem if not using ro_ops.
>>
>>>
>
> You can add new ops or tweak the current init function.

HDMI PLL is not different from other PLLS, so I think adding OPS is weird.

>
> Safest would be to do the following :
> * Check if the PLLs is already on.
> * Check if the 'pll->init_regs' matches what is already set
> - if so, you can skip the reset
> - if not, you need to reset as usual

static int meson_clk_pll_init(struct clk_hw *hw)

{

struct clk_regmap *clk = to_clk_regmap(hw);

struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);



if (pll->init_count) {

meson_parm_write(clk->map, &pll->rst, 1);

regmap_multi_reg_write(clk->map, pll->init_regs,

| pll->init_count);

meson_parm_write(clk->map, &pll->rst, 0);

}



return 0;

}

Because the init function looks like this. Therefore, HDMI PLL
init_count is not given. Can I change it like this?

2022-09-21 09:45:07

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver

Hi Jerome,

On 2022/8/30 15:37, Yu Tu wrote:
>
>
> On 2022/8/30 14:44, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>>
>> On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:
>>
>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>> [ EXTERNAL EMAIL ]
>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>>>
>>>>>>>> +
>>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>>> +        .en = {
>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>> +            .shift   = 28,
>>>>>>>> +            .width   = 1,
>>>>>>>> +        },
>>>>>>>> +        .m = {
>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>> +            .shift   = 0,
>>>>>>>> +            .width   = 8,
>>>>>>>> +        },
>>>>>>>> +        .n = {
>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>> +            .shift   = 10,
>>>>>>>> +            .width   = 5,
>>>>>>>> +        },
>>>>>>>> +        .frac = {
>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>>>> +            .shift   = 0,
>>>>>>>> +            .width   = 17,
>>>>>>>> +        },
>>>>>>>> +        .l = {
>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>> +            .shift   = 31,
>>>>>>>> +            .width   = 1,
>>>>>>>> +        },
>>>>>>>> +        .rst = {
>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>> +            .shift   = 29,
>>>>>>>> +            .width   = 1,
>>>>>>>> +        },
>>>>>>>> +    },
>>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>>> +        .name = "hdmi_pll_dco",
>>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>>>> +            { .fw_name = "xtal", }
>>>>>>>> +        },
>>>>>>>> +        .num_parents = 1,
>>>>>>>> +        /*
>>>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>>>> +         * possible
>>>>>>>> +         */
>>>>>>>
>>>>>>> Is it really ?
>>>>>>>
>>>>>>> Given that HDMI support for the s4 is there yet, the
>>>>>>> addresses have changes and the region is no longer a syscon, it
>>>>>>> is time
>>>>>>> for the HDMI driver to get fixed.
>>>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>>>> "ro_ops".
>>>> That's no reason to put NOCACHE or ro-ops
>>>> If you want the frequencies to be statically assinged, the correct way
>>>> would be through assigned-rate in DT I guess.
>>>
>>> Okay. You're right. However, when registering with OPS, HDMI PLL will be
>>> reset. It takes time for PLL to stabilize the output frequency, which
>>> will
>>> lead to the startup screen flashing.
>>>
>>> I would like to know how to solve this problem if not using ro_ops.
>>>
>>>>
>>
>> You can add new ops or tweak the current init function.
>
> HDMI PLL is not different from other PLLS, so I think adding OPS is weird.
>
>>
>> Safest would be to do the following :
>>   * Check if the PLLs is already on.
>>   * Check if the 'pll->init_regs' matches what is already set
>>     - if so, you can skip the reset
>>     - if not, you need to reset as usual
>
> static int meson_clk_pll_init(struct clk_hw *hw)
> {
>         struct clk_regmap *clk = to_clk_regmap(hw);
>         struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>
>
>         if (pll->init_count) {
>                 meson_parm_write(clk->map, &pll->rst, 1);
>                 regmap_multi_reg_write(clk->map, pll->init_regs,
>                                 |      pll->init_count);
>                 meson_parm_write(clk->map, &pll->rst, 0);
>         }
>
>
>         return 0;
> }
>
> Because the init function looks like this. Therefore, HDMI PLL
> init_count is not given. Can I change it like this?

I don't know if this change meets your requirements? Please give us your
valuable advice.

2022-09-28 16:09:34

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver


On Wed 21 Sep 2022 at 16:40, Yu Tu <[email protected]> wrote:

> Hi Jerome,
>
> On 2022/8/30 15:37, Yu Tu wrote:
>> On 2022/8/30 14:44, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>>
>>> On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:
>>>
>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>> [ EXTERNAL EMAIL ]
>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>>>>
>>>>>>>>> +
>>>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>>>> +        .en = {
>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>> +            .shift   = 28,
>>>>>>>>> +            .width   = 1,
>>>>>>>>> +        },
>>>>>>>>> +        .m = {
>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>> +            .shift   = 0,
>>>>>>>>> +            .width   = 8,
>>>>>>>>> +        },
>>>>>>>>> +        .n = {
>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>> +            .shift   = 10,
>>>>>>>>> +            .width   = 5,
>>>>>>>>> +        },
>>>>>>>>> +        .frac = {
>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>>>>> +            .shift   = 0,
>>>>>>>>> +            .width   = 17,
>>>>>>>>> +        },
>>>>>>>>> +        .l = {
>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>> +            .shift   = 31,
>>>>>>>>> +            .width   = 1,
>>>>>>>>> +        },
>>>>>>>>> +        .rst = {
>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>> +            .shift   = 29,
>>>>>>>>> +            .width   = 1,
>>>>>>>>> +        },
>>>>>>>>> +    },
>>>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>>>> +        .name = "hdmi_pll_dco",
>>>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>>>>> +            { .fw_name = "xtal", }
>>>>>>>>> +        },
>>>>>>>>> +        .num_parents = 1,
>>>>>>>>> +        /*
>>>>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>>>>> +         * possible
>>>>>>>>> +         */
>>>>>>>>
>>>>>>>> Is it really ?
>>>>>>>>
>>>>>>>> Given that HDMI support for the s4 is there yet, the
>>>>>>>> addresses have changes and the region is no longer a syscon, it is
>>>>>>>> time
>>>>>>>> for the HDMI driver to get fixed.
>>>>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>>>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>>>>> "ro_ops".
>>>>> That's no reason to put NOCACHE or ro-ops
>>>>> If you want the frequencies to be statically assinged, the correct way
>>>>> would be through assigned-rate in DT I guess.
>>>>
>>>> Okay. You're right. However, when registering with OPS, HDMI PLL will be
>>>> reset. It takes time for PLL to stabilize the output frequency, which
>>>> will
>>>> lead to the startup screen flashing.
>>>>
>>>> I would like to know how to solve this problem if not using ro_ops.
>>>>
>>>>>
>>>
>>> You can add new ops or tweak the current init function.
>> HDMI PLL is not different from other PLLS, so I think adding OPS is
>> weird.
>>
>>>
>>> Safest would be to do the following :
>>>   * Check if the PLLs is already on.
>>>   * Check if the 'pll->init_regs' matches what is already set
>>>     - if so, you can skip the reset
>>>     - if not, you need to reset as usual
>> static int meson_clk_pll_init(struct clk_hw *hw)
>> {
>>         struct clk_regmap *clk = to_clk_regmap(hw);
>>         struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>>         if (pll->init_count) {
>>                 meson_parm_write(clk->map, &pll->rst, 1);
>>                 regmap_multi_reg_write(clk->map, pll->init_regs,
>>                                 |      pll->init_count);
>>                 meson_parm_write(clk->map, &pll->rst, 0);
>>         }
>>         return 0;
>> }
>> Because the init function looks like this. Therefore, HDMI PLL init_count
>> is not given.

I don't get the remark. You've got pll->init_count right there.

>> Can I change it like this?

What change ? The function above looks a lot like meson_clk_pll_init()
in the actual source

>
> I don't know if this change meets your requirements? Please give us your
> valuable advice.

What change ?

2022-09-29 07:35:54

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver

Hi Jerome,
Thank you for your reply.

On 2022/9/28 23:27, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Wed 21 Sep 2022 at 16:40, Yu Tu <[email protected]> wrote:
>
>> Hi Jerome,
>>
>> On 2022/8/30 15:37, Yu Tu wrote:
>>> On 2022/8/30 14:44, Jerome Brunet wrote:
>>>> [ EXTERNAL EMAIL ]
>>>>
>>>>
>>>> On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:
>>>>
>>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>>> [ EXTERNAL EMAIL ]
>>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>>>>> +        .en = {
>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>> +            .shift   = 28,
>>>>>>>>>> +            .width   = 1,
>>>>>>>>>> +        },
>>>>>>>>>> +        .m = {
>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>> +            .shift   = 0,
>>>>>>>>>> +            .width   = 8,
>>>>>>>>>> +        },
>>>>>>>>>> +        .n = {
>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>> +            .shift   = 10,
>>>>>>>>>> +            .width   = 5,
>>>>>>>>>> +        },
>>>>>>>>>> +        .frac = {
>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>>>>>> +            .shift   = 0,
>>>>>>>>>> +            .width   = 17,
>>>>>>>>>> +        },
>>>>>>>>>> +        .l = {
>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>> +            .shift   = 31,
>>>>>>>>>> +            .width   = 1,
>>>>>>>>>> +        },
>>>>>>>>>> +        .rst = {
>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>> +            .shift   = 29,
>>>>>>>>>> +            .width   = 1,
>>>>>>>>>> +        },
>>>>>>>>>> +    },
>>>>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>>>>> +        .name = "hdmi_pll_dco",
>>>>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>>>>>> +            { .fw_name = "xtal", }
>>>>>>>>>> +        },
>>>>>>>>>> +        .num_parents = 1,
>>>>>>>>>> +        /*
>>>>>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>>>>>> +         * possible
>>>>>>>>>> +         */
>>>>>>>>>
>>>>>>>>> Is it really ?
>>>>>>>>>
>>>>>>>>> Given that HDMI support for the s4 is there yet, the
>>>>>>>>> addresses have changes and the region is no longer a syscon, it is
>>>>>>>>> time
>>>>>>>>> for the HDMI driver to get fixed.
>>>>>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>>>>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>>>>>> "ro_ops".
>>>>>> That's no reason to put NOCACHE or ro-ops
>>>>>> If you want the frequencies to be statically assinged, the correct way
>>>>>> would be through assigned-rate in DT I guess.
>>>>>
>>>>> Okay. You're right. However, when registering with OPS, HDMI PLL will be
>>>>> reset. It takes time for PLL to stabilize the output frequency, which
>>>>> will
>>>>> lead to the startup screen flashing.
>>>>>
>>>>> I would like to know how to solve this problem if not using ro_ops.
>>>>>
>>>>>>
>>>>
>>>> You can add new ops or tweak the current init function.
>>> HDMI PLL is not different from other PLLS, so I think adding OPS is
>>> weird.
>>>
>>>>
>>>> Safest would be to do the following :
>>>>   * Check if the PLLs is already on.
>>>>   * Check if the 'pll->init_regs' matches what is already set
>>>>     - if so, you can skip the reset
>>>>     - if not, you need to reset as usual
>>> static int meson_clk_pll_init(struct clk_hw *hw)
>>> {
>>>         struct clk_regmap *clk = to_clk_regmap(hw);
>>>         struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>>>         if (pll->init_count) {
>>>                 meson_parm_write(clk->map, &pll->rst, 1);
>>>                 regmap_multi_reg_write(clk->map, pll->init_regs,
>>>                                 |      pll->init_count);
>>>                 meson_parm_write(clk->map, &pll->rst, 0);
>>>         }
>>>         return 0;
>>> }
>>> Because the init function looks like this. Therefore, HDMI PLL init_count
>>> is not given.
>
> I don't get the remark. You've got pll->init_count right there.
>
>>> Can I change it like this?
>
> What change ? The function above looks a lot like meson_clk_pll_init()
> in the actual source
>
>>
>> I don't know if this change meets your requirements? Please give us your
>> valuable advice.
>
> What change ?

static struct clk_regmap s4_hdmi_pll_dco = {

.data = &(struct meson_clk_pll_data){

.en = {

.reg_off = ANACTRL_HDMIPLL_CTRL0,

.shift = 28,

.width = 1,

},

.m = {

.reg_off = ANACTRL_HDMIPLL_CTRL0,

.shift = 0,

.width = 8,

},

.n = {

.reg_off = ANACTRL_HDMIPLL_CTRL0,

.shift = 10,

.width = 5,

},

.frac = {

.reg_off = ANACTRL_HDMIPLL_CTRL1,

.shift = 0,

.width = 17,

},

.l = {

.reg_off = ANACTRL_HDMIPLL_CTRL0,

.shift = 31,

.width = 1,

},

.rst = {

.reg_off = ANACTRL_HDMIPLL_CTRL0,

.shift = 29,

.width = 1,

},

.range = &s4_gp0_pll_mult_range,

},

.hw.init = &(struct clk_init_data){

.name = "hdmi_pll_dco",

.ops = &meson_clk_pll_ops,

.parent_data = (const struct clk_parent_data []) {

{ .fw_name = "xtal", }

},

.num_parents = 1,

},

};

This is my code right now. Because init_count and init_regs are not
defined, HDMI PLL is not reset. In this way, the kernel will not blink
when the Uboot starts. Then in the kernel stage, if we want to change
the HDMI PLL frequency value, we can directly change M, N and OD. In
fact, we will not change the HDMI PLL frequency value later.

I wonder if you accept this change?



2022-10-22 12:52:02

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver


On Thu 29 Sep 2022 at 15:07, Yu Tu <[email protected]> wrote:

> Hi Jerome,
> Thank you for your reply.
>
> On 2022/9/28 23:27, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>> On Wed 21 Sep 2022 at 16:40, Yu Tu <[email protected]> wrote:
>>
>>> Hi Jerome,
>>>
>>> On 2022/8/30 15:37, Yu Tu wrote:
>>>> On 2022/8/30 14:44, Jerome Brunet wrote:
>>>>> [ EXTERNAL EMAIL ]
>>>>>
>>>>>
>>>>> On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:
>>>>>
>>>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>>>> [ EXTERNAL EMAIL ]
>>>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>>>>>>
>>>>>>>>>>> +
>>>>>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>>>>>> +        .en = {
>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>> +            .shift   = 28,
>>>>>>>>>>> +            .width   = 1,
>>>>>>>>>>> +        },
>>>>>>>>>>> +        .m = {
>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>> +            .shift   = 0,
>>>>>>>>>>> +            .width   = 8,
>>>>>>>>>>> +        },
>>>>>>>>>>> +        .n = {
>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>> +            .shift   = 10,
>>>>>>>>>>> +            .width   = 5,
>>>>>>>>>>> +        },
>>>>>>>>>>> +        .frac = {
>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>>>>>>> +            .shift   = 0,
>>>>>>>>>>> +            .width   = 17,
>>>>>>>>>>> +        },
>>>>>>>>>>> +        .l = {
>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>> +            .shift   = 31,
>>>>>>>>>>> +            .width   = 1,
>>>>>>>>>>> +        },
>>>>>>>>>>> +        .rst = {
>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>> +            .shift   = 29,
>>>>>>>>>>> +            .width   = 1,
>>>>>>>>>>> +        },
>>>>>>>>>>> +    },
>>>>>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>>>>>> +        .name = "hdmi_pll_dco",
>>>>>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>>>>>>> +            { .fw_name = "xtal", }
>>>>>>>>>>> +        },
>>>>>>>>>>> +        .num_parents = 1,
>>>>>>>>>>> +        /*
>>>>>>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>>>>>>> +         * possible
>>>>>>>>>>> +         */
>>>>>>>>>>
>>>>>>>>>> Is it really ?
>>>>>>>>>>
>>>>>>>>>> Given that HDMI support for the s4 is there yet, the
>>>>>>>>>> addresses have changes and the region is no longer a syscon, it is
>>>>>>>>>> time
>>>>>>>>>> for the HDMI driver to get fixed.
>>>>>>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>>>>>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>>>>>>> "ro_ops".
>>>>>>> That's no reason to put NOCACHE or ro-ops
>>>>>>> If you want the frequencies to be statically assinged, the correct way
>>>>>>> would be through assigned-rate in DT I guess.
>>>>>>
>>>>>> Okay. You're right. However, when registering with OPS, HDMI PLL will be
>>>>>> reset. It takes time for PLL to stabilize the output frequency, which
>>>>>> will
>>>>>> lead to the startup screen flashing.
>>>>>>
>>>>>> I would like to know how to solve this problem if not using ro_ops.
>>>>>>
>>>>>>>
>>>>>
>>>>> You can add new ops or tweak the current init function.
>>>> HDMI PLL is not different from other PLLS, so I think adding OPS is
>>>> weird.
>>>>
>>>>>
>>>>> Safest would be to do the following :
>>>>>   * Check if the PLLs is already on.
>>>>>   * Check if the 'pll->init_regs' matches what is already set
>>>>>     - if so, you can skip the reset
>>>>>     - if not, you need to reset as usual
>>>> static int meson_clk_pll_init(struct clk_hw *hw)
>>>> {
>>>>         struct clk_regmap *clk = to_clk_regmap(hw);
>>>>         struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>>>>         if (pll->init_count) {
>>>>                 meson_parm_write(clk->map, &pll->rst, 1);
>>>>                 regmap_multi_reg_write(clk->map, pll->init_regs,
>>>>                                 |      pll->init_count);
>>>>                 meson_parm_write(clk->map, &pll->rst, 0);
>>>>         }
>>>>         return 0;
>>>> }
>>>> Because the init function looks like this. Therefore, HDMI PLL init_count
>>>> is not given.
>> I don't get the remark. You've got pll->init_count right there.
>>
>>>> Can I change it like this?
>> What change ? The function above looks a lot like meson_clk_pll_init()
>> in the actual source
>>
>>>
>>> I don't know if this change meets your requirements? Please give us your
>>> valuable advice.
>> What change ?
>
> static struct clk_regmap s4_hdmi_pll_dco = { .data = &(struct
> meson_clk_pll_data){ .en = {
> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 28,
> .width = 1, }, .m = {
> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 0,
> .width = 8, }, .n = {
> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 10,
> .width = 5, }, .frac = {
> .reg_off = ANACTRL_HDMIPLL_CTRL1, .shift = 0,
> .width = 17, }, .l = {
> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 31,
> .width = 1, }, .rst = {
> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 29,
> .width = 1, }, .range =
> &s4_gp0_pll_mult_range, }, .hw.init = &(struct
> clk_init_data){ .name = "hdmi_pll_dco",
> .ops = &meson_clk_pll_ops, .parent_data = (const struct
> clk_parent_data []) { { .fw_name = "xtal", }
> }, .num_parents = 1, }, };

I'm sorry but I can't read this

>
> This is my code right now. Because init_count and init_regs are not
> defined, HDMI PLL is not reset. In this way, the kernel will not blink when
> the Uboot starts. Then in the kernel stage, if we want to change the HDMI
> PLL frequency value, we can directly change M, N and OD. In fact, we will
> not change the HDMI PLL frequency value later.
>
> I wonder if you accept this change?

I think it would be better to separate this topic from your new SoC support

I think you can make another version of the clock controller for the new
SoC, keeping the pll_ops as it is now.

With another series, you may submit something to avoid reseting the PLL if
is already correctly initialized. This can be useful to all the SoC, not
just the last one. It will be easier to discuss on properly formatted
patches

2022-10-24 12:04:15

by Yu Tu

[permalink] [raw]
Subject: Re: [PATCH V3 3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver

Hi Jerome,
Thank you for your reply.

On 2022/10/22 20:22, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
>
> On Thu 29 Sep 2022 at 15:07, Yu Tu <[email protected]> wrote:
>
>> Hi Jerome,
>> Thank you for your reply.
>>
>> On 2022/9/28 23:27, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Wed 21 Sep 2022 at 16:40, Yu Tu <[email protected]> wrote:
>>>
>>>> Hi Jerome,
>>>>
>>>> On 2022/8/30 15:37, Yu Tu wrote:
>>>>> On 2022/8/30 14:44, Jerome Brunet wrote:
>>>>>> [ EXTERNAL EMAIL ]
>>>>>>
>>>>>>
>>>>>> On Tue 30 Aug 2022 at 14:13, Yu Tu <[email protected]> wrote:
>>>>>>
>>>>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>>>>> [ EXTERNAL EMAIL ]
>>>>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <[email protected]> wrote:
>>>>>>>>
>>>>>>>>>>>> +
>>>>>>>>>>>> +static struct clk_regmap s4_hdmi_pll_dco = {
>>>>>>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>>>>>>> +        .en = {
>>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>>> +            .shift   = 28,
>>>>>>>>>>>> +            .width   = 1,
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +        .m = {
>>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>>> +            .shift   = 0,
>>>>>>>>>>>> +            .width   = 8,
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +        .n = {
>>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>>> +            .shift   = 10,
>>>>>>>>>>>> +            .width   = 5,
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +        .frac = {
>>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL1,
>>>>>>>>>>>> +            .shift   = 0,
>>>>>>>>>>>> +            .width   = 17,
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +        .l = {
>>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>>> +            .shift   = 31,
>>>>>>>>>>>> +            .width   = 1,
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +        .rst = {
>>>>>>>>>>>> +            .reg_off = ANACTRL_HDMIPLL_CTRL0,
>>>>>>>>>>>> +            .shift   = 29,
>>>>>>>>>>>> +            .width   = 1,
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +    },
>>>>>>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>>>>>>> +        .name = "hdmi_pll_dco",
>>>>>>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>>>>>>> +        .parent_data = (const struct clk_parent_data []) {
>>>>>>>>>>>> +            { .fw_name = "xtal", }
>>>>>>>>>>>> +        },
>>>>>>>>>>>> +        .num_parents = 1,
>>>>>>>>>>>> +        /*
>>>>>>>>>>>> +         * Display directly handle hdmi pll registers ATM, we need
>>>>>>>>>>>> +         * NOCACHE to keep our view of the clock as accurate as
>>>>>>>>>>>> +         * possible
>>>>>>>>>>>> +         */
>>>>>>>>>>>
>>>>>>>>>>> Is it really ?
>>>>>>>>>>>
>>>>>>>>>>> Given that HDMI support for the s4 is there yet, the
>>>>>>>>>>> addresses have changes and the region is no longer a syscon, it is
>>>>>>>>>>> time
>>>>>>>>>>> for the HDMI driver to get fixed.
>>>>>>>>> The HDMI PLL is configured in the Uboot phase and does not change the
>>>>>>>>> frequency in the kernel phase. So we use the NOCACHE flag and
>>>>>>>>> "ro_ops".
>>>>>>>> That's no reason to put NOCACHE or ro-ops
>>>>>>>> If you want the frequencies to be statically assinged, the correct way
>>>>>>>> would be through assigned-rate in DT I guess.
>>>>>>>
>>>>>>> Okay. You're right. However, when registering with OPS, HDMI PLL will be
>>>>>>> reset. It takes time for PLL to stabilize the output frequency, which
>>>>>>> will
>>>>>>> lead to the startup screen flashing.
>>>>>>>
>>>>>>> I would like to know how to solve this problem if not using ro_ops.
>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> You can add new ops or tweak the current init function.
>>>>> HDMI PLL is not different from other PLLS, so I think adding OPS is
>>>>> weird.
>>>>>
>>>>>>
>>>>>> Safest would be to do the following :
>>>>>>   * Check if the PLLs is already on.
>>>>>>   * Check if the 'pll->init_regs' matches what is already set
>>>>>>     - if so, you can skip the reset
>>>>>>     - if not, you need to reset as usual
>>>>> static int meson_clk_pll_init(struct clk_hw *hw)
>>>>> {
>>>>>         struct clk_regmap *clk = to_clk_regmap(hw);
>>>>>         struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>>>>>         if (pll->init_count) {
>>>>>                 meson_parm_write(clk->map, &pll->rst, 1);
>>>>>                 regmap_multi_reg_write(clk->map, pll->init_regs,
>>>>>                                 |      pll->init_count);
>>>>>                 meson_parm_write(clk->map, &pll->rst, 0);
>>>>>         }
>>>>>         return 0;
>>>>> }
>>>>> Because the init function looks like this. Therefore, HDMI PLL init_count
>>>>> is not given.
>>> I don't get the remark. You've got pll->init_count right there.
>>>
>>>>> Can I change it like this?
>>> What change ? The function above looks a lot like meson_clk_pll_init()
>>> in the actual source
>>>
>>>>
>>>> I don't know if this change meets your requirements? Please give us your
>>>> valuable advice.
>>> What change ?
>>
>> static struct clk_regmap s4_hdmi_pll_dco = { .data = &(struct
>> meson_clk_pll_data){ .en = {
>> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 28,
>> .width = 1, }, .m = {
>> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 0,
>> .width = 8, }, .n = {
>> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 10,
>> .width = 5, }, .frac = {
>> .reg_off = ANACTRL_HDMIPLL_CTRL1, .shift = 0,
>> .width = 17, }, .l = {
>> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 31,
>> .width = 1, }, .rst = {
>> .reg_off = ANACTRL_HDMIPLL_CTRL0, .shift = 29,
>> .width = 1, }, .range =
>> &s4_gp0_pll_mult_range, }, .hw.init = &(struct
>> clk_init_data){ .name = "hdmi_pll_dco",
>> .ops = &meson_clk_pll_ops, .parent_data = (const struct
>> clk_parent_data []) { { .fw_name = "xtal", }
>> }, .num_parents = 1, }, };
>
> I'm sorry but I can't read this
>
>>
>> This is my code right now. Because init_count and init_regs are not
>> defined, HDMI PLL is not reset. In this way, the kernel will not blink when
>> the Uboot starts. Then in the kernel stage, if we want to change the HDMI
>> PLL frequency value, we can directly change M, N and OD. In fact, we will
>> not change the HDMI PLL frequency value later.
>>
>> I wonder if you accept this change?

Sorry, the code is not formatted properly.

>
> I think it would be better to separate this topic from your new SoC support
>
> I think you can make another version of the clock controller for the new
> SoC, keeping the pll_ops as it is now.
>
> With another series, you may submit something to avoid reseting the PLL if
> is already correctly initialized. This can be useful to all the SoC, not
> just the last one. It will be easier to discuss on properly formatted
> patches

Do you mean that hdmi_pll, I will submit the next version of code
according to pll_ro_ops first, and then we will discuss how to reject
the patch of reset pll in another topic?