[PATCH 0/2] Fix clk-divider rates above 4.29 GHz on 64-bit

[PATCH 0/2] Fix clk-divider rates above 4.29 GHz on 64-bit

[Carlo Caione]

On 64-bit platforms, the clk-divider truncates requested rates above
4.29 GHz when computing a divisor. Fix it and add KUnit tests.

Sebastian Reichel fixed this in 2023 [1], but later versions moved to a
new generic macro and were never applied. Patch 1 resends his v1 with a
new commit message. It uses the existing DIV64_U64_ROUND_UP(), as
Stephen asked for then [2]. On 32-bit, the rate fits in 32 bits, so
div64_u64() does the same 64/32 division as today [3].

[1] https://lore.kernel.org/linux-clk/20230519190522.194729-3-sebastian.reichel@collabora.com/
[2] https://lore.kernel.org/linux-clk/5e9e776db6628d02d7081292b81ab102.sboyd@kernel.org/
[3] https://lore.kernel.org/linux-clk/1b74d2ea2c3a458694c4c74f2381fcab@AcuMS.aculab.com/

---
Carlo Caione (1):
      clk: divider: test: Add tests for rates above 4.29 GHz

Sebastian Reichel (1):
      clk: divider: Fix the divisor for rates above 4.29 GHz

 drivers/clk/clk-divider.c      |   6 +--
 drivers/clk/clk-divider_test.c | 117 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 118 insertions(+), 5 deletions(-)
---
base-commit: ab94dbf302331eed3679e262e1d8d1a73f308640
change-id: 20261102-upstream-clk-divider-large-rates-872467864e39

Best regards,
--  
Carlo Caione <ccaione@baylibre.com>

[PATCH 1/2] clk: divider: Fix the divisor for rates above 4.29 GHz

[Carlo Caione]

From: Sebastian Reichel <sebastian.reichel@collabora.com>

DIV_ROUND_UP_ULL() takes a 32-bit divisor, so on 64-bit platforms a
requested rate above 4.29 GHz is truncated. A divider fed by a 5.94 GHz
PLL then computes a divisor of 7, not 1, for 5.94 GHz.

Use DIV64_U64_ROUND_UP(), which takes a 64-bit divisor.

Fixes: 9556f9dad8f5 ("clk: divider: handle integer overflow when dividing large clock rates")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 drivers/clk/clk-divider.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index f68216c65d3d..a446abdedcd7 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -227,7 +227,7 @@ static int _div_round_up(const struct clk_div_table *table,
 			 unsigned long parent_rate, unsigned long rate,
 			 unsigned long flags)
 {
-	int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
+	int div = DIV64_U64_ROUND_UP(parent_rate, rate);
 
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		div = __roundup_pow_of_two(div);
@@ -244,7 +244,7 @@ static int _div_round_closest(const struct clk_div_table *table,
 	int up, down;
 	unsigned long up_rate, down_rate;
 
-	up = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
+	up = DIV64_U64_ROUND_UP(parent_rate, rate);
 	down = parent_rate / rate;
 
 	if (flags & CLK_DIVIDER_POWER_OF_TWO) {
@@ -423,7 +423,7 @@ int divider_get_val(unsigned long rate, unsigned long parent_rate,
 {
 	unsigned int div, value;
 
-	div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
+	div = DIV64_U64_ROUND_UP(parent_rate, rate);
 
 	if (!_is_valid_div(table, div, flags))
 		return -EINVAL;

-- 
2.56.0

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

[Carlo Caione]

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@baylibre.com>
---
 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,
+					 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),
+	{}
+};
+
+static struct kunit_suite clk_divider_large_rate_test_suite = {
+	.name = "clk_divider_large_rate",
+	.test_cases = clk_divider_large_rate_test_cases,
+};
+
+kunit_test_suites(&clk_divider_bestdiv_test_suite,
+		  &clk_divider_large_rate_test_suite);
 
 MODULE_DESCRIPTION("KUnit tests for clk divider");
 MODULE_LICENSE("GPL");

-- 
2.56.0

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@baylibre.com>
> ---
>  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@kernel.org>

Re: [PATCH 1/2] clk: divider: Fix the divisor for rates above 4.29 GHz

[Brian Masney]

> DIV_ROUND_UP_ULL() takes a 32-bit divisor, so on 64-bit platforms a
> requested rate above 4.29 GHz is truncated. A divider fed by a 5.94 GHz
> PLL then computes a divisor of 7, not 1, for 5.94 GHz.
> 
> Use DIV64_U64_ROUND_UP(), which takes a 64-bit divisor.
> 
> Fixes: 9556f9dad8f5 ("clk: divider: handle integer overflow when dividing large clock rates")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Reviewed-by: Brian Masney <masneyb@kernel.org>

Since this is a core change to the framework, I'd also like to see what
Jerome thinks before we merge this.

Brian

-- 
Brian Masney <masneyb@kernel.org>

Re: [PATCH 1/2] clk: divider: Fix the divisor for rates above 4.29 GHz

[David Laight]

On Wed, 04 Nov 2026 16:52:04 -0500
Brian Masney <masneyb@kernel.org> wrote:

> > DIV_ROUND_UP_ULL() takes a 32-bit divisor, so on 64-bit platforms a
> > requested rate above 4.29 GHz is truncated. A divider fed by a 5.94 GHz
> > PLL then computes a divisor of 7, not 1, for 5.94 GHz.
> > 
> > Use DIV64_U64_ROUND_UP(), which takes a 64-bit divisor.
> > 
> > Fixes: 9556f9dad8f5 ("clk: divider: handle integer overflow when dividing large clock rates")
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > Signed-off-by: Carlo Caione <ccaione@baylibre.com>  
> 
> Reviewed-by: Brian Masney <masneyb@kernel.org>
> 
> Since this is a core change to the framework, I'd also like to see what
> Jerome thinks before we merge this.

I've wondered about whether using 'unsigned long' for clock frequencies
was ever right.
Clearly the type should be the same size for 32 bit and 64 bit, and I'm
pretty sure the choice of long predates 64bit (I guess someone was worried
'int' might only be 16 bits?).

But if frequencies can be over 4GHz (which no one would have imagined a few
years back) they can be that high on 32 bit systems as well as 64bit ones.
So there is a big 'bag of worms' being opened.

Changing the type to 64 bits is hard to check (unless you manage to build
a kernel with the value embedded in a structure using accessor functions)
and will be a lot of bloat for most uses on 32 bit.
Alternatively you enforce a different api for clocks that might exceed 4GHz.

David

> 
> Brian
>