Re: [PATCH 07/16] clk: tests: Add Kunit testing for of_clk_get_parent_name()

Stephen Boyd
2026-04-12T01:29:51Z

Quoting Miquel Raynal (Schneider Electric) (2026-03-27 13:09:29)
> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index b814b45f1f7e..8a17ad0d185f 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -3651,9 +3651,19 @@ static void clk_parse_clkspec_with_incorrect_index_and_name(struct kunit *test)
> KUNIT_EXPECT_TRUE(test, IS_ERR(hw));
> }
>

Add a comment here to tell us what the test is expecting.

> +static void clk_parse_and_get_parent_name(struct kunit *test)
> +{
> + struct clk_parse_clkspec_ctx *ctx = test->priv;
> +
> + KUNIT_EXPECT_STREQ(test,
> + of_clk_get_parent_name(ctx->cons_np, 0),
> + clk_parse_clkspec_1_init_data.name);
> +}

Reading this test is pretty hard because all the context is in the
common setup. Maybe the common setup is too broad in this case and
should be simplified so that tests can show more setup and assert code
for the things it wants to do. For example, if the name can be passed
from this test directly it would clarify by moving context into test
function scope.

const char *pname = "expected-name";

KUNIT_ASSERT_EQ(test, 0, setup_parent(pname));

KUNIT_EXPECT_STREQ(test, pname,
of_clk_get_parent_name(ctx->cons_np, 0));

> +
> static struct kunit_case clk_parse_clkspec_test_cases[] = {
> KUNIT_CASE(clk_parse_clkspec_with_correct_index_and_name),
> KUNIT_CASE(clk_parse_clkspec_with_incorrect_index_and_name),
> + KUNIT_CASE(clk_parse_and_get_parent_name),

Better to call the test something like
of_clk_get_parent_name_gets_parent_name to indicate what we're testing.