From: Juan Manuel López Carrillo <redacted>

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 <redacted>
---
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
([ref]), 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(+)

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