Messages in this thread | | | From | "dbasehore ." <> | Date | Fri, 19 Oct 2018 16:07:21 -0700 | Subject | Re: [PATCH] clk: fix clk_calc_subtree compute duplications |
| |
On Fri, Aug 31, 2018 at 2:20 PM Derek Basehore <dbasehore@chromium.org> wrote: > > clk_calc_subtree was called at every step up the clk tree in > clk_calc_new_rates. Since it recursively calls itself for its > children, this means it would be called once on each clk for each > step above the top clk is. > > This is fixed by adding a non-recursive function called at every > step in clk_calc_new_rates that fills in new_rate, new_parent, etc. > Since the clks not called directly for clk_calc_new_rates can only > change their rate, we only set new_rate in clk_calc_subtree. > clk_calc_subtree is also only called on the top clk after it's found > via clk_calc_new_rates to remove the duplicate recursive calls. > > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > --- > drivers/clk/clk.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index d31055ae6ec6..52032fb1a8a2 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1609,11 +1609,18 @@ static int __clk_speculate_rates(struct clk_core *core, > return ret; > } > > -static void clk_calc_subtree(struct clk_core *core, unsigned long new_rate, > - struct clk_core *new_parent, u8 p_index) > +static void clk_calc_subtree(struct clk_core *core, unsigned long new_rate) > { > struct clk_core *child; > > + core->new_rate = new_rate; > + hlist_for_each_entry(child, &core->children, child_node) > + clk_calc_subtree(child, clk_recalc(child, new_rate)); > +} > + > +static void clk_set_change(struct clk_core *core, unsigned long new_rate, > + struct clk_core *new_parent, u8 p_index) > +{ > core->new_rate = new_rate; > core->new_parent = new_parent; > core->new_parent_index = p_index; > @@ -1621,11 +1628,6 @@ static void clk_calc_subtree(struct clk_core *core, unsigned long new_rate, > core->new_child = NULL; > if (new_parent && new_parent != core->parent) > new_parent->new_child = core; > - > - hlist_for_each_entry(child, &core->children, child_node) { > - child->new_rate = clk_recalc(child, new_rate); > - clk_calc_subtree(child, child->new_rate, NULL, 0); > - } > } > > /* > @@ -1709,7 +1711,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, > top = clk_calc_new_rates(parent, best_parent_rate); > > out: > - clk_calc_subtree(core, new_rate, parent, p_index); > + clk_set_change(core, new_rate, parent, p_index); > > return top; > } > @@ -1910,6 +1912,8 @@ static int clk_core_set_rate_nolock(struct clk_core *core, > if (ret) > return ret; > > + clk_calc_subtree(top, top->new_rate); > +
Oops. This is wrong. Calling here will overwrite new rate settings such as when determine_rate/round_rate return something different than clk_recalc(core, parent_rate). I'm working on a larger patch series where this will be fixed.
> /* notify that we are about to change rates */ > fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE); > if (fail_clk) { > -- > 2.19.0.rc1.350.ge57e33dbd1-goog >
| |