Skip to content

Commit 8cd9c39

Browse files
mripardbebarino
authored andcommitted
clk: Move clk_core_init_rate_req() from clk_core_round_rate_nolock() to its caller
The clk_rate_request structure is used internally as an argument for the clk_core_determine_round_nolock() and clk_core_round_rate_nolock(). In both cases, the clk_core_init_rate_req() function is used to initialize the clk_rate_request structure. However, the expectation on who gets to call that function is inconsistent between those two functions. Indeed, clk_core_determine_round_nolock() will assume the structure is properly initialized and will just use it. On the other hand, clk_core_round_rate_nolock() will call clk_core_init_rate_req() itself, expecting the caller to have filled only a minimal set of parameters (rate, min_rate and max_rate). If we ignore the calling convention inconsistency, this leads to a second inconsistency for drivers: * If they get called by the framework through clk_core_round_rate_nolock(), the rate, min_rate and max_rate fields will be filled by the caller, and the best_parent_rate and best_parent_hw fields will get filled by clk_core_init_rate_req(). * If they get called by a driver through __clk_determine_rate (and thus clk_core_round_rate_nolock), only best_parent_rate and best_parent_hw are being explicitly set by the framework. Even though we can reasonably expect rate to be set, only one of the 6 in-tree users explicitly set min_rate and max_rate. * If they get called by the framework through clk_core_determine_round_nolock(), then we have two callpaths. Either it will be called by clk_core_round_rate_nolock() itself, or it will be called by clk_calc_new_rates(), which will properly initialize rate, min_rate, max_rate itself, and best_parent_rate and best_parent_hw through clk_core_init_rate_req(). Even though the first and third case seems equivalent, they aren't when the clock has CLK_SET_RATE_PARENT. Indeed, in such a case clk_core_round_rate_nolock() will call itself on the current parent clock with the same clk_rate_request structure. The clk_core_init_rate_req() function will then be called on the parent clock, with the child clk_rate_request pointer and will fill the best_parent_rate and best_parent_hw fields with the parent context. When the whole recursion stops and the call returns, the initial caller will end up with a clk_rate_request structure with some information of the child clock (rate, min_rate, max_rate) and some others of the last clock up the tree whose child had CLK_SET_RATE_PARENT (best_parent_hw, best_parent_rate). In the most common case, best_parent_rate is going to be equal on all the parent clocks so it's not a big deal. However, best_parent_hw is going to point to a clock that never has been a valid parent for that clock which is definitely confusing. In order to fix the calling inconsistency, let's move the clk_core_init_rate_req() calls to the callers, which will also help a bit with the clk_core_round_rate_nolock() recursion. Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # imx8mp Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> # exynos4210, meson g12b Signed-off-by: Maxime Ripard <maxime@cerno.tech> Link: https://lore.kernel.org/r/20220816112530.1837489-16-maxime@cerno.tech Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
1 parent 718af79 commit 8cd9c39

File tree

1 file changed

+3
-5
lines changed

1 file changed

+3
-5
lines changed

drivers/clk/clk.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,8 +1415,6 @@ static int clk_core_round_rate_nolock(struct clk_core *core,
14151415
return 0;
14161416
}
14171417

1418-
clk_core_init_rate_req(core, req, req->rate);
1419-
14201418
if (clk_core_can_round(core))
14211419
return clk_core_determine_round_nolock(core, req);
14221420
else if (core->flags & CLK_SET_RATE_PARENT)
@@ -1464,8 +1462,8 @@ unsigned long clk_hw_round_rate(struct clk_hw *hw, unsigned long rate)
14641462
int ret;
14651463
struct clk_rate_request req;
14661464

1465+
clk_core_init_rate_req(hw->core, &req, rate);
14671466
clk_core_get_boundaries(hw->core, &req.min_rate, &req.max_rate);
1468-
req.rate = rate;
14691467

14701468
ret = clk_core_round_rate_nolock(hw->core, &req);
14711469
if (ret)
@@ -1497,8 +1495,8 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
14971495
if (clk->exclusive_count)
14981496
clk_core_rate_unprotect(clk->core);
14991497

1498+
clk_core_init_rate_req(clk->core, &req, rate);
15001499
clk_core_get_boundaries(clk->core, &req.min_rate, &req.max_rate);
1501-
req.rate = rate;
15021500

15031501
ret = clk_core_round_rate_nolock(clk->core, &req);
15041502

@@ -2209,8 +2207,8 @@ static unsigned long clk_core_req_round_rate_nolock(struct clk_core *core,
22092207
if (cnt < 0)
22102208
return cnt;
22112209

2210+
clk_core_init_rate_req(core, &req, req_rate);
22122211
clk_core_get_boundaries(core, &req.min_rate, &req.max_rate);
2213-
req.rate = req_rate;
22142212

22152213
ret = clk_core_round_rate_nolock(core, &req);
22162214

0 commit comments

Comments
 (0)