Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate

[Maxime Ripard]

On Sun, Jul 02, 2023 at 07:55:24PM +0200, Frank Oltmanns wrote:
> When finding the best rate for a NKM clock, consider rates that are
> higher than the requested rate, if the CCU_FEATURE_CLOSEST_RATE flag is
> set.
> 
> Accommodate ccu_mux_helper_determine_rate to this change.
> 
> Signed-off-by: Frank Oltmanns <frank@xxxxxxxxxxxx>
> ---
>  drivers/clk/sunxi-ng/ccu_mux.c | 23 +++++++++++++++-----
>  drivers/clk/sunxi-ng/ccu_nkm.c | 48 +++++++++++++++++++++++++++++++-----------
>  2 files changed, 54 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
> index 1d557e323169..8594d6a4addd 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.c
> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> @@ -113,7 +113,7 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common,
>  	}
>  
>  	for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
> -		unsigned long tmp_rate, parent_rate;
> +		unsigned long tmp_rate, parent_rate, best_diff = ULONG_MAX;
>  		struct clk_hw *parent;
>  
>  		parent = clk_hw_get_parent_by_index(hw, i);
> @@ -139,10 +139,23 @@ int ccu_mux_helper_determine_rate(struct ccu_common *common,
>  			goto out;
>  		}
>  
> -		if ((req->rate - tmp_rate) < (req->rate - best_rate)) {
> -			best_rate = tmp_rate;
> -			best_parent_rate = parent_rate;
> -			best_parent = parent;
> +		if (common->features & CCU_FEATURE_CLOSEST_RATE) {
> +			unsigned long tmp_diff = req->rate > tmp_rate ?
> +						 req->rate - tmp_rate :
> +						 tmp_rate - req->rate;
> +
> +			if (tmp_diff < best_diff) {
> +				best_rate = tmp_rate;
> +				best_parent_rate = parent_rate;
> +				best_parent = parent;
> +				best_diff = tmp_diff;
> +			}
> +		} else {
> +			if ((req->rate - tmp_rate) < (req->rate - best_rate)) {
> +				best_rate = tmp_rate;
> +				best_parent_rate = parent_rate;
> +				best_parent = parent;
> +			}
>  		}
>  	}

Like I said in the previous patch, I think we could do something like:

bool ccu_is_better_rate(struct ccu_common *common,
     			unsigned long target_rate,
			unsigned long current_rate,
			unsigned long best_rate)
{
	if (common->features & CCU_FEATURE_CLOSEST_RATE)
		return abs(current_rate - target_rate) < abs(best_rate - target_rate);

	return current_rate <= target_rate && current_rate > best_rate;
}

Then, the code above would look like:

if (ccu_is_better_rate(common, req->rate, tmp_rate, best_rate)) {
	best_rate = tmp_rate;
	best_parent_rate = parent_rate;
	best_parent = parent;
}

It's simpler, and we can share it easily between drivers.
Maxime

Attachment: signature.asc
Description: PGP signature