[PATCH v2 0/2] Fix clk_divider_bestdiv() to get max clk rate supported and add some kunit test suites

[PATCH v2 0/2] Fix clk_divider_bestdiv() to get max clk rate supported and add some kunit test suites

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi all,

This patch series includes two patches. The first patch fixes the
clk_divider_bestdiv() function in clk-divider driver to return the
maximum clock rate supported by the divider when the requested rate
is larger than the parent clock rate. The second patch adds some
kunit test suites for clk-divider driver to verify the fix.

kunit test case logs:
case #1 without the fix:
------------------------
[   44.288459] KTAP version 1
[   44.291655] 1..1
[   44.293844]     KTAP version 1
[   44.297411]     # Subtest: clk_divider_bestdiv
[   44.302575]     # module: clk_divider_test
[   44.302583]     1..2
[   44.310280]     # clk_divider_bestdiv_ulong_max_returns_max_rate: EXPECTATION FAILED at drivers/clk/clk-divider_test.c:71
[   44.310280]     Expected rate == 1000000000UL / 2, but
[   44.310280]         rate == 125000000 (0x7735940)
[   44.310280]         1000000000UL / 2 == 500000000 (0x1dcd6500)
[   44.310705]     not ok 1 clk_divider_bestdiv_ulong_max_returns_max_rate
[   44.341802]     # clk_divider_bestdiv_mux_ulong_max_returns_max_rate: EXPECTATION FAILED at drivers/clk/clk-divider_test.c:134
[   44.341802]     Expected rate == (4 * 1000000000UL) / 2, but
[   44.341802]         rate == 0 (0x0)
[   44.341802]         (4 * 1000000000UL) / 2 == 2000000000 (0x77359400)
[   44.349940]     not ok 2 clk_divider_bestdiv_mux_ulong_max_returns_max_rate
[   44.381047] # clk_divider_bestdiv: pass:0 fail:2 skip:0 total:2
[   44.388922] # Totals: pass:0 fail:2 skip:0 total:2
[   44.395783] not ok 1 clk_divider_bestdiv

case #2 with the fix:
---------------------
[   22.077769] KTAP version 1
[   22.080931] 1..1
[   22.083168]     KTAP version 1
[   22.086739]     # Subtest: clk_divider_bestdiv
[   22.091826]     # module: clk_divider_test
[   22.091835]     1..2
[   22.099869]     ok 1 clk_divider_bestdiv_ulong_max_returns_max_rate
[   22.100612]     ok 2 clk_divider_bestdiv_mux_ulong_max_returns_max_rate
[   22.107902] # clk_divider_bestdiv: pass:2 fail:0 skip:0 total:2
[   22.115589] # Totals: pass:2 fail:0 skip:0 total:2
[   22.122473] ok 1 clk_divider_bestdiv

v1->v2:
- Updated the patch#1 fixing review comments from Sashiko
- Added a new patch#2 to add some kunit test suites for clk-divider driver

Cheers,
Prabhakar

Lad Prabhakar (2):
  clk: divider: Fix clk_divider_bestdiv() returning min rate for large
    rate requests
  clk: divider: Add some kunit test suites

 drivers/clk/Kconfig            |   7 ++
 drivers/clk/Makefile           |   1 +
 drivers/clk/clk-divider.c      |  25 ++++--
 drivers/clk/clk-divider_test.c | 151 +++++++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+), 8 deletions(-)
 create mode 100644 drivers/clk/clk-divider_test.c

-- 
2.53.0

[PATCH v2 1/2] clk: divider: Fix clk_divider_bestdiv() returning min rate for large rate requests

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk_divider_bestdiv() clamps maxdiv using:

    maxdiv = min(ULONG_MAX / rate, maxdiv);

to avoid overflow in rate * i. However, requests like
clk_round_rate(clk, ULONG_MAX), which are used to determine the maximum
supported rate of a clock, result in maxdiv being clamped to 1. If no
valid divider of 1 exists in the table the loop is never entered and
bestdiv falls back to the maximum divider with the minimum parent rate,
causing clk_round_rate(clk, ULONG_MAX) to incorrectly return the minimum
supported rate instead of the maximum.

Fix this by removing the pre-loop maxdiv clamping and replacing the
unprotected rate * i multiplication with check_mul_overflow(). Guard
the exact-match short-circuit with !overflow to prevent a clamped
target_parent_rate of ULONG_MAX from falsely matching parent_rate_saved
and causing premature loop exit. Break out of the loop after evaluating
the first overflowing divider since clk_hw_round_rate(parent, ULONG_MAX)
returns a constant for all subsequent iterations, meaning no better
candidate can be found, and continuing would cause exponential recursive
calls in chained divider clocks.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/clk-divider.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 45e7ebde4a8b..9a6a2ad6f397 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/log2.h>
+#include <linux/overflow.h>
 
 /*
  * DOC: basic adjustable divider clock that cannot gate
@@ -301,6 +302,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 	int i, bestdiv = 0;
 	unsigned long parent_rate, best = 0, now, maxdiv;
 	unsigned long parent_rate_saved = *best_parent_rate;
+	unsigned long target_parent_rate;
 
 	if (!rate)
 		rate = 1;
@@ -315,15 +317,11 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 		return bestdiv;
 	}
 
-	/*
-	 * The maximum divider we can use without overflowing
-	 * unsigned long in rate * i below
-	 */
-	maxdiv = min(ULONG_MAX / rate, maxdiv);
-
 	for (i = _next_div(table, 0, flags); i <= maxdiv;
 					     i = _next_div(table, i, flags)) {
-		if (rate * i == parent_rate_saved) {
+		bool overflow = check_mul_overflow(rate, (unsigned long)i, &target_parent_rate);
+
+		if (!overflow && target_parent_rate == parent_rate_saved) {
 			/*
 			 * It's the most ideal case if the requested rate can be
 			 * divided from parent clock without needing to change
@@ -332,13 +330,24 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 			*best_parent_rate = parent_rate_saved;
 			return i;
 		}
-		parent_rate = clk_hw_round_rate(parent, rate * i);
+		/*
+		 * Clamp target_parent_rate to ULONG_MAX on overflow. The true
+		 * required parent rate exceeds what can be represented, so ask
+		 * the parent for the highest rate it can produce. There is no
+		 * point continuing the loop past this since larger dividers
+		 * only move further from the requested rate.
+		 */
+		if (overflow)
+			target_parent_rate = ULONG_MAX;
+		parent_rate = clk_hw_round_rate(parent, target_parent_rate);
 		now = DIV_ROUND_UP_ULL((u64)parent_rate, i);
 		if (_is_best_div(rate, now, best, flags)) {
 			bestdiv = i;
 			best = now;
 			*best_parent_rate = parent_rate;
 		}
+		if (overflow)
+			break;
 	}
 
 	if (!bestdiv) {
-- 
2.53.0

Re: [PATCH v2 1/2] clk: divider: Fix clk_divider_bestdiv() returning min rate for large rate requests

[Brian Masney]

On Mon, Apr 13, 2026 at 01:49:11PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> clk_divider_bestdiv() clamps maxdiv using:
> 
>     maxdiv = min(ULONG_MAX / rate, maxdiv);
> 
> to avoid overflow in rate * i. However, requests like
> clk_round_rate(clk, ULONG_MAX), which are used to determine the maximum
> supported rate of a clock, result in maxdiv being clamped to 1. If no
> valid divider of 1 exists in the table the loop is never entered and
> bestdiv falls back to the maximum divider with the minimum parent rate,
> causing clk_round_rate(clk, ULONG_MAX) to incorrectly return the minimum
> supported rate instead of the maximum.
> 
> Fix this by removing the pre-loop maxdiv clamping and replacing the
> unprotected rate * i multiplication with check_mul_overflow(). Guard
> the exact-match short-circuit with !overflow to prevent a clamped
> target_parent_rate of ULONG_MAX from falsely matching parent_rate_saved
> and causing premature loop exit. Break out of the loop after evaluating
> the first overflowing divider since clk_hw_round_rate(parent, ULONG_MAX)
> returns a constant for all subsequent iterations, meaning no better
> candidate can be found, and continuing would cause exponential recursive
> calls in chained divider clocks.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Brian Masney <bmasney@redhat.com>

[PATCH v2 2/2] clk: divider: Add some kunit test suites

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add KUnit tests to verify clk_divider_bestdiv() returns the maximum
achievable rate when clk_round_rate() is called with ULONG_MAX, which
is the canonical way to probe the maximum rate a clock can produce.

The first test uses a fixed-rate parent driving a table-based divider
with no div=1 entry. The second test places a two-input mux between
the divider and its root clocks to verify correct parent selection and
that the divider loop does not make redundant calls to
clk_hw_round_rate() for each remaining table entry after the first
overflow.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/Kconfig            |   7 ++
 drivers/clk/Makefile           |   1 +
 drivers/clk/clk-divider_test.c | 151 +++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/clk/clk-divider_test.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index cc8743b11bb1..c8f9eaef6f6b 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -573,4 +573,11 @@ config CLK_FD_KUNIT_TEST
 	help
 	  Kunit test for the clk-fractional-divider type.
 
+config CLK_DIVIDER_KUNIT_TEST
+	tristate "KUnit tests for clk divider bestdiv" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Kunit test for the clk-divider type.
+
 endif
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index a3e2862ebd7e..0c915c6cf3fa 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -20,6 +20,7 @@ clk-test-y			:= clk_test.o \
 				   kunit_clk_assigned_rates_zero_consumer.dtbo.o \
 				   kunit_clk_hw_get_dev_of_node.dtbo.o \
 				   kunit_clk_parent_data_test.dtbo.o
+obj-$(CONFIG_CLK_DIVIDER_KUNIT_TEST) += clk-divider_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-divider_test.c b/drivers/clk/clk-divider_test.c
new file mode 100644
index 000000000000..3a5e3adccb2e
--- /dev/null
+++ b/drivers/clk/clk-divider_test.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for clk_divider_bestdiv()
+ */
+#include <kunit/test.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.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)
+
+static u32 fake_reg_a, fake_reg_b;
+
+static const struct clk_div_table no_div1_table[] = {
+	{0, 2},
+	{1, 4},
+	{2, 8},
+	{0, 0},
+};
+
+static void unregister_fixed_rate(void *hw)
+{
+	clk_hw_unregister_fixed_rate(hw);
+}
+
+static void unregister_divider(void *hw)
+{
+	clk_hw_unregister_divider(hw);
+}
+
+static void unregister_mux(void *hw)
+{
+	clk_hw_unregister_mux(hw);
+}
+
+/*
+ * Test that clk_round_rate(clk, ULONG_MAX) returns the maximum achievable
+ * rate for a divider clock.
+ */
+static void clk_divider_bestdiv_ulong_max_returns_max_rate(struct kunit *test)
+{
+	struct clk_hw *parent_hw, *div_hw;
+	unsigned long rate;
+
+	parent_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-parent",
+					       NULL, 0, PARENT_RATE_1GHZ);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw);
+	kunit_add_action(test, unregister_fixed_rate, parent_hw);
+
+	fake_reg_a = 0;
+	div_hw = clk_hw_register_divider_table(NULL, "bestdiv-div",
+					       "bestdiv-parent",
+					       CLK_SET_RATE_PARENT,
+					       (void __iomem *)&fake_reg_a,
+					       0, 2, 0, no_div1_table, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
+	kunit_add_action(test, unregister_divider, div_hw);
+
+	/*
+	 * ULONG_MAX is the canonical way to probe the maximum rate a clock
+	 * can produce. With a parent at 1 GHz and the smallest table divider
+	 * being 2, the expected maximum is 500 MHz.
+	 *
+	 * Before the fix this returned 125 MHz (PARENT_RATE / 8), the
+	 * minimum rate, because the search loop was bypassed entirely.
+	 */
+	rate = clk_hw_round_rate(div_hw, ULONG_MAX);
+	KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_1GHZ / 2);
+}
+
+/*
+ * Test that clk_round_rate(clk, ULONG_MAX) returns the correct maximum rate when
+ * a mux clock sits between a divider and its parent candidates.
+ *
+ * Topology:
+ *
+ *   [fixed 4 GHz] --\
+ *                    +--> [mux CLK_SET_RATE_PARENT] --> [div {2,4,8} CLK_SET_RATE_PARENT]
+ *   [fixed 2 GHz] --/
+ *
+ */
+static void clk_divider_bestdiv_mux_ulong_max_returns_max_rate(struct kunit *test)
+{
+	static const char *const mux_parents[] = {
+		"bestdiv-mux-parent-a",
+		"bestdiv-mux-parent-b",
+	};
+	struct clk_hw *parent_a_hw, *parent_b_hw, *mux_hw, *div_hw;
+	unsigned long rate;
+
+	/* Higher-rate parent: the mux should select this for ULONG_MAX. */
+	parent_a_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-a",
+						 NULL, 0, PARENT_RATE_4GHZ);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_a_hw);
+	kunit_add_action(test, unregister_fixed_rate, parent_a_hw);
+
+	/* Lower-rate parent: should not be selected. */
+	parent_b_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-b",
+						 NULL, 0, PARENT_RATE_2GHZ);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_b_hw);
+	kunit_add_action(test, unregister_fixed_rate, parent_b_hw);
+
+	/*
+	 * 1-bit mux register selects between the two parents.
+	 * CLK_SET_RATE_PARENT allows the divider's rate request to
+	 * propagate into clk_mux_determine_rate().
+	 */
+	fake_reg_a = 0;
+	mux_hw = clk_hw_register_mux(NULL, "bestdiv-mux",
+				     mux_parents, ARRAY_SIZE(mux_parents),
+				     CLK_SET_RATE_PARENT,
+				     (void __iomem *)&fake_reg_a,
+				     0, 1, 0, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mux_hw);
+	kunit_add_action(test, unregister_mux, mux_hw);
+
+	fake_reg_b = 0;
+	div_hw = clk_hw_register_divider_table(NULL, "bestdiv-mux-div",
+					       "bestdiv-mux",
+					       CLK_SET_RATE_PARENT,
+					       (void __iomem *)&fake_reg_b,
+					       0, 2, 0, no_div1_table, NULL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
+	kunit_add_action(test, unregister_divider, div_hw);
+
+	/*
+	 * Expected maximum: mux selects the 4 GHz parent, divider applies
+	 * the smallest table entry (2): 4 GHz / 2 = 2 GHz.
+	 */
+	rate = clk_hw_round_rate(div_hw, ULONG_MAX);
+	KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_4GHZ / 2);
+}
+
+static struct kunit_case clk_divider_bestdiv_test_cases[] = {
+	KUNIT_CASE(clk_divider_bestdiv_ulong_max_returns_max_rate),
+	KUNIT_CASE(clk_divider_bestdiv_mux_ulong_max_returns_max_rate),
+	{}
+};
+
+static struct kunit_suite clk_divider_bestdiv_test_suite = {
+	.name = "clk_divider_bestdiv",
+	.test_cases = clk_divider_bestdiv_test_cases,
+};
+
+kunit_test_suite(clk_divider_bestdiv_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for clk_divider_bestdiv()");
+MODULE_LICENSE("GPL");
-- 
2.53.0

Re: [PATCH v2 2/2] clk: divider: Add some kunit test suites

[Brian Masney]

Hi Lad,

On Mon, Apr 13, 2026 at 01:49:12PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add KUnit tests to verify clk_divider_bestdiv() returns the maximum
> achievable rate when clk_round_rate() is called with ULONG_MAX, which
> is the canonical way to probe the maximum rate a clock can produce.
> 
> The first test uses a fixed-rate parent driving a table-based divider
> with no div=1 entry. The second test places a two-input mux between
> the divider and its root clocks to verify correct parent selection and
> that the divider loop does not make redundant calls to
> clk_hw_round_rate() for each remaining table entry after the first
> overflow.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/clk/Kconfig            |   7 ++
>  drivers/clk/Makefile           |   1 +
>  drivers/clk/clk-divider_test.c | 151 +++++++++++++++++++++++++++++++++
>  3 files changed, 159 insertions(+)
>  create mode 100644 drivers/clk/clk-divider_test.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index cc8743b11bb1..c8f9eaef6f6b 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -573,4 +573,11 @@ config CLK_FD_KUNIT_TEST
>  	help
>  	  Kunit test for the clk-fractional-divider type.
>  
> +config CLK_DIVIDER_KUNIT_TEST
> +	tristate "KUnit tests for clk divider bestdiv" if !KUNIT_ALL_TESTS
> +	depends on KUNIT

Since the clk divider calls writel(), you also will need to
unfortunately add:

    depends on !S390

This is already on CLK_GATE_KUNIT_TEST. For the reason why, look at
commit a6c3da03ead11 ("clk: disable clk gate tests for s390")

> +	default KUNIT_ALL_TESTS
> +	help
> +	  Kunit test for the clk-divider type.
> +
>  endif
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index a3e2862ebd7e..0c915c6cf3fa 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -20,6 +20,7 @@ clk-test-y			:= clk_test.o \
>  				   kunit_clk_assigned_rates_zero_consumer.dtbo.o \
>  				   kunit_clk_hw_get_dev_of_node.dtbo.o \
>  				   kunit_clk_parent_data_test.dtbo.o
> +obj-$(CONFIG_CLK_DIVIDER_KUNIT_TEST) += clk-divider_test.o
>  obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o

Swap the order of these two lines above for consistency with the
clk-fixed-rate and clk-gate tests where the actual implementation is
first, and then the test.

>  obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
>  obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
> diff --git a/drivers/clk/clk-divider_test.c b/drivers/clk/clk-divider_test.c
> new file mode 100644
> index 000000000000..3a5e3adccb2e
> --- /dev/null
> +++ b/drivers/clk/clk-divider_test.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for clk_divider_bestdiv()
> + */
> +#include <kunit/test.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.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)
> +
> +static u32 fake_reg_a, fake_reg_b;

Right now this limits this to one implementation. Put these in a
structure and use kunit_kzalloc() so that there can be multiple, and the
runner can execute them in parallel.

> +
> +static const struct clk_div_table no_div1_table[] = {
> +	{0, 2},
> +	{1, 4},
> +	{2, 8},
> +	{0, 0},
> +};

You can pass NULL for the table to simplify this code further. I don't
see where you are testing anything special related to the table. I think
you'll need to pass CLK_DIVIDER_ONE_BASED to the flags when you create
the divider if you use a NULL table.

> +
> +static void unregister_fixed_rate(void *hw)
> +{
> +	clk_hw_unregister_fixed_rate(hw);
> +}
> +
> +static void unregister_divider(void *hw)
> +{
> +	clk_hw_unregister_divider(hw);
> +}
> +
> +static void unregister_mux(void *hw)
> +{
> +	clk_hw_unregister_mux(hw);
> +}
> +
> +/*
> + * Test that clk_round_rate(clk, ULONG_MAX) returns the maximum achievable
> + * rate for a divider clock.
> + */
> +static void clk_divider_bestdiv_ulong_max_returns_max_rate(struct kunit *test)
> +{
> +	struct clk_hw *parent_hw, *div_hw;
> +	unsigned long rate;
> +
> +	parent_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-parent",
> +					       NULL, 0, PARENT_RATE_1GHZ);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw);
> +	kunit_add_action(test, unregister_fixed_rate, parent_hw);

You can put clk_hw_unregister_fixed_rate() in the call here, and then
drop unregister_fixed_rate(). There's some cases of this below as well.

Check the return value of kunit_add_action() here and below as well.

> +
> +	fake_reg_a = 0;
> +	div_hw = clk_hw_register_divider_table(NULL, "bestdiv-div",
> +					       "bestdiv-parent",
> +					       CLK_SET_RATE_PARENT,
> +					       (void __iomem *)&fake_reg_a,

You'll need __force for the cast for sparse as well.

> +					       0, 2, 0, no_div1_table, NULL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
> +	kunit_add_action(test, unregister_divider, div_hw);

Same here... you can just put clk_hw_unregister_divider() here and drop
the function above.

> +
> +	/*
> +	 * ULONG_MAX is the canonical way to probe the maximum rate a clock
> +	 * can produce. With a parent at 1 GHz and the smallest table divider
> +	 * being 2, the expected maximum is 500 MHz.
> +	 *
> +	 * Before the fix this returned 125 MHz (PARENT_RATE / 8), the
> +	 * minimum rate, because the search loop was bypassed entirely.

The "Before the fix" comment should go in the commit log. The comment in
the code should describe how the code is right now.

> +	 */
> +	rate = clk_hw_round_rate(div_hw, ULONG_MAX);
> +	KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_1GHZ / 2);
> +}
> +
> +/*
> + * Test that clk_round_rate(clk, ULONG_MAX) returns the correct maximum rate when
> + * a mux clock sits between a divider and its parent candidates.
> + *
> + * Topology:
> + *
> + *   [fixed 4 GHz] --\
> + *                    +--> [mux CLK_SET_RATE_PARENT] --> [div {2,4,8} CLK_SET_RATE_PARENT]
> + *   [fixed 2 GHz] --/
> + *
> + */
> +static void clk_divider_bestdiv_mux_ulong_max_returns_max_rate(struct kunit *test)
> +{
> +	static const char *const mux_parents[] = {
> +		"bestdiv-mux-parent-a",
> +		"bestdiv-mux-parent-b",
> +	};
> +	struct clk_hw *parent_a_hw, *parent_b_hw, *mux_hw, *div_hw;
> +	unsigned long rate;
> +
> +	/* Higher-rate parent: the mux should select this for ULONG_MAX. */
> +	parent_a_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-a",
> +						 NULL, 0, PARENT_RATE_4GHZ);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_a_hw);
> +	kunit_add_action(test, unregister_fixed_rate, parent_a_hw);
> +
> +	/* Lower-rate parent: should not be selected. */
> +	parent_b_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-b",
> +						 NULL, 0, PARENT_RATE_2GHZ);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_b_hw);
> +	kunit_add_action(test, unregister_fixed_rate, parent_b_hw);
> +
> +	/*
> +	 * 1-bit mux register selects between the two parents.
> +	 * CLK_SET_RATE_PARENT allows the divider's rate request to
> +	 * propagate into clk_mux_determine_rate().
> +	 */
> +	fake_reg_a = 0;
> +	mux_hw = clk_hw_register_mux(NULL, "bestdiv-mux",
> +				     mux_parents, ARRAY_SIZE(mux_parents),
> +				     CLK_SET_RATE_PARENT,
> +				     (void __iomem *)&fake_reg_a,
> +				     0, 1, 0, NULL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mux_hw);
> +	kunit_add_action(test, unregister_mux, mux_hw);

You can put clk_hw_unregister_mux() here and drop this function above.

> +
> +	fake_reg_b = 0;
> +	div_hw = clk_hw_register_divider_table(NULL, "bestdiv-mux-div",
> +					       "bestdiv-mux",
> +					       CLK_SET_RATE_PARENT,
> +					       (void __iomem *)&fake_reg_b,
> +					       0, 2, 0, no_div1_table, NULL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
> +	kunit_add_action(test, unregister_divider, div_hw);
> +
> +	/*
> +	 * Expected maximum: mux selects the 4 GHz parent, divider applies
> +	 * the smallest table entry (2): 4 GHz / 2 = 2 GHz.
> +	 */
> +	rate = clk_hw_round_rate(div_hw, ULONG_MAX);
> +	KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_4GHZ / 2);
> +}
> +
> +static struct kunit_case clk_divider_bestdiv_test_cases[] = {
> +	KUNIT_CASE(clk_divider_bestdiv_ulong_max_returns_max_rate),
> +	KUNIT_CASE(clk_divider_bestdiv_mux_ulong_max_returns_max_rate),
> +	{}
> +};

Usually I'd ask for a few other tests for basic functionality to be
added rather than just testing the maximum. However there's actually
some stuff broken with the existing dividers and the clk core where a
clk can change the rate of it's siblings. I have a series to address
this at:

https://lore.kernel.org/linux-clk/20260327-clk-scaling-v8-0-86cd0aba3c5f@redhat.com/

I think the tests that you have are fine.

Stephen: If you have time after the merge window closes I'd appreciate
it if you could take time to provide feedback about that series.

Puss in Boots please... :)
https://media2.giphy.com/media/v1.Y2lkPTc5MGI3NjExaXg5cGFhbGdmbTZjdnRkdWs4aXM5d3FlYnFmbTNudWFsZ3Fwc3o5NiZlcD12MV9pbnRlcm5hbF9naWZfYnlfaWQmY3Q9Zw/qUIm5wu6LAAog/giphy.gif

Brian

Re: [PATCH v2 2/2] clk: divider: Add some kunit test suites

[Brian Masney]

Hi Lad,

On Thu, Apr 16, 2026 at 5:35 PM Brian Masney <bmasney@redhat.com> wrote:
> On Mon, Apr 13, 2026 at 01:49:12PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > +     /*
> > +      * ULONG_MAX is the canonical way to probe the maximum rate a clock
> > +      * can produce. With a parent at 1 GHz and the smallest table divider
> > +      * being 2, the expected maximum is 500 MHz.
> > +      *
> > +      * Before the fix this returned 125 MHz (PARENT_RATE / 8), the
> > +      * minimum rate, because the search loop was bypassed entirely.
>
> The "Before the fix" comment should go in the commit log. The comment in
> the code should describe how the code is right now.

To demonstrate the existing issue, you could structure your series by:

- Introduce the test as the first patch and have it show the issue and
the current behavior.
- Put the divider fix in, and update the test for the new behavior.

Brian

Re: [PATCH v2 2/2] clk: divider: Add some kunit test suites

[Lad, Prabhakar]

Hi Brian,

On Fri, Apr 17, 2026 at 2:21 PM Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Lad,
>
> On Thu, Apr 16, 2026 at 5:35 PM Brian Masney <bmasney@redhat.com> wrote:
> > On Mon, Apr 13, 2026 at 01:49:12PM +0100, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > +     /*
> > > +      * ULONG_MAX is the canonical way to probe the maximum rate a clock
> > > +      * can produce. With a parent at 1 GHz and the smallest table divider
> > > +      * being 2, the expected maximum is 500 MHz.
> > > +      *
> > > +      * Before the fix this returned 125 MHz (PARENT_RATE / 8), the
> > > +      * minimum rate, because the search loop was bypassed entirely.
> >
> > The "Before the fix" comment should go in the commit log. The comment in
> > the code should describe how the code is right now.
>
> To demonstrate the existing issue, you could structure your series by:
>
> - Introduce the test as the first patch and have it show the issue and
> the current behavior.
> - Put the divider fix in, and update the test for the new behavior.
>
Sure to confirm,

patch #1,  have KUNIT_EXPECT_EQ() with a false positive.
Patch #2: Clock divider fix + updated KUNIT_EXPECT_EQ() with correct
expected values.

Cheers,
Prabhakar

Re: [PATCH v2 2/2] clk: divider: Add some kunit test suites

[Brian Masney]

On Fri, Apr 17, 2026 at 12:15 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Fri, Apr 17, 2026 at 2:21 PM Brian Masney <bmasney@redhat.com> wrote:
> > On Thu, Apr 16, 2026 at 5:35 PM Brian Masney <bmasney@redhat.com> wrote:
> > > On Mon, Apr 13, 2026 at 01:49:12PM +0100, Prabhakar wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > +     /*
> > > > +      * ULONG_MAX is the canonical way to probe the maximum rate a clock
> > > > +      * can produce. With a parent at 1 GHz and the smallest table divider
> > > > +      * being 2, the expected maximum is 500 MHz.
> > > > +      *
> > > > +      * Before the fix this returned 125 MHz (PARENT_RATE / 8), the
> > > > +      * minimum rate, because the search loop was bypassed entirely.
> > >
> > > The "Before the fix" comment should go in the commit log. The comment in
> > > the code should describe how the code is right now.
> >
> > To demonstrate the existing issue, you could structure your series by:
> >
> > - Introduce the test as the first patch and have it show the issue and
> > the current behavior.
> > - Put the divider fix in, and update the test for the new behavior.
> >
> Sure to confirm,
>
> patch #1,  have KUNIT_EXPECT_EQ() with a false positive.
> Patch #2: Clock divider fix + updated KUNIT_EXPECT_EQ() with correct
> expected values.

Yes, sounds good.

Hopefully the kunit change in the second patch is minimal. The issue
with having the kunit change for the fix in another patch is that
it'll break the tests if git bisect is used.

Brian

Re: [PATCH v2 2/2] clk: divider: Add some kunit test suites

[Lad, Prabhakar]

Hi Brian,

Thank you for the review.

On Thu, Apr 16, 2026 at 10:35 PM Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Lad,
>
> On Mon, Apr 13, 2026 at 01:49:12PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add KUnit tests to verify clk_divider_bestdiv() returns the maximum
> > achievable rate when clk_round_rate() is called with ULONG_MAX, which
> > is the canonical way to probe the maximum rate a clock can produce.
> >
> > The first test uses a fixed-rate parent driving a table-based divider
> > with no div=1 entry. The second test places a two-input mux between
> > the divider and its root clocks to verify correct parent selection and
> > that the divider loop does not make redundant calls to
> > clk_hw_round_rate() for each remaining table entry after the first
> > overflow.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/clk/Kconfig            |   7 ++
> >  drivers/clk/Makefile           |   1 +
> >  drivers/clk/clk-divider_test.c | 151 +++++++++++++++++++++++++++++++++
> >  3 files changed, 159 insertions(+)
> >  create mode 100644 drivers/clk/clk-divider_test.c
> >
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index cc8743b11bb1..c8f9eaef6f6b 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -573,4 +573,11 @@ config CLK_FD_KUNIT_TEST
> >       help
> >         Kunit test for the clk-fractional-divider type.
> >
> > +config CLK_DIVIDER_KUNIT_TEST
> > +     tristate "KUnit tests for clk divider bestdiv" if !KUNIT_ALL_TESTS
> > +     depends on KUNIT
>
> Since the clk divider calls writel(), you also will need to
> unfortunately add:
>
>     depends on !S390
>
Ok.

> This is already on CLK_GATE_KUNIT_TEST. For the reason why, look at
> commit a6c3da03ead11 ("clk: disable clk gate tests for s390")
>
Thank you for the pointer.

> > +     default KUNIT_ALL_TESTS
> > +     help
> > +       Kunit test for the clk-divider type.
> > +
> >  endif
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index a3e2862ebd7e..0c915c6cf3fa 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -20,6 +20,7 @@ clk-test-y                  := clk_test.o \
> >                                  kunit_clk_assigned_rates_zero_consumer.dtbo.o \
> >                                  kunit_clk_hw_get_dev_of_node.dtbo.o \
> >                                  kunit_clk_parent_data_test.dtbo.o
> > +obj-$(CONFIG_CLK_DIVIDER_KUNIT_TEST) += clk-divider_test.o
> >  obj-$(CONFIG_COMMON_CLK)     += clk-divider.o
>
> Swap the order of these two lines above for consistency with the
> clk-fixed-rate and clk-gate tests where the actual implementation is
> first, and then the test.
>
Ok.

> >  obj-$(CONFIG_COMMON_CLK)     += clk-fixed-factor.o
> >  obj-$(CONFIG_COMMON_CLK)     += clk-fixed-rate.o
> > diff --git a/drivers/clk/clk-divider_test.c b/drivers/clk/clk-divider_test.c
> > new file mode 100644
> > index 000000000000..3a5e3adccb2e
> > --- /dev/null
> > +++ b/drivers/clk/clk-divider_test.c
> > @@ -0,0 +1,151 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit tests for clk_divider_bestdiv()
> > + */
> > +#include <kunit/test.h>
> > +#include <linux/clk.h>
> > +#include <linux/clk-provider.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)
> > +
> > +static u32 fake_reg_a, fake_reg_b;
>
> Right now this limits this to one implementation. Put these in a
> structure and use kunit_kzalloc() so that there can be multiple, and the
> runner can execute them in parallel.
>
Ok.

> > +
> > +static const struct clk_div_table no_div1_table[] = {
> > +     {0, 2},
> > +     {1, 4},
> > +     {2, 8},
> > +     {0, 0},
> > +};
>
> You can pass NULL for the table to simplify this code further. I don't
> see where you are testing anything special related to the table. I think
> you'll need to pass CLK_DIVIDER_ONE_BASED to the flags when you create
> the divider if you use a NULL table.
>
Agreed.

> > +
> > +static void unregister_fixed_rate(void *hw)
> > +{
> > +     clk_hw_unregister_fixed_rate(hw);
> > +}
> > +
> > +static void unregister_divider(void *hw)
> > +{
> > +     clk_hw_unregister_divider(hw);
> > +}
> > +
> > +static void unregister_mux(void *hw)
> > +{
> > +     clk_hw_unregister_mux(hw);
> > +}
> > +
> > +/*
> > + * Test that clk_round_rate(clk, ULONG_MAX) returns the maximum achievable
> > + * rate for a divider clock.
> > + */
> > +static void clk_divider_bestdiv_ulong_max_returns_max_rate(struct kunit *test)
> > +{
> > +     struct clk_hw *parent_hw, *div_hw;
> > +     unsigned long rate;
> > +
> > +     parent_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-parent",
> > +                                            NULL, 0, PARENT_RATE_1GHZ);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_hw);
> > +     kunit_add_action(test, unregister_fixed_rate, parent_hw);
>
> You can put clk_hw_unregister_fixed_rate() in the call here, and then
> drop unregister_fixed_rate(). There's some cases of this below as well.
>
> Check the return value of kunit_add_action() here and below as well.
>
Or maybe have something like the following (while keeping the wrappers)?

KUNIT_ASSERT_EQ(test, 0, kunit_add_action_or_reset(test,
unregister_divider, div_hw));


> > +
> > +     fake_reg_a = 0;
> > +     div_hw = clk_hw_register_divider_table(NULL, "bestdiv-div",
> > +                                            "bestdiv-parent",
> > +                                            CLK_SET_RATE_PARENT,
> > +                                            (void __iomem *)&fake_reg_a,
>
> You'll need __force for the cast for sparse as well.
>
Agreed.

> > +                                            0, 2, 0, no_div1_table, NULL);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
> > +     kunit_add_action(test, unregister_divider, div_hw);
>
> Same here... you can just put clk_hw_unregister_divider() here and drop
> the function above.
>
> > +
> > +     /*
> > +      * ULONG_MAX is the canonical way to probe the maximum rate a clock
> > +      * can produce. With a parent at 1 GHz and the smallest table divider
> > +      * being 2, the expected maximum is 500 MHz.
> > +      *
> > +      * Before the fix this returned 125 MHz (PARENT_RATE / 8), the
> > +      * minimum rate, because the search loop was bypassed entirely.
>
> The "Before the fix" comment should go in the commit log. The comment in
> the code should describe how the code is right now.
>
Ok.

> > +      */
> > +     rate = clk_hw_round_rate(div_hw, ULONG_MAX);
> > +     KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_1GHZ / 2);
> > +}
> > +
> > +/*
> > + * Test that clk_round_rate(clk, ULONG_MAX) returns the correct maximum rate when
> > + * a mux clock sits between a divider and its parent candidates.
> > + *
> > + * Topology:
> > + *
> > + *   [fixed 4 GHz] --\
> > + *                    +--> [mux CLK_SET_RATE_PARENT] --> [div {2,4,8} CLK_SET_RATE_PARENT]
> > + *   [fixed 2 GHz] --/
> > + *
> > + */
> > +static void clk_divider_bestdiv_mux_ulong_max_returns_max_rate(struct kunit *test)
> > +{
> > +     static const char *const mux_parents[] = {
> > +             "bestdiv-mux-parent-a",
> > +             "bestdiv-mux-parent-b",
> > +     };
> > +     struct clk_hw *parent_a_hw, *parent_b_hw, *mux_hw, *div_hw;
> > +     unsigned long rate;
> > +
> > +     /* Higher-rate parent: the mux should select this for ULONG_MAX. */
> > +     parent_a_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-a",
> > +                                              NULL, 0, PARENT_RATE_4GHZ);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_a_hw);
> > +     kunit_add_action(test, unregister_fixed_rate, parent_a_hw);
> > +
> > +     /* Lower-rate parent: should not be selected. */
> > +     parent_b_hw = clk_hw_register_fixed_rate(NULL, "bestdiv-mux-parent-b",
> > +                                              NULL, 0, PARENT_RATE_2GHZ);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent_b_hw);
> > +     kunit_add_action(test, unregister_fixed_rate, parent_b_hw);
> > +
> > +     /*
> > +      * 1-bit mux register selects between the two parents.
> > +      * CLK_SET_RATE_PARENT allows the divider's rate request to
> > +      * propagate into clk_mux_determine_rate().
> > +      */
> > +     fake_reg_a = 0;
> > +     mux_hw = clk_hw_register_mux(NULL, "bestdiv-mux",
> > +                                  mux_parents, ARRAY_SIZE(mux_parents),
> > +                                  CLK_SET_RATE_PARENT,
> > +                                  (void __iomem *)&fake_reg_a,
> > +                                  0, 1, 0, NULL);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mux_hw);
> > +     kunit_add_action(test, unregister_mux, mux_hw);
>
> You can put clk_hw_unregister_mux() here and drop this function above.
>
> > +
> > +     fake_reg_b = 0;
> > +     div_hw = clk_hw_register_divider_table(NULL, "bestdiv-mux-div",
> > +                                            "bestdiv-mux",
> > +                                            CLK_SET_RATE_PARENT,
> > +                                            (void __iomem *)&fake_reg_b,
> > +                                            0, 2, 0, no_div1_table, NULL);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, div_hw);
> > +     kunit_add_action(test, unregister_divider, div_hw);
> > +
> > +     /*
> > +      * Expected maximum: mux selects the 4 GHz parent, divider applies
> > +      * the smallest table entry (2): 4 GHz / 2 = 2 GHz.
> > +      */
> > +     rate = clk_hw_round_rate(div_hw, ULONG_MAX);
> > +     KUNIT_EXPECT_EQ(test, rate, PARENT_RATE_4GHZ / 2);
> > +}
> > +
> > +static struct kunit_case clk_divider_bestdiv_test_cases[] = {
> > +     KUNIT_CASE(clk_divider_bestdiv_ulong_max_returns_max_rate),
> > +     KUNIT_CASE(clk_divider_bestdiv_mux_ulong_max_returns_max_rate),
> > +     {}
> > +};
>
> Usually I'd ask for a few other tests for basic functionality to be
> added rather than just testing the maximum. However there's actually
> some stuff broken with the existing dividers and the clk core where a
> clk can change the rate of it's siblings. I have a series to address
> this at:
>
> https://lore.kernel.org/linux-clk/20260327-clk-scaling-v8-0-86cd0aba3c5f@redhat.com/
>
> I think the tests that you have are fine.
>
Thanks for the context, that makes sense.

Cheers,
Prabhakar

>
> Stephen: If you have time after the merge window closes I'd appreciate
> it if you could take time to provide feedback about that series.
>
> Puss in Boots please... :)
> https://media2.giphy.com/media/v1.Y2lkPTc5MGI3NjExaXg5cGFhbGdmbTZjdnRkdWs4aXM5d3FlYnFmbTNudWFsZ3Fwc3o5NiZlcD12MV9pbnRlcm5hbF9naWZfYnlfaWQmY3Q9Zw/qUIm5wu6LAAog/giphy.gif
>
> Brian
>