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

[Sascha Hauer <sha@pengutronix.de>]

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 |