1 From 6d969d963a86c88ca1d6dcbc25447e2937e833b1 Mon Sep 17 00:00:00 2001
2 From: Maxime Ripard <maxime@cerno.tech>
3 Date: Sat, 2 Apr 2022 14:36:59 +0200
4 Subject: [PATCH] clk: Stop forwarding clk_rate_requests to the parent
6 If the clock cannot modify its rate and has CLK_SET_RATE_PARENT,
7 clk_mux_determine_rate_flags() and clk_core_round_rate_nolock() will
8 call clk_core_round_rate_nolock() with its parent clock but use the
9 request of the child node either directly (clk_core_round_rate_nolock())
10 or by copying it (clk_mux_determine_rate_flags()).
12 Both cases are problematic since the parent will now have a request with
13 the best parent fields of the child (so pointing to itself) and the
14 boundaries of the child as well.
16 clk_core_round_rate_nolock() is even worse since we would directly
17 modify the caller structure if the parent was ever to modify its own
18 parent or its parent rate, then returning to the caller a best parent
19 that isn't a parent of the clock we just called clk_determine_rate()
22 Let's create a new function that will create a new request to forward to
23 the parent, clk_core_forward_rate_req() and update the relevant call
24 sites to that new function.
26 Let's also add a test to make sure we avoid regressions there.
28 Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # imx8mp
29 Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> # exynos4210, meson g12b
30 Signed-off-by: Maxime Ripard <maxime@cerno.tech>
32 drivers/clk/clk.c | 58 ++++++++++++--
33 drivers/clk/clk_test.c | 176 +++++++++++++++++++++++++++++++++++++++++
34 2 files changed, 227 insertions(+), 7 deletions(-)
36 --- a/drivers/clk/clk.c
37 +++ b/drivers/clk/clk.c
38 @@ -542,6 +542,10 @@ static bool mux_is_better_rate(unsigned
39 return now <= rate && now > best;
42 +static void clk_core_init_rate_req(struct clk_core * const core,
43 + struct clk_rate_request *req,
44 + unsigned long rate);
46 static int clk_core_round_rate_nolock(struct clk_core *core,
47 struct clk_rate_request *req);
49 @@ -565,6 +569,24 @@ static bool clk_core_has_parent(struct c
54 +clk_core_forward_rate_req(struct clk_core * const core,
55 + struct clk_core * const parent,
56 + struct clk_rate_request * const old_req,
57 + struct clk_rate_request *req)
59 + if (WARN_ON(!clk_core_has_parent(core, parent)))
62 + clk_core_init_rate_req(parent, req, old_req->rate);
64 + if (req->min_rate < old_req->min_rate)
65 + req->min_rate = old_req->min_rate;
67 + if (req->max_rate > old_req->max_rate)
68 + req->max_rate = old_req->max_rate;
71 int clk_mux_determine_rate_flags(struct clk_hw *hw,
72 struct clk_rate_request *req,
74 @@ -572,17 +594,19 @@ int clk_mux_determine_rate_flags(struct
75 struct clk_core *core = hw->core, *parent, *best_parent = NULL;
76 int i, num_parents, ret;
77 unsigned long best = 0;
78 - struct clk_rate_request parent_req = *req;
80 /* if NO_REPARENT flag set, pass through to current parent */
81 if (core->flags & CLK_SET_RATE_NO_REPARENT) {
82 parent = core->parent;
83 if (core->flags & CLK_SET_RATE_PARENT) {
84 + struct clk_rate_request parent_req;
91 + clk_core_forward_rate_req(core, parent, req, &parent_req);
92 ret = clk_core_round_rate_nolock(parent, &parent_req);
95 @@ -600,23 +624,29 @@ int clk_mux_determine_rate_flags(struct
96 /* find the parent that can provide the fastest rate <= rate */
97 num_parents = core->num_parents;
98 for (i = 0; i < num_parents; i++) {
99 + unsigned long parent_rate;
101 parent = clk_core_get_parent_by_index(core, i);
105 if (core->flags & CLK_SET_RATE_PARENT) {
107 + struct clk_rate_request parent_req;
109 + clk_core_forward_rate_req(core, parent, req, &parent_req);
110 ret = clk_core_round_rate_nolock(parent, &parent_req);
114 + parent_rate = parent_req.rate;
116 - parent_req.rate = clk_core_get_rate_nolock(parent);
117 + parent_rate = clk_core_get_rate_nolock(parent);
120 - if (mux_is_better_rate(req->rate, parent_req.rate,
121 + if (mux_is_better_rate(req->rate, parent_rate,
123 best_parent = parent;
124 - best = parent_req.rate;
125 + best = parent_rate;
129 @@ -1449,6 +1479,8 @@ static bool clk_core_can_round(struct cl
130 static int clk_core_round_rate_nolock(struct clk_core *core,
131 struct clk_rate_request *req)
135 lockdep_assert_held(&prepare_lock);
138 @@ -1458,8 +1490,20 @@ static int clk_core_round_rate_nolock(st
140 if (clk_core_can_round(core))
141 return clk_core_determine_round_nolock(core, req);
142 - else if (core->flags & CLK_SET_RATE_PARENT)
143 - return clk_core_round_rate_nolock(core->parent, req);
145 + if (core->flags & CLK_SET_RATE_PARENT) {
146 + struct clk_rate_request parent_req;
148 + clk_core_forward_rate_req(core, core->parent, req, &parent_req);
149 + ret = clk_core_round_rate_nolock(core->parent, &parent_req);
153 + req->best_parent_rate = parent_req.rate;
154 + req->rate = parent_req.rate;
159 req->rate = core->rate;
161 --- a/drivers/clk/clk_test.c
162 +++ b/drivers/clk/clk_test.c
163 @@ -999,6 +999,34 @@ clk_test_single_parent_mux_set_range_dis
166 * Test that for a clock that can't modify its rate and with a single
167 + * parent, if we set a range on the parent and then call
168 + * clk_round_rate(), the boundaries of the parent are taken into
172 +clk_test_single_parent_mux_set_range_round_rate_parent_only(struct kunit *test)
174 + struct clk_single_parent_ctx *ctx = test->priv;
175 + struct clk_hw *hw = &ctx->hw;
176 + struct clk *clk = hw->clk;
177 + struct clk *parent;
178 + unsigned long rate;
181 + parent = clk_get_parent(clk);
182 + KUNIT_ASSERT_PTR_NE(test, parent, NULL);
184 + ret = clk_set_rate_range(parent, DUMMY_CLOCK_RATE_1, DUMMY_CLOCK_RATE_2);
185 + KUNIT_ASSERT_EQ(test, ret, 0);
187 + rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
188 + KUNIT_ASSERT_GT(test, rate, 0);
189 + KUNIT_EXPECT_GE(test, rate, DUMMY_CLOCK_RATE_1);
190 + KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2);
194 + * Test that for a clock that can't modify its rate and with a single
195 * parent, if we set a range on the parent and a more restrictive one on
196 * the child, and then call clk_round_rate(), the boundaries of the
197 * two clocks are taken into account.
198 @@ -1033,12 +1061,50 @@ clk_test_single_parent_mux_set_range_rou
199 KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
203 + * Test that for a clock that can't modify its rate and with a single
204 + * parent, if we set a range on the child and a more restrictive one on
205 + * the parent, and then call clk_round_rate(), the boundaries of the
206 + * two clocks are taken into account.
209 +clk_test_single_parent_mux_set_range_round_rate_parent_smaller(struct kunit *test)
211 + struct clk_single_parent_ctx *ctx = test->priv;
212 + struct clk_hw *hw = &ctx->hw;
213 + struct clk *clk = hw->clk;
214 + struct clk *parent;
215 + unsigned long rate;
218 + parent = clk_get_parent(clk);
219 + KUNIT_ASSERT_PTR_NE(test, parent, NULL);
221 + ret = clk_set_rate_range(parent, DUMMY_CLOCK_RATE_1 + 1000, DUMMY_CLOCK_RATE_2 - 1000);
222 + KUNIT_ASSERT_EQ(test, ret, 0);
224 + ret = clk_set_rate_range(clk, DUMMY_CLOCK_RATE_1, DUMMY_CLOCK_RATE_2);
225 + KUNIT_ASSERT_EQ(test, ret, 0);
227 + rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_1 - 1000);
228 + KUNIT_ASSERT_GT(test, rate, 0);
229 + KUNIT_EXPECT_GE(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
230 + KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
232 + rate = clk_round_rate(clk, DUMMY_CLOCK_RATE_2 + 1000);
233 + KUNIT_ASSERT_GT(test, rate, 0);
234 + KUNIT_EXPECT_GE(test, rate, DUMMY_CLOCK_RATE_1 + 1000);
235 + KUNIT_EXPECT_LE(test, rate, DUMMY_CLOCK_RATE_2 - 1000);
238 static struct kunit_case clk_single_parent_mux_test_cases[] = {
239 KUNIT_CASE(clk_test_single_parent_mux_get_parent),
240 KUNIT_CASE(clk_test_single_parent_mux_has_parent),
241 KUNIT_CASE(clk_test_single_parent_mux_set_range_disjoint_child_last),
242 KUNIT_CASE(clk_test_single_parent_mux_set_range_disjoint_parent_last),
243 KUNIT_CASE(clk_test_single_parent_mux_set_range_round_rate_child_smaller),
244 + KUNIT_CASE(clk_test_single_parent_mux_set_range_round_rate_parent_only),
245 + KUNIT_CASE(clk_test_single_parent_mux_set_range_round_rate_parent_smaller),
249 @@ -1945,7 +2011,117 @@ static struct kunit_suite clk_range_mini
250 .test_cases = clk_range_minimize_test_cases,
253 +struct clk_leaf_mux_ctx {
254 + struct clk_multiple_parent_ctx mux_ctx;
259 +clk_leaf_mux_set_rate_parent_test_init(struct kunit *test)
261 + struct clk_leaf_mux_ctx *ctx;
262 + const char *top_parents[2] = { "parent-0", "parent-1" };
265 + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
270 + ctx->mux_ctx.parents_ctx[0].hw.init = CLK_HW_INIT_NO_PARENT("parent-0",
271 + &clk_dummy_rate_ops,
273 + ctx->mux_ctx.parents_ctx[0].rate = DUMMY_CLOCK_RATE_1;
274 + ret = clk_hw_register(NULL, &ctx->mux_ctx.parents_ctx[0].hw);
278 + ctx->mux_ctx.parents_ctx[1].hw.init = CLK_HW_INIT_NO_PARENT("parent-1",
279 + &clk_dummy_rate_ops,
281 + ctx->mux_ctx.parents_ctx[1].rate = DUMMY_CLOCK_RATE_2;
282 + ret = clk_hw_register(NULL, &ctx->mux_ctx.parents_ctx[1].hw);
286 + ctx->mux_ctx.current_parent = 0;
287 + ctx->mux_ctx.hw.init = CLK_HW_INIT_PARENTS("test-mux", top_parents,
288 + &clk_multiple_parents_mux_ops,
290 + ret = clk_hw_register(NULL, &ctx->mux_ctx.hw);
294 + ctx->hw.init = CLK_HW_INIT_HW("test-clock", &ctx->mux_ctx.hw,
295 + &clk_dummy_single_parent_ops,
296 + CLK_SET_RATE_PARENT);
297 + ret = clk_hw_register(NULL, &ctx->hw);
304 +static void clk_leaf_mux_set_rate_parent_test_exit(struct kunit *test)
306 + struct clk_leaf_mux_ctx *ctx = test->priv;
308 + clk_hw_unregister(&ctx->hw);
309 + clk_hw_unregister(&ctx->mux_ctx.hw);
310 + clk_hw_unregister(&ctx->mux_ctx.parents_ctx[0].hw);
311 + clk_hw_unregister(&ctx->mux_ctx.parents_ctx[1].hw);
315 + * Test that, for a clock that will forward any rate request to its
316 + * parent, the rate request structure returned by __clk_determine_rate
317 + * is sane and will be what we expect.
319 +static void clk_leaf_mux_set_rate_parent_determine_rate(struct kunit *test)
321 + struct clk_leaf_mux_ctx *ctx = test->priv;
322 + struct clk_hw *hw = &ctx->hw;
323 + struct clk *clk = hw->clk;
324 + struct clk_rate_request req;
325 + unsigned long rate;
328 + rate = clk_get_rate(clk);
329 + KUNIT_ASSERT_EQ(test, rate, DUMMY_CLOCK_RATE_1);
331 + clk_hw_init_rate_request(hw, &req, DUMMY_CLOCK_RATE_2);
333 + ret = __clk_determine_rate(hw, &req);
334 + KUNIT_ASSERT_EQ(test, ret, 0);
336 + KUNIT_EXPECT_EQ(test, req.rate, DUMMY_CLOCK_RATE_2);
337 + KUNIT_EXPECT_EQ(test, req.best_parent_rate, DUMMY_CLOCK_RATE_2);
338 + KUNIT_EXPECT_PTR_EQ(test, req.best_parent_hw, &ctx->mux_ctx.hw);
341 +static struct kunit_case clk_leaf_mux_set_rate_parent_test_cases[] = {
342 + KUNIT_CASE(clk_leaf_mux_set_rate_parent_determine_rate),
347 + * Test suite for a clock whose parent is a mux with multiple parents.
348 + * The leaf clock has CLK_SET_RATE_PARENT, and will forward rate
349 + * requests to the mux, which will then select which parent is the best
350 + * fit for a given rate.
352 + * These tests are supposed to exercise the behaviour of muxes, and the
353 + * proper selection of parents.
355 +static struct kunit_suite clk_leaf_mux_set_rate_parent_test_suite = {
356 + .name = "clk-leaf-mux-set-rate-parent",
357 + .init = clk_leaf_mux_set_rate_parent_test_init,
358 + .exit = clk_leaf_mux_set_rate_parent_test_exit,
359 + .test_cases = clk_leaf_mux_set_rate_parent_test_cases,
363 + &clk_leaf_mux_set_rate_parent_test_suite,
365 &clk_multiple_parents_mux_test_suite,
366 &clk_orphan_transparent_multiple_parent_mux_test_suite,