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

Maxime Ripard
2022-04-01T13:36:01Z

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

On Fri, Apr 01, 2022 at 03:07:10PM +0200, Alexander Stein wrote:
> > Does it also happen if you only apply the patch I had above, and not all
> > the debugging?
>
> Yes, these are the last lines I see:
> ---
> [ 1.236306] mmcblk0rpmb: mmc0:0001 DA6016 4.00 MiB, chardev (235:0)
> [ 1.241031] i2c i2c-1: IMX I2C adapter registered
> [ 1.251771] i2c i2c-3: IMX I2C adapter registered
> [ 1.256957] i2c i2c-5: IMX I2C adapter registered

Could you add on top of next (so dropping everything we did so far)

---- >8 -----
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 91f863b7a824..552b1e16a82d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -540,6 +540,8 @@ static bool mux_is_better_rate(unsigned long rate, unsigned long now,
if (flags & CLK_MUX_ROUND_CLOSEST)
return abs(now - rate) < abs(best - rate);

+ pr_crit("%s +%d rate %lu now %lu best %lu\n", __func__, __LINE__, rate, now, best);
+
return now <= rate && now > best;
}

@@ -552,6 +554,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: %s: requested rate %lu\n", __func__, core->name, req->rate);
+
+ parent = core->parent;
+ pr_crit("%s: %s: current parent %s\n", __func__, core->name, parent ? parent->name : "(null)");
+ pr_crit("%s: %s: current parent rate %lu\n", __func__, core->name, 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,24 +586,37 @@ 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) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
parent_req = *req;
ret = __clk_determine_rate(parent->hw, &parent_req);
+ pr_crit("%s +%d %d\n", __func__, __LINE__, ret);
if (ret)
continue;
} else {
+ pr_crit("%s +%d\n", __func__, __LINE__);
parent_req.rate = clk_core_get_rate_nolock(parent);
}

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
if (mux_is_better_rate(req->rate, parent_req.rate,
best, flags)) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
best_parent = parent;
best = parent_req.rate;
}
}

- if (!best_parent)
+ if (!best_parent) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
return -EINVAL;
+ }

out:
if (best_parent)
@@ -603,6 +624,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);
@@ -1345,11 +1371,15 @@ static int clk_core_determine_round_nolock(struct clk_core *core,

lockdep_assert_held(&prepare_lock);

+ pr_crit("%s +%d %s\n", __func__, __LINE__, core->name);
if (!core)
return 0;

+ pr_crit("%s +%d\n", __func__, __LINE__);
req->rate = clamp(req->rate, req->min_rate, req->max_rate);

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
/*
* At this point, core protection will be disabled
* - if the provider is not protected at all
@@ -1357,10 +1387,13 @@ static int clk_core_determine_round_nolock(struct clk_core *core,
* over the provider
*/
if (clk_core_rate_is_protected(core)) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
req->rate = core->rate;
} else if (core->ops->determine_rate) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
return core->ops->determine_rate(core->hw, req);
} else if (core->ops->round_rate) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
rate = core->ops->round_rate(core->hw, req->rate,
&req->best_parent_rate);
if (rate < 0)
@@ -1368,6 +1401,7 @@ static int clk_core_determine_round_nolock(struct clk_core *core,

req->rate = rate;
} else {
+ pr_crit("%s +%d\n", __func__, __LINE__);
return -EINVAL;
}

@@ -1402,17 +1436,26 @@ static int clk_core_round_rate_nolock(struct clk_core *core,
{
lockdep_assert_held(&prepare_lock);

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
if (!core) {
req->rate = 0;
return 0;
}

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
clk_core_init_rate_req(core, req);

- if (clk_core_can_round(core))
+ pr_crit("%s +%d\n", __func__, __LINE__);
+
+ if (clk_core_can_round(core)) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
return clk_core_determine_round_nolock(core, req);
- else if (core->flags & CLK_SET_RATE_PARENT)
+ } else if (core->flags & CLK_SET_RATE_PARENT) {
+ pr_crit("%s +%d\n", __func__, __LINE__);
return clk_core_round_rate_nolock(core->parent, req);
+ }

req->rate = core->rate;
return 0;
@@ -2201,21 +2244,31 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
if (!core)
return 0;

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

+ pr_crit("%s: %s: rounded rate %lu\n", __func__, core->name, req_rate);
+
/* bail early if nothing to do */
if (rate == clk_core_get_rate_nolock(core))
return 0;

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
/* fail on a direct rate set of a protected provider */
if (clk_core_rate_is_protected(core))
return -EBUSY;

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
/* calculate new rates and get the topmost changed clock */
top = clk_calc_new_rates(core, req_rate);
if (!top)
return -EINVAL;

+ pr_crit("%s +%d\n", __func__, __LINE__);
+
ret = clk_pm_runtime_get(core);
if (ret)
return ret;
@@ -2367,6 +2420,16 @@ static int clk_set_rate_range_nolock(struct clk *clk,
goto out;
}

+ pr_crit("%s: %s: orphan ? %c\n",
+ __func__,
+ clk->core->name,
+ clk->core->orphan ? 'y' : 'n');
+
+ pr_crit("%s: %s: core req rate %lu\n",
+ __func__,
+ clk->core->name,
+ 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
@@ -2384,7 +2447,11 @@ static int clk_set_rate_range_nolock(struct clk *clk,
* - the determine_rate() callback does not really check for
* this corner case when determining the rate
*/
+
rate = clamp(clk->core->req_rate, min, max);
+
+ pr_crit("%s: %s: clamped rate %lu\n", __func__, clk->core->name, rate);
+
ret = clk_core_set_rate_nolock(clk->core, rate);
if (ret) {
/* rollback the changes */
@@ -2599,6 +2666,8 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
} else {
__clk_recalc_rates(core, POST_RATE_CHANGE);
__clk_recalc_accuracies(core);
+
+ core->req_rate = core->rate;
}

runtime_put:
---- >8 -----

Thanks!
Maxime

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

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