[PATCH] clk: spacemit: program the mux when setting a mix clock's rate

[PATCH] clk: spacemit: program the mux when setting a mix clock's rate

[Dave Klotz]

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.

Write the chosen parent's mux index and the divider together and latch
both with one frequency change. Enable the new parent first and reparent
the clock device after, so the framework follows the hardware. 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 <DaveWK225@protonmail.com>
---
Tested on an OrangePi R2S booting from eMMC, where the SDH2 io clock
moves from the BootROM's 409.6 MHz to 204.8 MHz in the SPL, together
with "clk: spacemit: ccu_mix: fix inverted condition in
ccu_mix_trigger_fc()" from the K3 clock series, and on an OrangePi RV2
booting from microSD.

 drivers/clk/spacemit/clk_mix.c | 54 +++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/spacemit/clk_mix.c b/drivers/clk/spacemit/clk_mix.c
index a1158512a9..3521ab9374 100644
--- a/drivers/clk/spacemit/clk_mix.c
+++ b/drivers/clk/spacemit/clk_mix.c
@@ -9,6 +9,7 @@
  */
 
 #include <dm/device.h>
+#include <dm/device-internal.h>
 #include <dm/uclass.h>
 #include <div64.h>
 #include <regmap.h>
@@ -136,26 +137,63 @@ ccu_mix_calc_best_rate(struct clk *clk, unsigned long rate,
 	return best_rate;
 }
 
+/*
+ * U-Boot's CCF has no determine_rate, so the parent is chosen here along with
+ * the divider. The mux is programmed from the chosen parent rather than left
+ * where it was found: init falls back to parents[0] when the hardware mux
+ * selects a source this build does not list (the SPL lists only a subset), and
+ * the clock would otherwise keep running from that source at a rate nobody
+ * computed. Mux and divider are written together and latched by one frequency
+ * change, and nothing is written when both already match, so a clock left
+ * running by an earlier stage is not disturbed.
+ */
 static unsigned long ccu_mix_set_rate(struct clk *clk, unsigned long rate)
 {
 	struct ccu_mix *mix = clk_to_ccu_mix(clk);
 	struct ccu_common *common = &mix->common;
 	struct ccu_div_config *div = &mix->div;
-	u32 current_div, target_div, mask;
+	struct ccu_mux_config *mux = &mix->mux;
+	struct clk *parent = NULL;
+	unsigned long parent_rate;
+	u32 target_div = 0, mask, val;
+	int i, ret;
 
-	ccu_mix_calc_best_rate(clk, rate, NULL, NULL, &target_div);
+	ccu_mix_calc_best_rate(clk, rate, &parent, &parent_rate, &target_div);
 
-	current_div = ccu_read(common, ctrl) >> div->shift;
-	current_div &= (1 << div->width) - 1;
+	mask = GENMASK(div->width + div->shift - 1, div->shift);
+	val = target_div << div->shift;
+
+	if (mux->width && parent) {
+		for (i = 0; i < common->num_parents; i++)
+			if (!strcmp(parent->dev->name, common->parents[i]))
+				break;
+		if (i == common->num_parents)
+			return -EINVAL;
+		mask |= GENMASK(mux->width + mux->shift - 1, mux->shift);
+		val |= i << mux->shift;
+	} else {
+		parent = NULL;
+	}
 
-	if (current_div == target_div)
+	if ((ccu_read(common, ctrl) & mask) == val)
 		return 0;
 
-	mask = GENMASK(div->width + div->shift - 1, div->shift);
+	if (parent && clk->dev->parent != parent->dev) {
+		ret = clk_enable(parent);
+		if (ret)
+			return ret;
+	}
 
-	ccu_update(common, ctrl, mask, target_div << div->shift);
+	ccu_update(common, ctrl, mask, val);
 
-	return ccu_mix_trigger_fc(clk);
+	ret = ccu_mix_trigger_fc(clk);
+	if (ret)
+		return ret;
+
+	if (parent && clk->dev->parent != parent->dev)
+		return device_reparent(clk->dev, parent->dev);
+
+	return 0;
 }
 
 static u8 ccu_mux_get_parent(struct clk *clk)

base-commit: a000f2af240eaa996cec92f8d44bad96fdf3f898
-- 
2.55.0

Re: [PATCH] clk: spacemit: program the mux when setting a mix clock's rate

[Dave Klotz]

Also tested on the OrangePi R2S with upstream's K1 SDHCI driver from
next: the SPL moves the eMMC io clock from 409.6 MHz to 204.8 MHz and
loads U-Boot, which then runs the eMMC in HS200. The tree is upstream
next plus the posted K1 fixes and board support that is not posted yet.

Re: [PATCH] clk: spacemit: program the mux when setting a mix clock's rate

[Yixun Lan]

hi Dave,

On 21:29 Fri 02 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.
> 
> Write the chosen parent's mux index and the divider together and latch
> both with one frequency change. Enable the new parent first and reparent
> the clock device after, so the framework follows the hardware. 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 <DaveWK225@protonmail.com>
> ---
> Tested on an OrangePi R2S booting from eMMC, where the SDH2 io clock
> moves from the BootROM's 409.6 MHz to 204.8 MHz in the SPL, together
> with "clk: spacemit: ccu_mix: fix inverted condition in
> ccu_mix_trigger_fc()" from the K3 clock series, and on an OrangePi RV2
> booting from microSD.
> 
>  drivers/clk/spacemit/clk_mix.c | 54 +++++++++++++++++++++++++++++-----
>  1 file changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/spacemit/clk_mix.c b/drivers/clk/spacemit/clk_mix.c
> index a1158512a9..3521ab9374 100644
> --- a/drivers/clk/spacemit/clk_mix.c
> +++ b/drivers/clk/spacemit/clk_mix.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <dm/device.h>
> +#include <dm/device-internal.h>
>  #include <dm/uclass.h>
>  #include <div64.h>
>  #include <regmap.h>
> @@ -136,26 +137,63 @@ ccu_mix_calc_best_rate(struct clk *clk, unsigned long rate,
>  	return best_rate;
>  }
>  
> +/*
> + * U-Boot's CCF has no determine_rate, so the parent is chosen here along with
> + * the divider. The mux is programmed from the chosen parent rather than left
> + * where it was found: init falls back to parents[0] when the hardware mux
> + * selects a source this build does not list (the SPL lists only a subset), and
> + * the clock would otherwise keep running from that source at a rate nobody
> + * computed. Mux and divider are written together and latched by one frequency
> + * change, and nothing is written when both already match, so a clock left
> + * running by an earlier stage is not disturbed.
> + */
>  static unsigned long ccu_mix_set_rate(struct clk *clk, unsigned long rate)
>  {
>  	struct ccu_mix *mix = clk_to_ccu_mix(clk);
>  	struct ccu_common *common = &mix->common;
>  	struct ccu_div_config *div = &mix->div;
> -	u32 current_div, target_div, mask;
> +	struct ccu_mux_config *mux = &mix->mux;
> +	struct clk *parent = NULL;
> +	unsigned long parent_rate;
> +	u32 target_div = 0, mask, val;
> +	int i, ret;
>  
> -	ccu_mix_calc_best_rate(clk, rate, NULL, NULL, &target_div);
> +	ccu_mix_calc_best_rate(clk, rate, &parent, &parent_rate, &target_div);
>  
> -	current_div = ccu_read(common, ctrl) >> div->shift;
> -	current_div &= (1 << div->width) - 1;
> +	mask = GENMASK(div->width + div->shift - 1, div->shift);
> +	val = target_div << div->shift;
> +
> +	if (mux->width && parent) {
> +		for (i = 0; i < common->num_parents; i++)
> +			if (!strcmp(parent->dev->name, common->parents[i]))
> +				break;
> +		if (i == common->num_parents)
> +			return -EINVAL;
> +		mask |= GENMASK(mux->width + mux->shift - 1, mux->shift);
> +		val |= i << mux->shift;
> +	} else {
> +		parent = NULL;
> +	}
>  
> -	if (current_div == target_div)
> +	if ((ccu_read(common, ctrl) & mask) == val)
>  		return 0;
>  
> -	mask = GENMASK(div->width + div->shift - 1, div->shift);
> +	if (parent && clk->dev->parent != parent->dev) {
> +		ret = clk_enable(parent);
> +		if (ret)
> +			return ret;
> +	}
>  
.. this will enable parent, would be it ok to combine below
device_reparent() together? to simply use clk_set_parent(clk,
parent)? it make sense to me to reparent first then program
the divider

> -	ccu_update(common, ctrl, mask, target_div << div->shift);
> +	ccu_update(common, ctrl, mask, val);
>  
> -	return ccu_mix_trigger_fc(clk);
> +	ret = ccu_mix_trigger_fc(clk);
> +	if (ret)
> +		return ret;
> +
..
> +	if (parent && clk->dev->parent != parent->dev)
> +		return device_reparent(clk->dev, parent->dev);
> +
reparent clk
> +	return 0;
>  }
>  

-- 
Yixun Lan (dlan)

Re: [PATCH] clk: spacemit: program the mux when setting a mix clock's rate

[Dave Klotz]

Hi Yixun,

On Sat, Oct 03, 2026 at 01:04:52AM +0000, Yixun Lan wrote:
> .. this will enable parent, would be it ok to combine below
> device_reparent() together? to simply use clk_set_parent(clk,
> parent)? it make sense to me to reparent first then program
> the divider

Yes, that is better. clk_set_parent() only enables the new parent when
the clock itself is enabled, which also fixes the unbalanced
clk_enable() in this version, and it matches Linux, where the core calls
ccu_mux_set_parent() and then ccu_mix_set_rate(), each latched by its
own frequency change.

The one difference from this version is that, between the two frequency
changes, the clock briefly runs from the new parent with the old
divider. Linux has the same window, so I'll keep it simple unless you
would rather have the divider written first whenever that window would
exceed both the old and the new rate.

v2 now reparents through clk_set_parent() first, then writes the divider
and triggers the frequency change, and still writes nothing when the mux
and divider already match. It also reparents when the hardware mux
selects a different source even if the framework already has the chosen
parent. The parent/divider selection algorithm is unchanged.

I have retested the integrated board-support lane on next bd2f5794b7e2,
with OpenSBI v1.9 and your FC fix from k3/for-next:

- OrangePi R2S: eMMC SPL boot, SDH2 I/O clock 204.8 MHz, U-Boot HS200
  at 200 MHz/8-bit, EFI and FreeBSD boot, and an 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, and SSH works after
boot. The R2S, RV2, generic K1 and RV2 SPI configurations build SPL and
U-Boot proper without compiler warnings; the patch passes checkpatch.
Generic K1 and RV2 SPI are build-only checks for this revision. I have
not run before/after performance benchmarks.

Thanks,
Dave

[PATCH v2] clk: spacemit: program the mux when setting a mix clock's rate

[Dave Klotz]

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 <DaveWK225@protonmail.com>
---
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)
+{
+	struct ccu_mix *mix = clk_to_ccu_mix(clk);
+	struct ccu_mux_config *mux = &mix->mux;
+	u8 parent;
+
+	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)
+{
+	int i;
+
+	for (i = 0; i < common->num_parents; i++)
+		if (!strcmp(parent->dev->name, common->parents[i]))
+			return i;
+
+	return -EINVAL;
+}
+
+/*
+ * U-Boot's CCF has no determine_rate step, so the parent giving the best rate
+ * is chosen here. As in Linux, the clock is reparented first and the divider
+ * programmed after. Reparent also when the framework already has the chosen
+ * parent but the mux selects another source: init registers a clock under
+ * parents[0] when the mux selects a source this build does not list (the SPL
+ * lists only a subset). Nothing is written when mux and divider already match,
+ * so a clock left running by an earlier stage is not disturbed.
+ */
 static unsigned long ccu_mix_set_rate(struct clk *clk, unsigned long rate)
 {
 	struct ccu_mix *mix = clk_to_ccu_mix(clk);
 	struct ccu_common *common = &mix->common;
 	struct ccu_div_config *div = &mix->div;
-	u32 current_div, target_div, mask;
-
-	ccu_mix_calc_best_rate(clk, rate, NULL, NULL, &target_div);
+	struct ccu_mux_config *mux = &mix->mux;
+	struct clk *parent = NULL;
+	unsigned long parent_rate;
+	u32 current_div, target_div = 0, mask;
+	int index, ret;
+
+	ccu_mix_calc_best_rate(clk, rate, &parent, &parent_rate, &target_div);
+
+	if (mux->width && parent) {
+		index = ccu_mux_parent_index(common, parent);
+		if (index < 0)
+			return index;
+
+		if (clk->dev->parent != parent->dev ||
+		    ccu_mux_get_parent(clk) != index) {
+			ret = clk_set_parent(clk, parent);
+			if (ret)
+				return ret;
+		}
+	}
 
 	current_div = ccu_read(common, ctrl) >> div->shift;
 	current_div &= (1 << div->width) - 1;
@@ -158,35 +207,19 @@ static unsigned long ccu_mix_set_rate(struct clk *clk, unsigned long rate)
 	return ccu_mix_trigger_fc(clk);
 }
 
-static u8 ccu_mux_get_parent(struct clk *clk)
-{
-	struct ccu_mix *mix = clk_to_ccu_mix(clk);
-	struct ccu_mux_config *mux = &mix->mux;
-	u8 parent;
-
-	parent = ccu_read(&mix->common, ctrl) >> mux->shift;
-	parent &= (1 << mux->width) - 1;
-
-	return parent;
-}
-
 static int ccu_mux_set_parent(struct clk *clk, struct clk *parent)
 {
 	struct ccu_common *common = clk_to_ccu_common(clk);
 	struct ccu_mix *mix = clk_to_ccu_mix(clk);
 	struct ccu_mux_config *mux = &mix->mux;
 	u32 mask;
-	int i = 0;
+	int i;
 
 	mask = GENMASK(mux->width + mux->shift - 1, mux->shift);
 
-	for (i = 0; i < common->num_parents; i++) {
-		if (!strcmp(parent->dev->name, common->parents[i]))
-			break;
-	}
-
-	if (i == common->num_parents)
-		return -EINVAL;
+	i = ccu_mux_parent_index(common, parent);
+	if (i < 0)
+		return i;
 
 	ccu_update(&mix->common, ctrl, mask, i << mux->shift);
 
-- 
2.55.0

Re: [PATCH v2] clk: spacemit: program the mux when setting a mix clock's rate

[Yixun Lan]

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 <DaveWK225@protonmail.com>
> ---
> 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 <dlan@kernel.org>

--
Yixun Lan (dlan)

Re: [PATCH v2] clk: spacemit: program the mux when setting a mix clock's rate

[Dave Klotz]

Hi Yixun,

Thanks for the review. I've prepared v3 with
ccu_mux_current_parent_index() and ccu_mux_wanted_parent_index(),
updated every caller, and renamed the hardware-index local to index.

There are no functional changes from v2, and I've added your
Reviewed-by. Exact-base replay and the naming-only comparison pass.
Both board configurations build successfully.

During testing I found a separate overlapping DDR firmware copy in SPL;
I've sent that fix separately. With it included, the combined firmware
passed DDR initialization, FIT verification and automatic EFI/FreeBSD
boot on both the 2 GiB R2S and the 4 GiB RV2. These are combined-firmware
warm-boot results, not isolated tests of the clock patch. Cold boot of
the corrected firmware remains untested.

Thanks,
Dave

[PATCH v3] clk: spacemit: program the mux when setting a mix clock's rate

[Dave Klotz]

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 <DaveWK225@protonmail.com>
Reviewed-by: Yixun Lan <dlan@kernel.org>
---
Changes in v3:
- Rename ccu_mux_get_parent() to ccu_mux_current_parent_index(), including
  the set-rate and clock-registration callers.
- Rename ccu_mux_parent_index() to ccu_mux_wanted_parent_index(), including
  the set-rate and set-parent callers.
- Name the hardware parent-index local variable index instead of parent.
- Carry Yixun Lan's Reviewed-by. No functional changes from v2.

Changes in v2:
- Reparent through clk_set_parent() before programming the divider.
- Reparent also when the hardware mux disagrees with the chosen parent.
- Share parent-index lookup with ccu_mux_set_parent() and preserve the
  no-write case when mux and divider already match.

v2: https://lists.u-boot-project.org/pipermail/u-boot/2026-October/633842.html
Review: https://lists.u-boot-project.org/pipermail/u-boot/2026-October/633911.html

Validation: exact-base replay and inverse-renaming comparison against
v2 pass, with zero checkpatch errors or warnings. Both R2S and RV2
configurations build successfully.

An integration containing this patch, the MMC corrections and a separate
DDR firmware overlap fix passed warm-boot tests on a 2 GiB OrangePi R2S
and a 4 GiB OrangePi RV2. Both reached DDR/SPL, FIT verification and
automatic EFI boot to their existing FreeBSD 16 installation, with the
expected memory and eight CPUs. These are combined-integration results,
not isolated tests of this patch. Cold boot of the corrected firmware
and clock error paths remain untested.

 drivers/clk/spacemit/clk_mix.c | 87 +++++++++++++++-------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/spacemit/clk_mix.c b/drivers/clk/spacemit/clk_mix.c
index a115851..b626295 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_current_parent_index(struct clk *clk)
+{
+	struct ccu_mix *mix = clk_to_ccu_mix(clk);
+	struct ccu_mux_config *mux = &mix->mux;
+	u8 index;
+
+	index = ccu_read(&mix->common, ctrl) >> mux->shift;
+	index &= (1 << mux->width) - 1;
+
+	return index;
+}
+
+static int ccu_mux_wanted_parent_index(struct ccu_common *common, struct clk *parent)
+{
+	int i;
+
+	for (i = 0; i < common->num_parents; i++)
+		if (!strcmp(parent->dev->name, common->parents[i]))
+			return i;
+
+	return -EINVAL;
+}
+
+/*
+ * U-Boot's CCF has no determine_rate step, so the parent giving the best rate
+ * is chosen here. As in Linux, the clock is reparented first and the divider
+ * programmed after. Reparent also when the framework already has the chosen
+ * parent but the mux selects another source: init registers a clock under
+ * parents[0] when the mux selects a source this build does not list (the SPL
+ * lists only a subset). Nothing is written when mux and divider already match,
+ * so a clock left running by an earlier stage is not disturbed.
+ */
 static unsigned long ccu_mix_set_rate(struct clk *clk, unsigned long rate)
 {
 	struct ccu_mix *mix = clk_to_ccu_mix(clk);
 	struct ccu_common *common = &mix->common;
 	struct ccu_div_config *div = &mix->div;
-	u32 current_div, target_div, mask;
-
-	ccu_mix_calc_best_rate(clk, rate, NULL, NULL, &target_div);
+	struct ccu_mux_config *mux = &mix->mux;
+	struct clk *parent = NULL;
+	unsigned long parent_rate;
+	u32 current_div, target_div = 0, mask;
+	int index, ret;
+
+	ccu_mix_calc_best_rate(clk, rate, &parent, &parent_rate, &target_div);
+
+	if (mux->width && parent) {
+		index = ccu_mux_wanted_parent_index(common, parent);
+		if (index < 0)
+			return index;
+
+		if (clk->dev->parent != parent->dev ||
+		    ccu_mux_current_parent_index(clk) != index) {
+			ret = clk_set_parent(clk, parent);
+			if (ret)
+				return ret;
+		}
+	}
 
 	current_div = ccu_read(common, ctrl) >> div->shift;
 	current_div &= (1 << div->width) - 1;
@@ -158,35 +207,19 @@ static unsigned long ccu_mix_set_rate(struct clk *clk, unsigned long rate)
 	return ccu_mix_trigger_fc(clk);
 }
 
-static u8 ccu_mux_get_parent(struct clk *clk)
-{
-	struct ccu_mix *mix = clk_to_ccu_mix(clk);
-	struct ccu_mux_config *mux = &mix->mux;
-	u8 parent;
-
-	parent = ccu_read(&mix->common, ctrl) >> mux->shift;
-	parent &= (1 << mux->width) - 1;
-
-	return parent;
-}
-
 static int ccu_mux_set_parent(struct clk *clk, struct clk *parent)
 {
 	struct ccu_common *common = clk_to_ccu_common(clk);
 	struct ccu_mix *mix = clk_to_ccu_mix(clk);
 	struct ccu_mux_config *mux = &mix->mux;
 	u32 mask;
-	int i = 0;
+	int i;
 
 	mask = GENMASK(mux->width + mux->shift - 1, mux->shift);
 
-	for (i = 0; i < common->num_parents; i++) {
-		if (!strcmp(parent->dev->name, common->parents[i]))
-			break;
-	}
-
-	if (i == common->num_parents)
-		return -EINVAL;
+	i = ccu_mux_wanted_parent_index(common, parent);
+	if (i < 0)
+		return i;
 
 	ccu_update(&mix->common, ctrl, mask, i << mux->shift);
 
@@ -238,7 +271,7 @@ int spacemit_mux_init(struct ccu_common *common)
 	struct clk *clk = &common->clk;
 	u8 index;
 
-	index = ccu_mux_get_parent(clk);
+	index = ccu_mux_current_parent_index(clk);
 	if (index >= common->num_parents)
 		index = 0;
 
@@ -304,7 +337,7 @@ int spacemit_mux_gate_init(struct ccu_common *common)
 	struct clk *clk = &common->clk;
 	u8 index;
 
-	index = ccu_mux_get_parent(clk);
+	index = ccu_mux_current_parent_index(clk);
 	if (index >= common->num_parents)
 		index = 0;
 
@@ -353,7 +386,7 @@ int spacemit_mux_div_init(struct ccu_common *common)
 	struct clk *clk = &common->clk;
 	u8 index;
 
-	index = ccu_mux_get_parent(clk);
+	index = ccu_mux_current_parent_index(clk);
 	if (index >= common->num_parents)
 		index = 0;
 
@@ -379,7 +412,7 @@ int spacemit_mux_div_gate_init(struct ccu_common *common)
 	struct clk *clk = &common->clk;
 	u8 index;
 
-	index = ccu_mux_get_parent(clk);
+	index = ccu_mux_current_parent_index(clk);
 	if (index >= common->num_parents)
 		index = 0;
 

Re: [PATCH v3] clk: spacemit: program the mux when setting a mix clock's rate

[Yixun Lan]

On Mon, 05 Oct 2026 17:40:25 +0000, 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.
> 
> [...]

Applied, thanks!

[1/1] clk: spacemit: program the mux when setting a mix clock's rate
      https://git.u-boot-project.org/u-boot/custodians/u-boot-spacemit/-/commit/43798d61492bee6e9305a2cfe731daa81a701295

Best regards,
-- 
Yixun Lan <dlan@kernel.org>