197 lines
7.1 KiB
Diff
197 lines
7.1 KiB
Diff
From c12b9d21ea40c608be608b83f803efdbdfe9657a Mon Sep 17 00:00:00 2001
|
|
From: Maxime Ripard <maxime@cerno.tech>
|
|
Date: Mon, 17 Jan 2022 16:43:13 +0100
|
|
Subject: [PATCH] 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>
|
|
---
|
|
drivers/clk/clk.c | 45 ++++++++++++++++----------------
|
|
drivers/clk/clk_test.c | 58 +++++++++++++++++++-----------------------
|
|
2 files changed, 49 insertions(+), 54 deletions(-)
|
|
|
|
--- a/drivers/clk/clk.c
|
|
+++ b/drivers/clk/clk.c
|
|
@@ -2385,28 +2385,29 @@ int clk_set_rate_range(struct clk *clk,
|
|
goto out;
|
|
}
|
|
|
|
- rate = clk_core_get_rate_nolock(clk->core);
|
|
- if (rate < min || rate > max) {
|
|
- /*
|
|
- * FIXME:
|
|
- * We are in bit of trouble here, current rate is outside the
|
|
- * the requested range. We are going try to request appropriate
|
|
- * range boundary but there is a catch. It may fail for the
|
|
- * usual reason (clock broken, clock protected, etc) but also
|
|
- * because:
|
|
- * - round_rate() was not favorable and fell on the wrong
|
|
- * side of the boundary
|
|
- * - the determine_rate() callback does not really check for
|
|
- * this corner case when determining the rate
|
|
- */
|
|
-
|
|
- rate = clamp(clk->core->req_rate, min, max);
|
|
- ret = clk_core_set_rate_nolock(clk->core, rate);
|
|
- if (ret) {
|
|
- /* rollback the changes */
|
|
- clk->min_rate = old_min;
|
|
- clk->max_rate = old_max;
|
|
- }
|
|
+ /*
|
|
+ * Since the boundaries have been changed, let's give the
|
|
+ * opportunity to the provider to adjust the clock rate based on
|
|
+ * the new boundaries.
|
|
+ *
|
|
+ * We also need to handle the case where the clock is currently
|
|
+ * outside of the boundaries. Clamping the last requested rate
|
|
+ * to the current minimum and maximum will also handle this.
|
|
+ *
|
|
+ * FIXME:
|
|
+ * There is a catch. It may fail for the usual reason (clock
|
|
+ * broken, clock protected, etc) but also because:
|
|
+ * - round_rate() was not favorable and fell on the wrong
|
|
+ * side of the boundary
|
|
+ * - the determine_rate() callback does not really check for
|
|
+ * this corner case when determining the rate
|
|
+ */
|
|
+ rate = clamp(clk->core->req_rate, min, max);
|
|
+ ret = clk_core_set_rate_nolock(clk->core, rate);
|
|
+ if (ret) {
|
|
+ /* rollback the changes */
|
|
+ clk->min_rate = old_min;
|
|
+ clk->max_rate = old_max;
|
|
}
|
|
|
|
out:
|
|
--- a/drivers/clk/clk_test.c
|
|
+++ b/drivers/clk/clk_test.c
|
|
@@ -549,13 +549,12 @@ static struct kunit_suite clk_range_test
|
|
};
|
|
|
|
/*
|
|
- * Test that if:
|
|
- * - we have several subsequent calls to clk_set_rate_range();
|
|
- * - and we have a round_rate ops that always return the maximum
|
|
- * frequency allowed;
|
|
+ * Test that if we have several subsequent calls to
|
|
+ * clk_set_rate_range(), the core will reevaluate whether a new rate is
|
|
+ * needed each and every time.
|
|
*
|
|
- * The clock will run at the minimum of all maximum boundaries
|
|
- * requested, even if those boundaries aren't there anymore.
|
|
+ * With clk_dummy_maximize_rate_ops, this means that the the rate will
|
|
+ * 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_rat
|
|
|
|
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:
|
|
- * - we have several subsequent calls to clk_set_rate_range(), across
|
|
- * multiple users;
|
|
- * - and we have a round_rate ops that always return the maximum
|
|
- * frequency allowed;
|
|
+ * Test that if we have several subsequent calls to
|
|
+ * clk_set_rate_range(), across multiple users, the core will reevaluate
|
|
+ * whether a new rate is needed each and every time.
|
|
*
|
|
- * The clock will run at the minimum of all maximum boundaries
|
|
- * requested, even if those boundaries aren't there anymore.
|
|
+ * With clk_dummy_maximize_rate_ops, this means that the the rate will
|
|
+ * 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_
|
|
|
|
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,14 +670,13 @@ static struct kunit_suite clk_range_maxi
|
|
};
|
|
|
|
/*
|
|
- * Test that if:
|
|
- * - we have several subsequent calls to clk_set_rate_range()
|
|
- * - and we have a round_rate ops that always return the minimum
|
|
- * frequency allowed;
|
|
+ * Test that if we have several subsequent calls to
|
|
+ * clk_set_rate_range(), the core will reevaluate whether a new rate is
|
|
+ * needed each and every time.
|
|
*
|
|
- * The clock will run at the maximum of all minimum boundaries
|
|
- * requested, even if those boundaries aren't there anymore.
|
|
-*/
|
|
+ * With clk_dummy_minimize_rate_ops, this means that the the rate will
|
|
+ * trail along the minimum as it evolves.
|
|
+ */
|
|
static void clk_range_test_set_range_rate_minimized(struct kunit *test)
|
|
{
|
|
struct clk_dummy_context *ctx = test->priv;
|
|
@@ -720,19 +716,17 @@ static void clk_range_test_set_range_rat
|
|
|
|
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:
|
|
- * - we have several subsequent calls to clk_set_rate_range(), across
|
|
- * multiple users;
|
|
- * - and we have a round_rate ops that always return the minimum
|
|
- * frequency allowed;
|
|
+ * Test that if we have several subsequent calls to
|
|
+ * clk_set_rate_range(), across multiple users, the core will reevaluate
|
|
+ * whether a new rate is needed each and every time.
|
|
*
|
|
- * The clock will run at the maximum of all minimum boundaries
|
|
- * requested, even if those boundaries aren't there anymore.
|
|
-*/
|
|
+ * With clk_dummy_minimize_rate_ops, this means that the the rate will
|
|
+ * trail along the minimum as it evolves.
|
|
+ */
|
|
static void clk_range_test_multiple_set_range_rate_minimized(struct kunit *test)
|
|
{
|
|
struct clk_dummy_context *ctx = test->priv;
|
|
@@ -773,7 +767,7 @@ static void clk_range_test_multiple_set_
|
|
|
|
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);
|