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

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4386 bytes --]

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.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]