On Tue, 4 Jun 2024 at 06:38, Stephen Boyd <sboyd@xxxxxxxxxx> wrote: > > Test that clks registered with 'struct clk_parent_data' work as > intended and can find their parents. > > Cc: Christian Marangi <ansuelsmth@xxxxxxxxx> > Cc: Brendan Higgins <brendan.higgins@xxxxxxxxx> > Cc: David Gow <davidgow@xxxxxxxxxx> > Cc: Rae Moar <rmoar@xxxxxxxxxx> > Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxx> > --- 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@xxxxxxxxxx> 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.o This 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. > obj-$(CONFIG_COMMON_CLK) += clk-divider.o > obj-$(CONFIG_COMMON_CLK) += clk-fixed-factor.o > obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o > diff --git a/drivers/clk/clk_parent_data_test.h b/drivers/clk/clk_parent_data_test.h > new file mode 100644 > index 000000000000..eedd53ae910d > --- /dev/null > +++ b/drivers/clk/clk_parent_data_test.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _CLK_PARENT_DATA_TEST_H > +#define _CLK_PARENT_DATA_TEST_H > + > +#define CLK_PARENT_DATA_1MHZ_NAME "1mhz_fixed_legacy" > +#define CLK_PARENT_DATA_PARENT1 "parent_fwname" > +#define CLK_PARENT_DATA_PARENT2 "50" > +#define CLK_PARENT_DATA_50MHZ_NAME "50_clk" > + > +#endif > 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" > > +#include <kunit/clk.h> > +#include <kunit/of.h> > +#include <kunit/platform_device.h> > #include <kunit/test.h> > > +#include "clk_parent_data_test.h" > + > static const struct clk_ops empty_clk_ops = { }; > > #define DUMMY_CLOCK_INIT_RATE (42 * 1000 * 1000) > @@ -2659,6 +2666,446 @@ static struct kunit_suite clk_mux_no_reparent_test_suite = { > .test_cases = clk_mux_no_reparent_test_cases, > }; > > +struct clk_register_clk_parent_data_test_case { > + const char *desc; > + struct clk_parent_data pdata; > +}; > + > +static void > +clk_register_clk_parent_data_test_case_to_desc( > + const struct clk_register_clk_parent_data_test_case *t, char *desc) > +{ > + strcpy(desc, t->desc); > +} > + > +static const struct clk_register_clk_parent_data_test_case > +clk_register_clk_parent_data_of_cases[] = { > + { > + /* > + * Test that a clk registered with a struct device_node can > + * find a parent based on struct clk_parent_data::index. > + */ > + .desc = "clk_parent_data_of_index_test", > + .pdata.index = 0, > + }, > + { > + /* > + * Test that a clk registered with a struct device_node can > + * find a parent based on struct clk_parent_data::fwname. > + */ > + .desc = "clk_parent_data_of_fwname_test", > + .pdata.fw_name = CLK_PARENT_DATA_PARENT1, > + }, > + { > + /* > + * Test that a clk registered with a struct device_node can > + * find a parent based on struct clk_parent_data::name. > + */ > + .desc = "clk_parent_data_of_name_test", > + /* The index must be negative to indicate firmware not used */ > + .pdata.index = -1, > + .pdata.name = CLK_PARENT_DATA_1MHZ_NAME, > + }, > + { > + /* > + * Test that a clk registered with a struct device_node can > + * find a parent based on struct > + * clk_parent_data::{fw_name,name}. > + */ > + .desc = "clk_parent_data_of_fwname_name_test", > + .pdata.fw_name = CLK_PARENT_DATA_PARENT1, > + .pdata.name = "not_matching", > + }, > + { > + /* > + * Test that a clk registered with a struct device_node can > + * find a parent based on struct clk_parent_data::{index,name}. > + * Index takes priority. > + */ > + .desc = "clk_parent_data_of_index_name_priority_test", > + .pdata.index = 0, > + .pdata.name = "not_matching", > + }, > + { > + /* > + * Test that a clk registered with a struct device_node can > + * find a parent based on struct > + * clk_parent_data::{index,fwname,name}. The fw_name takes > + * priority over index and name. > + */ > + .desc = "clk_parent_data_of_index_fwname_name_priority_test", > + .pdata.index = 1, > + .pdata.fw_name = CLK_PARENT_DATA_PARENT1, > + .pdata.name = "not_matching", > + }, > +}; > + > +KUNIT_ARRAY_PARAM(clk_register_clk_parent_data_of_test, clk_register_clk_parent_data_of_cases, > + clk_register_clk_parent_data_test_case_to_desc) > + > +/** > + * 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. > +} > + > +/* > + * Test that a clk registered with a struct device_node can find a parent based on > + * struct clk_parent_data when the hw member isn't set. > + */ > +static void clk_register_clk_parent_data_of_test(struct kunit *test) > +{ > + struct clk_register_clk_parent_data_of_ctx *ctx = test->priv; > + struct clk_hw *parent_hw; > + const struct clk_register_clk_parent_data_test_case *test_param; > + struct clk_init_data init = { }; > + struct clk *expected_parent, *actual_parent; > + > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->np); > + > + expected_parent = of_clk_get_kunit(test, ctx->np, 0); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, expected_parent); > + > + test_param = test->param_value; > + init.parent_data = &test_param->pdata; > + init.num_parents = 1; > + init.name = "parent_data_of_test_clk"; > + init.ops = &clk_dummy_single_parent_ops; > + ctx->hw.init = &init; > + KUNIT_ASSERT_EQ(test, 0, of_clk_hw_register_kunit(test, ctx->np, &ctx->hw)); > + > + parent_hw = clk_hw_get_parent(&ctx->hw); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw); > + > + actual_parent = clk_hw_get_clk_kunit(test, parent_hw, __func__); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, actual_parent); > + > + KUNIT_EXPECT_TRUE(test, clk_is_match(expected_parent, actual_parent)); > +} > + > +static struct kunit_case clk_register_clk_parent_data_of_test_cases[] = { > + KUNIT_CASE_PARAM(clk_register_clk_parent_data_of_test, > + clk_register_clk_parent_data_of_test_gen_params), > + {} > +}; > + > +/* > + * Test suite for registering clks with struct clk_parent_data and a struct > + * device_node. > + */ > +static struct kunit_suite clk_register_clk_parent_data_of_suite = { > + .name = "clk_register_clk_parent_data_of", > + .init = clk_register_clk_parent_data_of_test_init, > + .test_cases = clk_register_clk_parent_data_of_test_cases, > +}; > + > +/** > + * struct clk_register_clk_parent_data_device_ctx - Context for clk_parent_data device tests > + * @dev: device of clk under test > + * @hw: clk_hw for clk under test > + * @pdrv: driver to attach to find @dev > + */ > +struct clk_register_clk_parent_data_device_ctx { > + struct device *dev; > + struct clk_hw hw; > + struct platform_driver pdrv; > +}; > + > +static inline struct clk_register_clk_parent_data_device_ctx * > +clk_register_clk_parent_data_driver_to_test_context(struct platform_device *pdev) > +{ > + return container_of(to_platform_driver(pdev->dev.driver), > + struct clk_register_clk_parent_data_device_ctx, pdrv); > +} > + > +static int clk_register_clk_parent_data_device_probe(struct platform_device *pdev) > +{ > + struct clk_register_clk_parent_data_device_ctx *ctx; > + > + ctx = clk_register_clk_parent_data_driver_to_test_context(pdev); > + ctx->dev = &pdev->dev; > + > + return 0; > +} > + > +static void clk_register_clk_parent_data_device_driver(struct kunit *test) > +{ > + struct clk_register_clk_parent_data_device_ctx *ctx = test->priv; > + static const struct of_device_id match_table[] = { > + { .compatible = "test,clk-parent-data" }, > + { } > + }; > + > + ctx->pdrv.probe = clk_register_clk_parent_data_device_probe; > + ctx->pdrv.driver.of_match_table = match_table; > + ctx->pdrv.driver.name = __func__; > + ctx->pdrv.driver.owner = THIS_MODULE; > + > + KUNIT_ASSERT_EQ(test, 0, kunit_platform_driver_register(test, &ctx->pdrv)); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dev); > +} > + > +static const struct clk_register_clk_parent_data_test_case > +clk_register_clk_parent_data_device_cases[] = { > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::index. > + */ > + .desc = "clk_parent_data_device_index_test", > + .pdata.index = 1, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::fwname. > + */ > + .desc = "clk_parent_data_device_fwname_test", > + .pdata.fw_name = CLK_PARENT_DATA_PARENT2, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::name. > + */ > + .desc = "clk_parent_data_device_name_test", > + /* The index must be negative to indicate firmware not used */ > + .pdata.index = -1, > + .pdata.name = CLK_PARENT_DATA_50MHZ_NAME, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::{fw_name,name}. > + */ > + .desc = "clk_parent_data_device_fwname_name_test", > + .pdata.fw_name = CLK_PARENT_DATA_PARENT2, > + .pdata.name = "not_matching", > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::{index,name}. Index > + * takes priority. > + */ > + .desc = "clk_parent_data_device_index_name_priority_test", > + .pdata.index = 1, > + .pdata.name = "not_matching", > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::{index,fwname,name}. > + * The fw_name takes priority over index and name. > + */ > + .desc = "clk_parent_data_device_index_fwname_name_priority_test", > + .pdata.index = 0, > + .pdata.fw_name = CLK_PARENT_DATA_PARENT2, > + .pdata.name = "not_matching", > + }, > +}; > + > +KUNIT_ARRAY_PARAM(clk_register_clk_parent_data_device_test, > + clk_register_clk_parent_data_device_cases, > + clk_register_clk_parent_data_test_case_to_desc) > + > +/* > + * Test that a clk registered with a struct device can find a parent based on > + * struct clk_parent_data when the hw member isn't set. > + */ > +static void clk_register_clk_parent_data_device_test(struct kunit *test) > +{ > + struct clk_register_clk_parent_data_device_ctx *ctx; > + const struct clk_register_clk_parent_data_test_case *test_param; > + struct clk_hw *parent_hw; > + struct clk_init_data init = { }; > + struct clk *expected_parent, *actual_parent; > + > + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx); > + test->priv = ctx; > + > + clk_register_clk_parent_data_device_driver(test); > + > + expected_parent = clk_get_kunit(test, ctx->dev, "50"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, expected_parent); > + > + test_param = test->param_value; > + init.parent_data = &test_param->pdata; > + init.num_parents = 1; > + init.name = "parent_data_device_test_clk"; > + init.ops = &clk_dummy_single_parent_ops; > + ctx->hw.init = &init; > + KUNIT_ASSERT_EQ(test, 0, clk_hw_register_kunit(test, ctx->dev, &ctx->hw)); > + > + parent_hw = clk_hw_get_parent(&ctx->hw); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw); > + > + actual_parent = clk_hw_get_clk_kunit(test, parent_hw, __func__); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, actual_parent); > + > + KUNIT_EXPECT_TRUE(test, clk_is_match(expected_parent, actual_parent)); > +} > + > +static const struct clk_register_clk_parent_data_test_case > +clk_register_clk_parent_data_device_hw_cases[] = { > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw. > + */ > + .desc = "clk_parent_data_device_hw_index_test", > + /* The index must be negative to indicate firmware not used */ > + .pdata.index = -1, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw when > + * struct clk_parent_data::fw_name is set. > + */ > + .desc = "clk_parent_data_device_hw_fwname_test", > + .pdata.fw_name = CLK_PARENT_DATA_PARENT2, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw when struct > + * clk_parent_data::name is set. > + */ > + .desc = "clk_parent_data_device_hw_name_test", > + /* The index must be negative to indicate firmware not used */ > + .pdata.index = -1, > + .pdata.name = CLK_PARENT_DATA_50MHZ_NAME, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw when struct > + * clk_parent_data::{fw_name,name} are set. > + */ > + .desc = "clk_parent_data_device_hw_fwname_name_test", > + .pdata.fw_name = CLK_PARENT_DATA_PARENT2, > + .pdata.name = "not_matching", > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw when struct > + * clk_parent_data::index is set. The hw pointer takes > + * priority. > + */ > + .desc = "clk_parent_data_device_hw_index_priority_test", > + .pdata.index = 0, > + }, > + { > + /* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw when > + * struct clk_parent_data::{index,fwname,name} are set. > + * The hw pointer takes priority over everything else. > + */ > + .desc = "clk_parent_data_device_hw_index_fwname_name_priority_test", > + .pdata.index = 0, > + .pdata.fw_name = CLK_PARENT_DATA_PARENT2, > + .pdata.name = "not_matching", > + }, > +}; > + > +KUNIT_ARRAY_PARAM(clk_register_clk_parent_data_device_hw_test, > + clk_register_clk_parent_data_device_hw_cases, > + clk_register_clk_parent_data_test_case_to_desc) > + > +/* > + * Test that a clk registered with a struct device can find a > + * parent based on struct clk_parent_data::hw. > + */ > +static void clk_register_clk_parent_data_device_hw_test(struct kunit *test) > +{ > + struct clk_register_clk_parent_data_device_ctx *ctx; > + const struct clk_register_clk_parent_data_test_case *test_param; > + struct clk_dummy_context *parent; > + struct clk_hw *parent_hw; > + struct clk_parent_data pdata = { }; > + struct clk_init_data init = { }; > + > + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx); > + test->priv = ctx; > + > + clk_register_clk_parent_data_device_driver(test); > + > + parent = kunit_kzalloc(test, sizeof(*parent), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent); > + > + parent_hw = &parent->hw; > + parent_hw->init = CLK_HW_INIT_NO_PARENT("parenlookst-clk", > + &clk_dummy_rate_ops, 0); > + > + KUNIT_ASSERT_EQ(test, 0, clk_hw_register_kunit(test, ctx->dev, parent_hw)); > + > + test_param = test->param_value; > + memcpy(&pdata, &test_param->pdata, sizeof(pdata)); > + pdata.hw = parent_hw; > + init.parent_data = &pdata; > + init.num_parents = 1; > + init.ops = &clk_dummy_single_parent_ops; > + init.name = "parent_data_device_hw_test_clk"; > + ctx->hw.init = &init; > + KUNIT_ASSERT_EQ(test, 0, clk_hw_register_kunit(test, ctx->dev, &ctx->hw)); > + > + KUNIT_EXPECT_PTR_EQ(test, parent_hw, clk_hw_get_parent(&ctx->hw)); > +} > + > +static struct kunit_case clk_register_clk_parent_data_device_test_cases[] = { > + KUNIT_CASE_PARAM(clk_register_clk_parent_data_device_test, > + clk_register_clk_parent_data_device_test_gen_params), > + KUNIT_CASE_PARAM(clk_register_clk_parent_data_device_hw_test, > + clk_register_clk_parent_data_device_hw_test_gen_params), > + {} > +}; > + > +static int clk_register_clk_parent_data_device_init(struct kunit *test) > +{ > + KUNIT_ASSERT_EQ(test, 0, > + of_overlay_apply_kunit(test, kunit_clk_parent_data_test)); > + > + return 0; > +} > + > +/* > + * Test suite for registering clks with struct clk_parent_data and a struct > + * device. > + */ > +static struct kunit_suite clk_register_clk_parent_data_device_suite = { > + .name = "clk_register_clk_parent_data_device", > + .init = clk_register_clk_parent_data_device_init, > + .test_cases = clk_register_clk_parent_data_device_test_cases, > +}; > + > kunit_test_suites( > &clk_leaf_mux_set_rate_parent_test_suite, > &clk_test_suite, > @@ -2671,7 +3118,9 @@ kunit_test_suites( > &clk_range_test_suite, > &clk_range_maximize_test_suite, > &clk_range_minimize_test_suite, > + &clk_register_clk_parent_data_of_suite, > + &clk_register_clk_parent_data_device_suite, > &clk_single_parent_mux_test_suite, > - &clk_uncached_test_suite > + &clk_uncached_test_suite, > ); > MODULE_LICENSE("GPL v2"); > diff --git a/drivers/clk/kunit_clk_parent_data_test.dtso b/drivers/clk/kunit_clk_parent_data_test.dtso > new file mode 100644 > index 000000000000..7d3ed9a5a2e8 > --- /dev/null > +++ b/drivers/clk/kunit_clk_parent_data_test.dtso > @@ -0,0 +1,28 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/dts-v1/; > +/plugin/; > + > +#include "clk_parent_data_test.h" > + > +&{/} { > + fixed_50: kunit-clock-50MHz { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <50000000>; > + clock-output-names = CLK_PARENT_DATA_50MHZ_NAME; > + }; > + > + fixed_parent: kunit-clock-1MHz { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <1000000>; > + clock-output-names = CLK_PARENT_DATA_1MHZ_NAME; > + }; > + > + kunit-clock-controller { > + compatible = "test,clk-parent-data"; > + clocks = <&fixed_parent>, <&fixed_50>; > + clock-names = CLK_PARENT_DATA_PARENT1, CLK_PARENT_DATA_PARENT2; > + #clock-cells = <1>; > + }; > +}; > -- > https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/ > https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature