mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <sha@pengutronix.de>
To: Johannes Zink <j.zink@pengutronix.de>
Cc: barebox@lists.infradead.org, Ahmad Fatoum <a.fatoum@pengutronix.de>
Subject: Re: [PATCH] clks: imx7: fix initial clock setup with deep probe enabled
Date: Tue, 30 Aug 2022 17:05:22 +0200	[thread overview]
Message-ID: <20220830150522.GI24324@pengutronix.de> (raw)
In-Reply-To: <20220830083937.466171-1-j.zink@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 |



  reply	other threads:[~2022-08-30 15:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-30  8:39 Johannes Zink
2022-08-30 15:05 ` Sascha Hauer [this message]
2022-08-31  8:21   ` Johannes Zink
2022-09-02 14:02     ` Ahmad Fatoum
2022-09-02 18:38       ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220830150522.GI24324@pengutronix.de \
    --to=sha@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=j.zink@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox