Re: [PATCH v2 3/3] clk: meson: a1: add support for Amlogic A1 clock driver

From: Jian Hu
Date: Mon Nov 25 2019 - 07:01:36 EST




On 2019/11/25 18:14, Jerome Brunet wrote:

On Thu 21 Nov 2019 at 04:21, Jian Hu <jian.hu@xxxxxxxxxxx> wrote:

Hi, Jerome

On 2019/11/20 23:35, Jerome Brunet wrote:

On Wed 20 Nov 2019 at 10:28, Jian Hu <jian.hu@xxxxxxxxxxx> wrote:

Hi, jerome

Is there any problem about fixed_pll_dco's parent_data?

Now both name and fw_name are described in parent_data.

Yes, there is a problem. This approach is incorrect, as I've tried to
explain a couple times already. Let me try to re-summarize why this
approach is incorrect.

Both fw_name and name should be provided when it is possible that
the DT does not describe the input clock. IOW, it is only for controllers
which relied on the global name so far and are now starting to describe
the clock input in DT

This is not your case.
Your controller is new and DT will have the correct
info

You are trying work around an ordering issue by providing both fw_name
and name. This is not correct and I'll continue to nack it.

If the orphan clock is not reparented as you would expect, I suggest you
try to look a bit further at how the reparenting of orphans is done in
CCF and why it does not match your expectation.

I have debugged the handle for orphan clock in CCF, Maybe you are missing
the last email.

Nope, got it the first time

Even though the clock index exit, it will get failed for the orphan clock's
parent clock due to it has not beed added to the provider.

If the provider is not registered yet, of course any query to it won't
work. This why I have suggested to this debug *further* :

* Is the orphan reparenting done when a new provider is registered ?
* If not, should it be done ? is this your problem ?

Yes, the orphan reparenting is done when the new provider is registered.

Reparenting the orphan will be done when each clock is registered by devm_clk_hw_register. And at this time the provider has not been registered. After all clocks are registered by devm_clk_hw_register, the
provider will be registered by devm_of_clk_add_hw_provider.

Reparenting the orphan will fail when fw_name is added alone, the couse is that devm_clk_hw_register is always running ahead of devm_of_clk_add_hw_provider.

That is why it will failed to get parent for the orphan clock.




.