Re: [PATCH] clk: Ensure correct consumer's rate boundaries

[Brian Masney <bmasney@redhat.com>]

Hi Chuan,

On Fri, Jan 09, 2026 at 11:24:22AM +0800, Chuan Liu via B4 Relay wrote:
> From: Chuan Liu <chuan.liu@amlogic.com>
> 
> 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@amlogic.com>
> ---
>  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.

Brian