Re: [PATCH 2/2] clk: divider: test: Add tests for rates above 4.29 GHz

[Brian Masney]

Hi Carlo,

> On 64-bit platforms, the divider truncated rates above 4.29 GHz when
> computing a divisor.
> 
> Add tests rounding and setting such rates on a divider with a 6 GHz
> parent, one for each divisor computation. They are skipped on 32-bit
> platforms, where rates cannot exceed 4.29 GHz.
> 
> Signed-off-by: Carlo Caione <ccaione@xxxxxxxxxxxx>
> ---
>  drivers/clk/clk-divider_test.c | 117 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 115 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-divider_test.c b/drivers/clk/clk-divider_test.c
> index 8ee1614128d0..fc96ac07ecff 100644
> --- a/drivers/clk/clk-divider_test.c
> +++ b/drivers/clk/clk-divider_test.c
> @@ -1,16 +1,19 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * KUnit tests for clk_divider_bestdiv()
> + * KUnit tests for clk divider
>   */
> +#include <kunit/clk.h>
>  #include <kunit/test.h>
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
> +#include <linux/io.h>
>  #include <linux/limits.h>
>  #include <linux/units.h>
>  
>  #define PARENT_RATE_1GHZ	GIGA
>  #define PARENT_RATE_2GHZ	(2 * GIGA)
>  #define PARENT_RATE_4GHZ	(4 * GIGA)
> +#define PARENT_RATE_6GHZ	(6 * GIGA)
>  
>  KUNIT_DEFINE_ACTION_WRAPPER(clk_hw_unregister_fixed_rate_wrapper,
>  			    clk_hw_unregister_fixed_rate, struct clk_hw *);
> @@ -147,7 +150,117 @@ static struct kunit_suite clk_divider_bestdiv_test_suite = {
>  	.test_cases = clk_divider_bestdiv_test_cases,
>  };
>  
> -kunit_test_suite(clk_divider_bestdiv_test_suite);
> +/*
> + * Register a 6 GHz fixed-rate parent and a 3-bit divider (/1 to /8) using
> + * @fake_reg as its register.
> + */
> +static struct clk_hw *clk_divider_large_rate_init(struct kunit *test,
> +						  u32 *fake_reg,
> +						  unsigned long flags,
> +						  u8 div_flags)
> +{
> +	struct clk_hw *parent_hw, *div_hw;
> +
> +	if (BITS_PER_LONG < 64)
> +		kunit_skip(test, "rates above 4.29 GHz need a 64-bit unsigned long");
> +
> +	parent_hw = clk_hw_register_fixed_rate(NULL, "large-rate-parent",
> +					       NULL, 0, PARENT_RATE_6GHZ);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw);
> +	KUNIT_ASSERT_EQ(test, 0,
> +			kunit_add_action_or_reset(test, clk_hw_unregister_fixed_rate_wrapper,
> +						  parent_hw));
> +
> +	div_hw = clk_hw_register_divider(NULL, "large-rate-div",
> +					 "large-rate-parent", flags,
> +					 (void __iomem __force *)fake_reg,

The fake_reg causes issues on s390, and I see that CLK_DIVIDER_KUNIT_TEST
already excludes this test on that architecture, so we're good on that
front.

> +					 0, 3, div_flags, NULL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
> +	KUNIT_ASSERT_EQ(test, 0,
> +			kunit_add_action_or_reset(test, clk_hw_unregister_divider_wrapper,
> +						  div_hw));
> +
> +	return div_hw;
> +}
> +
> +/*
> + * Test that rounding a rate above 4.29 GHz computes the divisor from the
> + * full rate: the parent rate needs /1.
> + */
> +static void clk_divider_large_rate_round_up(struct kunit *test)
> +{
> +	struct clk_hw *div_hw;
> +	u32 *fake_reg;
> +
> +	fake_reg = kunit_kzalloc(test, sizeof(*fake_reg), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fake_reg);
> +
> +	div_hw = clk_divider_large_rate_init(test, fake_reg, 0, 0);
> +
> +	KUNIT_EXPECT_EQ(test, clk_hw_round_rate(div_hw, PARENT_RATE_6GHZ),
> +			PARENT_RATE_6GHZ);
> +}
> +
> +/*
> + * Same with CLK_DIVIDER_ROUND_CLOSEST: 4.4 GHz is closer to 3 GHz (/2) than
> + * to 6 GHz (/1).
> + */
> +static void clk_divider_large_rate_round_closest(struct kunit *test)
> +{
> +	struct clk_hw *div_hw;
> +	u32 *fake_reg;
> +
> +	fake_reg = kunit_kzalloc(test, sizeof(*fake_reg), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fake_reg);
> +
> +	div_hw = clk_divider_large_rate_init(test, fake_reg, 0,
> +					     CLK_DIVIDER_ROUND_CLOSEST);
> +
> +	KUNIT_EXPECT_EQ(test, clk_hw_round_rate(div_hw, 4400 * MEGA),
> +			PARENT_RATE_6GHZ / 2);
> +}
> +
> +/*
> + * Test that setting a rate above 4.29 GHz programs the right divisor.
> + * With CLK_SET_RATE_PARENT the rounding searches the parent rates and
> + * finds /1, so this only checks the set_rate path.
> + */
> +static void clk_divider_large_rate_set_rate(struct kunit *test)
> +{
> +	struct clk_hw *div_hw;
> +	struct clk *clk;
> +	u32 *fake_reg;
> +
> +	fake_reg = kunit_kzalloc(test, sizeof(*fake_reg), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fake_reg);
> +
> +	/* Start from /2 */
> +	writel(1, (void __iomem __force *)fake_reg);
> +
> +	div_hw = clk_divider_large_rate_init(test, fake_reg,
> +					     CLK_SET_RATE_PARENT, 0);
> +	clk = clk_hw_get_clk_kunit(test, div_hw, NULL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, clk);
> +
> +	KUNIT_ASSERT_EQ(test, clk_set_rate(clk, PARENT_RATE_6GHZ), 0);
> +	KUNIT_EXPECT_EQ(test, clk_get_rate(clk), PARENT_RATE_6GHZ);
> +	KUNIT_EXPECT_EQ(test, *fake_reg, 0);
> +}
> +
> +static struct kunit_case clk_divider_large_rate_test_cases[] = {
> +	KUNIT_CASE(clk_divider_large_rate_round_up),
> +	KUNIT_CASE(clk_divider_large_rate_round_closest),
> +	KUNIT_CASE(clk_divider_large_rate_set_rate),

Would it be useful to add a test case for round max? A consumer may
want to request the maximum rate possible with:

    clk_round_rate(clk, ULONG_MAX);

Brian

-- 
Brian Masney <masneyb@xxxxxxxxxx>