[RESEND PATCH 1/3] clk: Remove _clk_reparent from API and restructure code
Mike Turquette
mturquette at linaro.org
Wed Mar 20 11:03:15 EDT 2013
Quoting Rajendra Nayak (2013-03-20 03:13:36)
> On Wednesday 20 March 2013 03:15 PM, Ulf Hansson wrote:
> > On 19 March 2013 22:20, Mike Turquette <mturquette at linaro.org> wrote:
> >> Quoting Ulf Hansson (2013-03-12 12:20:48)
> >>> From: Ulf Hansson <ulf.hansson at linaro.org>
> >>>
> >>> There shall be no reason for keeping this API available for clock
> >>> providers. So we remove it from the API and restrcuture the code so
> >>> for example the COMMON_CLK_DEBUG part is separated.
> >>>
> >>
> >> Hi Ulf,
> >>
> >> There is one reason to keep this api. OMAP currently uses it to change
> >> the parent of a PLL during .set_rate. This is kind of a legacy
> >> mechanism however, and the reentrancy series I posted actually takes
> >> care of this one corner case:
> >> http://article.gmane.org/gmane.linux.kernel/1448449
> >
> > Hi Mike,
> >
> > It was a while ago I created this patch, so I guess this has beed
> > merged without me noticing, sorry.
> >
> >>
> >> Let me see how the OMAP folks feel about taking that patch in, which
> >> eliminates the only external user of the API. +Rajendra & Benoit
> >>
> >
> > I have no problem rebasing the patch and keep the API, if that is the
> > easiest way forward.
>
> Ulf, It would be good if you could keep the api for now. I seemed to have
> missed out on Mikes patch getting rid of the the api from OMAP PLL .set_rate.
> I will take a stab at that to get rid of that api along with the other OMAP
> only things that exist today as part of Common clk.
>
Rajendra,
Yeah, the patch I linked to above was actually just "example code"
showing how to reentrantly call clk_set_parent from within a .set_rate
callback. It's no suprise you missed it since the patch $SUBJECT starts
with "HACK:" ;-)
The trick is to recognize that OMAP's PLLs are mux clocks and have them
properly support clk_set_parent. This would have made my life easier in
the bad days of DPLL cascading back in 2011.
> regards,
> Rajendra
>
> > Although, I guess in the end you want to remove these kind of internal
> > clk API functions. So, if we get ack from Rajendra & Benoit, it is
> > better to remove the API right?
> >
Ulf,
Correct, as we discussed at LCA I do want to get rid of all of these
internal accessor functions.
Thanks,
Mike
> > Kind regards
> > Ulf Hansson
> >
> >> Thanks,
> >> Mike
> >>
> >>> This patch will also make it possible to hold the spinlock over the
> >>> actual update of the clock tree topology, which could not be done
> >>> before when both debugfs updates and clock rate updates was done
> >>> within the same function.
> >>>
> >>> Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
> >>> ---
> >>> drivers/clk/clk.c | 82 ++++++++++++++++++++++++------------------
> >>> include/linux/clk-provider.h | 1 -
> >>> 2 files changed, 48 insertions(+), 35 deletions(-)
> >>>
> >>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >>> index 593a2e4..2e10cc1 100644
> >>> --- a/drivers/clk/clk.c
> >>> +++ b/drivers/clk/clk.c
> >>> @@ -284,6 +284,39 @@ out:
> >>> }
> >>>
> >>> /**
> >>> + * clk_debug_reparent - reparent clk node in the debugfs clk tree
> >>> + * @clk: the clk being reparented
> >>> + * @new_parent: the new clk parent, may be NULL
> >>> + *
> >>> + * Rename clk entry in the debugfs clk tree if debugfs has been
> >>> + * initialized. Otherwise it bails out early since the debugfs clk tree
> >>> + * will be created lazily by clk_debug_init as part of a late_initcall.
> >>> + *
> >>> + * Caller must hold prepare_lock.
> >>> + */
> >>> +static void clk_debug_reparent(struct clk *clk, struct clk *new_parent)
> >>> +{
> >>> + struct dentry *d;
> >>> + struct dentry *new_parent_d;
> >>> +
> >>> + if (!inited)
> >>> + return;
> >>> +
> >>> + if (new_parent)
> >>> + new_parent_d = new_parent->dentry;
> >>> + else
> >>> + new_parent_d = orphandir;
> >>> +
> >>> + d = debugfs_rename(clk->dentry->d_parent, clk->dentry,
> >>> + new_parent_d, clk->name);
> >>> + if (d)
> >>> + clk->dentry = d;
> >>> + else
> >>> + pr_debug("%s: failed to rename debugfs entry for %s\n",
> >>> + __func__, clk->name);
> >>> +}
> >>> +
> >>> +/**
> >>> * clk_debug_init - lazily create the debugfs clk tree visualization
> >>> *
> >>> * clks are often initialized very early during boot before memory can
> >>> @@ -338,6 +371,9 @@ static int __init clk_debug_init(void)
> >>> late_initcall(clk_debug_init);
> >>> #else
> >>> static inline int clk_debug_register(struct clk *clk) { return 0; }
> >>> +static inline void clk_debug_reparent(struct clk *clk, struct clk *new_parent)
> >>> +{
> >>> +}
> >>> #endif
> >>>
> >>> /* caller must hold prepare_lock */
> >>> @@ -1179,16 +1215,8 @@ out:
> >>> return ret;
> >>> }
> >>>
> >>> -void __clk_reparent(struct clk *clk, struct clk *new_parent)
> >>> +static void clk_reparent(struct clk *clk, struct clk *new_parent)
> >>> {
> >>> -#ifdef CONFIG_COMMON_CLK_DEBUG
> >>> - struct dentry *d;
> >>> - struct dentry *new_parent_d;
> >>> -#endif
> >>> -
> >>> - if (!clk || !new_parent)
> >>> - return;
> >>> -
> >>> hlist_del(&clk->child_node);
> >>>
> >>> if (new_parent)
> >>> @@ -1196,28 +1224,7 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
> >>> else
> >>> hlist_add_head(&clk->child_node, &clk_orphan_list);
> >>>
> >>> -#ifdef CONFIG_COMMON_CLK_DEBUG
> >>> - if (!inited)
> >>> - goto out;
> >>> -
> >>> - if (new_parent)
> >>> - new_parent_d = new_parent->dentry;
> >>> - else
> >>> - new_parent_d = orphandir;
> >>> -
> >>> - d = debugfs_rename(clk->dentry->d_parent, clk->dentry,
> >>> - new_parent_d, clk->name);
> >>> - if (d)
> >>> - clk->dentry = d;
> >>> - else
> >>> - pr_debug("%s: failed to rename debugfs entry for %s\n",
> >>> - __func__, clk->name);
> >>> -out:
> >>> -#endif
> >>> -
> >>> clk->parent = new_parent;
> >>> -
> >>> - __clk_recalc_rates(clk, POST_RATE_CHANGE);
> >>> }
> >>>
> >>> static int __clk_set_parent(struct clk *clk, struct clk *parent)
> >>> @@ -1329,7 +1336,9 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
> >>> }
> >>>
> >>> /* propagate rate recalculation downstream */
> >>> - __clk_reparent(clk, parent);
> >>> + clk_reparent(clk, parent);
> >>> + clk_debug_reparent(clk, parent);
> >>> + __clk_recalc_rates(clk, POST_RATE_CHANGE);
> >>>
> >>> out:
> >>> mutex_unlock(&prepare_lock);
> >>> @@ -1453,14 +1462,19 @@ int __clk_init(struct device *dev, struct clk *clk)
> >>> hlist_for_each_entry_safe(orphan, tmp, tmp2, &clk_orphan_list, child_node) {
> >>> if (orphan->ops->get_parent) {
> >>> i = orphan->ops->get_parent(orphan->hw);
> >>> - if (!strcmp(clk->name, orphan->parent_names[i]))
> >>> - __clk_reparent(orphan, clk);
> >>> + if (!strcmp(clk->name, orphan->parent_names[i])) {
> >>> + clk_reparent(orphan, clk);
> >>> + clk_debug_reparent(orphan, clk);
> >>> + __clk_recalc_rates(orphan, POST_RATE_CHANGE);
> >>> + }
> >>> continue;
> >>> }
> >>>
> >>> for (i = 0; i < orphan->num_parents; i++)
> >>> if (!strcmp(clk->name, orphan->parent_names[i])) {
> >>> - __clk_reparent(orphan, clk);
> >>> + clk_reparent(orphan, clk);
> >>> + clk_debug_reparent(orphan, clk);
> >>> + __clk_recalc_rates(orphan, POST_RATE_CHANGE);
> >>> break;
> >>> }
> >>> }
> >>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> >>> index 4989b8a..87a7c2c 100644
> >>> --- a/include/linux/clk-provider.h
> >>> +++ b/include/linux/clk-provider.h
> >>> @@ -359,7 +359,6 @@ struct clk *__clk_lookup(const char *name);
> >>> */
> >>> int __clk_prepare(struct clk *clk);
> >>> void __clk_unprepare(struct clk *clk);
> >>> -void __clk_reparent(struct clk *clk, struct clk *new_parent);
> >>> unsigned long __clk_round_rate(struct clk *clk, unsigned long rate);
> >>>
> >>> struct of_device_id;
> >>> --
> >>> 1.7.10
More information about the linux-arm-kernel
mailing list