Re: [PATCH] clk: Ensure correct consumer's rate boundaries
From: Chuan Liu
Date: Wed Jan 14 2026 - 21:38:49 EST
Hi Brian,
On 1/15/2026 9:30 AM, Brian Masney wrote:
[ EXTERNAL EMAIL ]
Hi Chuan,
On Fri, Jan 09, 2026 at 11:24:22AM +0800, Chuan Liu via B4 Relay wrote:
From: Chuan Liu <chuan.liu@xxxxxxxxxxx>
If we were to have two users of the same clock, doing something like:
clk_set_rate_range(user1, 1000, 2000);
clk_set_rate_range(user2, 3000, 4000);
Even when user2's call returns -EINVAL, the min_rate and max_rate of
user2 are still incorrectly updated. This causes subsequent calls by
user1 to fail when setting the clock rate, as clk_core_get_boundaries()
returns corrupted boundaries (min_rate = 3000, max_rate = 2000).
To prevent this, clk_core_check_boundaries() now rollback to the old
boundaries when the check fails.
Signed-off-by: Chuan Liu <chuan.liu@xxxxxxxxxxx>
---
drivers/clk/clk.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 85d2f2481acf..0dfb16bf3f31 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2710,13 +2710,17 @@ static int clk_set_rate_range_nolock(struct clk *clk,
*/
rate = clamp(rate, min, max);
ret = clk_core_set_rate_nolock(clk->core, rate);
+
+out:
if (ret) {
- /* rollback the changes */
+ /*
+ * Rollback the consumer’s old boundaries if check_boundaries or
+ * set_rate fails.
+ */
clk->min_rate = old_min;
clk->max_rate = old_max;
}
-out:
if (clk->exclusive_count)
clk_core_rate_protect(clk->core);
This looks correct to me. Just a quick question though to possibly
simplify this further. Currently clk_set_rate_range_nolock() has the
following code:
/* Save the current values in case we need to rollback the change */
old_min = clk->min_rate;
old_max = clk->max_rate;
clk->min_rate = min;
clk->max_rate = max;
if (!clk_core_check_boundaries(clk->core, min, max)) {
ret = -EINVAL;
goto out;
}
Since clk_core_check_boundaries() is a readonly operation, what do you
think about moving clk_core_check_boundaries above the code that saves the
previous values? That way we only need to rollback in the case where
set_rate() fails.
Perhaps it would be more appropriate to move the clk_core_check_boundaries() check before saving the previous values, like this:
if (!clk_core_check_boundaries(clk->core, min, max)) {
ret = -EINVAL;
goto out;
}
/* Save the current values in case we need to rollback the change */
old_min = clk->min_rate;
old_max = clk->max_rate;
clk->min_rate = min;
clk->max_rate = max;
The changes in this patch are intended to avoid altering the original driver execution flow, while making the minimal modification to fix the issue where the range is incorrectly assigned.
Brian