Add a test case where the parent clk rate is changed to a rate that's
acceptable to both children, however the sibling clk rate is affected.
The tests in this commit use the following simplified clk tree with
the initial state:
parent
24 MHz
/ \
child1 child2
24 MHz 24 MHz
child1 and child2 both divider-only clocks that have CLK_SET_RATE_PARENT
set, and the parent is capable of achieving any rate.
child1 requests 32 MHz, and the tree should end up with the state:
parent
96 MHz
/ \
child1 child2
32 MHz 24 MHz
However, child2 ends up with it's parent rate due to the way the clk
core currently handles rate changes.
parent
96 MHz
/ \
child1 child2
32 MHz 96 MHz
^^^^^^
Incorrect. Should be 24 MHz.
Let's document this behavior with a kunit test since some boards are
unknowingly dependent on this behavior.
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-divider_test.c | 70 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/clk/clk-divider_test.c b/drivers/clk/clk-divider_test.c
index
2a01b0201e9d919c36bb70eeb21c9f4ae113254e..95a55835a29065fede0426f1c15ec158e8a703c1
100644
--- a/drivers/clk/clk-divider_test.c
+++ b/drivers/clk/clk-divider_test.c
@@ -54,6 +54,26 @@ static const struct clk_ops clk_dummy_div_ops = {
.set_rate = clk_dummy_div_set_rate,
};
+static int clk_dummy_div_lcm_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_hw *parent_hw = clk_hw_get_parent(hw);
+
+ if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) &&
req->best_parent_rate < req->rate)
+ return -EINVAL;
+
+ req->best_parent_rate = clk_hw_get_children_lcm(parent_hw, hw,
req->rate);
+ req->best_parent_hw = parent_hw;
+
+ return divider_determine_rate(hw, req, NULL, CLK_DUMMY_DIV_WIDTH,
CLK_DUMMY_DIV_FLAGS);
+}
+
+static const struct clk_ops clk_dummy_div_lcm_ops = {
+ .recalc_rate = clk_dummy_div_recalc_rate,
+ .determine_rate = clk_dummy_div_lcm_determine_rate,
+ .set_rate = clk_dummy_div_set_rate,
+};
+
struct clk_rate_change_divider_context {
struct clk_dummy_context parent;
struct clk_dummy_div child1, child2;
@@ -78,6 +98,18 @@ clk_rate_change_divider_test_regular_ops_params[] = {
KUNIT_ARRAY_PARAM_DESC(clk_rate_change_divider_test_regular_ops,
clk_rate_change_divider_test_regular_ops_params, desc)
+static const struct clk_rate_change_divider_test_param
+clk_rate_change_divider_test_lcm_ops_v1_params[] = {
+ {
+ .desc = "lcm_ops_v1",
+ .ops = &clk_dummy_div_lcm_ops,
+ .extra_child_flags = 0,
+ },
+};
+
+KUNIT_ARRAY_PARAM_DESC(clk_rate_change_divider_test_lcm_ops_v1,
+ clk_rate_change_divider_test_lcm_ops_v1_params, desc)
+
static int clk_rate_change_divider_test_init(struct kunit *test)
{
const struct clk_rate_change_divider_test_param *param =
test->param_value;
@@ -197,11 +229,49 @@ static void clk_test_rate_change_divider_2_v1(struct
kunit *test)
KUNIT_EXPECT_EQ(test, ctx->child2.div, 1);
}
+/*
+ * Test that, for a parent with two divider-only children with
CLK_SET_RATE_PARENT
+ * set and one requests a rate incompatible with the existing parent rate, the
+ * sibling rate is also affected. This preserves existing behavior in the clk
+ * core that some drivers may be unknowingly dependent on. This test
+ * demonstrates that even if the clk provider picks a parent rate that's
+ * suitable for both children, the child's rate change also affects the
+ * sibling's rate with the v1 rate negotiation logic.
+ */
+static void clk_test_rate_change_divider_3_v1(struct kunit *test)
+{
+ struct clk_rate_change_divider_context *ctx = test->priv;
+ int ret;
+
+ KUNIT_ASSERT_EQ(test, clk_get_rate(ctx->parent_clk), 24 * HZ_PER_MHZ);
+ KUNIT_ASSERT_EQ(test, clk_get_rate(ctx->child1_clk), 24 * HZ_PER_MHZ);
+ KUNIT_EXPECT_EQ(test, ctx->child1.div, 1);
+ KUNIT_ASSERT_EQ(test, clk_get_rate(ctx->child2_clk), 24 * HZ_PER_MHZ);
+ KUNIT_EXPECT_EQ(test, ctx->child2.div, 1);
+
+ ret = clk_set_rate(ctx->child1_clk, 32 * HZ_PER_MHZ);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ /*
+ * With LCM-based coordinated rate changes, the parent should be at
+ * 96 MHz (LCM of 32 and 24), child1 at 32 MHz, and child2 at 24 MHz.
+ * However, the clk core by default will clobber the sibling clk rate,
+ * so the sibling gets the parent rate of 96 MHz.
+ */
+ KUNIT_EXPECT_EQ(test, clk_get_rate(ctx->parent_clk), 96 * HZ_PER_MHZ);
+ KUNIT_EXPECT_EQ(test, clk_get_rate(ctx->child1_clk), 32 * HZ_PER_MHZ);
+ KUNIT_EXPECT_EQ(test, ctx->child1.div, 3);
+ KUNIT_EXPECT_EQ(test, clk_get_rate(ctx->child2_clk), 96 * HZ_PER_MHZ);
+ KUNIT_EXPECT_EQ(test, ctx->child2.div, 1);
+}
+
static struct kunit_case clk_rate_change_divider_cases[] = {
KUNIT_CASE_PARAM(clk_test_rate_change_divider_1,
clk_rate_change_divider_test_regular_ops_gen_params),
KUNIT_CASE_PARAM(clk_test_rate_change_divider_2_v1,
clk_rate_change_divider_test_regular_ops_gen_params),
+ KUNIT_CASE_PARAM(clk_test_rate_change_divider_3_v1,
+ clk_rate_change_divider_test_lcm_ops_v1_gen_params),
{}
};
--
2.53.0
