Re: [PATCH 02/22] clk: tests: Add test suites description

[Stephen Boyd <sboyd@kernel.org>]

Quoting Maxime Ripard (2022-04-08 02:10:17)
> We start to have a few test suites, and we'll add more, so it will get
> pretty confusing to figure out what is supposed to be tested in what
> suite.
> 
> Let's add some comments to explain what setup they create, and what we
> should be testing in every suite.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---

Thanks. Mostly nitpicks but otherwise looks good.

> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index fd2339cc5898..663b3dd388f7 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -250,6 +250,11 @@ static struct kunit_case clk_test_cases[] = {
>         {}
>  };
>  
> +/*
> + * Test suite for a basic rate clock, without any parent.
> + *
> + * These tests are supposed to exercise the rate API with simple scenarios

s/are supposed to//

Simplify please.

> + */
>  static struct kunit_suite clk_test_suite = {
>         .name = "clk-test",
>         .init = clk_test_init,
> @@ -336,6 +341,14 @@ static struct kunit_case clk_orphan_transparent_single_parent_mux_test_cases[] =
>         {}
>  };
>  
> +/*
> + * Test suite for a basic mux clock with one parent. The parent is
> + * registered after its child. The clock will thus be orphan when

s/be/be an/

> + * registered, but will no longer be when the tests run.
> + *
> + * These tests are supposed to make sure a clock that used to be orphan

s/are supposed to//

> + * has a sane, consistent, behaviour.
> + */
>  static struct kunit_suite clk_orphan_transparent_single_parent_test_suite = {
>         .name = "clk-orphan-transparent-single-parent-test",
>         .init = clk_orphan_transparent_single_parent_mux_test_init,
> @@ -645,6 +658,13 @@ static struct kunit_case clk_range_test_cases[] = {
>         {}
>  };
>  
> +/*
> + * Test suite for a basic rate clock, without any parent.
> + *
> + * These tests are supposed to exercise the rate range API

s/are supposed to//

s/API/API:/


> + * (clk_set_rate_range, clk_set_min_rate, clk_set_max_rate,

Functions have () after them. Drop parenthesis around the list and use
colon instead.

> + * clk_drop_range).
> + */
>  static struct kunit_suite clk_range_test_suite = {
>         .name = "clk-range-test",
>         .init = clk_test_init,
> @@ -822,6 +842,14 @@ static struct kunit_case clk_range_maximize_test_cases[] = {
>         {}
>  };
>  
> +/*
> + * Test suite for a basic rate clock, without any parent.
> + *
> + * These tests are supposed to exercise the rate range API

s/are supposed to//

> + * (clk_set_rate_range, clk_set_min_rate, clk_set_max_rate,

s/(//

> + * clk_drop_range), with a driver that will always try to run at the

s/)//

> + * highest possible rate.
> + */
>  static struct kunit_suite clk_range_maximize_test_suite = {
>         .name = "clk-range-maximize-test",
>         .init = clk_maximize_test_init,
> @@ -991,6 +1019,14 @@ static struct kunit_case clk_range_minimize_test_cases[] = {
>         {}
>  };
>  
> +/*
> + * Test suite for a basic rate clock, without any parent.
> + *
> + * These tests are supposed to exercise the rate range API

s/are supposed to//

> + * (clk_set_rate_range, clk_set_min_rate, clk_set_max_rate,
> + * clk_drop_range), with a driver that will always try to run at the
> + * lowest possible rate.
> + */
>  static struct kunit_suite clk_range_minimize_test_suite = {
>         .name = "clk-range-minimize-test",
>         .init = clk_minimize_test_init,