On 09/22/2014 10:18 PM, Stephen Boyd wrote:
On 08/21, Tero Kristo wrote:
In some cases, clocks can switch their parent with clk_set_rate, for
example clk_mux can do this in some cases. Current implementation of
clk_change_rate uses un-safe list iteration on the clock children, which
will cause wrong clocks to be parsed in case any of the clock children
change their parents during the change rate operation. Fixed by using
the safe list iterator instead.
The problem was detected due to some divide by zero errors generated
by clock init on dra7-evm board, see discussion under
http://article.gmane.org/gmane.linux.ports.arm.kernel/349180 for details.
Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
To: Mike Turquette <mturquette@xxxxxxxxxx>
Reported-by: Nishanth Menon <nm@xxxxxx>
---
drivers/clk/clk.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b76fa69..bacc06f 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1467,6 +1467,7 @@ static struct clk *clk_propagate_rate_change(struct clk *clk, unsigned long even
static void clk_change_rate(struct clk *clk)
{
struct clk *child;
+ struct hlist_node *tmp;
unsigned long old_rate;
unsigned long best_parent_rate = 0;
bool skip_set_rate = false;
@@ -1502,7 +1503,11 @@ 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) {
+ /*
+ * Use safe iteration, as change_rate can actually swap parents
+ * for certain clock types.
+ */
+ hlist_for_each_entry_safe(child, tmp, &clk->children, child_node) {
/* Skip children who will be reparented to another clock */
if (child->new_parent && child->new_parent != clk)
continue;
Are we not hitting the new_parent check here? I don't understand
how we can be changing parents here unless the check is being
avoided, in which case I wonder why determine_rate isn't being
used.
It depends how the clock underneath handles the situation. The error I
am seeing actually happens with a SoC specific compound clock (DPLL)
which integrates set_rate + mux functionality into a single clock node.
A call to the clk_set_rate changes the parent of this clock (from bypass
clock to reference clock), in addition to changing the rate (tune the
mul+div.) I looked at using the determine rate call with this type but
it breaks everything up... the parent gets changed but not the clock
rate, in addition to some other issues.
-Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
