Re: [PATCH v6 4/7] clk: test: introduce additional test case showing sibling clock rate change

[Brian Masney]

On Thu, Mar 19, 2026 at 5:22 AM Maxime Ripard <mripard@xxxxxxxxxx> 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@xxxxxxxxxx/
> > Link: https://lpc.events/event/19/contributions/2152/
> > Signed-off-by: Brian Masney <bmasney@xxxxxxxxxx>
> > ---
> >  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