Quoting David Gow (2024-06-13 00:56:08)
On Tue, 4 Jun 2024 at 06:38, Stephen Boyd sboyd@kernel.org wrote:
Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.
Cc: Christian Marangi ansuelsmth@gmail.com Cc: Brendan Higgins brendan.higgins@linux.dev Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com Signed-off-by: Stephen Boyd sboyd@kernel.org
This seems good to me overall, but will break if we can't compile the dtbo.o file. Maybe these need to live behind a #if IS_ENABLED(CONFIG_OF) or equivalent.
Also, there's a cast to kunit_action_t* which needs to use a wrapper.
Otherwise, Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 3 +- drivers/clk/clk_parent_data_test.h | 10 + drivers/clk/clk_test.c | 451 +++++++++++++++++++- drivers/clk/kunit_clk_parent_data_test.dtso | 28 ++ 5 files changed, 491 insertions(+), 2 deletions(-) create mode 100644 drivers/clk/clk_parent_data_test.h create mode 100644 drivers/clk/kunit_clk_parent_data_test.dtso
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index f649f2a0279c..c33fdf9fdcd6 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -508,6 +508,7 @@ config CLK_KUNIT_TEST tristate "Basic Clock Framework Kunit Tests" if !KUNIT_ALL_TESTS depends on KUNIT default KUNIT_ALL_TESTS
select OF_OVERLAY if OF help Kunit tests for the common clock framework.diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 7b57e3d22cee..ed4e1a0e6943 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -2,7 +2,8 @@ # common clock types obj-$(CONFIG_HAVE_CLK) += clk-devres.o clk-bulk.o clkdev.o obj-$(CONFIG_COMMON_CLK) += clk.o -obj-$(CONFIG_CLK_KUNIT_TEST) += clk_test.o +obj-$(CONFIG_CLK_KUNIT_TEST) += clk_test.o \
kunit_clk_parent_data_test.dtbo.oThis breaks if CONFIG_OF isn't enabled, as there's no rule to compile it: make[5]: *** No rule to make target 'drivers/clk/kunit_clk_parent_data_test.dtbo.o', needed by 'drivers/clk/modules.order'. Stop.
Ah, I see that I need to set CONFIG_DTC or the DT compiler (dtc) won't be built. Maybe I should just select OF_OVERLAY instead of being nice and letting OF be disabled? The problem is that I can't test the CONFIG_OF=n case easily then.
For now I'll go with 'select DTC', but it really feels like we should just get rid of that Kconfig and build 'dtc' if it is needed.
diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c index 39e2b5ff4f51..bdf3c4bb2243 100644 --- a/drivers/clk/clk_test.c +++ b/drivers/clk/clk_test.c @@ -4,12 +4,19 @@ */ #include <linux/clk.h> #include <linux/clk-provider.h> +#include <linux/of.h> +#include <linux/platform_device.h>
/* Needed for clk_hw_get_clk() */ #include "clk.h"
[...]
+/**
- struct clk_register_clk_parent_data_of_ctx - Context for clk_parent_data OF tests
- @np: device node of clk under test
- @hw: clk_hw for clk under test
- */
+struct clk_register_clk_parent_data_of_ctx {
struct device_node *np;struct clk_hw hw;+};
+static int clk_register_clk_parent_data_of_test_init(struct kunit *test) +{
struct clk_register_clk_parent_data_of_ctx *ctx;KUNIT_ASSERT_EQ(test, 0,of_overlay_apply_kunit(test, kunit_clk_parent_data_test));ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);if (!ctx)return -ENOMEM;test->priv = ctx;ctx->np = of_find_compatible_node(NULL, NULL, "test,clk-parent-data");if (!ctx->np)return -ENODEV;return kunit_add_action_or_reset(test, (kunit_action_t *)&of_node_put, ctx->np);We should use an action wrapper here (KUNIT_DEFINE_ACTION_WRAPPER()), as casting function pointers to kunit_action_t* breaks control-flow integrity.
Got it, thanks. Maybe there should be an of_node_put_kunit_exit() helper that does that and can be used anywhere.