* [PATCH] clks: imx7: fix initial clock setup with deep probe enabled
@ 2022-08-30 8:39 Johannes Zink
2022-08-30 15:05 ` Sascha Hauer
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Zink @ 2022-08-30 8:39 UTC (permalink / raw)
To: barebox; +Cc: Ahmad Fatoum, Johannes Zink
We register the i.MX7 clock controller driver at core_initcall level and
then do some initial clock setup/reparenting at postcore_initcall level.
This doesn't work as expected when deep probe is enabled, because while
the driver is registered at core_initcall level, it's only probed
later on, currently at postcore_initcall level because it's a dependency
of the timer for which of_ensure_device_probed is called.
As the initial clock setup is also at postcore_initcall level, it's no
longer guaranteed that the code executes in the same order. Fix this by
directly doing the setup at the end of the probe function.
Co-developed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
---
drivers/clk/imx/clk-imx7.c | 62 +++++++++++++++++---------------------
1 file changed, 27 insertions(+), 35 deletions(-)
diff --git a/drivers/clk/imx/clk-imx7.c b/drivers/clk/imx/clk-imx7.c
index ffa39d17b0..67876a8404 100644
--- a/drivers/clk/imx/clk-imx7.c
+++ b/drivers/clk/imx/clk-imx7.c
@@ -358,7 +358,32 @@ static int const clks_init_on[] __initconst = {
static struct clk_onecell_data clk_data;
-static int imx7_clk_initialized;
+static void imx7_clk_setup(void)
+{
+ int i;
+
+ clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
+
+ for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
+ clk_enable(clks[clks_init_on[i]]);
+
+ /* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
+ clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
+
+ /* set uart module clock's parent clock source that must be great then 80MHz */
+ clk_set_parent(clks[IMX7D_UART1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
+
+ clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
+ clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
+ clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
+ clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
+
+ clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
+ clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC], clks[IMX7D_PLL_SYS_PFD4_CLK]);
+ clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
+ clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
+ clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
+}
static int imx7_ccm_probe(struct device_d *dev)
{
@@ -806,43 +831,10 @@ static int imx7_ccm_probe(struct device_d *dev)
clk_data.clk_num = ARRAY_SIZE(clks);
of_clk_add_provider(dev->device_node, of_clk_src_onecell_get, &clk_data);
- imx7_clk_initialized = 1;
-
- return 0;
-}
-
-static int imx7_clk_setup(void)
-{
- int i;
-
- if (!imx7_clk_initialized)
- return 0;
-
- clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
-
- for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
- clk_enable(clks[clks_init_on[i]]);
-
- /* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
- clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
-
- /* set uart module clock's parent clock source that must be great then 80MHz */
- clk_set_parent(clks[IMX7D_UART1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
-
- clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
- clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
- clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
- clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
-
- clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
- clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC], clks[IMX7D_PLL_SYS_PFD4_CLK]);
- clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
- clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
- clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
+ imx7_clk_setup();
return 0;
}
-postcore_initcall(imx7_clk_setup);
static __maybe_unused struct of_device_id imx7_ccm_dt_ids[] = {
{
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clks: imx7: fix initial clock setup with deep probe enabled
2022-08-30 8:39 [PATCH] clks: imx7: fix initial clock setup with deep probe enabled Johannes Zink
@ 2022-08-30 15:05 ` Sascha Hauer
2022-08-31 8:21 ` Johannes Zink
0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2022-08-30 15:05 UTC (permalink / raw)
To: Johannes Zink; +Cc: barebox, Ahmad Fatoum
On Tue, Aug 30, 2022 at 10:39:37AM +0200, Johannes Zink wrote:
> We register the i.MX7 clock controller driver at core_initcall level and
> then do some initial clock setup/reparenting at postcore_initcall level.
> This doesn't work as expected when deep probe is enabled, because while
> the driver is registered at core_initcall level, it's only probed
> later on, currently at postcore_initcall level because it's a dependency
> of the timer for which of_ensure_device_probed is called.
>
> As the initial clock setup is also at postcore_initcall level, it's no
> longer guaranteed that the code executes in the same order. Fix this by
> directly doing the setup at the end of the probe function.
Does this still work with deep probe disabled?
I am asking because this effectively reverts bce79428773 ("clk: i.MX7: do
clock reparenting when all clocks are initialized"). Switching all
i.MX7 boards to deep probe might be a solution as well.
Sascha
>
> Co-developed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> ---
> drivers/clk/imx/clk-imx7.c | 62 +++++++++++++++++---------------------
> 1 file changed, 27 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx7.c b/drivers/clk/imx/clk-imx7.c
> index ffa39d17b0..67876a8404 100644
> --- a/drivers/clk/imx/clk-imx7.c
> +++ b/drivers/clk/imx/clk-imx7.c
> @@ -358,7 +358,32 @@ static int const clks_init_on[] __initconst = {
>
> static struct clk_onecell_data clk_data;
>
> -static int imx7_clk_initialized;
> +static void imx7_clk_setup(void)
> +{
> + int i;
> +
> + clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
> +
> + for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> + clk_enable(clks[clks_init_on[i]]);
> +
> + /* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
> + clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
> +
> + /* set uart module clock's parent clock source that must be great then 80MHz */
> + clk_set_parent(clks[IMX7D_UART1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
> +
> + clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> + clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> + clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> + clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> +
> + clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
> + clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC], clks[IMX7D_PLL_SYS_PFD4_CLK]);
> + clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
> + clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
> + clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
> +}
>
> static int imx7_ccm_probe(struct device_d *dev)
> {
> @@ -806,43 +831,10 @@ static int imx7_ccm_probe(struct device_d *dev)
> clk_data.clk_num = ARRAY_SIZE(clks);
> of_clk_add_provider(dev->device_node, of_clk_src_onecell_get, &clk_data);
>
> - imx7_clk_initialized = 1;
> -
> - return 0;
> -}
> -
> -static int imx7_clk_setup(void)
> -{
> - int i;
> -
> - if (!imx7_clk_initialized)
> - return 0;
> -
> - clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
> -
> - for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> - clk_enable(clks[clks_init_on[i]]);
> -
> - /* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
> - clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
> -
> - /* set uart module clock's parent clock source that must be great then 80MHz */
> - clk_set_parent(clks[IMX7D_UART1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
> -
> - clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> - clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> - clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> - clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> -
> - clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
> - clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC], clks[IMX7D_PLL_SYS_PFD4_CLK]);
> - clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
> - clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
> - clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
> + imx7_clk_setup();
>
> return 0;
> }
> -postcore_initcall(imx7_clk_setup);
>
> static __maybe_unused struct of_device_id imx7_ccm_dt_ids[] = {
> {
> --
> 2.30.2
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clks: imx7: fix initial clock setup with deep probe enabled
2022-08-30 15:05 ` Sascha Hauer
@ 2022-08-31 8:21 ` Johannes Zink
2022-09-02 14:02 ` Ahmad Fatoum
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Zink @ 2022-08-31 8:21 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox, Ahmad Fatoum
Hi Sascha,
On Tue, 2022-08-30 at 17:05 +0200, Sascha Hauer wrote:
> On Tue, Aug 30, 2022 at 10:39:37AM +0200, Johannes Zink wrote:
> > We register the i.MX7 clock controller driver at core_initcall
> > level and
> > then do some initial clock setup/reparenting at postcore_initcall
> > level.
> > This doesn't work as expected when deep probe is enabled, because
> > while
> > the driver is registered at core_initcall level, it's only probed
> > later on, currently at postcore_initcall level because it's a
> > dependency
> > of the timer for which of_ensure_device_probed is called.
> >
> > As the initial clock setup is also at postcore_initcall level, it's
> > no
> > longer guaranteed that the code executes in the same order. Fix
> > this by
> > directly doing the setup at the end of the probe function.
>
> Does this still work with deep probe disabled?
>
> I am asking because this effectively reverts bce79428773 ("clk:
> i.MX7: do
> clock reparenting when all clocks are initialized"). Switching all
> i.MX7 boards to deep probe might be a solution as well.
>
> Sascha
>
Though I would not _expect_ any problems here, I would like to check
that more thoroughly. This will probably take a bit, please stand by.
Best regards
Johannes
>
> >
> > Co-developed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > ---
> > drivers/clk/imx/clk-imx7.c | 62 +++++++++++++++++-----------------
> > ----
> > 1 file changed, 27 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx7.c b/drivers/clk/imx/clk-
> > imx7.c
> > index ffa39d17b0..67876a8404 100644
> > --- a/drivers/clk/imx/clk-imx7.c
> > +++ b/drivers/clk/imx/clk-imx7.c
> > @@ -358,7 +358,32 @@ static int const clks_init_on[] __initconst =
> > {
> >
> > static struct clk_onecell_data clk_data;
> >
> > -static int imx7_clk_initialized;
> > +static void imx7_clk_setup(void)
> > +{
> > + int i;
> > +
> > + clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
> > +
> > + for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> > + clk_enable(clks[clks_init_on[i]]);
> > +
> > + /* use old gpt clk setting, gpt1 root clk must be twice as
> > gpt counter freq */
> > + clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC],
> > clks[IMX7D_OSC_24M_CLK]);
> > +
> > + /* set uart module clock's parent clock source that must be
> > great then 80MHz */
> > + clk_set_parent(clks[IMX7D_UART1_ROOT_SRC],
> > clks[IMX7D_OSC_24M_CLK]);
> > +
> > + clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> > + clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> > + clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> > + clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> > +
> > + clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
> > + clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC],
> > clks[IMX7D_PLL_SYS_PFD4_CLK]);
> > + clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
> > + clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
> > + clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
> > +}
> >
> > static int imx7_ccm_probe(struct device_d *dev)
> > {
> > @@ -806,43 +831,10 @@ static int imx7_ccm_probe(struct device_d
> > *dev)
> > clk_data.clk_num = ARRAY_SIZE(clks);
> > of_clk_add_provider(dev->device_node,
> > of_clk_src_onecell_get, &clk_data);
> >
> > - imx7_clk_initialized = 1;
> > -
> > - return 0;
> > -}
> > -
> > -static int imx7_clk_setup(void)
> > -{
> > - int i;
> > -
> > - if (!imx7_clk_initialized)
> > - return 0;
> > -
> > - clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
> > -
> > - for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> > - clk_enable(clks[clks_init_on[i]]);
> > -
> > - /* use old gpt clk setting, gpt1 root clk must be twice as
> > gpt counter freq */
> > - clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC],
> > clks[IMX7D_OSC_24M_CLK]);
> > -
> > - /* set uart module clock's parent clock source that must be
> > great then 80MHz */
> > - clk_set_parent(clks[IMX7D_UART1_ROOT_SRC],
> > clks[IMX7D_OSC_24M_CLK]);
> > -
> > - clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> > - clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> > - clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
> > - clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC],
> > clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
> > -
> > - clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
> > - clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC],
> > clks[IMX7D_PLL_SYS_PFD4_CLK]);
> > - clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
> > - clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
> > - clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
> > + imx7_clk_setup();
> >
> > return 0;
> > }
> > -postcore_initcall(imx7_clk_setup);
> >
> > static __maybe_unused struct of_device_id imx7_ccm_dt_ids[] = {
> > {
> > --
> > 2.30.2
> >
> >
> >
>
--
Pengutronix e.K. | Johannes Zink |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clks: imx7: fix initial clock setup with deep probe enabled
2022-08-31 8:21 ` Johannes Zink
@ 2022-09-02 14:02 ` Ahmad Fatoum
2022-09-02 18:38 ` Sascha Hauer
0 siblings, 1 reply; 5+ messages in thread
From: Ahmad Fatoum @ 2022-09-02 14:02 UTC (permalink / raw)
To: Johannes Zink, Sascha Hauer; +Cc: barebox
Hello,
On 31.08.22 10:21, Johannes Zink wrote:
> Hi Sascha,
>
> On Tue, 2022-08-30 at 17:05 +0200, Sascha Hauer wrote:
>> On Tue, Aug 30, 2022 at 10:39:37AM +0200, Johannes Zink wrote:
>>> We register the i.MX7 clock controller driver at core_initcall
>>> level and
>>> then do some initial clock setup/reparenting at postcore_initcall
>>> level.
>>> This doesn't work as expected when deep probe is enabled, because
>>> while
>>> the driver is registered at core_initcall level, it's only probed
>>> later on, currently at postcore_initcall level because it's a
>>> dependency
>>> of the timer for which of_ensure_device_probed is called.
>>>
>>> As the initial clock setup is also at postcore_initcall level, it's
>>> no
>>> longer guaranteed that the code executes in the same order. Fix
>>> this by
>>> directly doing the setup at the end of the probe function.
>>
>> Does this still work with deep probe disabled?
>>
>> I am asking because this effectively reverts bce79428773 ("clk:
>> i.MX7: do
>> clock reparenting when all clocks are initialized"). Switching all
>> i.MX7 boards to deep probe might be a solution as well.
fixed clocks are registered via CLK_OF_DECLARE, so by the time the
imx7 clock probe runs, they will always be available, independent of
whether deep probe is enabled.
> Though I would not _expect_ any problems here, I would like to check
> that more thoroughly. This will probably take a bit, please stand by.
I looked at this with Johannes and I am puzzled how the problem
that bce79428773 aimed to fix occurred. Could you elaborate?
Thanks,
Ahmad
>
> Best regards
> Johannes
>
>>
>>>
>>> Co-developed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
>>> ---
>>> drivers/clk/imx/clk-imx7.c | 62 +++++++++++++++++-----------------
>>> ----
>>> 1 file changed, 27 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/clk/imx/clk-imx7.c b/drivers/clk/imx/clk-
>>> imx7.c
>>> index ffa39d17b0..67876a8404 100644
>>> --- a/drivers/clk/imx/clk-imx7.c
>>> +++ b/drivers/clk/imx/clk-imx7.c
>>> @@ -358,7 +358,32 @@ static int const clks_init_on[] __initconst =
>>> {
>>>
>>> static struct clk_onecell_data clk_data;
>>>
>>> -static int imx7_clk_initialized;
>>> +static void imx7_clk_setup(void)
>>> +{
>>> + int i;
>>> +
>>> + clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
>>> + clk_enable(clks[clks_init_on[i]]);
>>> +
>>> + /* use old gpt clk setting, gpt1 root clk must be twice as
>>> gpt counter freq */
>>> + clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC],
>>> clks[IMX7D_OSC_24M_CLK]);
>>> +
>>> + /* set uart module clock's parent clock source that must be
>>> great then 80MHz */
>>> + clk_set_parent(clks[IMX7D_UART1_ROOT_SRC],
>>> clks[IMX7D_OSC_24M_CLK]);
>>> +
>>> + clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
>>> + clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
>>> + clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
>>> + clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
>>> +
>>> + clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
>>> + clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC],
>>> clks[IMX7D_PLL_SYS_PFD4_CLK]);
>>> + clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
>>> + clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
>>> + clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
>>> +}
>>>
>>> static int imx7_ccm_probe(struct device_d *dev)
>>> {
>>> @@ -806,43 +831,10 @@ static int imx7_ccm_probe(struct device_d
>>> *dev)
>>> clk_data.clk_num = ARRAY_SIZE(clks);
>>> of_clk_add_provider(dev->device_node,
>>> of_clk_src_onecell_get, &clk_data);
>>>
>>> - imx7_clk_initialized = 1;
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static int imx7_clk_setup(void)
>>> -{
>>> - int i;
>>> -
>>> - if (!imx7_clk_initialized)
>>> - return 0;
>>> -
>>> - clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
>>> -
>>> - for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
>>> - clk_enable(clks[clks_init_on[i]]);
>>> -
>>> - /* use old gpt clk setting, gpt1 root clk must be twice as
>>> gpt counter freq */
>>> - clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC],
>>> clks[IMX7D_OSC_24M_CLK]);
>>> -
>>> - /* set uart module clock's parent clock source that must be
>>> great then 80MHz */
>>> - clk_set_parent(clks[IMX7D_UART1_ROOT_SRC],
>>> clks[IMX7D_OSC_24M_CLK]);
>>> -
>>> - clk_set_parent(clks[IMX7D_ENET1_REF_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
>>> - clk_set_parent(clks[IMX7D_ENET1_TIME_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
>>> - clk_set_parent(clks[IMX7D_ENET2_REF_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_125M_CLK]);
>>> - clk_set_parent(clks[IMX7D_ENET2_TIME_ROOT_SRC],
>>> clks[IMX7D_PLL_ENET_MAIN_100M_CLK]);
>>> -
>>> - clk_set_rate(clks[IMX7D_PLL_SYS_PFD4_CLK], 392000000);
>>> - clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC],
>>> clks[IMX7D_PLL_SYS_PFD4_CLK]);
>>> - clk_set_rate(clks[IMX7D_ENET_AXI_ROOT_CLK], 197000000);
>>> - clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
>>> - clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
>>> + imx7_clk_setup();
>>>
>>> return 0;
>>> }
>>> -postcore_initcall(imx7_clk_setup);
>>>
>>> static __maybe_unused struct of_device_id imx7_ccm_dt_ids[] = {
>>> {
>>> --
>>> 2.30.2
>>>
>>>
>>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clks: imx7: fix initial clock setup with deep probe enabled
2022-09-02 14:02 ` Ahmad Fatoum
@ 2022-09-02 18:38 ` Sascha Hauer
0 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2022-09-02 18:38 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: Johannes Zink, barebox
On Fri, Sep 02, 2022 at 04:02:32PM +0200, Ahmad Fatoum wrote:
> Hello,
>
> On 31.08.22 10:21, Johannes Zink wrote:
> > Hi Sascha,
> >
> > On Tue, 2022-08-30 at 17:05 +0200, Sascha Hauer wrote:
> >> On Tue, Aug 30, 2022 at 10:39:37AM +0200, Johannes Zink wrote:
> >>> We register the i.MX7 clock controller driver at core_initcall
> >>> level and
> >>> then do some initial clock setup/reparenting at postcore_initcall
> >>> level.
> >>> This doesn't work as expected when deep probe is enabled, because
> >>> while
> >>> the driver is registered at core_initcall level, it's only probed
> >>> later on, currently at postcore_initcall level because it's a
> >>> dependency
> >>> of the timer for which of_ensure_device_probed is called.
> >>>
> >>> As the initial clock setup is also at postcore_initcall level, it's
> >>> no
> >>> longer guaranteed that the code executes in the same order. Fix
> >>> this by
> >>> directly doing the setup at the end of the probe function.
> >>
> >> Does this still work with deep probe disabled?
> >>
> >> I am asking because this effectively reverts bce79428773 ("clk:
> >> i.MX7: do
> >> clock reparenting when all clocks are initialized"). Switching all
> >> i.MX7 boards to deep probe might be a solution as well.
>
> fixed clocks are registered via CLK_OF_DECLARE, so by the time the
> imx7 clock probe runs, they will always be available, independent of
> whether deep probe is enabled.
So this problem is already fixed, great.
>
> > Though I would not _expect_ any problems here, I would like to check
> > that more thoroughly. This will probably take a bit, please stand by.
>
> I looked at this with Johannes and I am puzzled how the problem
> that bce79428773 aimed to fix occurred. Could you elaborate?
Have a look at the tree at bce79428773. Back then the i.MX7 clock driver
was registered at core_initcall level, the same level that of_probe()
was running which first called of_platform_populate() and then
of_clk_init().
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-02 18:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-30 8:39 [PATCH] clks: imx7: fix initial clock setup with deep probe enabled Johannes Zink
2022-08-30 15:05 ` Sascha Hauer
2022-08-31 8:21 ` Johannes Zink
2022-09-02 14:02 ` Ahmad Fatoum
2022-09-02 18:38 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox