Hi Dave,
On 02:39 Sat 03 Oct , Dave Klotz wrote:
> ccu_mix_set_rate() picks the best parent and divider but writes only
> the divider. U-Boot's CCF has no determine_rate step that would reparent
> the clock first, so the clock keeps running from whatever source the
> mux selects, at a rate the caller never asked for. Worse, when the
> hardware mux selects a source that this build's parent list does not
> contain (the K1 SPL lists only pll1_d6_409p6 for the SDH clocks), init
> registers the clock under parents[0] and the clock framework's idea of
> the parent no longer matches the hardware.
>
> Reparent the clock to the chosen parent with clk_set_parent() before
> programming the divider, as the clock core does in Linux. Do so also
> when the framework already has that parent but the mux selects another
> source. Nothing is written when the mux and divider already match, so a
> clock left running at the requested rate by an earlier stage is not
> disturbed.
>
> Signed-off-by: Dave Klotz <[email protected]>
> ---
> Changes in v2:
> - Reparent through clk_set_parent() before programming the divider, as
> suggested by Yixun Lan. CCF enables the new parent only when the clock
> itself is enabled.
> - Reparent also when only the hardware mux disagrees with the chosen
> parent, including the SPL's single-parent clock registration case.
> - Share parent-index lookup with ccu_mux_set_parent(). Keep the no-write
> case when the mux and divider already match.
>
> v1: https://lists.u-boot-project.org/pipermail/u-boot/2026-October/633805.html
>
> The parent/divider selection algorithm is unchanged from v1. This
> revision changes how the selected parent is applied.
>
> Retested on 2026-10-02, with the integrated board-support lane on next
> bd2f5794b7e2, OpenSBI v1.9, and "clk: spacemit: ccu_mix: fix inverted
> condition in ccu_mix_trigger_fc()" from Yixun's k3/for-next tree:
> - OrangePi R2S: eMMC SPL boot, SDH2 I/O clock 204.8 MHz instead of the
> BootROM's 409.6 MHz; U-Boot HS200 at 200 MHz/8-bit, normal EFI and
> FreeBSD boot, and a final unattended reboot.
> - OrangePi RV2: microSD SPL boot, U-Boot SD High Speed at 50 MHz, EFI and
> FreeBSD boot with root on NVMe; two boots checked.
>
> Both CPU clusters remain at 1600 MHz on both boards, with SSH working
> following boot. The R2S, RV2, generic K1 and RV2 SPI configurations build
> SPL and U-Boot proper without compiler warnings; checkpatch passes.
> Generic K1 and RV2 SPI are BUILD-ONLY for this revision. No before/after
> performance benchmarks were run.
>
> drivers/clk/spacemit/clk_mix.c | 79 ++++++++++++++++++++++++----------
> 1 file changed, 56 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/clk/spacemit/clk_mix.c b/drivers/clk/spacemit/clk_mix.c
> index a1158512a9..fc8e0fd41d 100644
> --- a/drivers/clk/spacemit/clk_mix.c
> +++ b/drivers/clk/spacemit/clk_mix.c
> @@ -136,14 +136,63 @@ ccu_mix_calc_best_rate(struct clk *clk, unsigned long
> rate,
> return best_rate;
> }
>
..
> +static u8 ccu_mux_get_parent(struct clk *clk)
This problem isn't introduced in this patch, but while review it..
I have difficulty to understand the function name with what it exactly do
here.. I think it trys to retrieve current parent index where the hardware
programmed to, so I think it would be better to rename as
ccu_mux_current_parent_index() or ccu_mux_get_parent_index()
> +{
> + struct ccu_mix *mix = clk_to_ccu_mix(clk);
> + struct ccu_mux_config *mux = &mix->mux;
..
> + u8 parent;
I'd like to simply name it as 'index' instead of 'parent', we already have
"struct clk *parent" which will brings confusion..
> +
> + parent = ccu_read(&mix->common, ctrl) >> mux->shift;
> + parent &= (1 << mux->width) - 1;
> +
> + return parent;
> +}
> +
> +static int ccu_mux_parent_index(struct ccu_common *common, struct clk
> *parent)
same reasone, how about ccu_mux_wanted_parent_index()? it's a parent index that
expected to program to the hardware..
> +{
> + int i;
> +
> + for (i = 0; i < common->num_parents; i++)
> + if (!strcmp(parent->dev->name, common->parents[i]))
> + return i;
> +
> + return -EINVAL;
> +}
> +
The patch looks good to me, so with or without changes
Reviewed-by: Yixun Lan <[email protected]>
--
Yixun Lan (dlan)
