Re: [PATCH 06/16] clk: tests: Add clk_parse_clkspec() Kunit testing

Brian Masney
2026-03-30T14:48:43Z

Hi Miquel,

On Fri, Mar 27, 2026 at 09:09:28PM +0100, Miquel Raynal (Schneider Electric) wrote:
> Create a new set of kunit tests to make sure clk_parse_clkspec() is
> working as expected. We currently verify if we get a proper device when
> using indexes and names. If we make an out of bounds request we expect
> an error.
>
> For testing purposes, we must ensure of_clk_get_hw()'s symbol is
> exported.
>
> Suggested-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
> ---
> drivers/clk/Makefile | 1 +
> drivers/clk/clk.c | 1 +
> drivers/clk/clk_test.c | 124 +++++++++++++++++++++++++++++++
> drivers/clk/kunit_clk_parse_clkspec.dtso | 21 ++++++
> 4 files changed, 147 insertions(+)
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index f7bce3951a30..97b621456bf5 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -19,6 +19,7 @@ clk-test-y := clk_test.o \
> kunit_clk_assigned_rates_zero.dtbo.o \
> kunit_clk_assigned_rates_zero_consumer.dtbo.o \
> kunit_clk_hw_get_dev_of_node.dtbo.o \
> + kunit_clk_parse_clkspec.dtbo.o \
> kunit_clk_parent_data_test.dtbo.o
> obj-$(CONFIG_COMMON_CLK) += clk-divider.o
> obj-$(CONFIG_COMMON_CLK) += clk-fixed-factor.o
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 47093cda9df3..1795246b10a0 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5312,6 +5312,7 @@ struct clk_hw *of_clk_get_hw(struct device_node *np, int index,
>
> return hw;
> }
> +EXPORT_SYMBOL_GPL(of_clk_get_hw);

So that we don't unnecessarily broaden the API that's available to the
clk providers, you can use EXPORT_SYMBOL_IF_KUNIT so that this is only
available to the kunit tests.

Note that Chen-Yu posted a separate patch to add the includes for a
separate test. The two patches will conflict since Stephen hasn't picked
this up yet.

https://lore.kernel.org/linux-clk/20260225083413.3384950-1-wenst@chromium.org/

>
> static struct clk *__of_clk_get(struct device_node *np,
> int index, const char *dev_id,
> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index a268d7b5d4cb..b814b45f1f7e 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -3541,10 +3541,134 @@ static struct kunit_suite clk_hw_get_dev_of_node_test_suite = {
> .test_cases = clk_hw_get_dev_of_node_test_cases,
> };
>
> +static const struct clk_init_data clk_parse_clkspec_1_init_data = {
> + .name = "clk_parse_clkspec_1",
> + .ops = &empty_clk_ops,
> +};
> +
> +static const struct clk_init_data clk_parse_clkspec_2_init_data = {
> + .name = "clk_parse_clkspec_2",
> + .ops = &empty_clk_ops,
> +};
> +
> +static struct clk_hw *kunit_clk_get(struct of_phandle_args *clkspec, void *data)
> +{
> + return (struct clk_hw *)data;
> +}
> +
> +struct clk_parse_clkspec_ctx {
> + struct device_node *prov1_np;
> + struct device_node *prov2_np;
> + struct device_node *cons_np;
> +};
> +
> +static int clk_parse_clkspec_init(struct kunit *test)
> +{
> + struct clk_parse_clkspec_ctx *ctx;
> + struct clk_hw *hw1, *hw2;
> +
> + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> + test->priv = ctx;
> +
> + KUNIT_ASSERT_EQ(test, 0, of_overlay_apply_kunit(test, kunit_clk_parse_clkspec));
> +
> + /* Register provider 1 */
> + hw1 = kunit_kzalloc(test, sizeof(*hw1), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw1);
> + hw1->init = &clk_parse_clkspec_1_init_data;
> +
> + ctx->prov1_np = of_find_compatible_node(NULL, NULL, "test,clock-provider1");
> + KUNIT_ASSERT_NOT_NULL(test, ctx->prov1_np);
> +
> + KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, ctx->prov1_np, hw1));
> + of_clk_add_hw_provider(ctx->prov1_np, kunit_clk_get, hw1);

Can you just use of_clk_hw_simple_get() and drop kunit_clk_get() above?

> + of_node_put(ctx->prov1_np);
> +
> + /* Register provider 2 */
> + hw2 = kunit_kzalloc(test, sizeof(*hw2), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, hw2);
> + hw2->init = &clk_parse_clkspec_2_init_data;
> +
> + ctx->prov2_np = of_find_compatible_node(NULL, NULL, "test,clock-provider2");
> + KUNIT_ASSERT_NOT_NULL(test, ctx->prov2_np);
> +
> + KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, ctx->prov2_np, hw2));
> + of_clk_add_hw_provider(ctx->prov2_np, kunit_clk_get, hw2);
> + of_node_put(ctx->prov2_np);
> +
> + ctx->cons_np = of_find_compatible_node(NULL, NULL, "test,clock-consumer");
> + KUNIT_ASSERT_NOT_NULL(test, ctx->cons_np);
> +
> + return 0;
> +}
> +
> +static void clk_parse_clkspec_exit(struct kunit *test)
> +{
> + struct clk_parse_clkspec_ctx *ctx = test->priv;
> +
> + of_node_put(ctx->prov1_np);
> + of_node_put(ctx->prov2_np);

Is there a double free of prov1_np and prov2_np? If this is dropped from
the test exit, then they should't need to be in the ctx struct.

Brian