Re: [PATCH v2 2/3] clk: add support for clock reparent on set_rate
From: Mike Turquette
Date: Tue May 14 2013 - 14:13:28 EST
Quoting James Hogan (2013-04-19 09:28:23)
> Add core support to allow clock implementations to select the best
> parent clock when rounding a rate, e.g. the one which can provide the
> closest clock rate to that requested. This is by way of adding a new
> clock op, determine_rate(), which is like round_rate() but has an extra
> parameter to allow the clock implementation to optionally select a
> different parent clock. The core then takes care of reparenting the
> clock when setting the rate.
>
Hi James,
Can you rebase this onto -rc1? It does not apply cleanly.
Thanks,
Mike
> The parent change takes place with the help of two new private data
> members. struct clk::new_parent specifies a clock's new parent (NULL
> indicates no change), and struct clk::new_child specifies a clock's new
> child (whose new_parent member points back to it). The purpose of these
> are to allow correct walking of the future tree for notifications prior
> to actually reparenting any clocks, specifically to skip child clocks
> who are being reparented to another clock (they will be notified via the
> new parent), and to include any new child clock. These pointers are set
> by clk_calc_subtree(), and the new_child pointer gets cleared when a
> child is actually reparented to avoid duplicate POST_RATE_CHANGE
> notifications.
>
> Each place where round_rate() is called, determine_rate is checked first
> and called in preference, passing NULL to the new parent argument if not
> needed (e.g. in __clk_round_rate). This restructures a few of the call
> sites to simplify the logic into if/else blocks.
>
> A new __clk_set_parent_no_recalc() is created similar to
> clk_set_parent() but without calling __clk_recalc_rates(). This is for
> clk_change_rate() to use, where rate recalculation and notifications are
> already handled.
>
> Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
> ---
> Documentation/clk.txt | 4 ++
> drivers/clk/clk.c | 146 ++++++++++++++++++++++++++++++-------------
> include/linux/clk-private.h | 2 +
> include/linux/clk-provider.h | 7 +++
> 4 files changed, 116 insertions(+), 43 deletions(-)
>
> diff --git a/Documentation/clk.txt b/Documentation/clk.txt
> index 1943fae..502c033 100644
> --- a/Documentation/clk.txt
> +++ b/Documentation/clk.txt
> @@ -70,6 +70,10 @@ the operations defined in clk.h:
> unsigned long parent_rate);
> long (*round_rate)(struct clk_hw *hw, unsigned long,
> unsigned long *);
> + long (*determine_rate)(struct clk_hw *hw,
> + unsigned long rate,
> + unsigned long *best_parent_rate,
> + struct clk **best_parent_clk);
> int (*set_parent)(struct clk_hw *hw, u8 index);
> u8 (*get_parent)(struct clk_hw *hw);
> int (*set_rate)(struct clk_hw *hw, unsigned long);
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 79d5deb..cc0e618 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -27,6 +27,8 @@ static HLIST_HEAD(clk_root_list);
> static HLIST_HEAD(clk_orphan_list);
> static LIST_HEAD(clk_notifier_list);
>
> +static int __clk_set_parent_no_recalc(struct clk *clk, struct clk *parent);
> +
> /*** debugfs support ***/
>
> #ifdef CONFIG_COMMON_CLK_DEBUG
> @@ -727,17 +729,20 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned long rate)
> if (!clk)
> return 0;
>
> - if (!clk->ops->round_rate) {
> - if (clk->flags & CLK_SET_RATE_PARENT)
> - return __clk_round_rate(clk->parent, rate);
> - else
> - return clk->rate;
> - }
>
> if (clk->parent)
> parent_rate = clk->parent->rate;
>
> - return clk->ops->round_rate(clk->hw, rate, &parent_rate);
> + if (clk->ops->determine_rate)
> + return clk->ops->determine_rate(clk->hw, rate, &parent_rate,
> + NULL);
> + else if (clk->ops->round_rate)
> + return clk->ops->round_rate(clk->hw, rate, &parent_rate);
> + else if (clk->flags & CLK_SET_RATE_PARENT)
> + return __clk_round_rate(clk->parent, rate);
> + else
> + return clk->rate;
> +
> }
>
> /**
> @@ -906,18 +911,23 @@ out:
> return ret;
> }
>
> -static void clk_calc_subtree(struct clk *clk, unsigned long new_rate)
> +static void clk_calc_subtree(struct clk *clk, unsigned long new_rate,
> + struct clk *new_parent)
> {
> struct clk *child;
>
> clk->new_rate = new_rate;
> + clk->new_parent = new_parent;
> + clk->new_child = NULL;
> + if (new_parent && new_parent != clk->parent)
> + new_parent->new_child = clk;
>
> hlist_for_each_entry(child, &clk->children, child_node) {
> if (child->ops->recalc_rate)
> child->new_rate = child->ops->recalc_rate(child->hw, new_rate);
> else
> child->new_rate = new_rate;
> - clk_calc_subtree(child, child->new_rate);
> + clk_calc_subtree(child, child->new_rate, NULL);
> }
> }
>
> @@ -928,6 +938,7 @@ static void clk_calc_subtree(struct clk *clk, unsigned long new_rate)
> static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate)
> {
> struct clk *top = clk;
> + struct clk *old_parent, *parent;
> unsigned long best_parent_rate = 0;
> unsigned long new_rate;
>
> @@ -936,42 +947,43 @@ static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate)
> return NULL;
>
> /* save parent rate, if it exists */
> - if (clk->parent)
> - best_parent_rate = clk->parent->rate;
> -
> - /* never propagate up to the parent */
> - if (!(clk->flags & CLK_SET_RATE_PARENT)) {
> - if (!clk->ops->round_rate) {
> - clk->new_rate = clk->rate;
> - return NULL;
> - }
> - new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate);
> + parent = old_parent = clk->parent;
> + if (parent)
> + best_parent_rate = parent->rate;
> +
> + /* find the closest rate and parent clk/rate */
> + if (clk->ops->determine_rate) {
> + new_rate = clk->ops->determine_rate(clk->hw, rate,
> + &best_parent_rate,
> + &parent);
> + } else if (clk->ops->round_rate) {
> + new_rate = clk->ops->round_rate(clk->hw, rate,
> + &best_parent_rate);
> + } else if (!parent || !(clk->flags & CLK_SET_RATE_PARENT)) {
> + /* pass-through clock without adjustable parent */
> + clk->new_rate = clk->rate;
> + return NULL;
> + } else {
> + /* pass-through clock with adjustable parent */
> + top = clk_calc_new_rates(parent, rate);
> + new_rate = parent->new_rate;
> goto out;
> }
>
> - /* need clk->parent from here on out */
> - if (!clk->parent) {
> - pr_debug("%s: %s has NULL parent\n", __func__, clk->name);
> + /* some clocks must be gated to change parent */
> + if (parent != old_parent &&
> + (clk->flags & CLK_SET_PARENT_GATE) && clk->prepare_count) {
> + pr_debug("%s: %s not gated but wants to reparent\n",
> + __func__, clk->name);
> return NULL;
> }
>
> - if (!clk->ops->round_rate) {
> - top = clk_calc_new_rates(clk->parent, rate);
> - new_rate = clk->parent->new_rate;
> -
> - goto out;
> - }
> -
> - new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate);
> -
> - if (best_parent_rate != clk->parent->rate) {
> - top = clk_calc_new_rates(clk->parent, best_parent_rate);
> -
> - goto out;
> - }
> + if ((clk->flags & CLK_SET_RATE_PARENT) && parent &&
> + best_parent_rate != parent->rate)
> + top = clk_calc_new_rates(parent, best_parent_rate);
>
> out:
> - clk_calc_subtree(clk, new_rate);
> + clk_calc_subtree(clk, new_rate, parent);
>
> return top;
> }
> @@ -983,7 +995,7 @@ out:
> */
> static struct clk *clk_propagate_rate_change(struct clk *clk, unsigned long event)
> {
> - struct clk *child, *fail_clk = NULL;
> + struct clk *child, *tmp_clk, *fail_clk = NULL;
> int ret = NOTIFY_DONE;
>
> if (clk->rate == clk->new_rate)
> @@ -996,9 +1008,19 @@ static struct clk *clk_propagate_rate_change(struct clk *clk, unsigned long even
> }
>
> hlist_for_each_entry(child, &clk->children, child_node) {
> - clk = clk_propagate_rate_change(child, event);
> - if (clk)
> - fail_clk = clk;
> + /* Skip children who will be reparented to another clock */
> + if (child->new_parent && child->new_parent != clk)
> + continue;
> + tmp_clk = clk_propagate_rate_change(child, event);
> + if (tmp_clk)
> + fail_clk = tmp_clk;
> + }
> +
> + /* handle the new child who might not be in clk->children yet */
> + if (clk->new_child) {
> + tmp_clk = clk_propagate_rate_change(clk->new_child, event);
> + if (tmp_clk)
> + fail_clk = tmp_clk;
> }
>
> return fail_clk;
> @@ -1016,6 +1038,10 @@ static void clk_change_rate(struct clk *clk)
>
> old_rate = clk->rate;
>
> + /* set parent */
> + if (clk->new_parent && clk->new_parent != clk->parent)
> + __clk_set_parent_no_recalc(clk, clk->new_parent);
> +
> if (clk->parent)
> best_parent_rate = clk->parent->rate;
>
> @@ -1030,8 +1056,15 @@ static void clk_change_rate(struct clk *clk)
> if (clk->notifier_count && old_rate != clk->rate)
> __clk_notify(clk, POST_RATE_CHANGE, old_rate, clk->rate);
>
> - hlist_for_each_entry(child, &clk->children, child_node)
> + hlist_for_each_entry(child, &clk->children, child_node) {
> + /* Skip children who will be reparented to another clock */
> + if (child->new_parent && child->new_parent != clk)
> + continue;
> clk_change_rate(child);
> + }
> +
> + if (clk->new_child)
> + clk_change_rate(clk->new_child);
> }
>
> /**
> @@ -1169,7 +1202,7 @@ out:
> return ret;
> }
>
> -void __clk_reparent(struct clk *clk, struct clk *new_parent)
> +void __clk_reparent_no_recalc(struct clk *clk, struct clk *new_parent)
> {
> #ifdef CONFIG_COMMON_CLK_DEBUG
> struct dentry *d;
> @@ -1179,6 +1212,9 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
> if (!clk || !new_parent)
> return;
>
> + if (new_parent->new_child == clk)
> + new_parent->new_child = NULL;
> +
> hlist_del(&clk->child_node);
>
> if (new_parent)
> @@ -1206,6 +1242,11 @@ out:
> #endif
>
> clk->parent = new_parent;
> +}
> +
> +void __clk_reparent(struct clk *clk, struct clk *new_parent)
> +{
> + __clk_reparent_no_recalc(clk, new_parent);
>
> __clk_recalc_rates(clk, POST_RATE_CHANGE);
> }
> @@ -1270,6 +1311,25 @@ out:
> return ret;
> }
>
> +static int __clk_set_parent_no_recalc(struct clk *clk, struct clk *parent)
> +{
> + int ret = 0;
> +
> + if (clk->parent == parent)
> + goto out;
> +
> + /* only re-parent if the clock is not in use */
> + ret = __clk_set_parent(clk, parent);
> + if (ret)
> + goto out;
> +
> + /* reparent, but don't propagate rate recalculation downstream */
> + __clk_reparent_no_recalc(clk, parent);
> +
> +out:
> + return ret;
> +}
> +
> /**
> * clk_set_parent - switch the parent of a mux clk
> * @clk: the mux clk whose input we are switching
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index 9c7f580..0826a60 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -35,6 +35,8 @@ struct clk {
> u8 num_parents;
> unsigned long rate;
> unsigned long new_rate;
> + struct clk *new_parent;
> + struct clk *new_child;
> unsigned long flags;
> unsigned int enable_count;
> unsigned int prepare_count;
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4e0b634..5fe1d38 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -71,6 +71,10 @@ struct clk_hw;
> * @round_rate: Given a target rate as input, returns the closest rate actually
> * supported by the clock.
> *
> + * @determine_rate: Given a target rate as input, returns the closest rate
> + * actually supported by the clock, and optionally the parent clock
> + * that should be used to provide the clock rate.
> + *
> * @get_parent: Queries the hardware to determine the parent of a clock. The
> * return value is a u8 which specifies the index corresponding to
> * the parent clock. This index can be applied to either the
> @@ -116,6 +120,9 @@ struct clk_ops {
> unsigned long parent_rate);
> long (*round_rate)(struct clk_hw *hw, unsigned long,
> unsigned long *);
> + long (*determine_rate)(struct clk_hw *hw, unsigned long rate,
> + unsigned long *best_parent_rate,
> + struct clk **best_parent_clk);
> int (*set_parent)(struct clk_hw *hw, u8 index);
> u8 (*get_parent)(struct clk_hw *hw);
> int (*set_rate)(struct clk_hw *hw, unsigned long,
> --
> 1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/