Re: [PATCH v2 3/3] clk: Drop the rate range on clk_put

[Maxime Ripard <maxime@cerno.tech>]

[-- Attachment #1.1: Type: text/plain, Size: 3443 bytes --]

Hi Tony,

On Thu, Mar 31, 2022 at 06:00:42PM +0300, Tony Lindgren wrote:
> * Maxime Ripard <maxime@cerno.tech> [220331 09:52]:
> > On Thu, Mar 31, 2022 at 12:42:10PM +0300, Tony Lindgren wrote:
> > > It seems the dts assigned-clock-parents no longer works now?
> > 
> > That would make some kind of sense, __set_clk_parents calls clk_put on
> > both the assigned clock and its parent.
> > 
> > Could you see what parent (and why?) it tries to enforce then?
> 
> It picks the other option available for the mux clock that only has
> two options. No idea why, but if you have some debug patch in mind I
> can give it a try.
> 
> > It looks like the gpt1_fck driver might favor another parent for that
> > rate, which, if it's an invalid configuration, shouldn't really happen?
> 
> Hmm there's a gate clock and a mux clock, there's not really a rate
> selection available here for the sources.

If I followed the OMAP driver properly, clk_mux_determine_rate_flags is
doing the heavy lifting, could you run your test with

-- >8 --

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 91f863b7a824..c2ab1be62f96 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -552,6 +552,12 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 	unsigned long best = 0;
 	struct clk_rate_request parent_req = *req;

+	pr_crit("%s: requested rate %lu\n", __func__, req->rate);
+
+	parent = core->parent;
+	pr_crit("%s: current parent %s\n", __func__, parent->name);
+	pr_crit("%s: current parent rate %lu\n", __func__, clk_core_get_rate_nolock(parent));
+
 	/* if NO_REPARENT flag set, pass through to current parent */
 	if (core->flags & CLK_SET_RATE_NO_REPARENT) {
 		parent = core->parent;
@@ -578,6 +584,11 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 		if (!parent)
 			continue;

+		pr_crit("%s: Trying parent %s (%lu)\n",
+			__func__,
+			parent->name,
+			clk_core_get_rate_nolock(parent));
+
 		if (core->flags & CLK_SET_RATE_PARENT) {
 			parent_req = *req;
 			ret = __clk_determine_rate(parent->hw, &parent_req);
@@ -603,6 +614,11 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 	req->best_parent_rate = best;
 	req->rate = best;

+	pr_crit("%s: Best parent %s (%lu)\n",
+		__func__,
+		best_parent->name,
+		best);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(clk_mux_determine_rate_flags);
@@ -2201,8 +2217,12 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
 	if (!core)
 		return 0;

+	pr_crit("%s: rate %lu\n", __func__, req_rate);
+
 	rate = clk_core_req_round_rate_nolock(core, req_rate);

+	pr_crit("%s: rounded rate %lu\n", __func__, req_rate);
+
 	/* bail early if nothing to do */
 	if (rate == clk_core_get_rate_nolock(core))
 		return 0;
@@ -2367,6 +2387,8 @@ static int clk_set_rate_range_nolock(struct clk *clk,
 		goto out;
 	}

+	pr_crit("%s: core req rate %lu\n", __func__, clk->core->req_rate);
+
 	/*
 	 * Since the boundaries have been changed, let's give the
 	 * opportunity to the provider to adjust the clock rate based on
@@ -2385,6 +2407,9 @@ static int clk_set_rate_range_nolock(struct clk *clk,
 	 *   this corner case when determining the rate
 	 */
 	rate = clamp(clk->core->req_rate, min, max);
+
+	pr_crit("%s: clamped rate %lu\n", __func__, rate);
+
 	ret = clk_core_set_rate_nolock(clk->core, rate);
 	if (ret) {
 		/* rollback the changes */

-- >8 --

Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel