Re: [PATCH v3 00/65] clk: Make determine_rate mandatory for muxes

[Stephen Boyd <sboyd@kernel.org>]

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);
-- 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/
https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git