1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
|
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
@@ -2475,28 +2475,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);
|
