Add a test case where the parent clk rate is set 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 calculates 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/aUSWU7UymULCXOeF@redhat.com/
Link: https://lpc.events/event/19/contributions/2152/
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/clk_test.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
index 325da7c84ab2ecdcf6b7a023ce4c2c4ef2d49862..40bc01a0259d8d49ca4c1983b6c10a3684a95f0b 100644
--- a/drivers/clk/clk_test.c
+++ b/drivers/clk/clk_test.c
@@ -181,6 +181,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_multiple_parent_ctx {
struct clk_dummy_context parents_ctx[2];
struct clk_hw hw;
@@ -677,6 +697,18 @@ clk_rate_change_sibling_div_div_test_regular_ops_params[] = {
KUNIT_ARRAY_PARAM_DESC(clk_rate_change_sibling_div_div_test_regular_ops,
clk_rate_change_sibling_div_div_test_regular_ops_params, desc)
+static const struct clk_rate_change_sibling_div_div_test_param
+clk_rate_change_sibling_div_div_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_sibling_div_div_test_lcm_ops_v1,
+ clk_rate_change_sibling_div_div_test_lcm_ops_v1_params, desc)
+
static int clk_rate_change_sibling_div_div_test_init(struct kunit *test)
{
const struct clk_rate_change_sibling_div_div_test_param *param = test->param_value;
@@ -777,11 +809,40 @@ static void clk_test_rate_change_sibling_div_div_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.
+ */
+static void clk_test_rate_change_sibling_div_div_3_v1(struct kunit *test)
+{
+ struct clk_rate_change_sibling_div_div_context *ctx = test->priv;
+ int ret;
+
+ 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 (div=3), and child2
+ * at 24 MHz (div=4). However, the clk core by default will clobber
+ * the sibling clk rate, so the sibling gets the parent rate.
+ */
+ 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_sibling_div_div_cases[] = {
KUNIT_CASE_PARAM(clk_test_rate_change_sibling_div_div_1,
clk_rate_change_sibling_div_div_test_regular_ops_gen_params),
KUNIT_CASE_PARAM(clk_test_rate_change_sibling_div_div_2_v1,
clk_rate_change_sibling_div_div_test_regular_ops_gen_params),
+ KUNIT_CASE_PARAM(clk_test_rate_change_sibling_div_div_3_v1,
+ clk_rate_change_sibling_div_div_test_lcm_ops_v1_gen_params),
{}
};
--
2.53.0On Fri, Mar 13, 2026 at 12:43:11PM -0400, Brian Masney wrote:
> Add a test case where the parent clk rate is set 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 calculates 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/aUSWU7UymULCXOeF@redhat.com/
> Link: https://lpc.events/event/19/contributions/2152/
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> ---
> drivers/clk/clk_test.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index 325da7c84ab2ecdcf6b7a023ce4c2c4ef2d49862..40bc01a0259d8d49ca4c1983b6c10a3684a95f0b 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -181,6 +181,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_multiple_parent_ctx {
> struct clk_dummy_context parents_ctx[2];
> struct clk_hw hw;
> @@ -677,6 +697,18 @@ clk_rate_change_sibling_div_div_test_regular_ops_params[] = {
> KUNIT_ARRAY_PARAM_DESC(clk_rate_change_sibling_div_div_test_regular_ops,
> clk_rate_change_sibling_div_div_test_regular_ops_params, desc)
>
> +static const struct clk_rate_change_sibling_div_div_test_param
> +clk_rate_change_sibling_div_div_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_sibling_div_div_test_lcm_ops_v1,
> + clk_rate_change_sibling_div_div_test_lcm_ops_v1_params, desc)
> +
> static int clk_rate_change_sibling_div_div_test_init(struct kunit *test)
> {
> const struct clk_rate_change_sibling_div_div_test_param *param = test->param_value;
> @@ -777,11 +809,40 @@ static void clk_test_rate_change_sibling_div_div_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 description is identical to the one for
clk_test_rate_change_sibling_div_div_2_v1(), so it's not clear to me
what the difference between those two tests are.
Maxime
On Thu, Mar 19, 2026 at 5:22 AM Maxime Ripard <mripard@kernel.org> wrote:
> On Fri, Mar 13, 2026 at 12:43:11PM -0400, Brian Masney wrote:
> > Add a test case where the parent clk rate is set 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 calculates 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/aUSWU7UymULCXOeF@redhat.com/
> > Link: https://lpc.events/event/19/contributions/2152/
> > Signed-off-by: Brian Masney <bmasney@redhat.com>
> > ---
> > drivers/clk/clk_test.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 61 insertions(+)
> >
> > diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> > index 325da7c84ab2ecdcf6b7a023ce4c2c4ef2d49862..40bc01a0259d8d49ca4c1983b6c10a3684a95f0b 100644
> > --- a/drivers/clk/clk_test.c
> > +++ b/drivers/clk/clk_test.c
> > @@ -181,6 +181,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_multiple_parent_ctx {
> > struct clk_dummy_context parents_ctx[2];
> > struct clk_hw hw;
> > @@ -677,6 +697,18 @@ clk_rate_change_sibling_div_div_test_regular_ops_params[] = {
> > KUNIT_ARRAY_PARAM_DESC(clk_rate_change_sibling_div_div_test_regular_ops,
> > clk_rate_change_sibling_div_div_test_regular_ops_params, desc)
> >
> > +static const struct clk_rate_change_sibling_div_div_test_param
> > +clk_rate_change_sibling_div_div_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_sibling_div_div_test_lcm_ops_v1,
> > + clk_rate_change_sibling_div_div_test_lcm_ops_v1_params, desc)
> > +
> > static int clk_rate_change_sibling_div_div_test_init(struct kunit *test)
> > {
> > const struct clk_rate_change_sibling_div_div_test_param *param = test->param_value;
> > @@ -777,11 +809,40 @@ static void clk_test_rate_change_sibling_div_div_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 description is identical to the one for
> clk_test_rate_change_sibling_div_div_2_v1(), so it's not clear to me
> what the difference between those two tests are.
I put this test here to demonstrate that there are actually two
separate items that need to be addressed in order to fix the clk
scaling issue. So basically in the series:
- Patch 2 demonstrates the current behavior of the clk core with
today's behavior with the v1 scaling.
- Patches 3/4 (This patch): introduce LCM for the parent rate, and a
test showing the current behavior with this one change. This
demonstrates that this change alone is not enough since the sibling
rate is squashed and set to the parent rate.
- Patches 5/6/7: add the v2 negotiation flag, along with a test
showing how the two changes together give the desired behavior.
I'll update the test description here.
Also, thank you for all of the feedback.
Brian
Brian
© 2016 - 2026 Red Hat, Inc.