[PATCH master v2] clks: imx7: fix initial clock setup with deep probe enabled

[PATCH master v2] clks: imx7: fix initial clock setup with deep probe enabled

[Ahmad Fatoum]

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 for the deep
probe case. In non-deep-probe systems, we maintain the existing initcall
ordering to avoid regressions.

Co-developed-by: Johannes Zink <j.zink@pengutronix.de>
Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - maintain initcall ordering for non-deep-probe systems (Sascha)
---
 drivers/clk/imx/clk-imx7.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-imx7.c b/drivers/clk/imx/clk-imx7.c
index ffa39d17b0eb..588e9cbe5ce4 100644
--- a/drivers/clk/imx/clk-imx7.c
+++ b/drivers/clk/imx/clk-imx7.c
@@ -6,6 +6,7 @@
 #include <common.h>
 #include <init.h>
 #include <driver.h>
+#include <deep-probe.h>
 #include <linux/clk.h>
 #include <io.h>
 #include <of.h>
@@ -358,7 +359,9 @@ static int const clks_init_on[] __initconst = {
 
 static struct clk_onecell_data clk_data;
 
-static int imx7_clk_initialized;
+static struct device_node *ccm_np;
+
+static int imx7_clk_setup(void);
 
 static int imx7_ccm_probe(struct device_d *dev)
 {
@@ -806,19 +809,35 @@ 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;
+	ccm_np = dev->device_node;
+
+	/*
+	 * imx7_clk_setup() requires both the CCM and fixed-clock osc devices
+	 * to be available.
+	 * With deep probe enabled, we can instead just directly call
+	 * imx7_clk_setup because the osc fixed-clock will just be probed
+	 * on demand if not yet available. Otherwise, the imx7_clk_setup
+	 * will run at postcore_initcall level.
+	 */
+	if (deep_probe_is_supported())
+		return imx7_clk_setup();
 
 	return 0;
 }
 
 static int imx7_clk_setup(void)
 {
+	struct clk *clk;
 	int i;
 
-	if (!imx7_clk_initialized)
+	if (!ccm_np)
 		return 0;
 
-	clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
+	clk = of_clk_get_by_name(ccm_np, "osc");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	clks[IMX7D_OSC_24M_CLK] = clk;
 
 	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
 		clk_enable(clks[clks_init_on[i]]);
@@ -840,6 +859,8 @@ static int imx7_clk_setup(void)
 	clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
 	clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
 
+	ccm_np = NULL;
+
 	return 0;
 }
 postcore_initcall(imx7_clk_setup);
-- 
2.30.2

Re: [PATCH master v2] clks: imx7: fix initial clock setup with deep probe enabled

[Sascha Hauer]

On Wed, Oct 05, 2022 at 09:41:08PM +0200, Ahmad Fatoum 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 for the deep
> probe case. In non-deep-probe systems, we maintain the existing initcall
> ordering to avoid regressions.
> 
> Co-developed-by: Johannes Zink <j.zink@pengutronix.de>
> Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>   - maintain initcall ordering for non-deep-probe systems (Sascha)
> ---
>  drivers/clk/imx/clk-imx7.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/clk/imx/clk-imx7.c b/drivers/clk/imx/clk-imx7.c
> index ffa39d17b0eb..588e9cbe5ce4 100644
> --- a/drivers/clk/imx/clk-imx7.c
> +++ b/drivers/clk/imx/clk-imx7.c
> @@ -6,6 +6,7 @@
>  #include <common.h>
>  #include <init.h>
>  #include <driver.h>
> +#include <deep-probe.h>
>  #include <linux/clk.h>
>  #include <io.h>
>  #include <of.h>
> @@ -358,7 +359,9 @@ static int const clks_init_on[] __initconst = {
>  
>  static struct clk_onecell_data clk_data;
>  
> -static int imx7_clk_initialized;
> +static struct device_node *ccm_np;
> +
> +static int imx7_clk_setup(void);
>  
>  static int imx7_ccm_probe(struct device_d *dev)
>  {
> @@ -806,19 +809,35 @@ 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;
> +	ccm_np = dev->device_node;
> +
> +	/*
> +	 * imx7_clk_setup() requires both the CCM and fixed-clock osc devices
> +	 * to be available.
> +	 * With deep probe enabled, we can instead just directly call
> +	 * imx7_clk_setup because the osc fixed-clock will just be probed
> +	 * on demand if not yet available. Otherwise, the imx7_clk_setup
> +	 * will run at postcore_initcall level.
> +	 */
> +	if (deep_probe_is_supported())
> +		return imx7_clk_setup();
>  
>  	return 0;
>  }
>  
>  static int imx7_clk_setup(void)
>  {
> +	struct clk *clk;
>  	int i;
>  
> -	if (!imx7_clk_initialized)
> +	if (!ccm_np)
>  		return 0;
>  
> -	clks[IMX7D_OSC_24M_CLK] = clk_lookup("osc");
> +	clk = of_clk_get_by_name(ccm_np, "osc");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	clks[IMX7D_OSC_24M_CLK] = clk;
>  
>  	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
>  		clk_enable(clks[clks_init_on[i]]);
> @@ -840,6 +859,8 @@ static int imx7_clk_setup(void)
>  	clk_set_rate(clks[IMX7D_ENET1_TIME_ROOT_CLK], 25000000);
>  	clk_set_rate(clks[IMX7D_ENET2_TIME_ROOT_CLK], 25000000);
>  
> +	ccm_np = NULL;
> +
>  	return 0;
>  }
>  postcore_initcall(imx7_clk_setup);
> -- 
> 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 |