diff mbox series

[3/6] clk: change rates via list iteration

Message ID 20181024013132.115907-4-dbasehore@chromium.org (mailing list archive)
State New, archived
Headers show
Series Coordinated Clks | expand

Commit Message

Derek Basehore Oct. 24, 2018, 1:31 a.m. UTC
This changes the clk_set_rate code to use lists instead of recursion.
While making this change, also add error handling for clk_set_rate.
This means that errors in the set_rate/set_parent/set_rate_and_parent
functions will not longer be ignored. When an error occurs, the clk
rates and parents are reset, unless an error occurs here, in which we
bail and cross our fingers.

Signed-off-by: Derek Basehore <dbasehore@chromium.org>
---
 drivers/clk/clk.c | 225 +++++++++++++++++++++++++++++++---------------
 1 file changed, 153 insertions(+), 72 deletions(-)

Comments

Derek Basehore Oct. 26, 2018, 3:29 a.m. UTC | #1
On Tue, Oct 23, 2018 at 6:31 PM Derek Basehore <dbasehore@chromium.org> wrote:
>
> This changes the clk_set_rate code to use lists instead of recursion.
> While making this change, also add error handling for clk_set_rate.
> This means that errors in the set_rate/set_parent/set_rate_and_parent
> functions will not longer be ignored. When an error occurs, the clk
> rates and parents are reset, unless an error occurs here, in which we
> bail and cross our fingers.
>
> Signed-off-by: Derek Basehore <dbasehore@chromium.org>
> ---
>  drivers/clk/clk.c | 225 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 153 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 61de8ad3e4cf..1db44b4e46b0 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -42,6 +42,13 @@ static LIST_HEAD(clk_notifier_list);
>
>  /***    private data structures    ***/
>
> +struct clk_change {
> +       struct list_head        change_list;
> +       unsigned long           rate;
> +       struct clk_core         *core;
> +       struct clk_core         *parent;
> +};
> +
>  struct clk_core {
>         const char              *name;
>         const struct clk_ops    *ops;
> @@ -52,11 +59,9 @@ struct clk_core {
>         const char              **parent_names;
>         struct clk_core         **parents;
>         u8                      num_parents;
> -       u8                      new_parent_index;
>         unsigned long           rate;
>         unsigned long           req_rate;
> -       unsigned long           new_rate;
> -       struct clk_core         *new_parent;
> +       struct clk_change       change;
>         struct clk_core         *new_child;
>         unsigned long           flags;
>         bool                    orphan;
> @@ -1719,20 +1724,53 @@ static int __clk_speculate_rates(struct clk_core *core,
>
>  static void clk_calc_subtree(struct clk_core *core)
>  {
> -       struct clk_core *child;
> +       LIST_HEAD(tmp_list);
>
> -       hlist_for_each_entry(child, &core->children, child_node) {
> -               child->new_rate = clk_recalc(child, core->new_rate);
> -               clk_calc_subtree(child);
> +       list_add(&core->change.change_list, &tmp_list);
> +       while (!list_empty(&tmp_list)) {
> +               struct clk_change *change = list_first_entry(&tmp_list,
> +                               struct clk_change, change_list);
> +               struct clk_core *tmp = change->core;
> +               struct clk_core *child;
> +
> +               hlist_for_each_entry(child, &tmp->children, child_node) {
> +                       child->change.rate = clk_recalc(child,
> +                                       tmp->change.rate);
> +                       list_add_tail(&child->change.change_list, &tmp_list);
> +               }
> +
> +               list_del_init(&change->change_list);
> +       }
> +}
> +
> +static void clk_prepare_changes(struct list_head *change_list,
> +                               struct clk_core *core)
> +{
> +       LIST_HEAD(tmp_list);
> +
> +       list_add(&core->change.change_list, &tmp_list);
> +       while (!list_empty(&tmp_list)) {
> +               struct clk_change *change = list_first_entry(&tmp_list,
> +                               struct clk_change, change_list);
> +               struct clk_core *tmp = change->core;
> +               struct clk_core *child;
> +
> +               hlist_for_each_entry(child, &tmp->children, child_node)
> +                       list_add_tail(&child->change.change_list, &tmp_list);
> +
> +               child = tmp->new_child;
> +               if (child)
> +                       list_add_tail(&child->change.change_list, &tmp_list);
> +
> +               list_move_tail(&tmp->change.change_list, change_list);
>         }
>  }
>
>  static void clk_set_change(struct clk_core *core, unsigned long new_rate,
> -                          struct clk_core *new_parent, u8 p_index)
> +                          struct clk_core *new_parent)
>  {
> -       core->new_rate = new_rate;
> -       core->new_parent = new_parent;
> -       core->new_parent_index = p_index;
> +       core->change.rate = new_rate;
> +       core->change.parent = new_parent;
>         /* include clk in new parent's PRE_RATE_CHANGE notifications */
>         core->new_child = NULL;
>         if (new_parent && new_parent != core->parent)
> @@ -1752,7 +1790,6 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core,
>         unsigned long new_rate;
>         unsigned long min_rate;
>         unsigned long max_rate;
> -       int p_index = 0;
>         long ret;
>
>         /* sanity */
> @@ -1788,14 +1825,13 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core,
>                         return NULL;
>         } else if (!parent || !(core->flags & CLK_SET_RATE_PARENT)) {
>                 /* pass-through clock without adjustable parent */
> -               core->new_rate = core->rate;
>                 return NULL;
>         } else {
>                 /* pass-through clock with adjustable parent */
>                 top = clk_calc_new_rates(parent, rate);
> -               new_rate = parent->new_rate;
> +               new_rate = parent->change.rate;
>                 hlist_for_each_entry(child, &parent->children, child_node)
> -                       child->new_rate = clk_recalc(child, new_rate);
> +                       child->change.rate = clk_recalc(child, new_rate);
>                 goto out;
>         }
>
> @@ -1807,25 +1843,16 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core,
>                 return NULL;
>         }
>
> -       /* try finding the new parent index */
> -       if (parent && core->num_parents > 1) {
> -               p_index = clk_fetch_parent_index(core, parent);
> -               if (p_index < 0) {
> -                       pr_debug("%s: clk %s can not be parent of clk %s\n",
> -                                __func__, parent->name, core->name);
> -                       return NULL;
> -               }
> -       }
> -
>         if ((core->flags & CLK_SET_RATE_PARENT) && parent &&
>             best_parent_rate != parent->rate) {
>                 top = clk_calc_new_rates(parent, best_parent_rate);
>                 hlist_for_each_entry(child, &parent->children, child_node)
> -                       child->new_rate = clk_recalc(child, parent->new_rate);
> +                       child->change.rate = clk_recalc(child,
> +                                       parent->change.rate);
>         }
>
>  out:
> -       clk_set_change(core, new_rate, parent, p_index);
> +       clk_set_change(core, new_rate, parent);
>
>         return top;
>  }
> @@ -1841,18 +1868,18 @@ static struct clk_core *clk_propagate_rate_change(struct clk_core *core,
>         struct clk_core *child, *tmp_clk, *fail_clk = NULL;
>         int ret = NOTIFY_DONE;
>
> -       if (core->rate == core->new_rate)
> +       if (core->rate == core->change.rate)
>                 return NULL;
>
>         if (core->notifier_count) {
> -               ret = __clk_notify(core, event, core->rate, core->new_rate);
> +               ret = __clk_notify(core, event, core->rate, core->change.rate);
>                 if (ret & NOTIFY_STOP_MASK)
>                         fail_clk = core;
>         }
>
>         hlist_for_each_entry(child, &core->children, child_node) {
>                 /* Skip children who will be reparented to another clock */
> -               if (child->new_parent && child->new_parent != core)
> +               if (child->change.parent && child->change.parent != core)
>                         continue;
>                 tmp_clk = clk_propagate_rate_change(child, event);
>                 if (tmp_clk)
> @@ -1873,28 +1900,30 @@ static struct clk_core *clk_propagate_rate_change(struct clk_core *core,
>   * walk down a subtree and set the new rates notifying the rate
>   * change on the way
>   */
> -static void clk_change_rate(struct clk_core *core)
> +static int clk_change_rate(struct clk_change *change)
>  {
> -       struct clk_core *child;
> -       struct hlist_node *tmp;
> +       struct clk_core *core = change->core;
>         unsigned long old_rate;
>         unsigned long best_parent_rate = 0;
>         bool skip_set_rate = false;
> -       struct clk_core *old_parent;
> +       struct clk_core *old_parent = NULL;
>         struct clk_core *parent = NULL;
> +       int p_index;
> +       int ret = 0;
>
>         old_rate = core->rate;
>
> -       if (core->new_parent) {
> -               parent = core->new_parent;
> -               best_parent_rate = core->new_parent->rate;
> +       if (change->parent) {
> +               parent = change->parent;
> +               best_parent_rate = parent->rate;
>         } else if (core->parent) {
>                 parent = core->parent;
> -               best_parent_rate = core->parent->rate;
> +               best_parent_rate = parent->rate;
>         }
>
> -       if (clk_pm_runtime_get(core))
> -               return;
> +       ret = clk_pm_runtime_get(core);
> +       if (ret)
> +               return ret;
>
>         if (core->flags & CLK_SET_RATE_UNGATE) {
>                 unsigned long flags;
> @@ -1905,35 +1934,55 @@ static void clk_change_rate(struct clk_core *core)
>                 clk_enable_unlock(flags);
>         }
>
> -       if (core->new_parent && core->new_parent != core->parent) {
> -               old_parent = __clk_set_parent_before(core, core->new_parent);
> -               trace_clk_set_parent(core, core->new_parent);
> +       if (core->flags & CLK_OPS_PARENT_ENABLE)
> +               clk_core_prepare_enable(parent);
> +
> +       if (parent != core->parent) {
> +               p_index = clk_fetch_parent_index(core, parent);
> +               if (p_index < 0) {
> +                       pr_debug("%s: clk %s can not be parent of clk %s\n",
> +                                __func__, parent->name, core->name);
> +                       ret = p_index;
> +                       goto out;
> +               }
> +               old_parent = __clk_set_parent_before(core, parent);
> +
> +               trace_clk_set_parent(core, change->parent);
>
>                 if (core->ops->set_rate_and_parent) {
>                         skip_set_rate = true;
> -                       core->ops->set_rate_and_parent(core->hw, core->new_rate,
> +                       ret = core->ops->set_rate_and_parent(core->hw,
> +                                       change->rate,
>                                         best_parent_rate,
> -                                       core->new_parent_index);
> +                                       p_index);
>                 } else if (core->ops->set_parent) {
> -                       core->ops->set_parent(core->hw, core->new_parent_index);
> +                       ret = core->ops->set_parent(core->hw, p_index);
>                 }
>
> -               trace_clk_set_parent_complete(core, core->new_parent);
> -               __clk_set_parent_after(core, core->new_parent, old_parent);
> -       }
> +               trace_clk_set_parent_complete(core, change->parent);
> +               __clk_set_parent_after(core, change->parent, old_parent);
> +               if (ret)
> +                       goto out;

Need to handle the set_parent op failing here. __clk_set_parent_before
needs to be undone, and we need to not call __clk_set_parent_after
with the failed state.

>
> -       if (core->flags & CLK_OPS_PARENT_ENABLE)
> -               clk_core_prepare_enable(parent);
> +       }
>
> -       trace_clk_set_rate(core, core->new_rate);
> +       trace_clk_set_rate(core, change->rate);
>
>         if (!skip_set_rate && core->ops->set_rate)
> -               core->ops->set_rate(core->hw, core->new_rate, best_parent_rate);
> +               ret = core->ops->set_rate(core->hw, change->rate,
> +                               best_parent_rate);
>
> -       trace_clk_set_rate_complete(core, core->new_rate);
> +       trace_clk_set_rate_complete(core, change->rate);
>
>         core->rate = clk_recalc(core, best_parent_rate);
>
> +out:
> +       if (core->flags & CLK_OPS_PARENT_ENABLE)
> +               clk_core_disable_unprepare(parent);
> +
> +       if (core->notifier_count && old_rate != core->rate)
> +               __clk_notify(core, POST_RATE_CHANGE, old_rate, core->rate);
> +
>         if (core->flags & CLK_SET_RATE_UNGATE) {
>                 unsigned long flags;
>
> @@ -1943,31 +1992,44 @@ static void clk_change_rate(struct clk_core *core)
>                 clk_core_unprepare(core);
>         }
>
> -       if (core->flags & CLK_OPS_PARENT_ENABLE)
> -               clk_core_disable_unprepare(parent);
> -
> -       if (core->notifier_count && old_rate != core->rate)
> -               __clk_notify(core, POST_RATE_CHANGE, old_rate, core->rate);
> +       clk_pm_runtime_put(core);
>
>         if (core->flags & CLK_RECALC_NEW_RATES)
> -               (void)clk_calc_new_rates(core, core->new_rate);
> +               (void)clk_calc_new_rates(core, change->rate);
>
>         /*
> -        * Use safe iteration, as change_rate can actually swap parents
> -        * for certain clock types.
> +        * Keep track of old parent and requested rate in case we have
> +        * to undo the change due to an error.
>          */
> -       hlist_for_each_entry_safe(child, tmp, &core->children, child_node) {
> -               /* Skip children who will be reparented to another clock */
> -               if (child->new_parent && child->new_parent != core)
> -                       continue;
> -               clk_change_rate(child);
> +       change->parent = old_parent;
> +       change->rate = old_rate;
> +       return ret;
> +}
> +
> +static int clk_change_rates(struct list_head *list)
> +{
> +       struct clk_change *change, *tmp;
> +       int ret = 0;
> +
> +       list_for_each_entry(change, list, change_list) {
> +               ret = clk_change_rate(change);
> +               if (ret)
> +                       goto err;
>         }
>
> -       /* handle the new child who might not be in core->children yet */
> -       if (core->new_child)
> -               clk_change_rate(core->new_child);
> +       return 0;
> +err:
> +       list_for_each_entry_safe_continue(change, tmp, list, change_list)
> +               list_del_init(&change->change_list);
>
> -       clk_pm_runtime_put(core);
> +       list_for_each_entry(change, list, change_list) {
> +               /* Give up. Driver might want to shutdown/reboot. */
> +               ret = clk_change_rate(change);
> +               if (WARN_ON(ret))
> +                       return ret;
> +       }
> +
> +       return ret;
>  }
>
>  static unsigned long clk_core_req_round_rate_nolock(struct clk_core *core,
> @@ -2001,7 +2063,9 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
>                                     unsigned long req_rate)
>  {
>         struct clk_core *top, *fail_clk;
> +       struct clk_change *change, *tmp;
>         unsigned long rate;
> +       LIST_HEAD(changes);
>         int ret = 0;
>
>         if (!core)
> @@ -2028,6 +2092,9 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
>
>         clk_calc_subtree(core);
>
> +       /* Construct the list of changes */
> +       clk_prepare_changes(&changes, top);
> +
>         /* notify that we are about to change rates */
>         fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
>         if (fail_clk) {
> @@ -2039,7 +2106,19 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
>         }
>
>         /* change the rates */
> -       clk_change_rate(top);
> +       ret = clk_change_rates(&changes);
> +       list_for_each_entry_safe(change, tmp, &changes, change_list) {
> +               change->rate = 0;
> +               change->parent = NULL;
> +               list_del_init(&change->change_list);
> +       }
> +
> +       if (ret) {
> +               pr_debug("%s: failed to set %s rate via top clk %s\n", __func__,
> +                               core->name, top->name);
> +               clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
> +               goto err;
> +       }
>
>         core->req_rate = req_rate;
>  err:
> @@ -3306,6 +3385,8 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
>         core->max_rate = ULONG_MAX;
>         INIT_LIST_HEAD(&core->prepare_list);
>         INIT_LIST_HEAD(&core->enable_list);
> +       INIT_LIST_HEAD(&core->change.change_list);
> +       core->change.core = core;
>         hw->core = core;
>
>         /* allocate local copy in case parent_names is __initdata */
> --
> 2.19.1.568.g152ad8e336-goog
>
diff mbox series

Patch

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 61de8ad3e4cf..1db44b4e46b0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -42,6 +42,13 @@  static LIST_HEAD(clk_notifier_list);
 
 /***    private data structures    ***/
 
+struct clk_change {
+	struct list_head	change_list;
+	unsigned long		rate;
+	struct clk_core		*core;
+	struct clk_core		*parent;
+};
+
 struct clk_core {
 	const char		*name;
 	const struct clk_ops	*ops;
@@ -52,11 +59,9 @@  struct clk_core {
 	const char		**parent_names;
 	struct clk_core		**parents;
 	u8			num_parents;
-	u8			new_parent_index;
 	unsigned long		rate;
 	unsigned long		req_rate;
-	unsigned long		new_rate;
-	struct clk_core		*new_parent;
+	struct clk_change	change;
 	struct clk_core		*new_child;
 	unsigned long		flags;
 	bool			orphan;
@@ -1719,20 +1724,53 @@  static int __clk_speculate_rates(struct clk_core *core,
 
 static void clk_calc_subtree(struct clk_core *core)
 {
-	struct clk_core *child;
+	LIST_HEAD(tmp_list);
 
-	hlist_for_each_entry(child, &core->children, child_node) {
-		child->new_rate = clk_recalc(child, core->new_rate);
-		clk_calc_subtree(child);
+	list_add(&core->change.change_list, &tmp_list);
+	while (!list_empty(&tmp_list)) {
+		struct clk_change *change = list_first_entry(&tmp_list,
+				struct clk_change, change_list);
+		struct clk_core *tmp = change->core;
+		struct clk_core *child;
+
+		hlist_for_each_entry(child, &tmp->children, child_node) {
+			child->change.rate = clk_recalc(child,
+					tmp->change.rate);
+			list_add_tail(&child->change.change_list, &tmp_list);
+		}
+
+		list_del_init(&change->change_list);
+	}
+}
+
+static void clk_prepare_changes(struct list_head *change_list,
+				struct clk_core *core)
+{
+	LIST_HEAD(tmp_list);
+
+	list_add(&core->change.change_list, &tmp_list);
+	while (!list_empty(&tmp_list)) {
+		struct clk_change *change = list_first_entry(&tmp_list,
+				struct clk_change, change_list);
+		struct clk_core *tmp = change->core;
+		struct clk_core *child;
+
+		hlist_for_each_entry(child, &tmp->children, child_node)
+			list_add_tail(&child->change.change_list, &tmp_list);
+
+		child = tmp->new_child;
+		if (child)
+			list_add_tail(&child->change.change_list, &tmp_list);
+
+		list_move_tail(&tmp->change.change_list, change_list);
 	}
 }
 
 static void clk_set_change(struct clk_core *core, unsigned long new_rate,
-			   struct clk_core *new_parent, u8 p_index)
+			   struct clk_core *new_parent)
 {
-	core->new_rate = new_rate;
-	core->new_parent = new_parent;
-	core->new_parent_index = p_index;
+	core->change.rate = new_rate;
+	core->change.parent = new_parent;
 	/* include clk in new parent's PRE_RATE_CHANGE notifications */
 	core->new_child = NULL;
 	if (new_parent && new_parent != core->parent)
@@ -1752,7 +1790,6 @@  static struct clk_core *clk_calc_new_rates(struct clk_core *core,
 	unsigned long new_rate;
 	unsigned long min_rate;
 	unsigned long max_rate;
-	int p_index = 0;
 	long ret;
 
 	/* sanity */
@@ -1788,14 +1825,13 @@  static struct clk_core *clk_calc_new_rates(struct clk_core *core,
 			return NULL;
 	} else if (!parent || !(core->flags & CLK_SET_RATE_PARENT)) {
 		/* pass-through clock without adjustable parent */
-		core->new_rate = core->rate;
 		return NULL;
 	} else {
 		/* pass-through clock with adjustable parent */
 		top = clk_calc_new_rates(parent, rate);
-		new_rate = parent->new_rate;
+		new_rate = parent->change.rate;
 		hlist_for_each_entry(child, &parent->children, child_node)
-			child->new_rate = clk_recalc(child, new_rate);
+			child->change.rate = clk_recalc(child, new_rate);
 		goto out;
 	}
 
@@ -1807,25 +1843,16 @@  static struct clk_core *clk_calc_new_rates(struct clk_core *core,
 		return NULL;
 	}
 
-	/* try finding the new parent index */
-	if (parent && core->num_parents > 1) {
-		p_index = clk_fetch_parent_index(core, parent);
-		if (p_index < 0) {
-			pr_debug("%s: clk %s can not be parent of clk %s\n",
-				 __func__, parent->name, core->name);
-			return NULL;
-		}
-	}
-
 	if ((core->flags & CLK_SET_RATE_PARENT) && parent &&
 	    best_parent_rate != parent->rate) {
 		top = clk_calc_new_rates(parent, best_parent_rate);
 		hlist_for_each_entry(child, &parent->children, child_node)
-			child->new_rate = clk_recalc(child, parent->new_rate);
+			child->change.rate = clk_recalc(child,
+					parent->change.rate);
 	}
 
 out:
-	clk_set_change(core, new_rate, parent, p_index);
+	clk_set_change(core, new_rate, parent);
 
 	return top;
 }
@@ -1841,18 +1868,18 @@  static struct clk_core *clk_propagate_rate_change(struct clk_core *core,
 	struct clk_core *child, *tmp_clk, *fail_clk = NULL;
 	int ret = NOTIFY_DONE;
 
-	if (core->rate == core->new_rate)
+	if (core->rate == core->change.rate)
 		return NULL;
 
 	if (core->notifier_count) {
-		ret = __clk_notify(core, event, core->rate, core->new_rate);
+		ret = __clk_notify(core, event, core->rate, core->change.rate);
 		if (ret & NOTIFY_STOP_MASK)
 			fail_clk = core;
 	}
 
 	hlist_for_each_entry(child, &core->children, child_node) {
 		/* Skip children who will be reparented to another clock */
-		if (child->new_parent && child->new_parent != core)
+		if (child->change.parent && child->change.parent != core)
 			continue;
 		tmp_clk = clk_propagate_rate_change(child, event);
 		if (tmp_clk)
@@ -1873,28 +1900,30 @@  static struct clk_core *clk_propagate_rate_change(struct clk_core *core,
  * walk down a subtree and set the new rates notifying the rate
  * change on the way
  */
-static void clk_change_rate(struct clk_core *core)
+static int clk_change_rate(struct clk_change *change)
 {
-	struct clk_core *child;
-	struct hlist_node *tmp;
+	struct clk_core *core = change->core;
 	unsigned long old_rate;
 	unsigned long best_parent_rate = 0;
 	bool skip_set_rate = false;
-	struct clk_core *old_parent;
+	struct clk_core *old_parent = NULL;
 	struct clk_core *parent = NULL;
+	int p_index;
+	int ret = 0;
 
 	old_rate = core->rate;
 
-	if (core->new_parent) {
-		parent = core->new_parent;
-		best_parent_rate = core->new_parent->rate;
+	if (change->parent) {
+		parent = change->parent;
+		best_parent_rate = parent->rate;
 	} else if (core->parent) {
 		parent = core->parent;
-		best_parent_rate = core->parent->rate;
+		best_parent_rate = parent->rate;
 	}
 
-	if (clk_pm_runtime_get(core))
-		return;
+	ret = clk_pm_runtime_get(core);
+	if (ret)
+		return ret;
 
 	if (core->flags & CLK_SET_RATE_UNGATE) {
 		unsigned long flags;
@@ -1905,35 +1934,55 @@  static void clk_change_rate(struct clk_core *core)
 		clk_enable_unlock(flags);
 	}
 
-	if (core->new_parent && core->new_parent != core->parent) {
-		old_parent = __clk_set_parent_before(core, core->new_parent);
-		trace_clk_set_parent(core, core->new_parent);
+	if (core->flags & CLK_OPS_PARENT_ENABLE)
+		clk_core_prepare_enable(parent);
+
+	if (parent != core->parent) {
+		p_index = clk_fetch_parent_index(core, parent);
+		if (p_index < 0) {
+			pr_debug("%s: clk %s can not be parent of clk %s\n",
+				 __func__, parent->name, core->name);
+			ret = p_index;
+			goto out;
+		}
+		old_parent = __clk_set_parent_before(core, parent);
+
+		trace_clk_set_parent(core, change->parent);
 
 		if (core->ops->set_rate_and_parent) {
 			skip_set_rate = true;
-			core->ops->set_rate_and_parent(core->hw, core->new_rate,
+			ret = core->ops->set_rate_and_parent(core->hw,
+					change->rate,
 					best_parent_rate,
-					core->new_parent_index);
+					p_index);
 		} else if (core->ops->set_parent) {
-			core->ops->set_parent(core->hw, core->new_parent_index);
+			ret = core->ops->set_parent(core->hw, p_index);
 		}
 
-		trace_clk_set_parent_complete(core, core->new_parent);
-		__clk_set_parent_after(core, core->new_parent, old_parent);
-	}
+		trace_clk_set_parent_complete(core, change->parent);
+		__clk_set_parent_after(core, change->parent, old_parent);
+		if (ret)
+			goto out;
 
-	if (core->flags & CLK_OPS_PARENT_ENABLE)
-		clk_core_prepare_enable(parent);
+	}
 
-	trace_clk_set_rate(core, core->new_rate);
+	trace_clk_set_rate(core, change->rate);
 
 	if (!skip_set_rate && core->ops->set_rate)
-		core->ops->set_rate(core->hw, core->new_rate, best_parent_rate);
+		ret = core->ops->set_rate(core->hw, change->rate,
+				best_parent_rate);
 
-	trace_clk_set_rate_complete(core, core->new_rate);
+	trace_clk_set_rate_complete(core, change->rate);
 
 	core->rate = clk_recalc(core, best_parent_rate);
 
+out:
+	if (core->flags & CLK_OPS_PARENT_ENABLE)
+		clk_core_disable_unprepare(parent);
+
+	if (core->notifier_count && old_rate != core->rate)
+		__clk_notify(core, POST_RATE_CHANGE, old_rate, core->rate);
+
 	if (core->flags & CLK_SET_RATE_UNGATE) {
 		unsigned long flags;
 
@@ -1943,31 +1992,44 @@  static void clk_change_rate(struct clk_core *core)
 		clk_core_unprepare(core);
 	}
 
-	if (core->flags & CLK_OPS_PARENT_ENABLE)
-		clk_core_disable_unprepare(parent);
-
-	if (core->notifier_count && old_rate != core->rate)
-		__clk_notify(core, POST_RATE_CHANGE, old_rate, core->rate);
+	clk_pm_runtime_put(core);
 
 	if (core->flags & CLK_RECALC_NEW_RATES)
-		(void)clk_calc_new_rates(core, core->new_rate);
+		(void)clk_calc_new_rates(core, change->rate);
 
 	/*
-	 * Use safe iteration, as change_rate can actually swap parents
-	 * for certain clock types.
+	 * Keep track of old parent and requested rate in case we have
+	 * to undo the change due to an error.
 	 */
-	hlist_for_each_entry_safe(child, tmp, &core->children, child_node) {
-		/* Skip children who will be reparented to another clock */
-		if (child->new_parent && child->new_parent != core)
-			continue;
-		clk_change_rate(child);
+	change->parent = old_parent;
+	change->rate = old_rate;
+	return ret;
+}
+
+static int clk_change_rates(struct list_head *list)
+{
+	struct clk_change *change, *tmp;
+	int ret = 0;
+
+	list_for_each_entry(change, list, change_list) {
+		ret = clk_change_rate(change);
+		if (ret)
+			goto err;
 	}
 
-	/* handle the new child who might not be in core->children yet */
-	if (core->new_child)
-		clk_change_rate(core->new_child);
+	return 0;
+err:
+	list_for_each_entry_safe_continue(change, tmp, list, change_list)
+		list_del_init(&change->change_list);
 
-	clk_pm_runtime_put(core);
+	list_for_each_entry(change, list, change_list) {
+		/* Give up. Driver might want to shutdown/reboot. */
+		ret = clk_change_rate(change);
+		if (WARN_ON(ret))
+			return ret;
+	}
+
+	return ret;
 }
 
 static unsigned long clk_core_req_round_rate_nolock(struct clk_core *core,
@@ -2001,7 +2063,9 @@  static int clk_core_set_rate_nolock(struct clk_core *core,
 				    unsigned long req_rate)
 {
 	struct clk_core *top, *fail_clk;
+	struct clk_change *change, *tmp;
 	unsigned long rate;
+	LIST_HEAD(changes);
 	int ret = 0;
 
 	if (!core)
@@ -2028,6 +2092,9 @@  static int clk_core_set_rate_nolock(struct clk_core *core,
 
 	clk_calc_subtree(core);
 
+	/* Construct the list of changes */
+	clk_prepare_changes(&changes, top);
+
 	/* notify that we are about to change rates */
 	fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
 	if (fail_clk) {
@@ -2039,7 +2106,19 @@  static int clk_core_set_rate_nolock(struct clk_core *core,
 	}
 
 	/* change the rates */
-	clk_change_rate(top);
+	ret = clk_change_rates(&changes);
+	list_for_each_entry_safe(change, tmp, &changes, change_list) {
+		change->rate = 0;
+		change->parent = NULL;
+		list_del_init(&change->change_list);
+	}
+
+	if (ret) {
+		pr_debug("%s: failed to set %s rate via top clk %s\n", __func__,
+				core->name, top->name);
+		clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
+		goto err;
+	}
 
 	core->req_rate = req_rate;
 err:
@@ -3306,6 +3385,8 @@  struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 	core->max_rate = ULONG_MAX;
 	INIT_LIST_HEAD(&core->prepare_list);
 	INIT_LIST_HEAD(&core->enable_list);
+	INIT_LIST_HEAD(&core->change.change_list);
+	core->change.core = core;
 	hw->core = core;
 
 	/* allocate local copy in case parent_names is __initdata */