As demonstrated by the kunit tests, clk_calc_subtree() in the clk core
can overwrite a sibling clk with the parent rate. Clocks that are used
for some subsystems like DRM and sound are particularly sensitive to
this issue.
I consider this to be a logic bug in the clk subsystem, however this
functionality has existed since the clk core was introduced with
commit b2476490ef11 ("clk: introduce the common clock framework"),
and some boards are unknowingly dependent on this behavior.
Let's add support for a v2 rate negotiation logic that addresses the
logic error. Clks can opt into this new behavior by adding the flag
CLK_V2_RATE_NEGOTIATION.
Link: https://lore.kernel.org/linux-clk/[email protected]/
Link: https://lpc.events/event/19/contributions/2152/
Signed-off-by: Brian Masney <[email protected]>
---
drivers/clk/clk.c | 58 ++++++++++++++++++++++++++++++++++----------
include/linux/clk-provider.h | 3 +++
2 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index
f1afd6c93eba49b9fc6c5c0e1db11d46c79069e9..514f2e2eb62a09df4f797b0207aa9b1448ddaf3d
100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -887,6 +887,31 @@ unsigned long clk_hw_get_children_lcm(struct clk_hw *hw,
struct clk_hw *requesti
}
EXPORT_SYMBOL_GPL(clk_hw_get_children_lcm);
+/**
+ * clk_has_v2_rate_negotiation - Check if a clk should use v2 rate negotiation
+ * @core: The clock core to check
+ *
+ * This function recursively checks if the clk or any of its descendants have
+ * the CLK_V2_RATE_NEGOTIATION flag set.
+ *
+ * Returns: true if the v2 logic should be used; false otherwise
+ */
+bool clk_has_v2_rate_negotiation(const struct clk_core *core)
+{
+ struct clk_core *child;
+
+ if (core->flags & CLK_V2_RATE_NEGOTIATION)
+ return true;
+
+ hlist_for_each_entry(child, &core->children, child_node) {
+ if (clk_has_v2_rate_negotiation(child))
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(clk_has_v2_rate_negotiation);
+
/*
* __clk_mux_determine_rate - clk_ops::determine_rate implementation for a mux
type clk
* @hw: mux type clk to determine rate on
@@ -2294,7 +2319,8 @@ static int __clk_speculate_rates(struct clk_core *core,
}
static void clk_calc_subtree(struct clk_core *core, unsigned long new_rate,
- struct clk_core *new_parent, u8 p_index)
+ struct clk_core *new_parent, u8 p_index,
+ struct clk_core *initiating_clk)
{
struct clk_core *child;
@@ -2307,8 +2333,12 @@ static void clk_calc_subtree(struct clk_core *core,
unsigned long new_rate,
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);
+ if (child != initiating_clk &&
clk_has_v2_rate_negotiation(child))
+ child->new_rate = child->rate;
+ else
+ child->new_rate = clk_recalc(child, new_rate);
+
+ clk_calc_subtree(child, child->new_rate, NULL, 0,
initiating_clk);
}
}
@@ -2317,7 +2347,8 @@ static void clk_calc_subtree(struct clk_core *core,
unsigned long new_rate,
* changed.
*/
static struct clk_core *clk_calc_new_rates(struct clk_core *core,
- unsigned long rate)
+ unsigned long rate,
+ struct clk_core *initiating_clk)
{
struct clk_core *top = core;
struct clk_core *old_parent, *parent;
@@ -2365,7 +2396,7 @@ static struct clk_core *clk_calc_new_rates(struct
clk_core *core,
return NULL;
} else {
/* pass-through clock with adjustable parent */
- top = clk_calc_new_rates(parent, rate);
+ top = clk_calc_new_rates(parent, rate, initiating_clk);
new_rate = parent->new_rate;
goto out;
}
@@ -2390,10 +2421,10 @@ static struct clk_core *clk_calc_new_rates(struct
clk_core *core,
if ((core->flags & CLK_SET_RATE_PARENT) && parent &&
best_parent_rate != parent->rate)
- top = clk_calc_new_rates(parent, best_parent_rate);
+ top = clk_calc_new_rates(parent, best_parent_rate,
initiating_clk);
out:
- clk_calc_subtree(core, new_rate, parent, p_index);
+ clk_calc_subtree(core, new_rate, parent, p_index, initiating_clk);
return top;
}
@@ -2441,7 +2472,7 @@ 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 void clk_change_rate(struct clk_core *core, struct clk_core
*initiating_clk)
{
struct clk_core *child;
struct hlist_node *tmp;
@@ -2510,7 +2541,7 @@ static void clk_change_rate(struct clk_core *core)
__clk_notify(core, POST_RATE_CHANGE, old_rate, core->rate);
if (core->flags & CLK_RECALC_NEW_RATES)
- (void)clk_calc_new_rates(core, core->new_rate);
+ (void)clk_calc_new_rates(core, core->new_rate, initiating_clk);
/*
* Use safe iteration, as change_rate can actually swap parents
@@ -2520,12 +2551,12 @@ static void clk_change_rate(struct clk_core *core)
/* Skip children who will be reparented to another clock */
if (child->new_parent && child->new_parent != core)
continue;
- clk_change_rate(child);
+ clk_change_rate(child, initiating_clk);
}
/* handle the new child who might not be in core->children yet */
if (core->new_child)
- clk_change_rate(core->new_child);
+ clk_change_rate(core->new_child, initiating_clk);
clk_pm_runtime_put(core);
}
@@ -2581,7 +2612,7 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
return -EBUSY;
/* calculate new rates and get the topmost changed clock */
- top = clk_calc_new_rates(core, req_rate);
+ top = clk_calc_new_rates(core, req_rate, core);
if (!top)
return -EINVAL;
@@ -2600,7 +2631,7 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
}
/* change the rates */
- clk_change_rate(top);
+ clk_change_rate(top, core);
core->req_rate = req_rate;
err:
@@ -3587,6 +3618,7 @@ static const struct {
ENTRY(CLK_IS_CRITICAL),
ENTRY(CLK_OPS_PARENT_ENABLE),
ENTRY(CLK_DUTY_CYCLE_PARENT),
+ ENTRY(CLK_V2_RATE_NEGOTIATION),
#undef ENTRY
};
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index
2699b9759e13d0c1f0b54f4fa4f7f7bdd42e8dde..e0fc0bd347e5920e999ac96dbed9fc247f9443fa
100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -32,6 +32,8 @@
#define CLK_OPS_PARENT_ENABLE BIT(12)
/* duty cycle call may be forwarded to the parent clock */
#define CLK_DUTY_CYCLE_PARENT BIT(13)
+/* clock participates in v2 rate negotiation */
+#define CLK_V2_RATE_NEGOTIATION BIT(14)
struct clk;
struct clk_hw;
@@ -1432,6 +1434,7 @@ void clk_hw_set_rate_range(struct clk_hw *hw, unsigned
long min_rate,
unsigned long max_rate);
unsigned long clk_hw_get_children_lcm(struct clk_hw *hw, struct clk_hw
*requesting_hw,
unsigned long requesting_rate);
+bool clk_has_v2_rate_negotiation(const struct clk_core *core);
static inline void __clk_hw_set_clk(struct clk_hw *dst, struct clk_hw *src)
{
--
2.53.0
