Re: [PATCH 2/5] clk: spacemit: make MIX rate selection consistent

From: Yao Zi

Date: Thu Sep 10 2026 - 09:28:47 EST


On Wed, Sep 09, 2026 at 10:07:02PM +0800, Troy Mitchell wrote:
> CCF passes the selected parent's rate to set_rate(). Searching other
> parents at that point can produce a divider for a different source,
> making the programmed rate disagree with CCF's selection.
>
> Restrict divider selection to the supplied parent rate and use the same
> rounding as divider_recalc_rate(). Track the best error separately so
> low-rate requests do not leave the initial zero-Hz candidate selected.
> Skip zero-rate parents and have determine_rate() reject requests when no
> usable parent exists.
>
> Fixes: 1b72c59db0ad ("clk: spacemit: Add clock support for SpacemiT K1 SoC")
> Signed-off-by: Troy Mitchell <troy.mitchell@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/clk/spacemit/ccu_mix.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/spacemit/ccu_mix.c b/drivers/clk/spacemit/ccu_mix.c
> index a8b407049bf4d..da3c5685d4f65 100644
> --- a/drivers/clk/spacemit/ccu_mix.c
> +++ b/drivers/clk/spacemit/ccu_mix.c
> @@ -107,22 +107,27 @@ ccu_mix_calc_best_rate(struct clk_hw *hw, unsigned long rate,
> struct ccu_mix *mix = hw_to_ccu_mix(hw);
> unsigned int parent_num = clk_hw_get_num_parents(hw);
> struct ccu_div_config *div = &mix->div;
> - u32 div_max = 1 << div->width;
> unsigned long best_rate = 0;
> + unsigned long best_delta = ULONG_MAX;
>
> for (int i = 0; i < parent_num; i++) {
> struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
> unsigned long parent_rate;
> + u32 div_max = 1 << div->width;

div_max should be invariant across iterations. Is there a reason moving
it inside the loop?

> if (!parent)
> continue;
>
> parent_rate = clk_hw_get_rate(parent);
> + if (!parent_rate)
> + continue;
>
> for (int j = 1; j <= div_max; j++) {
> - unsigned long tmp = DIV_ROUND_CLOSEST_ULL(parent_rate, j);
> + unsigned long tmp = DIV_ROUND_UP_ULL(parent_rate, j);
> + unsigned long delta = abs_diff(tmp, rate);
>
> - if (abs(tmp - rate) < abs(best_rate - rate)) {
> + if (delta < best_delta) {
> + best_delta = delta;
> best_rate = tmp;
>
> if (div_val)

Regards,
Yao Zi