On Wed, Feb 23, 2022 at 2:56 AM Maxime Ripard maxime@cerno.tech wrote:
Let's test various parts of the rate-related clock API with the kunit testing framework.
Cc: kunit-dev@googlegroups.com Suggested-by: Stephen Boyd sboyd@kernel.org Signed-off-by: Maxime Ripard maxime@cerno.tech
Tested-by: Daniel Latypov dlatypov@google.com
Looks good to me on the KUnit side. Two small nits below.
FYI, I computed the incremental coverage for this series, i.e.: 1) applied the full series 2) computed the absolute coverage
$ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk --make_options=CC=/usr/bin/gcc-6 --kconfig_add=CONFIG_DEBUG_KERNEL=y --kconfig_add=CONFIG_DEBUG_INFO=y --kconfig_add=CONFIG_GCOV=y $ lcov -t "clk_tests" -o coverage.info -c -d .kunit/ --gcov-tool=/usr/bin/gcov-6
3) intersected that with the total diff
Incremental coverage for 3/9 files in --diff_file Total incremental: 99.29% coverage (281/283 lines) drivers/clk/clk.c: 84.62% coverage (11/13 lines) drivers/clk/clk_test.c: 100.00% coverage (269/269 lines) include/linux/clk.h: 100.00% coverage (1/1 lines)
Missing lines are drivers/clk/clk.c:2397-8, i.e. this part of the diff: + if (ret) { + /* rollback the changes */ + clk->min_rate = old_min; <- 2397 + clk->max_rate = old_max; <- 2398
These are from before and were just moved around.
Note: this filters out code that wasn't compiled in and wasn't executable. So that's why it's missing clk-raspberrypi.c and friends and it says clk.c had 13 changed lines (since most of the lines are comments).
drivers/clk/.kunitconfig | 1 + drivers/clk/Kconfig | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk_test.c | 786 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 795 insertions(+) create mode 100644 drivers/clk/clk_test.c
diff --git a/drivers/clk/.kunitconfig b/drivers/clk/.kunitconfig index 3754fdb9485a..cdbc7d7deba9 100644 --- a/drivers/clk/.kunitconfig +++ b/drivers/clk/.kunitconfig @@ -1,3 +1,4 @@ CONFIG_KUNIT=y CONFIG_COMMON_CLK=y +CONFIG_CLK_KUNIT_TEST=y CONFIG_CLK_GATE_KUNIT_TEST=y diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 3cdf33470a75..2ef6eca297ff 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -429,6 +429,13 @@ source "drivers/clk/xilinx/Kconfig" source "drivers/clk/zynqmp/Kconfig"
# Kunit test cases +config CLK_KUNIT_TEST
tristate "Basic Clock Framework Kunit Tests" if !KUNIT_ALL_TESTSdepends on KUNITdefault KUNIT_ALL_TESTShelpKunit tests for the common clock framework.config CLK_GATE_KUNIT_TEST tristate "Basic gate type Kunit test" if !KUNIT_ALL_TESTS depends on KUNIT diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 6a98291350b6..8f9b1daba411 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -2,6 +2,7 @@ # 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_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_test.c b/drivers/clk/clk_test.c new file mode 100644 index 000000000000..0ca6cd391c8e --- /dev/null +++ b/drivers/clk/clk_test.c @@ -0,0 +1,786 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Kunit test for clk rate management
- */
+#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/slab.h>
Very nit: Is this #include <linux/slab.h> necessary? I removed it and added a check that its header guard is not defined:
+#ifdef _LINUX_SLAB_H +#error "Included slab.h indirectly" +#endif
The test still compiled, so afaik, nothing here needs it.
+/* Needed for clk_hw_get_clk() */ +#include "clk.h"
+#include <kunit/test.h>
+#define DUMMY_CLOCK_INIT_RATE (42 * 1000 * 1000) +#define DUMMY_CLOCK_RATE_1 (142 * 1000 * 1000) +#define DUMMY_CLOCK_RATE_2 (242 * 1000 * 1000)
+struct clk_dummy_context {
struct clk_hw hw;unsigned long rate;+};
+static unsigned long clk_dummy_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)+{
struct clk_dummy_context *ctx =container_of(hw, struct clk_dummy_context, hw);return ctx->rate;+}
+static int clk_dummy_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)+{
/* Just return the same rate without modifying it */return 0;+}
+static int clk_dummy_maximize_rate(struct clk_hw *hw,
struct clk_rate_request *req)+{
/** If there's a maximum set, always run the clock at the maximum* allowed.*/if (req->max_rate < ULONG_MAX)req->rate = req->max_rate;return 0;+}
+static int clk_dummy_minimize_rate(struct clk_hw *hw,
struct clk_rate_request *req)+{
/** If there's a minimum set, always run the clock at the minimum* allowed.*/if (req->min_rate > 0)req->rate = req->min_rate;return 0;+}
+static int clk_dummy_set_rate(struct clk_hw *hw,
unsigned long rate,unsigned long parent_rate)+{
struct clk_dummy_context *ctx =container_of(hw, struct clk_dummy_context, hw);ctx->rate = rate;return 0;+}
+static const struct clk_ops clk_dummy_rate_ops = {
.recalc_rate = clk_dummy_recalc_rate,.determine_rate = clk_dummy_determine_rate,.set_rate = clk_dummy_set_rate,+};
+static const struct clk_ops clk_dummy_maximize_rate_ops = {
.recalc_rate = clk_dummy_recalc_rate,.determine_rate = clk_dummy_maximize_rate,.set_rate = clk_dummy_set_rate,+};
+static const struct clk_ops clk_dummy_minimize_rate_ops = {
.recalc_rate = clk_dummy_recalc_rate,.determine_rate = clk_dummy_minimize_rate,.set_rate = clk_dummy_set_rate,+};
+static int clk_test_init_with_ops(struct kunit *test, const struct clk_ops *ops) +{
struct clk_dummy_context *ctx;struct clk_init_data init = { };int ret;ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);if (!ctx)return -ENOMEM;ctx->rate = DUMMY_CLOCK_INIT_RATE;test->priv = ctx;init.name = "test_dummy_rate";init.ops = ops;ctx->hw.init = &init;ret = clk_hw_register(NULL, &ctx->hw);if (ret)return ret;return 0;+}
+static int clk_test_init(struct kunit *test) +{
return clk_test_init_with_ops(test, &clk_dummy_rate_ops);+}
+static int clk_maximize_test_init(struct kunit *test) +{
return clk_test_init_with_ops(test, &clk_dummy_maximize_rate_ops);+}
+static int clk_minimize_test_init(struct kunit *test) +{
return clk_test_init_with_ops(test, &clk_dummy_minimize_rate_ops);+}
+static void clk_test_exit(struct kunit *test) +{
struct clk_dummy_context *ctx = test->priv;clk_hw_unregister(&ctx->hw);+}
+/*
- Test that the actual rate matches what is returned by clk_get_rate()
- */
+static void clk_test_get_rate(struct kunit *test) +{
struct clk_dummy_context *ctx = test->priv;struct clk_hw *hw = &ctx->hw;struct clk *clk = hw->clk;unsigned long rate;rate = clk_get_rate(clk);KUNIT_ASSERT_TRUE(test, rate > 0);
nit: KUNIT_ASSERT_GT(test, rate, 0);
Looks like we updated the others below but forgot this one.