Re: [PATCH 11/16] clk: tests: Add Kunit testing for nexus nodes

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Stephen,

On 11/04/2026 at 18:40:29 -07, Stephen Boyd <sboyd@kernel.org> wrote:

> Quoting Miquel Raynal (Schneider Electric) (2026-03-27 13:09:33)
>> Add a nexus node with a child requesting a mapped clock in the fake DT
>> overlay to verify that the parsing is also correctly working.
>> 
>> Suggested-by: Stephen Boyd <sboyd@kernel.org>
>> Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
>> ---
>>  drivers/clk/clk_test.c                   | 20 ++++++++++++++++++++
>>  drivers/clk/kunit_clk_parse_clkspec.dtso | 10 ++++++++++
>>  2 files changed, 30 insertions(+)
>> 
>> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
>> index 8a17ad0d185f..cb0071955146 100644
>> --- a/drivers/clk/clk_test.c
>> +++ b/drivers/clk/clk_test.c
>> @@ -3660,10 +3660,30 @@ static void clk_parse_and_get_parent_name(struct kunit *test)
>>                            clk_parse_clkspec_1_init_data.name);
>>  }
>>  
>> +static void clk_parse_and_get_nexus(struct kunit *test)
>
> of_clk_get_hw_maps_thru_nexus()
>
>> +{
>> +       struct clk_parse_clkspec_ctx *ctx = test->priv;
>> +       struct clk_hw *hw1, *hw2;
>> +       struct device_node *np;
>> +
>> +       /* Get clocks by index */
>> +       np = of_find_node_by_name(NULL, "kunit-clock-nexus-child");
>> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np);
>> +       hw1 = of_clk_get_hw(np, 0, NULL);
>> +       KUNIT_EXPECT_NOT_ERR_OR_NULL(test, hw1);
>> +
>> +       hw2 = of_clk_get_hw(ctx->cons_np, 1, NULL);
>> +       KUNIT_EXPECT_NOT_ERR_OR_NULL(test, hw2);
>> +       KUNIT_EXPECT_PTR_EQ(test, hw1, hw2);
>
> I think hw2 is expected? Rewrite it like this?

Correct.

> 	struct clk_parse_clkspec_ctx *ctx = test->priv;
> 	struct clk_hw *expected;
> 	struct device_node *np;
> 	
> 	np = of_find_node_by_name(NULL, "kunit-clock-nexus-child");
> 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np);
> 	expected = of_clk_get_hw(ctx->cons_np, 1, NULL);
> 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, expected);
>
> 	KUNIT_EXPECT_PTR_EQ(test, expected, of_clk_get_hw(np, 0, NULL));

I took your proposal.

> We're also leaking the of_node if an assert triggers so we need to use a
> kunit wrapper version of of_find_node_by_name() or do a hand code
> version of putting the node when kunit asserts.

Indeed, I just create the _kunit() variant to handle it. I see most
other similar cases in the test file are handled like that and it looks
pretty neat afterwards.

Thanks for the suggestions,
Miquèl