| Message ID | 20240510090933.19464-1-ddrokosov@salutedevices.com |
|---|---|
| Headers | show |
| Series | clk: meson: introduce Amlogic A1 SoC Family CPU clock controller driver | expand |
On Fri 10 May 2024 at 12:08, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > When dealing with certain PLLs, it is necessary to avoid modifying them > if they have already been initialized by lower levels. For instance, in > the A1 SoC Family, the sys_pll is enabled as the parent for the cpuclk, > and it cannot be disabled during the initialization sequence. Therefore, > initialization phase must be skipped. > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > --- > drivers/clk/meson/clk-pll.c | 37 +++++++++++++++++++++---------------- > drivers/clk/meson/clk-pll.h | 1 + > 2 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index 78d17b2415af..47b22a6be2e4 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -289,11 +289,32 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) > return -ETIMEDOUT; > } > > +static int meson_clk_pll_is_enabled(struct clk_hw *hw) > +{ > + struct clk_regmap *clk = to_clk_regmap(hw); > + struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > + > + if (MESON_PARM_APPLICABLE(&pll->rst) && > + meson_parm_read(clk->map, &pll->rst)) > + return 0; > + > + if (!meson_parm_read(clk->map, &pll->en) || > + !meson_parm_read(clk->map, &pll->l)) > + return 0; > + > + return 1; > +} > + > 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); > > + /* Do not init already enabled PLL which marked with 'init_once' > */ That is decribing the code, which we can read. So not really helpful Saying why you do it, like "Keep the clock running from the bootloader stage and avoid glitching it ..." gives more context about what you are trying to do. > + if ((pll->flags & CLK_MESON_PLL_INIT_ONCE) && I don't like INIT_ONCE. It gives the false impression that * The PLL is going to be initialized once in Linux if it has the flag * Is initialised multiple times otherwise I agree that currently that carefully reading the code clears that up but it is misleading CLK_MESON_PLL_EN_NOINIT ? > + meson_clk_pll_is_enabled(hw)) > + return 0; > + > if (pll->init_count) { > if (MESON_PARM_APPLICABLE(&pll->rst)) > meson_parm_write(clk->map, &pll->rst, 1); > @@ -308,22 +329,6 @@ static int meson_clk_pll_init(struct clk_hw *hw) > return 0; > } > > -static int meson_clk_pll_is_enabled(struct clk_hw *hw) > -{ > - struct clk_regmap *clk = to_clk_regmap(hw); > - struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > - > - if (MESON_PARM_APPLICABLE(&pll->rst) && > - meson_parm_read(clk->map, &pll->rst)) > - return 0; > - > - if (!meson_parm_read(clk->map, &pll->en) || > - !meson_parm_read(clk->map, &pll->l)) > - return 0; > - > - return 1; > -} > - > static int meson_clk_pcie_pll_enable(struct clk_hw *hw) > { > int retries = 10; > diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h > index a2228c0fdce5..23195ea4eae1 100644 > --- a/drivers/clk/meson/clk-pll.h > +++ b/drivers/clk/meson/clk-pll.h > @@ -28,6 +28,7 @@ struct pll_mult_range { > } > > #define CLK_MESON_PLL_ROUND_CLOSEST BIT(0) > +#define CLK_MESON_PLL_INIT_ONCE BIT(1) > > struct meson_clk_pll_data { > struct parm en;
On Fri 10 May 2024 at 12:08, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > The 'syspll' PLL, also known as the system PLL, is a general and > essential PLL responsible for generating the CPU clock frequency. > With its wide-ranging capabilities, it is designed to accommodate > frequencies within the range of 768MHz to 1536MHz. > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > --- > drivers/clk/meson/a1-pll.c | 79 ++++++++++++++++++++++++++++++++++++++ > drivers/clk/meson/a1-pll.h | 6 +++ > 2 files changed, 85 insertions(+) > > diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c > index 60b2e53e7e51..af47ba308bbe 100644 > --- a/drivers/clk/meson/a1-pll.c > +++ b/drivers/clk/meson/a1-pll.c > @@ -138,6 +138,82 @@ static struct clk_regmap hifi_pll = { > }, > }; > > +static const struct pll_mult_range sys_pll_mult_range = { > + .min = 32, > + .max = 64, > +}; > + > +static const struct reg_sequence sys_pll_init_regs[] = { > + { .reg = ANACTRL_SYSPLL_CTRL1, .def = 0x01800000 }, > + { .reg = ANACTRL_SYSPLL_CTRL2, .def = 0x00001100 }, > + { .reg = ANACTRL_SYSPLL_CTRL3, .def = 0x10022300 }, > + { .reg = ANACTRL_SYSPLL_CTRL4, .def = 0x00300000 }, > + { .reg = ANACTRL_SYSPLL_CTRL0, .def = 0x01f18432 }, > +}; > + > +static struct clk_regmap sys_pll = { > + .data = &(struct meson_clk_pll_data){ > + .en = { > + .reg_off = ANACTRL_SYSPLL_CTRL0, > + .shift = 28, > + .width = 1, > + }, > + .m = { > + .reg_off = ANACTRL_SYSPLL_CTRL0, > + .shift = 0, > + .width = 8, > + }, > + .n = { > + .reg_off = ANACTRL_SYSPLL_CTRL0, > + .shift = 10, > + .width = 5, > + }, > + .frac = { > + .reg_off = ANACTRL_SYSPLL_CTRL1, > + .shift = 0, > + .width = 19, > + }, > + .l = { > + .reg_off = ANACTRL_SYSPLL_STS, > + .shift = 31, > + .width = 1, > + }, > + .current_en = { > + .reg_off = ANACTRL_SYSPLL_CTRL0, > + .shift = 26, > + .width = 1, > + }, > + .l_detect = { > + .reg_off = ANACTRL_SYSPLL_CTRL2, > + .shift = 6, > + .width = 1, > + }, > + .range = &sys_pll_mult_range, > + .init_regs = sys_pll_init_regs, > + .init_count = ARRAY_SIZE(sys_pll_init_regs), Like other 'fishy' flags, I would like a clear comment why this flag is required so, 2y from now, we will know why it was put there and how we can deal with it. > + .flags = CLK_MESON_PLL_INIT_ONCE, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "sys_pll", > + .ops = &meson_clk_pll_ops, > + .parent_names = (const char *[]){ "syspll_in" }, > + .num_parents = 1, > + }, > +}; > + > +static struct clk_fixed_factor sys_pll_div16 = { > + .mult = 1, > + .div = 16, > + .hw.init = &(struct clk_init_data){ > + .name = "sys_pll_div16", > + .ops = &clk_fixed_factor_ops, > + .parent_hws = (const struct clk_hw *[]) { > + &sys_pll.hw > + }, > + .num_parents = 1, > + }, > +}; Unlike the fdivs, this fixed divider is not part of the diagram describing the syspll clock. IMO, it could as well be in peripheral controller because it exists (from what I can see) just testing purposes, to make the sys pll observable through tst_out or gen_clk. It also looks less awkward in the bindings. > + > static struct clk_fixed_factor fclk_div2_div = { > .mult = 1, > .div = 2, > @@ -283,6 +359,8 @@ static struct clk_hw *a1_pll_hw_clks[] = { > [CLKID_FCLK_DIV5] = &fclk_div5.hw, > [CLKID_FCLK_DIV7] = &fclk_div7.hw, > [CLKID_HIFI_PLL] = &hifi_pll.hw, > + [CLKID_SYS_PLL] = &sys_pll.hw, > + [CLKID_SYS_PLL_DIV16] = &sys_pll_div16.hw, > }; > > static struct clk_regmap *const a1_pll_regmaps[] = { > @@ -293,6 +371,7 @@ static struct clk_regmap *const a1_pll_regmaps[] = { > &fclk_div5, > &fclk_div7, > &hifi_pll, > + &sys_pll, > }; > > static struct regmap_config a1_pll_regmap_cfg = { > diff --git a/drivers/clk/meson/a1-pll.h b/drivers/clk/meson/a1-pll.h > index 4be17b2bf383..666d9b2137e9 100644 > --- a/drivers/clk/meson/a1-pll.h > +++ b/drivers/clk/meson/a1-pll.h > @@ -18,6 +18,12 @@ > #define ANACTRL_FIXPLL_CTRL0 0x0 > #define ANACTRL_FIXPLL_CTRL1 0x4 > #define ANACTRL_FIXPLL_STS 0x14 > +#define ANACTRL_SYSPLL_CTRL0 0x80 > +#define ANACTRL_SYSPLL_CTRL1 0x84 > +#define ANACTRL_SYSPLL_CTRL2 0x88 > +#define ANACTRL_SYSPLL_CTRL3 0x8c > +#define ANACTRL_SYSPLL_CTRL4 0x90 > +#define ANACTRL_SYSPLL_STS 0x94 > #define ANACTRL_HIFIPLL_CTRL0 0xc0 > #define ANACTRL_HIFIPLL_CTRL1 0xc4 > #define ANACTRL_HIFIPLL_CTRL2 0xc8
On Mon, May 13, 2024 at 02:48:58PM +0200, Jerome Brunet wrote: > > On Fri 10 May 2024 at 12:08, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > > > The 'syspll' PLL, also known as the system PLL, is a general and > > essential PLL responsible for generating the CPU clock frequency. > > With its wide-ranging capabilities, it is designed to accommodate > > frequencies within the range of 768MHz to 1536MHz. > > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > --- > > drivers/clk/meson/a1-pll.c | 79 ++++++++++++++++++++++++++++++++++++++ > > drivers/clk/meson/a1-pll.h | 6 +++ > > 2 files changed, 85 insertions(+) > > > > diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c > > index 60b2e53e7e51..af47ba308bbe 100644 > > --- a/drivers/clk/meson/a1-pll.c > > +++ b/drivers/clk/meson/a1-pll.c > > @@ -138,6 +138,82 @@ static struct clk_regmap hifi_pll = { > > }, > > }; > > > > +static const struct pll_mult_range sys_pll_mult_range = { > > + .min = 32, > > + .max = 64, > > +}; > > + > > +static const struct reg_sequence sys_pll_init_regs[] = { > > + { .reg = ANACTRL_SYSPLL_CTRL1, .def = 0x01800000 }, > > + { .reg = ANACTRL_SYSPLL_CTRL2, .def = 0x00001100 }, > > + { .reg = ANACTRL_SYSPLL_CTRL3, .def = 0x10022300 }, > > + { .reg = ANACTRL_SYSPLL_CTRL4, .def = 0x00300000 }, > > + { .reg = ANACTRL_SYSPLL_CTRL0, .def = 0x01f18432 }, > > +}; > > + > > +static struct clk_regmap sys_pll = { > > + .data = &(struct meson_clk_pll_data){ > > + .en = { > > + .reg_off = ANACTRL_SYSPLL_CTRL0, > > + .shift = 28, > > + .width = 1, > > + }, > > + .m = { > > + .reg_off = ANACTRL_SYSPLL_CTRL0, > > + .shift = 0, > > + .width = 8, > > + }, > > + .n = { > > + .reg_off = ANACTRL_SYSPLL_CTRL0, > > + .shift = 10, > > + .width = 5, > > + }, > > + .frac = { > > + .reg_off = ANACTRL_SYSPLL_CTRL1, > > + .shift = 0, > > + .width = 19, > > + }, > > + .l = { > > + .reg_off = ANACTRL_SYSPLL_STS, > > + .shift = 31, > > + .width = 1, > > + }, > > + .current_en = { > > + .reg_off = ANACTRL_SYSPLL_CTRL0, > > + .shift = 26, > > + .width = 1, > > + }, > > + .l_detect = { > > + .reg_off = ANACTRL_SYSPLL_CTRL2, > > + .shift = 6, > > + .width = 1, > > + }, > > + .range = &sys_pll_mult_range, > > + .init_regs = sys_pll_init_regs, > > + .init_count = ARRAY_SIZE(sys_pll_init_regs), > > Like other 'fishy' flags, I would like a clear comment why this flag is > required so, 2y from now, we will know why it was put there and how we > can deal with it. > Yep, you are totally correct. The proper comment is required for that. > > + .flags = CLK_MESON_PLL_INIT_ONCE, > > + }, > > + .hw.init = &(struct clk_init_data){ > > + .name = "sys_pll", > > + .ops = &meson_clk_pll_ops, > > + .parent_names = (const char *[]){ "syspll_in" }, > > + .num_parents = 1, > > + }, > > +}; > > + > > +static struct clk_fixed_factor sys_pll_div16 = { > > + .mult = 1, > > + .div = 16, > > + .hw.init = &(struct clk_init_data){ > > + .name = "sys_pll_div16", > > + .ops = &clk_fixed_factor_ops, > > + .parent_hws = (const struct clk_hw *[]) { > > + &sys_pll.hw > > + }, > > + .num_parents = 1, > > + }, > > +}; > > Unlike the fdivs, this fixed divider is not part of the diagram > describing the syspll clock. > > IMO, it could as well be in peripheral controller because it exists > (from what I can see) just testing purposes, to make the sys pll > observable through tst_out or gen_clk. > > It also looks less awkward in the bindings. > In any case, it is necessary to introduce a new connection. Instead of using 'sys_pll_div16', it will now be called 'sys_pll'. I agree with you that this change will make the code more elegant. > > + > > static struct clk_fixed_factor fclk_div2_div = { > > .mult = 1, > > .div = 2, > > @@ -283,6 +359,8 @@ static struct clk_hw *a1_pll_hw_clks[] = { > > [CLKID_FCLK_DIV5] = &fclk_div5.hw, > > [CLKID_FCLK_DIV7] = &fclk_div7.hw, > > [CLKID_HIFI_PLL] = &hifi_pll.hw, > > + [CLKID_SYS_PLL] = &sys_pll.hw, > > + [CLKID_SYS_PLL_DIV16] = &sys_pll_div16.hw, > > }; > > > > static struct clk_regmap *const a1_pll_regmaps[] = { > > @@ -293,6 +371,7 @@ static struct clk_regmap *const a1_pll_regmaps[] = { > > &fclk_div5, > > &fclk_div7, > > &hifi_pll, > > + &sys_pll, > > }; > > > > static struct regmap_config a1_pll_regmap_cfg = { > > diff --git a/drivers/clk/meson/a1-pll.h b/drivers/clk/meson/a1-pll.h > > index 4be17b2bf383..666d9b2137e9 100644 > > --- a/drivers/clk/meson/a1-pll.h > > +++ b/drivers/clk/meson/a1-pll.h > > @@ -18,6 +18,12 @@ > > #define ANACTRL_FIXPLL_CTRL0 0x0 > > #define ANACTRL_FIXPLL_CTRL1 0x4 > > #define ANACTRL_FIXPLL_STS 0x14 > > +#define ANACTRL_SYSPLL_CTRL0 0x80 > > +#define ANACTRL_SYSPLL_CTRL1 0x84 > > +#define ANACTRL_SYSPLL_CTRL2 0x88 > > +#define ANACTRL_SYSPLL_CTRL3 0x8c > > +#define ANACTRL_SYSPLL_CTRL4 0x90 > > +#define ANACTRL_SYSPLL_STS 0x94 > > #define ANACTRL_HIFIPLL_CTRL0 0xc0 > > #define ANACTRL_HIFIPLL_CTRL1 0xc4 > > #define ANACTRL_HIFIPLL_CTRL2 0xc8 > > > -- > Jerome
On Mon, May 13, 2024 at 02:44:06PM +0200, Jerome Brunet wrote: > > On Fri 10 May 2024 at 12:08, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: > > > When dealing with certain PLLs, it is necessary to avoid modifying them > > if they have already been initialized by lower levels. For instance, in > > the A1 SoC Family, the sys_pll is enabled as the parent for the cpuclk, > > and it cannot be disabled during the initialization sequence. Therefore, > > initialization phase must be skipped. > > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > --- > > drivers/clk/meson/clk-pll.c | 37 +++++++++++++++++++++---------------- > > drivers/clk/meson/clk-pll.h | 1 + > > 2 files changed, 22 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > > index 78d17b2415af..47b22a6be2e4 100644 > > --- a/drivers/clk/meson/clk-pll.c > > +++ b/drivers/clk/meson/clk-pll.c > > @@ -289,11 +289,32 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) > > return -ETIMEDOUT; > > } > > > > +static int meson_clk_pll_is_enabled(struct clk_hw *hw) > > +{ > > + struct clk_regmap *clk = to_clk_regmap(hw); > > + struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > > + > > + if (MESON_PARM_APPLICABLE(&pll->rst) && > > + meson_parm_read(clk->map, &pll->rst)) > > + return 0; > > + > > + if (!meson_parm_read(clk->map, &pll->en) || > > + !meson_parm_read(clk->map, &pll->l)) > > + return 0; > > + > > + return 1; > > +} > > + > > 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); > > > > + /* Do not init already enabled PLL which marked with 'init_once' > > */ > > That is decribing the code, which we can read. So not really helpful > Saying why you do it, like "Keep the clock running from the bootloader > stage and avoid glitching it ..." gives more context about what you are > trying to do. > Yes, I agree with you. "Instead of describing the action, provide the reasoning behind it." I will incorporate your feedback in the upcoming version. > > + if ((pll->flags & CLK_MESON_PLL_INIT_ONCE) && > > I don't like INIT_ONCE. It gives the false impression that > > * The PLL is going to be initialized once in Linux if it has the flag > * Is initialised multiple times otherwise But that's how things happen. For previous clocks on other platforms, we assumed that the PLL could be initialized multiple times: once from the bootloader and once from Linux. We didn't have the ability to disable initialization from the Linux side before, so it meant that multiple initializations were potentially possible by default. > > I agree that currently that carefully reading the code clears that up > but it is misleading > > CLK_MESON_PLL_EN_NOINIT ? > I have been considering this name and its derivatives, such as: CLK_MESON_PLL_SKIP_ENABLED CLK_MESON_PLL_NOINIT_ENABLED CLK_MESON_PLL_INIT_DISABLED_ONLY However, I find all of these names to be quite long and bulky. It reminded me of the WARN_ONCE() function, which ensures that a warning message is only printed once. In my opinion, the name "INIT_ONCE" accurately reflects the situation. Nevertheless, if it is your requirement for me to change the flag name, I am more than willing to do so, it's not a problem. > > + meson_clk_pll_is_enabled(hw)) > > + return 0; > > + > > if (pll->init_count) { > > if (MESON_PARM_APPLICABLE(&pll->rst)) > > meson_parm_write(clk->map, &pll->rst, 1); > > @@ -308,22 +329,6 @@ static int meson_clk_pll_init(struct clk_hw *hw) > > return 0; > > } > > > > -static int meson_clk_pll_is_enabled(struct clk_hw *hw) > > -{ > > - struct clk_regmap *clk = to_clk_regmap(hw); > > - struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > > - > > - if (MESON_PARM_APPLICABLE(&pll->rst) && > > - meson_parm_read(clk->map, &pll->rst)) > > - return 0; > > - > > - if (!meson_parm_read(clk->map, &pll->en) || > > - !meson_parm_read(clk->map, &pll->l)) > > - return 0; > > - > > - return 1; > > -} > > - > > static int meson_clk_pcie_pll_enable(struct clk_hw *hw) > > { > > int retries = 10; > > diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h > > index a2228c0fdce5..23195ea4eae1 100644 > > --- a/drivers/clk/meson/clk-pll.h > > +++ b/drivers/clk/meson/clk-pll.h > > @@ -28,6 +28,7 @@ struct pll_mult_range { > > } > > > > #define CLK_MESON_PLL_ROUND_CLOSEST BIT(0) > > +#define CLK_MESON_PLL_INIT_ONCE BIT(1) > > > > struct meson_clk_pll_data { > > struct parm en; > > > -- > Jerome
On Tue 14 May 2024 at 00:47, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote: >> >> I agree that currently that carefully reading the code clears that up >> but it is misleading >> >> CLK_MESON_PLL_EN_NOINIT ? >> > > I have been considering this name and its derivatives, such as: > > CLK_MESON_PLL_SKIP_ENABLED > CLK_MESON_PLL_NOINIT_ENABLED That one accurately describes what you do. Use this one please
The CPU clock controller plays a general role in the Amlogic A1 SoC family by generating CPU clocks. As an APB slave module, it offers the capability to inherit the CPU clock from two sources: the internal fixed clock known as 'cpu fixed clock' and the external input provided by the A1 PLL clock controller, referred to as 'syspll'. It is important for the driver to handle the cpu_clk rate switching effectively by transitioning to the CPU fixed clock to avoid any potential execution freezes. Validation: * to double-check all clk flags, run the below helper script: ``` pushd /sys/kernel/debug/clk for f in *; do if [[ -f "$f/clk_flags" ]]; then flags="$(cat $f/clk_flags | awk '{$1=$1};1' | sed ':a;N;$!ba;s/\n/ | /g')" echo -e "$f: $flags" fi done popd ``` * to trace the current clks state, use the '/sys/kernel/debug/clk/clk_dump' node with jq post-processing: ``` $ cat /sys/kernel/debug/clk/clk_dump | jq '.' > clk_dump.json ``` * to see the CPU clock hierarchy, use the '/sys/kernel/debug/clk/clk_summary' node with jq post-processing: ``` $ cat /sys/kernel/debug/clk/clk_summary | jq '.' > clk_dump.json ``` when cpu_clk is inherited from sys_pll, it should be: ``` syspll_in 1 1 0 24000000 0 0 50000 Y deviceless no_connection_id sys_pll 2 2 0 1200000000 0 0 50000 Y deviceless no_connection_id cpu_clk 1 1 0 1200000000 0 0 50000 Y cpu0 no_connection_id cpu0 no_connection_id fd000000.clock-controller dvfs deviceless no_connection_id ``` and from cpu fixed clock: ``` fclk_div3_div 1 1 0 512000000 0 0 50000 Y deviceless no_connection_id fclk_div3 4 4 0 512000000 0 0 50000 Y deviceless no_connection_id cpu_fsource_sel0 1 1 0 512000000 0 0 50000 Y deviceless no_connection_id cpu_fsource_div0 1 1 0 128000000 0 0 50000 Y deviceless no_connection_id cpu_fsel0 1 1 0 128000000 0 0 50000 Y deviceless no_connection_id cpu_fclk 1 1 0 128000000 0 0 50000 Y deviceless no_connection_id cpu_clk 1 1 0 128000000 0 0 50000 Y cpu0 no_connection_id cpu0 no_connection_id fd000000.clock-controller dvfs deviceless no_connection_id ``` * to debug cpu clk rate propagation and proper parent switching, compile kernel with the following definition: $ sed -i "s/undef CLOCK_ALLOW_WRITE_DEBUGFS/define CLOCK_ALLOW_WRITE_DEBUGFS/g" drivers/clk/clk.c after that, clk_rate debug node for each clock will be available for write operation Changes v2 since v1 at [1]: - introduce new 'INIT_ONCE' flag to eliminate init for already enabled PLL - explain why we need to break ABI for a1-pll driver by adding sys_pll connections - implement sys_pll init sequence, which is applicable when sys_pll is disabled - remove CLK_IS_CRITICAL from sys_pll - move sys_pll_div16 binding to the end per Rob's suggestion - add Rob's RvB - remove holes from the beginning of the cpu clock controller regmap - move a1-cpu.h registers offsets definition to a1-cpu.c - set CLK_SET_RATE_GATE for parallel cpu fixed clock source trees per Martin's and Jerome's suggestion - redesign clock notifier block from cpu_clk to sys_pll to keep cpu_clock working continuously (the same implementation is located in the g12a clock driver) Links: [1] https://lore.kernel.org/all/20240329205904.25002-1-ddrokosov@salutedevices.com/ Dmitry Rokosov (7): clk: meson: introduce 'INIT_ONCE' flag to eliminate init for enabled PLL dt-bindings: clock: meson: a1: pll: introduce new syspll bindings clk: meson: a1: pll: support 'syspll' general-purpose PLL for CPU clock dt-bindings: clock: meson: a1: peripherals: support sys_pll_div16 input clk: meson: a1: peripherals: support 'sys_pll_div16' clock as GEN input dt-bindings: clock: meson: add A1 CPU clock controller bindings clk: meson: a1: add Amlogic A1 CPU clock controller driver .../bindings/clock/amlogic,a1-cpu-clkc.yaml | 64 ++++ .../clock/amlogic,a1-peripherals-clkc.yaml | 7 +- .../bindings/clock/amlogic,a1-pll-clkc.yaml | 7 +- drivers/clk/meson/Kconfig | 10 + drivers/clk/meson/Makefile | 1 + drivers/clk/meson/a1-cpu.c | 331 ++++++++++++++++++ drivers/clk/meson/a1-peripherals.c | 4 +- drivers/clk/meson/a1-pll.c | 79 +++++ drivers/clk/meson/a1-pll.h | 6 + drivers/clk/meson/clk-pll.c | 37 +- drivers/clk/meson/clk-pll.h | 1 + .../dt-bindings/clock/amlogic,a1-cpu-clkc.h | 19 + .../dt-bindings/clock/amlogic,a1-pll-clkc.h | 2 + 13 files changed, 546 insertions(+), 22 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-cpu-clkc.yaml create mode 100644 drivers/clk/meson/a1-cpu.c create mode 100644 include/dt-bindings/clock/amlogic,a1-cpu-clkc.h