diff mbox series

[V3,3/6] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver

Message ID 20220805085716.5635-4-yu.tu@amlogic.com (mailing list archive)
State Superseded
Delegated to: Neil Armstrong
Headers show
Series Add S4 SoC PLL and Peripheral clock controller | expand

Commit Message

Yu Tu Aug. 5, 2022, 8:57 a.m. UTC
Add the S4 PLL clock controller found in the s4 SoC family.

Signed-off-by: Yu Tu <yu.tu@amlogic.com>
---
 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

Comments

Jerome Brunet Aug. 10, 2022, 1:47 p.m. UTC | #1
On Fri 05 Aug 2022 at 16:57, Yu Tu <yu.tu@amlogic.com> wrote:

> Add the S4 PLL clock controller found in the s4 SoC family.
>
> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> ---
>  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 <yu.tu@amlogic.com>
> + */
> +
> +#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 <yu.tu@amlogic.com>
> + */
> +
> +#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__ */
Yu Tu Aug. 15, 2022, 6:34 a.m. UTC | #2
Hi Jerome,

On 2022/8/10 21:47, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> 
> On Fri 05 Aug 2022 at 16:57, Yu Tu <yu.tu@amlogic.com> wrote:
> 
>> Add the S4 PLL clock controller found in the s4 SoC family.
>>
>> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
>> ---
>>   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 <yu.tu@amlogic.com>
>> + */
>> +
>> +#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 <yu.tu@amlogic.com>
>> + */
>> +
>> +#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__ */
> 
> .
Yu Tu Aug. 15, 2022, 1:20 p.m. UTC | #3
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 <yu.tu@amlogic.com> wrote:
>>
>>> Add the S4 PLL clock controller found in the s4 SoC family.
>>>
>>> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
>>> ---
>>>   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 <yu.tu@amlogic.com>
>>> + */
>>> +
>>> +#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 <yu.tu@amlogic.com>
>>> + */
>>> +
>>> +#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__ */
>>
>> .
Jerome Brunet Aug. 29, 2022, 9:46 a.m. UTC | #4
On Mon 15 Aug 2022 at 14:34, Yu Tu <yu.tu@amlogic.com> wrote:

> Hi Jerome,
>
> On 2022/8/10 21:47, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>> On Fri 05 Aug 2022 at 16:57, Yu Tu <yu.tu@amlogic.com> 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
Jerome Brunet Aug. 29, 2022, 9:48 a.m. UTC | #5
On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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.
Yu Tu Aug. 30, 2022, 6:08 a.m. UTC | #6
On 2022/8/29 17:46, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> 
> On Mon 15 Aug 2022 at 14:34, Yu Tu <yu.tu@amlogic.com> wrote:
> 
>> Hi Jerome,
>>
>> On 2022/8/10 21:47, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Fri 05 Aug 2022 at 16:57, Yu Tu <yu.tu@amlogic.com> 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?

> 
> 
> .
Yu Tu Aug. 30, 2022, 6:13 a.m. UTC | #7
On 2022/8/29 17:48, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> 
> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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.

>
Jerome Brunet Aug. 30, 2022, 6:44 a.m. UTC | #8
On Tue 30 Aug 2022 at 14:13, Yu Tu <yu.tu@amlogic.com> wrote:

> On 2022/8/29 17:48, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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
Yu Tu Aug. 30, 2022, 7:37 a.m. UTC | #9
On 2022/8/30 14:44, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> 
> On Tue 30 Aug 2022 at 14:13, Yu Tu <yu.tu@amlogic.com> wrote:
> 
>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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?
Yu Tu Sept. 21, 2022, 8:40 a.m. UTC | #10
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 <yu.tu@amlogic.com> wrote:
>>
>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>> [ EXTERNAL EMAIL ]
>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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.
Jerome Brunet Sept. 28, 2022, 3:27 p.m. UTC | #11
On Wed 21 Sep 2022 at 16:40, Yu Tu <yu.tu@amlogic.com> 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 <yu.tu@amlogic.com> wrote:
>>>
>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>> [ EXTERNAL EMAIL ]
>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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 ?
Yu Tu Sept. 29, 2022, 7:07 a.m. UTC | #12
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 <yu.tu@amlogic.com> 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 <yu.tu@amlogic.com> wrote:
>>>>
>>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>>> [ EXTERNAL EMAIL ]
>>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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?
Jerome Brunet Oct. 22, 2022, 12:22 p.m. UTC | #13
On Thu 29 Sep 2022 at 15:07, Yu Tu <yu.tu@amlogic.com> 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 <yu.tu@amlogic.com> 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 <yu.tu@amlogic.com> wrote:
>>>>>
>>>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>>>> [ EXTERNAL EMAIL ]
>>>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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
Yu Tu Oct. 24, 2022, 11:33 a.m. UTC | #14
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 <yu.tu@amlogic.com> 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 <yu.tu@amlogic.com> 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 <yu.tu@amlogic.com> wrote:
>>>>>>
>>>>>>> On 2022/8/29 17:48, Jerome Brunet wrote:
>>>>>>>> [ EXTERNAL EMAIL ]
>>>>>>>> On Mon 15 Aug 2022 at 21:20, Yu Tu <yu.tu@amlogic.com> 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?
diff mbox series

Patch

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 <yu.tu@amlogic.com>
+ */
+
+#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 <yu.tu@amlogic.com>
+ */
+
+#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__ */