clk: sunxi-ng: div: implement set_rate_and_parent

From: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>

When a rate change on a ccu_div clock also switches its parent, the
clk core, in the absence of a .set_rate_and_parent op, programs the
parent first and the divider second.  If the new parent is faster than
the old one, the clock transiently runs at new_parent_rate/old_divider
between the two register writes, overshooting both the old and the
requested rate and potentially violating the consumer's maximum
allowed frequency.

This is not theoretical.  On the Allwinner A523/T527 the GPU clock is
a ccu_div muxing between pll-gpu and the fixed pll-periph0 outputs:
going from 600 MHz (pll-periph0-600M, M=1) down to 400 or 200 MHz
(both derived from pll-periph0-800M) makes the Mali G57 run at 800 MHz
for the window between the two writes, 33% above the vendor's maximum
operating point.  Observed and validated on an Orange Pi 4A (T527)
with a downstream GPU devfreq setup; mainline does not yet describe
GPU OPPs for this SoC, but any ccu_div consumer whose set_rate ends up
crossing parents is affected.

Implement .set_rate_and_parent with the same ordering rule as
clk_composite_set_rate_and_parent(): if keeping the current divider
while switching the mux would overshoot the requested rate, program
the divider first, otherwise switch the mux first.  The intermediate
rate then never exceeds both the old and the new rate.

Clocks using a prediv feature keep the historical mux-then-divider
order: the prediv helpers look the prediv up by the current parent,
which is ambiguous while both are changing.

Fixes: e9b93213103f ("clk: sunxi-ng: Add divider")
Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
---
Note: this touches the same area of ccu_div.c as patch 2/4 of the
pending series "clk: sun6i-rtc: Add support for Allwinner A733 SoC" v5
(<20260717-a733-rtc-v5-2-3874cc26abf7@baylibre.com>), which adds
ccu_rodiv_ops right after ccu_div_ops.  The conflict is trivial
(context only); happy to rebase on top of it if it lands first.

 drivers/clk/sunxi-ng/ccu_div.c | 36 ++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)


base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
  

On Sun, Jul 19, 2026 at 11:31 PM Aureal
<juanmanuellopezcarrillo@gmail.com> wrote:
>
> From: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>
>
> When a rate change on a ccu_div clock also switches its parent, the
> clk core, in the absence of a .set_rate_and_parent op, programs the
> parent first and the divider second.  If the new parent is faster than
> the old one, the clock transiently runs at new_parent_rate/old_divider
> between the two register writes, overshooting both the old and the
> requested rate and potentially violating the consumer's maximum
> allowed frequency.
>
> This is not theoretical.  On the Allwinner A523/T527 the GPU clock is
> a ccu_div muxing between pll-gpu and the fixed pll-periph0 outputs:
> going from 600 MHz (pll-periph0-600M, M=1) down to 400 or 200 MHz
> (both derived from pll-periph0-800M) makes the Mali G57 run at 800 MHz
> for the window between the two writes, 33% above the vendor's maximum
> operating point.  Observed and validated on an Orange Pi 4A (T527)
> with a downstream GPU devfreq setup; mainline does not yet describe
> GPU OPPs for this SoC, but any ccu_div consumer whose set_rate ends up
> crossing parents is affected.
>
> Implement .set_rate_and_parent with the same ordering rule as
> clk_composite_set_rate_and_parent(): if keeping the current divider
> while switching the mux would overshoot the requested rate, program
> the divider first, otherwise switch the mux first.  The intermediate
> rate then never exceeds both the old and the new rate.
>
> Clocks using a prediv feature keep the historical mux-then-divider
> order: the prediv helpers look the prediv up by the current parent,
> which is ambiguous while both are changing.
>
> Fixes: e9b93213103f ("clk: sunxi-ng: Add divider")
> Signed-off-by: Juan Manuel López Carrillo <juanmanuellopezcarrillo@gmail.com>

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

So the patch itself is OK, but I think there is also something wrong
with the GPU mod clock. The GPU clock is not a standard divider, but
actually a fractional one. The formula for the output is

    Clock Source * ((16-M)/16)

with M being 0~15.

And we're also missing a notifier to switch the GPU clock away from
the PLL while the PLL is being changed.


Do you plan on implementing GPU DVFS on mainline?

> ---
> Note: this touches the same area of ccu_div.c as patch 2/4 of the
> pending series "clk: sun6i-rtc: Add support for Allwinner A733 SoC" v5
> (<20260717-a733-rtc-v5-2-3874cc26abf7@baylibre.com>), which adds
> ccu_rodiv_ops right after ccu_div_ops.  The conflict is trivial
> (context only); happy to rebase on top of it if it lands first.

That is manageable. Thanks for the heads up.


ChenYu

>
>  drivers/clk/sunxi-ng/ccu_div.c | 36 ++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> index 62d680ccb..9024bcd8c 100644
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -130,6 +130,41 @@ static int ccu_div_set_parent(struct clk_hw *hw, u8 index)
>         return ccu_mux_helper_set_parent(&cd->common, &cd->mux, index);
>  }
>
> +static int ccu_div_set_rate_and_parent(struct clk_hw *hw, unsigned long rate,
> +                                      unsigned long parent_rate, u8 index)
> +{
> +       struct ccu_div *cd = hw_to_ccu_div(hw);
> +
> +       /*
> +        * The predivider helpers look it up through the current parent,
> +        * which is ambiguous while both the parent and the divider are
> +        * changing, so keep the mux-then-divider order the core would
> +        * have used for those clocks.
> +        */
> +       if (cd->common.features & (CCU_FEATURE_VARIABLE_PREDIV |
> +                                  CCU_FEATURE_FIXED_PREDIV |
> +                                  CCU_FEATURE_ALL_PREDIV)) {
> +               ccu_div_set_parent(hw, index);
> +               return ccu_div_set_rate(hw, rate, parent_rate);
> +       }
> +
> +       /*
> +        * Same ordering rule as clk_composite_set_rate_and_parent(): if
> +        * switching the mux with the current divider would overshoot the
> +        * requested rate, program the divider first, so the intermediate
> +        * rate never exceeds both the old and the new rate.
> +        */
> +       if (ccu_div_recalc_rate(hw, parent_rate) > rate) {
> +               ccu_div_set_rate(hw, rate, parent_rate);
> +               ccu_div_set_parent(hw, index);
> +       } else {
> +               ccu_div_set_parent(hw, index);
> +               ccu_div_set_rate(hw, rate, parent_rate);
> +       }
> +
> +       return 0;
> +}
> +
>  const struct clk_ops ccu_div_ops = {
>         .disable        = ccu_div_disable,
>         .enable         = ccu_div_enable,
> @@ -141,5 +176,6 @@ const struct clk_ops ccu_div_ops = {
>         .determine_rate = ccu_div_determine_rate,
>         .recalc_rate    = ccu_div_recalc_rate,
>         .set_rate       = ccu_div_set_rate,
> +       .set_rate_and_parent = ccu_div_set_rate_and_parent,
>  };
>  EXPORT_SYMBOL_NS_GPL(ccu_div_ops, "SUNXI_CCU");
>
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
> --
> 2.47.3
>
  

> Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

Thanks a lot for the quick review!

> So the patch itself is OK, but I think there is also something wrong
> with the GPU mod clock. The GPU clock is not a standard divider, but
> actually a fractional one. The formula for the output is
>
>     Clock Source * ((16-M)/16)
>
> with M being 0~15.

You were right, and it was worse than I expected.  I checked the T527
user manual (v0.92, section 2.7.6.58: "FACTOR_M: mask M cycles at 16
cycles") and then measured it on the Orange Pi 4A, forcing each OPP
and reading the real GPU frequency with the Mali cycle counter.  With
the current linear model:

  OPP request    programmed    measured
  150 MHz        600M, M=3     ~487 MHz
  200 MHz        800M, M=3      648 MHz
  300 MHz        600M, M=1      560 MHz
  400 MHz        800M, M=1      749 MHz
  600 MHz        600M, M=0      599 MHz

so every OPP below 600 MHz silently overclocks (thermal throttling to
"400 MHz" actually raises the clock to 750 MHz).  Interestingly the
vendor BSP models this register as a linear divider too, so the
vendor kernel has the same mislabelling; it also removed the 800M
parent from its parent list citing GPU job faults.

I have a fix working: a small ccu type implementing the cycle-masking
semantics, with the A523 GPU clock switched over to it, the 800M
parent dropped from the selectable parents, and the OPP table for the
Orange Pi 4A.  Re-measured with the same cycle-counter method the five
OPPs now come out at 149/199/300/399/597 MHz from the intended
parents.  I'll send it as a follow-up series referencing this thread.

> And we're also missing a notifier to switch the GPU clock away from
> the PLL while the PLL is being changed.

Agreed — the series also registers the existing sunxi-ng mux notifier
on pll-gpu (cpux precedent), parking the GPU on pll-periph0-600M
during PLL rate changes.  The manual states the mux switch is
glitch-free.  Nothing retunes pll-gpu at runtime yet with the standard
OPPs, but the higher speed-bin operating points (648-792 MHz) will
need it as a live parent.

> Do you plan on implementing GPU DVFS on mainline?

Yes — see above; the series is on its way.  The speed-bin OPPs (SID
efuse gated) are left for a follow-up.  Happy to test any related
series on this board in the meantime.

Juan Manuel