Re: [PATCH] Revert "clk: fix __clk_init_parent() for single parent clocks"

[Stephen Boyd]

Quoting Jerome Brunet (2018-12-04 08:32:57)
> This reverts commit 2430a94d1e719b7b4af2a65b781a4c036eb22e64.
> 
> From the original commit message:
>   It turned out a problem because there are some single-parent clocks
>   that implement .get_parent() callback and return non-zero index.
>   The SOCFPGA clock is the case; the commit broke the SOCFPGA boards.
> 
> It is wrong for a clock to return an index >= num_parents. CCF checks
> for this condition in clk_core_get_parent_by_index(). This function sets
> the parent to NULL if index is incoherent, which seems like the only
> sane choice.
> 
> commit 2430a94d1e71 ("clk: fix __clk_init_parent() for single parent clocks")
> appears to be a work around installed in the core framework for a problem
> that is platform specific, and should probably be fixed in the platform code.

Ouch. I see that I even pointed that out in 2016 but never got a reply
or a fix patch[1].

> 
> Further more, it introduces a problem in a corner case of the mux clock.
> Take mux with multiple parents, but only one is known, the rest being
> undocumented. The register reset has one of unknown parent set.
> 
> Before commit 2430a94d1e71 ("clk: fix __clk_init_parent() for single parent clocks"):
>  * get_parent() is called, register is read and give an unknown index.
>    -> the mux is orphaned.
>  * a call to set_rate() will reparent the mux to the only known parent.
> 
> With commit 2430a94d1e71 ("clk: fix __clk_init_parent() for single parent clocks"):
>  * the register is never read.
>  * parent is wrongly assumed to be the only known one.
>    As a consequence, all the calculation deriving from the mux will be
>    wrong
>  * Since we believe the only know parent to be set, set_parent() won't
>    ever be called and the register won't be set with the correct value.

Isn't this the broken bad case all over again? Why register a clk as a
mux and then only say it has one parent?

> 
> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> ---

Is this related to the other patch you sent? Can you send series for
related patches please?

[1] https://lkml.kernel.org/r/20160209181833.GA24167@xxxxxxxxxxxxxx