From: Maxime Ripard <maxime@cerno.tech>
To: Mike Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>
Cc: linux-clk@vger.kernel.org, dri-devel@lists.freedesktop.org,
Dave Stevenson <dave.stevenson@raspberrypi.com>,
Phil Elwell <phil@raspberrypi.com>,
Tim Gover <tim.gover@raspberrypi.com>,
Dom Cobley <dom@raspberrypi.com>,
Maxime Ripard <maxime@cerno.tech>
Subject: [PATCH v3 04/10] clk: Always set the rate on clk_set_range_rate
Date: Thu, 20 Jan 2022 15:34:11 +0100 [thread overview]
Message-ID: <20220120143417.543744-5-maxime@cerno.tech> (raw)
In-Reply-To: <20220120143417.543744-1-maxime@cerno.tech>
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.
Also add a few new test cases to make sure that behaviour is not broken
in the future.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/clk/clk-rate-test.c | 281 +++++++++++++++++++++++++++++++++++-
drivers/clk/clk.c | 45 +++---
2 files changed, 303 insertions(+), 23 deletions(-)
diff --git a/drivers/clk/clk-rate-test.c b/drivers/clk/clk-rate-test.c
index a13b02702d20..baf0ea315322 100644
--- a/drivers/clk/clk-rate-test.c
+++ b/drivers/clk/clk-rate-test.c
@@ -6,6 +6,9 @@
#include <linux/clk-provider.h>
#include <linux/slab.h>
+/* Needed for clk_hw_create_clk() */
+#include "clk.h"
+
#include <kunit/test.h>
#define DUMMY_CLOCK_INIT_RATE (42 * 1000 * 1000)
@@ -33,6 +36,32 @@ static int clk_dummy_rate_determine_rate(struct clk_hw *hw,
return 0;
}
+static int clk_dummy_rate_maximize_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ /*
+ * If there's a maximum set, always run the clock at the maximum
+ * allowed.
+ */
+ if (req->max_rate < ULONG_MAX)
+ req->rate = req->max_rate;
+
+ return 0;
+}
+
+static int clk_dummy_rate_minimize_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ /*
+ * If there's a minimum set, always run the clock at the minimum
+ * allowed.
+ */
+ if (req->min_rate > 0)
+ req->rate = req->min_rate;
+
+ return 0;
+}
+
static int clk_dummy_rate_set_rate(struct clk_hw *hw,
unsigned long rate,
unsigned long parent_rate)
@@ -50,6 +79,18 @@ const static struct clk_ops clk_dummy_rate_ops = {
.set_rate = clk_dummy_rate_set_rate,
};
+const static struct clk_ops clk_dummy_maximize_rate_ops = {
+ .recalc_rate = clk_dummy_rate_recalc_rate,
+ .determine_rate = clk_dummy_rate_maximize_rate,
+ .set_rate = clk_dummy_rate_set_rate,
+};
+
+const static struct clk_ops clk_dummy_minimize_rate_ops = {
+ .recalc_rate = clk_dummy_rate_recalc_rate,
+ .determine_rate = clk_dummy_rate_minimize_rate,
+ .set_rate = clk_dummy_rate_set_rate,
+};
+
static int clk_rate_test_init_with_ops(struct kunit *test,
const struct clk_ops *ops)
{
@@ -79,6 +120,16 @@ static int clk_rate_test_init(struct kunit *test)
return clk_rate_test_init_with_ops(test, &clk_dummy_rate_ops);
}
+static int clk_rate_maximize_test_init(struct kunit *test)
+{
+ return clk_rate_test_init_with_ops(test, &clk_dummy_maximize_rate_ops);
+}
+
+static int clk_rate_minimize_test_init(struct kunit *test)
+{
+ return clk_rate_test_init_with_ops(test, &clk_dummy_minimize_rate_ops);
+}
+
static void clk_rate_test_exit(struct kunit *test)
{
struct clk_dummy_rate_context *ctx = test->priv;
@@ -317,8 +368,236 @@ static struct kunit_suite clk_rate_range_test_suite = {
.test_cases = clk_rate_range_test_cases,
};
+/*
+ * 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.
+ *
+ * With clk_dummy_maximize_rate_ops, this means that the the rate will
+ * trail along the maximum as it evolves.
+ */
+static void clk_rate_range_test_set_range_rate_maximized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ unsigned long rate;
+ int ret;
+
+ ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2 + 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2 - 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+}
+/*
+ * 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.
+ *
+ * With clk_dummy_maximize_rate_ops, this means that the the rate will
+ * trail along the maximum as it evolves.
+ */
+static void clk_rate_range_test_multiple_set_range_rate_maximized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ struct clk *user1, *user2;
+ unsigned long rate;
+ int ret;
+
+ user1 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user1));
+
+ user2 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user2));
+
+ ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2 + 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = clk_set_rate_range(user1,
+ 0,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ ret = clk_set_rate_range(user2,
+ 0,
+ DUMMY_CLOCK_RATE_1);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ ret = clk_set_rate_range(user2, 0, ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ clk_put(user2);
+ clk_put(user1);
+}
+
+static struct kunit_case clk_rate_range_maximize_test_cases[] = {
+ KUNIT_CASE(clk_rate_range_test_set_range_rate_maximized),
+ KUNIT_CASE(clk_rate_range_test_multiple_set_range_rate_maximized),
+ {}
+};
+
+static struct kunit_suite clk_rate_range_maximize_test_suite = {
+ .name = "clk-rate-range-maximize-test",
+ .init = clk_rate_maximize_test_init,
+ .exit = clk_rate_test_exit,
+ .test_cases = clk_rate_range_maximize_test_cases,
+};
+
+/*
+ * 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.
+ *
+ * With clk_dummy_minimize_rate_ops, this means that the the rate will
+ * trail along the minimum as it evolves.
+ */
+static void clk_rate_range_test_set_range_rate_minimized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ unsigned long rate;
+ int ret;
+
+ ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1 + 1000,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+}
+
+/*
+ * 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.
+ *
+ * With clk_dummy_minimize_rate_ops, this means that the the rate will
+ * trail along the minimum as it evolves.
+ */
+static void clk_rate_range_test_multiple_set_range_rate_minimized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ struct clk *user1, *user2;
+ unsigned long rate;
+ int ret;
+
+ user1 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user1));
+
+ user2 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user2));
+
+ ret = clk_set_rate_range(user1,
+ DUMMY_CLOCK_RATE_1,
+ ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ ret = clk_set_rate_range(user2,
+ DUMMY_CLOCK_RATE_2,
+ ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ ret = clk_set_rate_range(user2, 0, ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ clk_put(user2);
+ clk_put(user1);
+}
+
+static struct kunit_case clk_rate_range_minimize_test_cases[] = {
+ KUNIT_CASE(clk_rate_range_test_set_range_rate_minimized),
+ KUNIT_CASE(clk_rate_range_test_multiple_set_range_rate_minimized),
+ {}
+};
+
+static struct kunit_suite clk_rate_range_minimize_test_suite = {
+ .name = "clk-rate-range-minimize-test",
+ .init = clk_rate_minimize_test_init,
+ .exit = clk_rate_test_exit,
+ .test_cases = clk_rate_range_minimize_test_cases,
+};
+
kunit_test_suites(
&clk_rate_test_suite,
- &clk_rate_range_test_suite
+ &clk_rate_range_test_suite,
+ &clk_rate_range_maximize_test_suite,
+ &clk_rate_range_minimize_test_suite
);
MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 150d1bc0985b..718cab23f706 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2350,28 +2350,29 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
clk->min_rate = min;
clk->max_rate = max;
- 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;
}
if (clk->exclusive_count)
--
2.34.1
WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Mike Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>
Cc: Dom Cobley <dom@raspberrypi.com>,
Tim Gover <tim.gover@raspberrypi.com>,
Dave Stevenson <dave.stevenson@raspberrypi.com>,
dri-devel@lists.freedesktop.org, linux-clk@vger.kernel.org,
Maxime Ripard <maxime@cerno.tech>,
Phil Elwell <phil@raspberrypi.com>
Subject: [PATCH v3 04/10] clk: Always set the rate on clk_set_range_rate
Date: Thu, 20 Jan 2022 15:34:11 +0100 [thread overview]
Message-ID: <20220120143417.543744-5-maxime@cerno.tech> (raw)
In-Reply-To: <20220120143417.543744-1-maxime@cerno.tech>
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.
Also add a few new test cases to make sure that behaviour is not broken
in the future.
Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
drivers/clk/clk-rate-test.c | 281 +++++++++++++++++++++++++++++++++++-
drivers/clk/clk.c | 45 +++---
2 files changed, 303 insertions(+), 23 deletions(-)
diff --git a/drivers/clk/clk-rate-test.c b/drivers/clk/clk-rate-test.c
index a13b02702d20..baf0ea315322 100644
--- a/drivers/clk/clk-rate-test.c
+++ b/drivers/clk/clk-rate-test.c
@@ -6,6 +6,9 @@
#include <linux/clk-provider.h>
#include <linux/slab.h>
+/* Needed for clk_hw_create_clk() */
+#include "clk.h"
+
#include <kunit/test.h>
#define DUMMY_CLOCK_INIT_RATE (42 * 1000 * 1000)
@@ -33,6 +36,32 @@ static int clk_dummy_rate_determine_rate(struct clk_hw *hw,
return 0;
}
+static int clk_dummy_rate_maximize_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ /*
+ * If there's a maximum set, always run the clock at the maximum
+ * allowed.
+ */
+ if (req->max_rate < ULONG_MAX)
+ req->rate = req->max_rate;
+
+ return 0;
+}
+
+static int clk_dummy_rate_minimize_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ /*
+ * If there's a minimum set, always run the clock at the minimum
+ * allowed.
+ */
+ if (req->min_rate > 0)
+ req->rate = req->min_rate;
+
+ return 0;
+}
+
static int clk_dummy_rate_set_rate(struct clk_hw *hw,
unsigned long rate,
unsigned long parent_rate)
@@ -50,6 +79,18 @@ const static struct clk_ops clk_dummy_rate_ops = {
.set_rate = clk_dummy_rate_set_rate,
};
+const static struct clk_ops clk_dummy_maximize_rate_ops = {
+ .recalc_rate = clk_dummy_rate_recalc_rate,
+ .determine_rate = clk_dummy_rate_maximize_rate,
+ .set_rate = clk_dummy_rate_set_rate,
+};
+
+const static struct clk_ops clk_dummy_minimize_rate_ops = {
+ .recalc_rate = clk_dummy_rate_recalc_rate,
+ .determine_rate = clk_dummy_rate_minimize_rate,
+ .set_rate = clk_dummy_rate_set_rate,
+};
+
static int clk_rate_test_init_with_ops(struct kunit *test,
const struct clk_ops *ops)
{
@@ -79,6 +120,16 @@ static int clk_rate_test_init(struct kunit *test)
return clk_rate_test_init_with_ops(test, &clk_dummy_rate_ops);
}
+static int clk_rate_maximize_test_init(struct kunit *test)
+{
+ return clk_rate_test_init_with_ops(test, &clk_dummy_maximize_rate_ops);
+}
+
+static int clk_rate_minimize_test_init(struct kunit *test)
+{
+ return clk_rate_test_init_with_ops(test, &clk_dummy_minimize_rate_ops);
+}
+
static void clk_rate_test_exit(struct kunit *test)
{
struct clk_dummy_rate_context *ctx = test->priv;
@@ -317,8 +368,236 @@ static struct kunit_suite clk_rate_range_test_suite = {
.test_cases = clk_rate_range_test_cases,
};
+/*
+ * 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.
+ *
+ * With clk_dummy_maximize_rate_ops, this means that the the rate will
+ * trail along the maximum as it evolves.
+ */
+static void clk_rate_range_test_set_range_rate_maximized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ unsigned long rate;
+ int ret;
+
+ ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2 + 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2 - 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+}
+/*
+ * 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.
+ *
+ * With clk_dummy_maximize_rate_ops, this means that the the rate will
+ * trail along the maximum as it evolves.
+ */
+static void clk_rate_range_test_multiple_set_range_rate_maximized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ struct clk *user1, *user2;
+ unsigned long rate;
+ int ret;
+
+ user1 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user1));
+
+ user2 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user2));
+
+ ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_2 + 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = clk_set_rate_range(user1,
+ 0,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ ret = clk_set_rate_range(user2,
+ 0,
+ DUMMY_CLOCK_RATE_1);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ ret = clk_set_rate_range(user2, 0, ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ clk_put(user2);
+ clk_put(user1);
+}
+
+static struct kunit_case clk_rate_range_maximize_test_cases[] = {
+ KUNIT_CASE(clk_rate_range_test_set_range_rate_maximized),
+ KUNIT_CASE(clk_rate_range_test_multiple_set_range_rate_maximized),
+ {}
+};
+
+static struct kunit_suite clk_rate_range_maximize_test_suite = {
+ .name = "clk-rate-range-maximize-test",
+ .init = clk_rate_maximize_test_init,
+ .exit = clk_rate_test_exit,
+ .test_cases = clk_rate_range_maximize_test_cases,
+};
+
+/*
+ * 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.
+ *
+ * With clk_dummy_minimize_rate_ops, this means that the the rate will
+ * trail along the minimum as it evolves.
+ */
+static void clk_rate_range_test_set_range_rate_minimized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ unsigned long rate;
+ int ret;
+
+ ret = clk_set_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1 + 1000,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
+
+ ret = clk_set_rate_range(clk,
+ DUMMY_CLOCK_RATE_1,
+ DUMMY_CLOCK_RATE_2);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+}
+
+/*
+ * 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.
+ *
+ * With clk_dummy_minimize_rate_ops, this means that the the rate will
+ * trail along the minimum as it evolves.
+ */
+static void clk_rate_range_test_multiple_set_range_rate_minimized(struct kunit *test)
+{
+ struct clk_dummy_rate_context *ctx = test->priv;
+ struct clk_hw *hw = &ctx->hw;
+ struct clk *clk = hw->clk;
+ struct clk *user1, *user2;
+ unsigned long rate;
+ int ret;
+
+ user1 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user1));
+
+ user2 = clk_hw_create_clk(NULL, hw, NULL, NULL);
+ KUNIT_ASSERT_FALSE(test, IS_ERR(user2));
+
+ ret = clk_set_rate_range(user1,
+ DUMMY_CLOCK_RATE_1,
+ ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ ret = clk_set_rate_range(user2,
+ DUMMY_CLOCK_RATE_2,
+ ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_2);
+
+ ret = clk_set_rate_range(user2, 0, ULONG_MAX);
+ KUNIT_ASSERT_EQ(test, ret, 0);
+
+ rate = clk_get_rate(clk);
+ KUNIT_ASSERT_TRUE(test, rate > 0);
+ KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
+
+ clk_put(user2);
+ clk_put(user1);
+}
+
+static struct kunit_case clk_rate_range_minimize_test_cases[] = {
+ KUNIT_CASE(clk_rate_range_test_set_range_rate_minimized),
+ KUNIT_CASE(clk_rate_range_test_multiple_set_range_rate_minimized),
+ {}
+};
+
+static struct kunit_suite clk_rate_range_minimize_test_suite = {
+ .name = "clk-rate-range-minimize-test",
+ .init = clk_rate_minimize_test_init,
+ .exit = clk_rate_test_exit,
+ .test_cases = clk_rate_range_minimize_test_cases,
+};
+
kunit_test_suites(
&clk_rate_test_suite,
- &clk_rate_range_test_suite
+ &clk_rate_range_test_suite,
+ &clk_rate_range_maximize_test_suite,
+ &clk_rate_range_minimize_test_suite
);
MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 150d1bc0985b..718cab23f706 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2350,28 +2350,29 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
clk->min_rate = min;
clk->max_rate = max;
- 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;
}
if (clk->exclusive_count)
--
2.34.1
next prev parent reply other threads:[~2022-01-20 14:34 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-20 14:34 [PATCH v3 00/10] clk: Improve clock range handling Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 01/10] clk: Add Kunit tests for rate Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 21:31 ` Stephen Boyd
2022-01-20 21:31 ` Stephen Boyd
2022-01-20 21:56 ` Daniel Latypov
2022-01-20 21:56 ` Daniel Latypov
2022-01-21 4:34 ` Stephen Boyd
2022-01-21 4:34 ` Stephen Boyd
2022-01-21 5:25 ` Daniel Latypov
2022-01-21 5:25 ` Daniel Latypov
2022-01-22 1:51 ` Stephen Boyd
2022-01-22 1:51 ` Stephen Boyd
2022-01-20 14:34 ` [PATCH v3 02/10] clk: Always clamp the rounded rate Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 03/10] clk: Use clamp instead of open-coding our own Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard [this message]
2022-01-20 14:34 ` [PATCH v3 04/10] clk: Always set the rate on clk_set_range_rate Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 05/10] clk: Add clk_drop_range Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 06/10] clk: bcm: rpi: Add variant structure Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 07/10] clk: bcm: rpi: Set a default minimum rate Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 08/10] clk: bcm: rpi: Run some clocks at the minimum rate allowed Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 09/10] drm/vc4: Add logging and comments Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
2022-01-20 14:34 ` [PATCH v3 10/10] drm/vc4: hdmi: Remove clock rate initialization Maxime Ripard
2022-01-20 14:34 ` Maxime Ripard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220120143417.543744-5-maxime@cerno.tech \
--to=maxime@cerno.tech \
--cc=dave.stevenson@raspberrypi.com \
--cc=dom@raspberrypi.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-clk@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=phil@raspberrypi.com \
--cc=sboyd@kernel.org \
--cc=tim.gover@raspberrypi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.