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 <[email protected]>
---
drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/a1-pll.h | 6 +++
2 files changed, 84 insertions(+)
diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
index 60b2e53e7e51..02fd2d325cc6 100644
--- a/drivers/clk/meson/a1-pll.c
+++ b/drivers/clk/meson/a1-pll.c
@@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
},
};
+static const struct pll_mult_range sys_pll_mult_range = {
+ .min = 32,
+ .max = 64,
+};
+
+/*
+ * We assume that the sys_pll_clk has already been set up by the low-level
+ * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
+ * run the initialization sequence.
+ */
+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,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "sys_pll",
+ .ops = &meson_clk_pll_ops,
+ .parent_names = (const char *[]){ "syspll_in" },
+ .num_parents = 1,
+ /*
+ * This clock is used as the main CPU PLL source in low-level
+ * bootloaders, and it is necessary to mark it as critical.
+ */
+ .flags = CLK_IS_CRITICAL,
+ },
+};
+
+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,
+ },
+};
+
static struct clk_fixed_factor fclk_div2_div = {
.mult = 1,
.div = 2,
@@ -283,6 +358,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 +370,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
--
2.43.0
On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <[email protected]> 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 <[email protected]>
> ---
> drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/a1-pll.h | 6 +++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
> index 60b2e53e7e51..02fd2d325cc6 100644
> --- a/drivers/clk/meson/a1-pll.c
> +++ b/drivers/clk/meson/a1-pll.c
> @@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
> },
> };
>
> +static const struct pll_mult_range sys_pll_mult_range = {
> + .min = 32,
> + .max = 64,
> +};
> +
> +/*
> + * We assume that the sys_pll_clk has already been set up by the low-level
> + * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
> + * run the initialization sequence.
> + */
I see no reason to make such assumption.
This clock is no read-only, it apparently is able to re-lock so assuming
anything from the bootloader is just asking from trouble
> +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,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sys_pll",
> + .ops = &meson_clk_pll_ops,
> + .parent_names = (const char *[]){ "syspll_in" },
> + .num_parents = 1,
> + /*
> + * This clock is used as the main CPU PLL source in low-level
> + * bootloaders, and it is necessary to mark it as critical.
> + */
> + .flags = CLK_IS_CRITICAL,
No I don't think so. Downstream consumer maybe critical but that one is
not, unless it is read-only.
A CPU pll, like on the g12 family, is unlikely to be read-only since the
PLL will need to relock to change rates. During this phase, there will
be no reate coming from the PLL so the PLL is not critical and you must
be able to "park" your CPU an another clock while poking this one
> + },
> +};
> +
> +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,
> + },
> +};
> +
> static struct clk_fixed_factor fclk_div2_div = {
> .mult = 1,
> .div = 2,
> @@ -283,6 +358,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 +370,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 Tue, Apr 02, 2024 at 11:00:42AM +0200, Jerome Brunet wrote:
>
> On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <[email protected]> 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 <[email protected]>
> > ---
> > drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
> > drivers/clk/meson/a1-pll.h | 6 +++
> > 2 files changed, 84 insertions(+)
> >
> > diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
> > index 60b2e53e7e51..02fd2d325cc6 100644
> > --- a/drivers/clk/meson/a1-pll.c
> > +++ b/drivers/clk/meson/a1-pll.c
> > @@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
> > },
> > };
> >
> > +static const struct pll_mult_range sys_pll_mult_range = {
> > + .min = 32,
> > + .max = 64,
> > +};
> > +
> > +/*
> > + * We assume that the sys_pll_clk has already been set up by the low-level
> > + * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
> > + * run the initialization sequence.
> > + */
>
> I see no reason to make such assumption.
> This clock is no read-only, it apparently is able to re-lock so assuming
> anything from the bootloader is just asking from trouble
>
Indeed, I have implemented the following initialization sequence. I have
dumped the bootloader setup and included it in the definition of my
sys_pll. However, I have encountered an issue with the enable bit. If I
leave the enable bit switched on by default, there is a possibility that
the bootloader selects a fixed CPU clock while the sys_pll should be
switched off. On the other hand, if I keep the enable bit switched off
by default, the bootloader might configure the CPU clock to use sys_pll,
resulting in the execution halting when the initialization sequence is
run. This situation has led me to assume that we should place our trust
in the bootloader setup.
If you believe it is necessary to include the initialization sequence, I
can prepare it with the sys_pll enabled by default.
> > +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,
> > + },
> > + .hw.init = &(struct clk_init_data){
> > + .name = "sys_pll",
> > + .ops = &meson_clk_pll_ops,
> > + .parent_names = (const char *[]){ "syspll_in" },
> > + .num_parents = 1,
> > + /*
> > + * This clock is used as the main CPU PLL source in low-level
> > + * bootloaders, and it is necessary to mark it as critical.
> > + */
> > + .flags = CLK_IS_CRITICAL,
>
> No I don't think so. Downstream consumer maybe critical but that one is
> not, unless it is read-only.
>
> A CPU pll, like on the g12 family, is unlikely to be read-only since the
> PLL will need to relock to change rates. During this phase, there will
> be no reate coming from the PLL so the PLL is not critical and you must
> be able to "park" your CPU an another clock while poking this one
>
Initially, I tagged it with CLK_IS_CRITICAL because I observed in the
kernel start that CCF disables it. However, upon further understanding,
I realized that this happened due to other reasons. I believe that if I
provide an init sequence where sys_pll is enabled by default, CCF will
not disable this clock.
> > + },
> > +};
> > +
> > +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,
> > + },
> > +};
> > +
> > static struct clk_fixed_factor fclk_div2_div = {
> > .mult = 1,
> > .div = 2,
> > @@ -283,6 +358,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 +370,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
--
Thank you,
Dmitry
On Tue 02 Apr 2024 at 15:15, Dmitry Rokosov <[email protected]> wrote:
> On Tue, Apr 02, 2024 at 11:00:42AM +0200, Jerome Brunet wrote:
>>
>> On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <[email protected]> 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 <[email protected]>
>> > ---
>> > drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
>> > drivers/clk/meson/a1-pll.h | 6 +++
>> > 2 files changed, 84 insertions(+)
>> >
>> > diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
>> > index 60b2e53e7e51..02fd2d325cc6 100644
>> > --- a/drivers/clk/meson/a1-pll.c
>> > +++ b/drivers/clk/meson/a1-pll.c
>> > @@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
>> > },
>> > };
>> >
>> > +static const struct pll_mult_range sys_pll_mult_range = {
>> > + .min = 32,
>> > + .max = 64,
>> > +};
>> > +
>> > +/*
>> > + * We assume that the sys_pll_clk has already been set up by the low-level
>> > + * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
>> > + * run the initialization sequence.
>> > + */
>>
>> I see no reason to make such assumption.
>> This clock is no read-only, it apparently is able to re-lock so assuming
>> anything from the bootloader is just asking from trouble
>>
>
> Indeed, I have implemented the following initialization sequence. I have
> dumped the bootloader setup and included it in the definition of my
> sys_pll. However, I have encountered an issue with the enable bit. If I
> leave the enable bit switched on by default, there is a possibility that
> the bootloader selects a fixed CPU clock while the sys_pll should be
> switched off. On the other hand, if I keep the enable bit switched off
> by default, the bootloader might configure the CPU clock to use sys_pll,
> resulting in the execution halting when the initialization sequence is
> run. This situation has led me to assume that we should place our trust
> in the bootloader setup.
>
> If you believe it is necessary to include the initialization sequence, I
> can prepare it with the sys_pll enabled by default.
I just noted your initial comment is misleading.
You could submit a patch to apply the init sequence only if the PLL is
not already enabled. Maybe even condition that to flag in the pll data
to avoid applying it to the other platforms for now.
>
>> > +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,
>> > + },
>> > + .hw.init = &(struct clk_init_data){
>> > + .name = "sys_pll",
>> > + .ops = &meson_clk_pll_ops,
>> > + .parent_names = (const char *[]){ "syspll_in" },
>> > + .num_parents = 1,
>> > + /*
>> > + * This clock is used as the main CPU PLL source in low-level
>> > + * bootloaders, and it is necessary to mark it as critical.
>> > + */
>> > + .flags = CLK_IS_CRITICAL,
>>
>> No I don't think so. Downstream consumer maybe critical but that one is
>> not, unless it is read-only.
>>
>> A CPU pll, like on the g12 family, is unlikely to be read-only since the
>> PLL will need to relock to change rates. During this phase, there will
>> be no reate coming from the PLL so the PLL is not critical and you must
>> be able to "park" your CPU an another clock while poking this one
>>
>
> Initially, I tagged it with CLK_IS_CRITICAL because I observed in the
> kernel start that CCF disables it.
> However, upon further understanding,
> I realized that this happened due to other reasons. I believe that if I
> provide an init sequence where sys_pll is enabled by default, CCF will
> not disable this clock.
>
>> > + },
>> > +};
>> > +
>> > +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,
>> > + },
>> > +};
>> > +
>> > static struct clk_fixed_factor fclk_div2_div = {
>> > .mult = 1,
>> > .div = 2,
>> > @@ -283,6 +358,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 +370,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 Tue, Apr 02, 2024 at 04:27:17PM +0200, Jerome Brunet wrote:
>
> On Tue 02 Apr 2024 at 15:15, Dmitry Rokosov <[email protected]> wrote:
>
> > On Tue, Apr 02, 2024 at 11:00:42AM +0200, Jerome Brunet wrote:
> >>
> >> On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <[email protected]> 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 <[email protected]>
> >> > ---
> >> > drivers/clk/meson/a1-pll.c | 78 ++++++++++++++++++++++++++++++++++++++
> >> > drivers/clk/meson/a1-pll.h | 6 +++
> >> > 2 files changed, 84 insertions(+)
> >> >
> >> > diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
> >> > index 60b2e53e7e51..02fd2d325cc6 100644
> >> > --- a/drivers/clk/meson/a1-pll.c
> >> > +++ b/drivers/clk/meson/a1-pll.c
> >> > @@ -138,6 +138,81 @@ static struct clk_regmap hifi_pll = {
> >> > },
> >> > };
> >> >
> >> > +static const struct pll_mult_range sys_pll_mult_range = {
> >> > + .min = 32,
> >> > + .max = 64,
> >> > +};
> >> > +
> >> > +/*
> >> > + * We assume that the sys_pll_clk has already been set up by the low-level
> >> > + * bootloaders as the main CPU PLL source. Therefore, it is not necessary to
> >> > + * run the initialization sequence.
> >> > + */
> >>
> >> I see no reason to make such assumption.
> >> This clock is no read-only, it apparently is able to re-lock so assuming
> >> anything from the bootloader is just asking from trouble
> >>
> >
> > Indeed, I have implemented the following initialization sequence. I have
> > dumped the bootloader setup and included it in the definition of my
> > sys_pll. However, I have encountered an issue with the enable bit. If I
> > leave the enable bit switched on by default, there is a possibility that
> > the bootloader selects a fixed CPU clock while the sys_pll should be
> > switched off. On the other hand, if I keep the enable bit switched off
> > by default, the bootloader might configure the CPU clock to use sys_pll,
> > resulting in the execution halting when the initialization sequence is
> > run. This situation has led me to assume that we should place our trust
> > in the bootloader setup.
> >
> > If you believe it is necessary to include the initialization sequence, I
> > can prepare it with the sys_pll enabled by default.
>
> I just noted your initial comment is misleading.
>
> You could submit a patch to apply the init sequence only if the PLL is
> not already enabled. Maybe even condition that to flag in the pll data
> to avoid applying it to the other platforms for now.
>
Ah, I see. Okay, no problem, I will prepare such patch to common meson
clk-pll.
[...]
--
Thank you,
Dmitry