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@xxxxxxxxxxxxxx>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>>> Signed-off-by: Johannes Zink <j.zink@xxxxxxxxxxxxxx>
>>> ---
>>> 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 |
