clk: Always set the rate on clk_set_range_rate

When we change a clock minimum or maximum using clk_set_rate_range(),
clk_set_min_rate() or clk_set_max_rate(), the current code will only
trigger a new rate change if the rate is outside of the new boundaries.

However, a clock driver might want to always keep the clock rate to
one of its boundary, for example the minimum to keep the power
consumption as low as possible.

Since they don't always get called though, clock providers don't have the
opportunity to implement this behaviour.

Let's trigger a clk_set_rate() on the previous requested rate every time
clk_set_rate_range() is called. That way, providers that care about the
new boundaries have a chance to adjust the rate, while providers that
don't care about those new boundaries will return the same rate than
before, which will be ignored by clk_set_rate() and won't result in a
new rate change.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
Link: https://lore.kernel.org/r/20220225143534.405820-7-maxime@cerno.tech
Signed-off-by: Stephen Boyd <sboyd@kernel.org>

drivers/clk/clk.c

@@ -2373,28 +2373,29 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
 		goto out;
 	}
 
-	rate = clk_core_get_rate_nolock(clk->core);
+	/*
-	if (rate < min || rate > max) {
+	 * Since the boundaries have been changed, let's give the
-		/*
+	 * opportunity to the provider to adjust the clock rate based on
-		 * FIXME:
+	 * the new boundaries.
-		 * We are in bit of trouble here, current rate is outside the
+	 *
-		 * the requested range. We are going try to request appropriate
+	 * We also need to handle the case where the clock is currently
-		 * range boundary but there is a catch. It may fail for the
+	 * outside of the boundaries. Clamping the last requested rate
-		 * usual reason (clock broken, clock protected, etc) but also
+	 * to the current minimum and maximum will also handle this.
-		 * because:
+	 *
-		 * - round_rate() was not favorable and fell on the wrong
+	 * FIXME:
-		 *   side of the boundary
+	 * There is a catch. It may fail for the usual reason (clock
-		 * - the determine_rate() callback does not really check for
+	 * broken, clock protected, etc) but also because:
-		 *   this corner case when determining the rate
+	 * - round_rate() was not favorable and fell on the wrong
-		 */
+	 *   side of the boundary
-
+	 * - the determine_rate() callback does not really check for
-		rate = clamp(clk->core->req_rate, min, max);
+	 *   this corner case when determining the rate
-		ret = clk_core_set_rate_nolock(clk->core, rate);
+	 */
-		if (ret) {
+	rate = clamp(clk->core->req_rate, min, max);
-			/* rollback the changes */
+	ret = clk_core_set_rate_nolock(clk->core, rate);
-			clk->min_rate = old_min;
+	if (ret) {
-			clk->max_rate = old_max;
+		/* rollback the changes */
-		}
+		clk->min_rate = old_min;
+		clk->max_rate = old_max;
 	}
 
 out:

drivers/clk/clk_test.c

@@ -549,13 +549,12 @@ static struct kunit_suite clk_range_test_suite = {
 };
 
 /*
- * Test that if:
+ * Test that if we have several subsequent calls to
- * - we have several subsequent calls to clk_set_rate_range();
+ * clk_set_rate_range(), the core will reevaluate whether a new rate is
- * - and we have a round_rate ops that always return the maximum
+ * needed each and every time.
- *   frequency allowed;
  *
- * The clock will run at the minimum of all maximum boundaries
+ * With clk_dummy_maximize_rate_ops, this means that the rate will
- * requested, even if those boundaries aren't there anymore.
+ * trail along the maximum as it evolves.
  */
 static void clk_range_test_set_range_rate_maximized(struct kunit *test)
 {
@@ -596,18 +595,16 @@ static void clk_range_test_set_range_rate_maximized(struct kunit *test)
 
 	rate = clk_get_rate(clk);
 	KUNIT_ASSERT_GT(test, rate, 0);
-	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
 }
 
 /*
- * Test that if:
+ * Test that if we have several subsequent calls to
- * - we have several subsequent calls to clk_set_rate_range(), across
+ * clk_set_rate_range(), across multiple users, the core will reevaluate
- *   multiple users;
+ * whether a new rate is needed each and every time.
- * - and we have a round_rate ops that always return the maximum
- *   frequency allowed;
  *
- * The clock will run at the minimum of all maximum boundaries
+ * With clk_dummy_maximize_rate_ops, this means that the rate will
- * requested, even if those boundaries aren't there anymore.
+ * trail along the maximum as it evolves.
  */
 static void clk_range_test_multiple_set_range_rate_maximized(struct kunit *test)
 {
@@ -653,7 +650,7 @@ static void clk_range_test_multiple_set_range_rate_maximized(struct kunit *test)
 
 	rate = clk_get_rate(clk);
 	KUNIT_ASSERT_GT(test, rate, 0);
-	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
 
 	clk_put(user2);
 	clk_put(user1);
@@ -673,13 +670,12 @@ static struct kunit_suite clk_range_maximize_test_suite = {
 };
 
 /*
- * Test that if:
+ * Test that if we have several subsequent calls to
- * - we have several subsequent calls to clk_set_rate_range()
+ * clk_set_rate_range(), the core will reevaluate whether a new rate is
- * - and we have a round_rate ops that always return the minimum
+ * needed each and every time.
- *   frequency allowed;
  *
- * The clock will run at the maximum of all minimum boundaries
+ * With clk_dummy_minimize_rate_ops, this means that the rate will
- * requested, even if those boundaries aren't there anymore.
+ * trail along the minimum as it evolves.
  */
 static void clk_range_test_set_range_rate_minimized(struct kunit *test)
 {
@@ -720,18 +716,16 @@ static void clk_range_test_set_range_rate_minimized(struct kunit *test)
 
 	rate = clk_get_rate(clk);
 	KUNIT_ASSERT_GT(test, rate, 0);
-	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
 }
 
 /*
- * Test that if:
+ * Test that if we have several subsequent calls to
- * - we have several subsequent calls to clk_set_rate_range(), across
+ * clk_set_rate_range(), across multiple users, the core will reevaluate
- *   multiple users;
+ * whether a new rate is needed each and every time.
- * - and we have a round_rate ops that always return the minimum
- *   frequency allowed;
  *
- * The clock will run at the maximum of all minimum boundaries
+ * With clk_dummy_minimize_rate_ops, this means that the rate will
- * requested, even if those boundaries aren't there anymore.
+ * trail along the minimum as it evolves.
  */
 static void clk_range_test_multiple_set_range_rate_minimized(struct kunit *test)
 {
@@ -773,7 +767,7 @@ static void clk_range_test_multiple_set_range_rate_minimized(struct kunit *test)
 
 	rate = clk_get_rate(clk);
 	KUNIT_ASSERT_GT(test, rate, 0);
-	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+	KUNIT_EXPECT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
 
 	clk_put(user2);
 	clk_put(user1);