Re: [PATCH v9 4/6] clk: Add KUnit tests for assigned-clock-sscs

[Stephen Boyd <sboyd@kernel.org>]

Quoting Peng Fan (OSS) (2026-03-11 23:58:20)
> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index a268d7b5d4cb28ec1f029f828c31107f8e130556..97113b61c2841701a44603ca9935638374000a2e 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -3203,6 +3223,9 @@ static void clk_assigned_rates_assigns_one(struct kunit *test)
>         struct clk_assigned_rates_context *ctx = test->priv;
>  
>         KUNIT_EXPECT_EQ(test, ctx->clk0.rate, ASSIGNED_RATES_0_RATE);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.modfreq_hz, ASSIGNED_SSCS_0_MODFREQ);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.spread_bp, ASSIGNED_SSCS_0_SPREAD);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.method, ASSIGNED_SSCS_0_METHOD);
>  }
>  
>  static void clk_assigned_rates_assigns_multiple(struct kunit *test)
> @@ -3210,7 +3233,13 @@ static void clk_assigned_rates_assigns_multiple(struct kunit *test)
>         struct clk_assigned_rates_context *ctx = test->priv;
>  
>         KUNIT_EXPECT_EQ(test, ctx->clk0.rate, ASSIGNED_RATES_0_RATE);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.modfreq_hz, ASSIGNED_SSCS_0_MODFREQ);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.spread_bp, ASSIGNED_SSCS_0_SPREAD);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.method, ASSIGNED_SSCS_0_METHOD);
>         KUNIT_EXPECT_EQ(test, ctx->clk1.rate, ASSIGNED_RATES_1_RATE);
> +       KUNIT_EXPECT_EQ(test, ctx->clk1.sscs.modfreq_hz, ASSIGNED_SSCS_1_MODFREQ);
> +       KUNIT_EXPECT_EQ(test, ctx->clk1.sscs.spread_bp, ASSIGNED_SSCS_1_SPREAD);
> +       KUNIT_EXPECT_EQ(test, ctx->clk1.sscs.method, ASSIGNED_SSCS_1_METHOD);
>  }
>  
>  static void clk_assigned_rates_skips(struct kunit *test)
> @@ -3222,6 +3251,19 @@ static void clk_assigned_rates_skips(struct kunit *test)
>         KUNIT_EXPECT_EQ(test, ctx->clk0.rate, test_param->rate0);
>  }
>  
> +static void clk_assigned_sscs_skips(struct kunit *test)
> +{
> +       struct clk_assigned_rates_context *ctx = test->priv;
> +       const struct clk_assigned_rates_test_param *test_param = test->param_value;
> +
> +       KUNIT_EXPECT_NE(test, ctx->clk0.sscs.modfreq_hz, ASSIGNED_SSCS_0_MODFREQ);
> +       KUNIT_EXPECT_NE(test, ctx->clk0.sscs.spread_bp, ASSIGNED_SSCS_0_SPREAD);
> +       KUNIT_EXPECT_NE(test, ctx->clk0.sscs.method, ASSIGNED_SSCS_0_METHOD);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.modfreq_hz, test_param->sscs.modfreq_hz);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.spread_bp, test_param->sscs.spread_bp);
> +       KUNIT_EXPECT_EQ(test, ctx->clk0.sscs.method, test_param->sscs.method);
> +}
> +
>  OF_OVERLAY_DECLARE(kunit_clk_assigned_rates_one);
>  OF_OVERLAY_DECLARE(kunit_clk_assigned_rates_one_consumer);
>  OF_OVERLAY_DECLARE(kunit_clk_assigned_rates_u64_one);
> @@ -3384,6 +3426,77 @@ KUNIT_ARRAY_PARAM_DESC(clk_assigned_rates_skips,
>                        clk_assigned_rates_skips_test_params,
>                        desc)
>  
> +OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_without);
> +OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_without_consumer);
> +OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_zero);
> +OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_zero_consumer);
> +OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_null);
> +OF_OVERLAY_DECLARE(kunit_clk_assigned_sscs_null_consumer);
> +
> +/* Test cases that skip changing the sscs due to malformed DT */
> +static const struct clk_assigned_rates_test_param clk_assigned_sscs_skips_test_params[] = {
> +       {
> +               /*
> +                * Test that an assigned-clock-sscs property without an assigned-clocks
> +                * property fails when the property is in the provider.
> +                */
> +               .desc = "provider missing assigned-clocks",
> +               TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_without),
> +               .sscs = {50000, 60000, 3},
> +       },
> +       {
> +               /*
> +                * Test that an assigned-clock-rates property without an assigned-clocks

It is?

> +                * property fails when the property is in the consumer.
> +                */
> +               .desc = "consumer missing assigned-clocks",
> +               TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_without_consumer),
> +               .sscs = {50000, 60000, 3},
> +               .consumer_test = true,
> +       },
> +       {
> +               /*
> +                * Test that an assigned-clock-rates property of zero doesn't

Typo?

> +                * set a rate when the property is in the provider.
> +                */
> +               .desc = "provider assigned-clock-sscs of zero",
> +               TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_zero),
> +               .sscs = {50000, 60000, 3},
> +       },
> +       {
> +               /*
> +                * Test that an assigned-clock-rates property of zero doesn't
> +                * set a rate when the property is in the consumer.
> +                */
> +               .desc = "consumer assigned-clock-sscs of zero",
> +               TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_zero_consumer),
> +               .sscs = {50000, 60000, 3},
> +               .consumer_test = true,
> +       },
> +       {
> +               /*
> +                * Test that an assigned-clocks property with a null phandle
> +                * doesn't set a rate when the property is in the provider.
> +                */
> +               .desc = "provider assigned-clocks null phandle",
> +               TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_null),
> +               .sscs = {50000, 60000, 3},
> +       },
> +       {
> +               /*
> +                * Test that an assigned-clocks property with a null phandle
> +                * doesn't set a rate when the property is in the consumer.

None of these comments are correct.

> +                */
> +               .desc = "provider assigned-clocks null phandle",
> +               TEST_PARAM_OVERLAY(kunit_clk_assigned_sscs_null_consumer),
> +               .sscs = {50000, 60000, 3},
> +               .consumer_test = true,
> +       },
> +};
> +KUNIT_ARRAY_PARAM_DESC(clk_assigned_sscs_skips,
> +                      clk_assigned_sscs_skips_test_params,
> +                      desc)
> +
>  static struct kunit_case clk_assigned_rates_test_cases[] = {
>         KUNIT_CASE_PARAM(clk_assigned_rates_assigns_one,
>                          clk_assigned_rates_assigns_one_gen_params),
> @@ -3391,6 +3504,8 @@ static struct kunit_case clk_assigned_rates_test_cases[] = {
>                          clk_assigned_rates_assigns_multiple_gen_params),
>         KUNIT_CASE_PARAM(clk_assigned_rates_skips,
>                          clk_assigned_rates_skips_gen_params),
> +       KUNIT_CASE_PARAM(clk_assigned_sscs_skips,
> +                        clk_assigned_sscs_skips_gen_params),
>         {}
>  };

Instead of adding on another case just copy the entire thing,
kunit_case, test_params, etc. and implement the tests you want. Test
code is the opposite of DRY (DAMP?) so don't be afraid to just copy a
bunch of stuff. The reason why that is encouraged is because existing
tests are unchanged, and we don't have to worry that this patch breaks
the existing tests. It also helps the reviewer see the whole picture
because all the test code is in the patch instead of in the context.

For example, clk_assigned_rates_assigns_multiple() is saying that a
clock-assigned-rates property with multiple rates assigns multiple
rates. It's not supposed to be testing ssc. Don't modify it.