| Message ID | 20220120143417.543744-2-maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clk: Improve clock range handling | expand |
Quoting Maxime Ripard (2022-01-20 06:34:08) > 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> > --- This is great! Thanks for doing this. > drivers/clk/Kconfig | 7 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-rate-test.c | 278 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 286 insertions(+) > create mode 100644 drivers/clk/clk-rate-test.c I was thinking this would be more generic so that one file tests clk.c and all the code in there, but I guess there may be config dependencies like CONFIG_OF that we may want to extract out and depend on differently. I'm not sure how kunit will handle testing different paths depending on build configuration so this approach may head off future problems. If it doesn't then we can always slam the test together. > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index f5807d190ba2..7ae48a91f738 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -436,4 +436,11 @@ config CLK_GATE_TEST > help > Kunit test for the basic clk gate type. > > +config CLK_RATE_TEST See v3 here[1] and have it follow that. > + tristate "Basic Core Rate Kunit Tests" > + depends on KUNIT > + default KUNIT > + help > + Kunit test for the basic clock rate management. > + > endif > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index b940c6d35922..0238a595167a 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_RATE_TEST) += clk-rate-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-rate-test.c b/drivers/clk/clk-rate-test.c > new file mode 100644 > index 000000000000..f2d3df791b5a > --- /dev/null > +++ b/drivers/clk/clk-rate-test.c > @@ -0,0 +1,278 @@ > +// 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> > + > +#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_rate_context { > + struct clk_hw hw; > + unsigned long rate; > +}; > + > +static unsigned long clk_dummy_rate_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct clk_dummy_rate_context *ctx = > + container_of(hw, struct clk_dummy_rate_context, hw); > + > + return ctx->rate; > +} > + > +static int clk_dummy_rate_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_rate_set_rate(struct clk_hw *hw, > + unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_dummy_rate_context *ctx = > + container_of(hw, struct clk_dummy_rate_context, hw); > + > + ctx->rate = rate; > + return 0; > +} > + > +const static struct clk_ops clk_dummy_rate_ops = { static const? > + .recalc_rate = clk_dummy_rate_recalc_rate, > + .determine_rate = clk_dummy_rate_determine_rate, > + .set_rate = clk_dummy_rate_set_rate, > +}; > + > +static int clk_rate_test_init_with_ops(struct kunit *test, > + const struct clk_ops *ops) > +{ > + struct clk_dummy_rate_context *ctx; > + struct clk_init_data init = { }; > + int ret; > + > + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); Use kunit_kzalloc() here > + 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_rate_test_init(struct kunit *test) > +{ > + return clk_rate_test_init_with_ops(test, &clk_dummy_rate_ops); > +} > + > +static void clk_rate_test_exit(struct kunit *test) > +{ > + struct clk_dummy_rate_context *ctx = test->priv; > + > + clk_hw_unregister(&ctx->hw); > + kfree(ctx); And drop the kfree as it is now test managed. > +} > + > +/* > + * Test that the actual rate matches what is returned by clk_get_rate() > + */ > +static void clk_rate_test_get_rate(struct kunit *test) > +{ > + struct clk_dummy_rate_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); > + KUNIT_ASSERT_EQ(test, rate, ctx->rate); These should be KUNIT_EXPECT_*() as we don't want to stop the test if the rate is wrong, we want to check that the rate is what we expected it to be. Assertions are about making sure things are sane and if not we should stop testing, whereas expectations are about testing the code. A test must have an EXPECT while it can have an ASSERT. Maybe kunit should check that there was an EXPECT on return from the test. Daniel? > +} > + > +/* > + * Test that, after a call to clk_set_rate(), the rate returned by > + * clk_get_rate() matches. > + * > + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't > + * modify the requested rate, which is our case in clk_dummy_rate_ops. > + */ > +static void clk_rate_test_set_get_rate(struct kunit *test) > +{ > + struct clk_dummy_rate_context *ctx = test->priv; > + struct clk_hw *hw = &ctx->hw; > + struct clk *clk = hw->clk; > + unsigned long rate; > + int ret; > + > + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1); > + KUNIT_ASSERT_EQ(test, ret, 0); I'd like to throw the ret check directly into KUNIT_ASSERT_EQ() here. KUNIT_ASSERT_EQ(test, clk_set_rate(clk, DUMMY_CLOCK_RATE_1), 0); so we can easily see if something goes wrong that the set rate failed. Good use of assert here, we don't want to continue with the test unless the set rate actually worked. > + > + rate = clk_get_rate(clk); > + KUNIT_ASSERT_TRUE(test, rate > 0); > + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1); > +} > + > +/* > + * Test that, after several calls to clk_set_rate(), the rate returned > + * by clk_get_rate() matches the last one. > + * > + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't > + * modify the requested rate, which is our case in clk_dummy_rate_ops. > + */ > +static void clk_rate_test_set_set_get_rate(struct kunit *test) > +{ > + struct clk_dummy_rate_context *ctx = test->priv; > + struct clk_hw *hw = &ctx->hw; > + struct clk *clk = hw->clk; > + unsigned long rate; > + int ret; > + > + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + rate = clk_get_rate(clk); > + KUNIT_ASSERT_TRUE(test, rate > 0); > + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2); > +} > + > +static struct kunit_case clk_rate_test_cases[] = { > + KUNIT_CASE(clk_rate_test_get_rate), > + KUNIT_CASE(clk_rate_test_set_get_rate), > + KUNIT_CASE(clk_rate_test_set_set_get_rate), > + {} > +}; > + > +static struct kunit_suite clk_rate_test_suite = { > + .name = "clk-rate-test", > + .init = clk_rate_test_init, > + .exit = clk_rate_test_exit, > + .test_cases = clk_rate_test_cases, > +}; > + > +/* > + * Test that clk_set_rate_range won't return an error for a valid range. > + */ > +static void clk_rate_range_test_set_range(struct kunit *test) > +{ > + struct clk_dummy_rate_context *ctx = test->priv; > + struct clk_hw *hw = &ctx->hw; > + struct clk *clk = hw->clk; > + int ret; > + > + ret = clk_set_rate_range(clk, > + DUMMY_CLOCK_RATE_1, > + DUMMY_CLOCK_RATE_2); > + KUNIT_ASSERT_EQ(test, ret, 0); Also make sure that the rate is within the bounds of rate_1 and rate_2? > +} > + > +/* > + * Test that calling clk_set_rate_range with a minimum rate higher than > + * the maximum rate returns an error. > + */ > +static void clk_rate_range_test_set_range_invalid(struct kunit *test) > +{ > + struct clk_dummy_rate_context *ctx = test->priv; > + struct clk_hw *hw = &ctx->hw; > + struct clk *clk = hw->clk; > + int ret; > + > + ret = clk_set_rate_range(clk, > + DUMMY_CLOCK_RATE_1 + 1000, > + DUMMY_CLOCK_RATE_1); > + KUNIT_ASSERT_EQ(test, ret, -EINVAL); Let's not check for a specific error, but a negative value instead. I'd rather not encode particular error values unless we need them to be particular. > +} > + > +/* > + * Test that if our clock has a rate lower than the minimum set by a > + * call to clk_set_rate_range(), the rate will be raised to match the > + * new minimum. > + * > + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't > + * modify the requested rate, which is our case in clk_dummy_rate_ops. > + */ > +static void clk_rate_range_test_set_range_get_rate_raised(struct kunit *test) > +{ > + struct clk_dummy_rate_context *ctx = test->priv; > + struct clk_hw *hw = &ctx->hw; > + struct clk *clk = hw->clk; > + unsigned long rate; > + int ret; > + > + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1 - 1000); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + ret = clk_set_rate_range(clk, > + DUMMY_CLOCK_RATE_1, > + DUMMY_CLOCK_RATE_2); > + KUNIT_ASSERT_EQ(test, ret, 0); > + > + rate = clk_get_rate(clk); > + KUNIT_ASSERT_TRUE(test, rate > 0); KUNIT_EXPECT_LE(test, clk_get_rate(clk), DUMMY_CLOCK_RATE_2); Or just drop it entirely as DUMMY_CLOCK_RATE_1 is greater than 0 and it's tested next. > + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1); KUNIT_EXPECT_EQ(test, clk_get_rate(clk), DUMMY_CLOCK_RATE_1); > +} > +
On Thu, Jan 20, 2022 at 1:31 PM Stephen Boyd <sboyd@kernel.org> wrote: > I was thinking this would be more generic so that one file tests clk.c > and all the code in there, but I guess there may be config dependencies > like CONFIG_OF that we may want to extract out and depend on > differently. I'm not sure how kunit will handle testing different paths > depending on build configuration so this approach may head off future > problems. If it doesn't then we can always slam the test together. KUnit doesn't have hard technical limitations in this regard. You could have something like this static void my_optional_kunit_test(struct kunit *test) { #ifdef CONFIG_OPTIONAL_FEATURE # else kunit_skip(test, "CONFIG_OPTIONAL_FEATURE is not enabled"); #endif } I think it's just a matter of what's least confusing to users. > > +} > > + > > +/* > > + * Test that the actual rate matches what is returned by clk_get_rate() > > + */ > > +static void clk_rate_test_get_rate(struct kunit *test) > > +{ > > + struct clk_dummy_rate_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); KUNIT_EXPECT_GT(test, rate, 0); > > + KUNIT_ASSERT_EQ(test, rate, ctx->rate); > > These should be KUNIT_EXPECT_*() as we don't want to stop the test if > the rate is wrong, we want to check that the rate is what we expected it > to be. Assertions are about making sure things are sane and if not we > should stop testing, whereas expectations are about testing the code. A > test must have an EXPECT while it can have an ASSERT. > > Maybe kunit should check that there was an EXPECT on return from the > test. Daniel? Sorry, I'm not sure I understand the question. Are you saying you want kunit to flag cases like static void empty_test(struct kunit *) {} ?
Quoting Daniel Latypov (2022-01-20 13:56:39) > On Thu, Jan 20, 2022 at 1:31 PM Stephen Boyd <sboyd@kernel.org> wrote: > > I was thinking this would be more generic so that one file tests clk.c > > and all the code in there, but I guess there may be config dependencies > > like CONFIG_OF that we may want to extract out and depend on > > differently. I'm not sure how kunit will handle testing different paths > > depending on build configuration so this approach may head off future > > problems. If it doesn't then we can always slam the test together. > > KUnit doesn't have hard technical limitations in this regard. > > You could have something like this > > static void my_optional_kunit_test(struct kunit *test) > { > #ifdef CONFIG_OPTIONAL_FEATURE > > # else > kunit_skip(test, "CONFIG_OPTIONAL_FEATURE is not enabled"); > #endif > } > > I think it's just a matter of what's least confusing to users. Ok, I see. Is there some way to have multiple configs checked into the tree so we can test different kernel configuration code paths? This discussion isn't really relevant to this patch so we can take this up in another thread if you like. > > > > > Maybe kunit should check that there was an EXPECT on return from the > > test. Daniel? > > Sorry, I'm not sure I understand the question. > > Are you saying you want kunit to flag cases like > static void empty_test(struct kunit *) {} > ? Yes. I'd like kunit to enforce that all tests have at least one EXPECT_*() in them.
On Thu, Jan 20, 2022 at 8:34 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Daniel Latypov (2022-01-20 13:56:39) > > On Thu, Jan 20, 2022 at 1:31 PM Stephen Boyd <sboyd@kernel.org> wrote: > > KUnit doesn't have hard technical limitations in this regard. > > > > You could have something like this > > > > static void my_optional_kunit_test(struct kunit *test) > > { > > #ifdef CONFIG_OPTIONAL_FEATURE > > > > # else > > kunit_skip(test, "CONFIG_OPTIONAL_FEATURE is not enabled"); > > #endif > > } > > > > I think it's just a matter of what's least confusing to users. > > Ok, I see. Is there some way to have multiple configs checked into the > tree so we can test different kernel configuration code paths? This Multiple kunitconfigs? There's no restrictions on those $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk/kunitconfig.foo $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk/kunitconfig.bar The first one will assume drivers/clk/.kunitconfig. But there's no reason you need to have a file called that. One could just have multiple standalone kunitconfigs, named however they like. --kunitconfig is new enough (5.12+) that there's no real conventions yet. Another option is $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk \ --kconfig_add=CONFIG_RARELY_USED=y This is another case where we can do whatever is least confusing. > discussion isn't really relevant to this patch so we can take this up in > another thread if you like. > > > > > > > > > Maybe kunit should check that there was an EXPECT on return from the > > > test. Daniel? > > > > Sorry, I'm not sure I understand the question. > > > > Are you saying you want kunit to flag cases like > > static void empty_test(struct kunit *) {} > > ? > > Yes. I'd like kunit to enforce that all tests have at least one > EXPECT_*() in them. I totally understand the rationale. It's a bit misleading to say PASSED if no expectation/assertion passed. One might want a NO_STATUS (or maybe SKIPPED) result instead. But other unit test frameworks act the way KUnit does here, so there's an argument for consistency with others so users don't have to have a whole new mental model. Some examples below for reference. All of these output something like test result: ok. 1 passed; ... E.g. in Python import unittest class ExampleTest(unittest.TestCase): def test_foo(self): pass if __name__ == '__main__': unittest.main() In Golang: package example import "testing" func TestFoo(t *testing.T) {} In C++ using Googletest: #include "gtest/gtest.h" TEST(Suite, Case) { } In Rust: #[cfg(test)] mod tests { #[test] fn test_empty() {} }
Quoting Daniel Latypov (2022-01-20 21:25:03) > On Thu, Jan 20, 2022 at 8:34 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Daniel Latypov (2022-01-20 13:56:39) > > > On Thu, Jan 20, 2022 at 1:31 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > KUnit doesn't have hard technical limitations in this regard. > > > > > > You could have something like this > > > > > > static void my_optional_kunit_test(struct kunit *test) > > > { > > > #ifdef CONFIG_OPTIONAL_FEATURE > > > > > > # else > > > kunit_skip(test, "CONFIG_OPTIONAL_FEATURE is not enabled"); > > > #endif > > > } > > > > > > I think it's just a matter of what's least confusing to users. > > > > Ok, I see. Is there some way to have multiple configs checked into the > > tree so we can test different kernel configuration code paths? This > > Multiple kunitconfigs? > There's no restrictions on those > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk/kunitconfig.foo > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk/kunitconfig.bar > > The first one will assume drivers/clk/.kunitconfig. > But there's no reason you need to have a file called that. > One could just have multiple standalone kunitconfigs, named however they like. > > --kunitconfig is new enough (5.12+) that there's no real conventions yet. > > Another option is > $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/clk \ > --kconfig_add=CONFIG_RARELY_USED=y > > This is another case where we can do whatever is least confusing. Got it, thanks. > > > discussion isn't really relevant to this patch so we can take this up in > > another thread if you like. > > > > > > > > > > > > > Maybe kunit should check that there was an EXPECT on return from the > > > > test. Daniel? > > > > > > Sorry, I'm not sure I understand the question. > > > > > > Are you saying you want kunit to flag cases like > > > static void empty_test(struct kunit *) {} > > > ? > > > > Yes. I'd like kunit to enforce that all tests have at least one > > EXPECT_*() in them. > > I totally understand the rationale. > It's a bit misleading to say PASSED if no expectation/assertion passed. > One might want a NO_STATUS (or maybe SKIPPED) result instead. > > But other unit test frameworks act the way KUnit does here, so there's > an argument for consistency with others so users don't have to have a > whole new mental model. Ok if other test frameworks don't care then there's nothing to do.
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index f5807d190ba2..7ae48a91f738 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -436,4 +436,11 @@ config CLK_GATE_TEST help Kunit test for the basic clk gate type. +config CLK_RATE_TEST + tristate "Basic Core Rate Kunit Tests" + depends on KUNIT + default KUNIT + help + Kunit test for the basic clock rate management. + endif diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index b940c6d35922..0238a595167a 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_RATE_TEST) += clk-rate-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-rate-test.c b/drivers/clk/clk-rate-test.c new file mode 100644 index 000000000000..f2d3df791b5a --- /dev/null +++ b/drivers/clk/clk-rate-test.c @@ -0,0 +1,278 @@ +// 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> + +#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_rate_context { + struct clk_hw hw; + unsigned long rate; +}; + +static unsigned long clk_dummy_rate_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_dummy_rate_context *ctx = + container_of(hw, struct clk_dummy_rate_context, hw); + + return ctx->rate; +} + +static int clk_dummy_rate_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_rate_set_rate(struct clk_hw *hw, + unsigned long rate, + unsigned long parent_rate) +{ + struct clk_dummy_rate_context *ctx = + container_of(hw, struct clk_dummy_rate_context, hw); + + ctx->rate = rate; + return 0; +} + +const static struct clk_ops clk_dummy_rate_ops = { + .recalc_rate = clk_dummy_rate_recalc_rate, + .determine_rate = clk_dummy_rate_determine_rate, + .set_rate = clk_dummy_rate_set_rate, +}; + +static int clk_rate_test_init_with_ops(struct kunit *test, + const struct clk_ops *ops) +{ + struct clk_dummy_rate_context *ctx; + struct clk_init_data init = { }; + int ret; + + ctx = kzalloc(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_rate_test_init(struct kunit *test) +{ + return clk_rate_test_init_with_ops(test, &clk_dummy_rate_ops); +} + +static void clk_rate_test_exit(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + + clk_hw_unregister(&ctx->hw); + kfree(ctx); +} + +/* + * Test that the actual rate matches what is returned by clk_get_rate() + */ +static void clk_rate_test_get_rate(struct kunit *test) +{ + struct clk_dummy_rate_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); + KUNIT_ASSERT_EQ(test, rate, ctx->rate); +} + +/* + * Test that, after a call to clk_set_rate(), the rate returned by + * clk_get_rate() matches. + * + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't + * modify the requested rate, which is our case in clk_dummy_rate_ops. + */ +static void clk_rate_test_set_get_rate(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + struct clk_hw *hw = &ctx->hw; + struct clk *clk = hw->clk; + unsigned long rate; + int ret; + + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1); + KUNIT_ASSERT_EQ(test, ret, 0); + + rate = clk_get_rate(clk); + KUNIT_ASSERT_TRUE(test, rate > 0); + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1); +} + +/* + * Test that, after several calls to clk_set_rate(), the rate returned + * by clk_get_rate() matches the last one. + * + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't + * modify the requested rate, which is our case in clk_dummy_rate_ops. + */ +static void clk_rate_test_set_set_get_rate(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + struct clk_hw *hw = &ctx->hw; + struct clk *clk = hw->clk; + unsigned long rate; + int ret; + + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2); + KUNIT_ASSERT_EQ(test, ret, 0); + + rate = clk_get_rate(clk); + KUNIT_ASSERT_TRUE(test, rate > 0); + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2); +} + +static struct kunit_case clk_rate_test_cases[] = { + KUNIT_CASE(clk_rate_test_get_rate), + KUNIT_CASE(clk_rate_test_set_get_rate), + KUNIT_CASE(clk_rate_test_set_set_get_rate), + {} +}; + +static struct kunit_suite clk_rate_test_suite = { + .name = "clk-rate-test", + .init = clk_rate_test_init, + .exit = clk_rate_test_exit, + .test_cases = clk_rate_test_cases, +}; + +/* + * Test that clk_set_rate_range won't return an error for a valid range. + */ +static void clk_rate_range_test_set_range(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + struct clk_hw *hw = &ctx->hw; + struct clk *clk = hw->clk; + int ret; + + ret = clk_set_rate_range(clk, + DUMMY_CLOCK_RATE_1, + DUMMY_CLOCK_RATE_2); + KUNIT_ASSERT_EQ(test, ret, 0); +} + +/* + * Test that calling clk_set_rate_range with a minimum rate higher than + * the maximum rate returns an error. + */ +static void clk_rate_range_test_set_range_invalid(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + struct clk_hw *hw = &ctx->hw; + struct clk *clk = hw->clk; + int ret; + + ret = clk_set_rate_range(clk, + DUMMY_CLOCK_RATE_1 + 1000, + DUMMY_CLOCK_RATE_1); + KUNIT_ASSERT_EQ(test, ret, -EINVAL); +} + +/* + * Test that if our clock has a rate lower than the minimum set by a + * call to clk_set_rate_range(), the rate will be raised to match the + * new minimum. + * + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't + * modify the requested rate, which is our case in clk_dummy_rate_ops. + */ +static void clk_rate_range_test_set_range_get_rate_raised(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + struct clk_hw *hw = &ctx->hw; + struct clk *clk = hw->clk; + unsigned long rate; + int ret; + + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1 - 1000); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = clk_set_rate_range(clk, + DUMMY_CLOCK_RATE_1, + DUMMY_CLOCK_RATE_2); + KUNIT_ASSERT_EQ(test, ret, 0); + + rate = clk_get_rate(clk); + KUNIT_ASSERT_TRUE(test, rate > 0); + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1); +} + +/* + * Test that if our clock has a rate higher than the maximum set by a + * call to clk_set_rate_range(), the rate will be lowered to match the + * new maximum. + * + * This assumes that clk_ops.determine_rate or clk_ops.round_rate won't + * modify the requested rate, which is our case in clk_dummy_rate_ops. + */ +static void clk_rate_range_test_set_range_get_rate_lowered(struct kunit *test) +{ + struct clk_dummy_rate_context *ctx = test->priv; + struct clk_hw *hw = &ctx->hw; + struct clk *clk = hw->clk; + unsigned long rate; + int ret; + + ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2 + 1000); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = clk_set_rate_range(clk, + DUMMY_CLOCK_RATE_1, + DUMMY_CLOCK_RATE_2); + KUNIT_ASSERT_EQ(test, ret, 0); + + rate = clk_get_rate(clk); + KUNIT_ASSERT_TRUE(test, rate > 0); + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2); +} + +static struct kunit_case clk_rate_range_test_cases[] = { + KUNIT_CASE(clk_rate_range_test_set_range), + KUNIT_CASE(clk_rate_range_test_set_range_invalid), + KUNIT_CASE(clk_rate_range_test_set_range_get_rate_raised), + KUNIT_CASE(clk_rate_range_test_set_range_get_rate_lowered), + {} +}; + +static struct kunit_suite clk_rate_range_test_suite = { + .name = "clk-rate-range-test", + .init = clk_rate_test_init, + .exit = clk_rate_test_exit, + .test_cases = clk_rate_range_test_cases, +}; + +kunit_test_suites( + &clk_rate_test_suite, + &clk_rate_range_test_suite +); +MODULE_LICENSE("GPL v2");
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> --- drivers/clk/Kconfig | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-rate-test.c | 278 ++++++++++++++++++++++++++++++++++++ 3 files changed, 286 insertions(+) create mode 100644 drivers/clk/clk-rate-test.c