linux-kernel - Re: [PATCH v2 2/2] clk: tests: Add tests for fractional divisor approximation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <871qif21db.fsf@oltmanns.dev>
Date:   Tue, 13 Jun 2023 10:49:20 +0200
From:   Frank Oltmanns <frank@...manns.dev>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     Michael Turquette <mturquette@...libre.com>,
        "A.s. Dong" <aisheng.dong@....com>,
        Abel Vesa <abelvesa@...nel.org>,
        Fabio Estevam <festevam@...il.com>,
        linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, NXP Linux Team <linux-imx@....com>,
        Peng Fan <peng.fan@....com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>
Subject: Re: [PATCH v2 2/2] clk: tests: Add tests for fractional divisor
 approximation

Hi Stephen,

On 2023-06-13 at 10:36:26 +0200, Frank Oltmanns <frank@...manns.dev> wrote:
> In light of the recent discovery that the fractional divisor
> approximation does not utilize the full available range for clocks that
> are flagged CLK_FRAC_DIVIDER_ZERO_BASED, implement tests for the edge
> cases of this clock type.
>
> Signed-off-by: Frank Oltmanns <frank@...manns.dev>
> Link: https://lore.kernel.org/lkml/20230529133433.56215-1-frank@oltmanns.dev
> ---
>  drivers/clk/clk_test.c | 69 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk_test.c b/drivers/clk/clk_test.c
> index f9a5c2964c65..b247ba841cbd 100644
> --- a/drivers/clk/clk_test.c
> +++ b/drivers/clk/clk_test.c
> @@ -8,6 +8,9 @@
>  /* Needed for clk_hw_get_clk() */
>  #include "clk.h"
>
> +/* Needed for clk_fractional_divider_general_approximation */
> +#include "clk-fractional-divider.h"
> +
>  #include <kunit/test.h>
>
>  #define DUMMY_CLOCK_INIT_RATE	(42 * 1000 * 1000)
> @@ -2394,6 +2397,69 @@ static struct kunit_suite clk_mux_notifier_test_suite = {
>  	.test_cases = clk_mux_notifier_test_cases,
>  };
>
> +
> +/*
> + * Test that clk_fractional_divider_general_approximation will work with the
> + * highest available numerator and denominator.
> + */
> +static void clk_fd_test_round_rate_max_mn(struct kunit *test)
> +{
> +	struct clk_fractional_divider *fd;
> +	struct clk_hw *hw;
> +	unsigned long rate, parent_rate, m, n;
> +
> +	fd = kunit_kzalloc(test, sizeof(*fd), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, fd);
> +
> +	fd->mwidth = 3;
> +	fd->nwidth = 3;
> +
> +	hw = &fd->hw;
> +
> +	rate = DUMMY_CLOCK_RATE_1;
> +
> +	// Highest denominator, no flags
> +	parent_rate = 10 * DUMMY_CLOCK_RATE_1;
> +	clk_fractional_divider_general_approximation(hw, rate, &parent_rate, &m, &n);
> +	KUNIT_EXPECT_EQ(test, m, 1);
> +	KUNIT_EXPECT_EQ(test, n, 7);
> +
> +	// Highest numerator, no flags
> +	parent_rate = DUMMY_CLOCK_RATE_1 / 10;
> +	clk_fractional_divider_general_approximation(hw, rate, &parent_rate, &m, &n);
> +	KUNIT_EXPECT_EQ(test, m, 7);
> +	KUNIT_EXPECT_EQ(test, n, 1);

The two calls above aim at proving that the change does not break
existing functionality.

> +
> +	// Highest denominator, zero based
> +	parent_rate = 10 * DUMMY_CLOCK_RATE_1;
> +	fd->flags = CLK_FRAC_DIVIDER_ZERO_BASED;
> +	clk_fractional_divider_general_approximation(hw, rate, &parent_rate, &m, &n);
> +	KUNIT_EXPECT_EQ(test, m, 1);
> +	KUNIT_EXPECT_EQ(test, n, 8);
> +
> +	// Highest numerator, zero based
> +	parent_rate = DUMMY_CLOCK_RATE_1 / 10;
> +	clk_fractional_divider_general_approximation(hw, rate, &parent_rate, &m, &n);
> +	KUNIT_EXPECT_EQ(test, m, 8);
> +	KUNIT_EXPECT_EQ(test, n, 1);
> +}
> +
> +static struct kunit_case clk_fd_test_cases[] = {
> +	KUNIT_CASE(clk_fd_test_round_rate_max_mn),
> +	{}
> +};
> +
> +/*
> + * Test suite for a fractional divider clock.
> + *
> + * These tests exercise the fractional divider API: clk_recalc_rate,
> + * clk_set_rate(), clk_round_rate().
> + */
> +static struct kunit_suite clk_fd_test_suite = {
> +	.name = "clk-fd-test",
> +	.test_cases = clk_fd_test_cases,
> +};

Unfortunately, the style of the tests does not really match with the
style of the existing tests, because those where all aimed at the
framework itself and not at specific functions.

Please let me know, if you require any changes.

Thanks,
  Frank

> +
>  kunit_test_suites(
>  	&clk_leaf_mux_set_rate_parent_test_suite,
>  	&clk_test_suite,
> @@ -2406,6 +2472,7 @@ kunit_test_suites(
>  	&clk_range_maximize_test_suite,
>  	&clk_range_minimize_test_suite,
>  	&clk_single_parent_mux_test_suite,
> -	&clk_uncached_test_suite
> +	&clk_uncached_test_suite,
> +	&clk_fd_test_suite
>  );
>  MODULE_LICENSE("GPL v2");