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

[Sascha Hauer <sha@pengutronix.de>]

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 |