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

From: Stephen Boyd
Date: Tue Dec 04 2018 - 13:05:56 EST


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