Quoting Maxime Ripard (2023-04-04 03:10:50)
Hi,
This is a follow-up to a previous series that was printing a warning when a mux has a set_parent implementation but is missing determine_rate().
The rationale is that set_parent() is very likely to be useful when changing the rate, but it's determine_rate() that takes the parenting decision. If we're missing it, then the current parent is always going to be used, and thus set_parent() will not be used. The only exception being a direct call to clk_set_parent(), but those are fairly rare compared to clk_set_rate().
Stephen then asked to promote the warning to an error, and to fix up all the muxes that are in that situation first. So here it is :)
Thanks for resending.
I was thinking that we apply this patch first and then set determine_rate clk_ops without setting the clk flag. The function name is up for debate.
---8<--- diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 27c30a533759..057dd3ca8920 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -594,45 +594,57 @@ clk_core_forward_rate_req(struct clk_core *core, req->max_rate = old_req->max_rate; }
-int clk_mux_determine_rate_flags(struct clk_hw *hw, - struct clk_rate_request *req, - unsigned long flags) +static int +clk_core_determine_rate_noreparent(struct clk_core *core, + struct clk_rate_request *req) { - struct clk_core *core = hw->core, *parent, *best_parent = NULL; - int i, num_parents, ret; + struct clk_core *parent; + int ret; unsigned long best = 0;
- /* if NO_REPARENT flag set, pass through to current parent */ - if (core->flags & CLK_SET_RATE_NO_REPARENT) { - parent = core->parent; - if (core->flags & CLK_SET_RATE_PARENT) { - struct clk_rate_request parent_req; - - if (!parent) { - req->rate = 0; - return 0; - } + parent = core->parent; + if (core->flags & CLK_SET_RATE_PARENT) { + struct clk_rate_request parent_req;
- clk_core_forward_rate_req(core, req, parent, &parent_req, req->rate); + if (!parent) { + req->rate = 0; + return 0; + }
- trace_clk_rate_request_start(&parent_req); + clk_core_forward_rate_req(core, req, parent, &parent_req, req->rate);
- ret = clk_core_round_rate_nolock(parent, &parent_req); - if (ret) - return ret; + trace_clk_rate_request_start(&parent_req);
- trace_clk_rate_request_done(&parent_req); + ret = clk_core_round_rate_nolock(parent, &parent_req); + if (ret) + return ret;
- best = parent_req.rate; - } else if (parent) { - best = clk_core_get_rate_nolock(parent); - } else { - best = clk_core_get_rate_nolock(core); - } + trace_clk_rate_request_done(&parent_req);
- goto out; + best = parent_req.rate; + } else if (parent) { + best = clk_core_get_rate_nolock(parent); + } else { + best = clk_core_get_rate_nolock(core); }
+ req->rate = best; + + return 0; +} + +int clk_mux_determine_rate_flags(struct clk_hw *hw, + struct clk_rate_request *req, + unsigned long flags) +{ + struct clk_core *core = hw->core, *parent, *best_parent = NULL; + int i, num_parents, ret; + unsigned long best = 0; + + /* if NO_REPARENT flag set, pass through to current parent */ + if (core->flags & CLK_SET_RATE_NO_REPARENT) + return clk_core_determine_rate_noreparent(core, req); + /* find the parent that can provide the fastest rate <= rate */ num_parents = core->num_parents; for (i = 0; i < num_parents; i++) { @@ -670,9 +682,7 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw, if (!best_parent) return -EINVAL;
-out: - if (best_parent) - req->best_parent_hw = best_parent->hw; + req->best_parent_hw = best_parent->hw; req->best_parent_rate = best; req->rate = best;
@@ -772,6 +782,24 @@ int __clk_mux_determine_rate_closest(struct clk_hw *hw, } EXPORT_SYMBOL_GPL(__clk_mux_determine_rate_closest);
+/** + * clk_hw_determine_rate_noreparent - clk_ops::determine_rate implementation for a clk that doesn't reparent + * @hw: clk to determine rate on + * @req: rate request + * + * Helper for finding best parent rate to provide a given frequency. This can + * be used directly as a determine_rate callback (e.g. for a mux), or from a + * more complex clock that may combine a mux with other operations. + * + * Returns: 0 on success, -EERROR value on error + */ +int clk_hw_determine_rate_noreparent(struct clk_hw *hw, + struct clk_rate_request *req) +{ + return clk_core_determine_rate_noreparent(hw->core, req); +} +EXPORT_SYMBOL_GPL(clk_hw_determine_rate_noreparent); + /*** clk api ***/
static void clk_core_rate_unprotect(struct clk_core *core) diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 28ff6f1a6ada..958977231ff7 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -1333,6 +1333,8 @@ int __clk_mux_determine_rate_closest(struct clk_hw *hw, int clk_mux_determine_rate_flags(struct clk_hw *hw, struct clk_rate_request *req, unsigned long flags); +int clk_hw_determine_rate_noreparent(struct clk_hw *hw, + struct clk_rate_request *req); void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent); void clk_hw_get_rate_range(struct clk_hw *hw, unsigned long *min_rate, unsigned long *max_rate);