clk: sunxi-ng: ccu_mp: fix clocks without P dividers

Some sunxi-ng MP clocks have an M divider but no P divider. The A523 MBUS, IOMMU and DRAM clocks use this layout and also require the update bit when changing their rate.

ccu_mp_set_rate() unconditionally builds and applies a mask for the P field. With a zero-width P field this produces an invalid GENMASK() range and can clear bits outside a P divider, including the clock gate.

The callback also ignores CCU_FEATURE_UPDATE_BIT, so hardware that requires the update bit may not latch the new divider value.

Only update the P field when it exists, and set CCU_SUNXI_UPDATE_BIT for MP clocks carrying the feature. This matches the existing sunxi-ng div, mux and gate helper behavior.

---
Based on clk-next 8cdeaa50eae8 (Linux 7.2-rc2).
Tested with strict checkpatch and an arm64 W=1 build of ccu_mp.o. No hardware runtime claim is made.

 drivers/clk/sunxi-ng/ccu_mp.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

index 7cdb0eedc69b..aa6cb20447f1 100644
@@ -237,12 +237,17 @@ static int ccu_mp_set_rate(struct clk_hw *hw, unsigned long rate,
 
 	reg = readl(cmp->common.base + cmp->common.reg);
 	reg &= ~GENMASK(cmp->m.width + cmp->m.shift - 1, cmp->m.shift);
-	reg &= ~GENMASK(cmp->p.width + cmp->p.shift - 1, cmp->p.shift);
+	if (cmp->p.width)
+		reg &= ~GENMASK(cmp->p.width + cmp->p.shift - 1, cmp->p.shift);
+	if (cmp->common.features & CCU_FEATURE_UPDATE_BIT)
+		reg |= CCU_SUNXI_UPDATE_BIT;
 	reg |= (m - cmp->m.offset) << cmp->m.shift;
-	if (shift)
-		reg |= ilog2(p) << cmp->p.shift;
-	else
-		reg |= (p - cmp->p.offset) << cmp->p.shift;
+	if (cmp->p.width) {
+		if (shift)
+			reg |= ilog2(p) << cmp->p.shift;
+		else
+			reg |= (p - cmp->p.offset) << cmp->p.shift;
+	}
 
 	writel(reg, cmp->common.base + cmp->common.reg);
 

2.53.0

Hi,

Does that mean it's not tested on hardware?

No. If the clock only has one divider, then it should use the single divider clk class. That one also supports CCU_FEATURE_UPDATE_BIT.

ChenYu

Hi Andre, Chen-Yu,

Correct, this was not tested on hardware; it was build-tested only.

Thanks for the review. Chen-Yu's point is right: these one-divider clocks should use the single-divider clock class, which already supports CCU_FEATURE_UPDATE_BIT. I'll drop this generic ccu_mp patch; there will be no v2.

This analysis was done with AI assistance and each finding was checked against the cited sources.

Regards,
Enzo

For future patches, if you only build-tested it (which is a hard requirement), please mark the patches with "RFT" (so the subject prefix is "[PATCH RFT]") and note in the footer that it needs to be tested on actual hardware.

ChenYu

But there clocks that use a _MP_ initialiser with width = 0, both for M and P. Which leads to GENMASK(-1, 0), as you mentioned. So we need patches to fix: to convert the HSTIMER clocks to use SUNXI_CCU_P_DATA_WITH_MUX_GATE, which is a pure div clock that doesn't have that problem. Same with the r-timer clocks in ccu-sun55i-a523-r.c. And there is mbus, iommu and dram that have no P.

Are you going to make and post patches for these?

Cheers,
Andre

Andre asked whether we would post the A523/T527 conversions for the clocks whose ccu_mp descriptors declare one divider field with width zero. This series implements those conversions.

Patch 1 adds a feature-aware M-divider helper so M-only clocks can retain the CCU update-bit handling. Patch 2 converts MBUS, IOMMU, and DRAM to that M-only class and converts HSTIMER0-5 and R-TIMER0-2 to the existing P-only class.

I do not have matching A523/T527 hardware, so this is deliberately an RFT series and carries no Tested-by claim. The series has passed semantic and register-bit preservation models, KUnit tests, strict checkpatch, sparse, GCC and Clang builds, full arm64 Image/modules/DTB builds, reproducibility checks, and hostile review. It also applies and builds cleanly together with the current A523/T527 GPU clock-model v2 series.

not tested on A523/T527 hardware; testing on actual hardware is needed.

In particular, testing of clock rate changes and affected users of MBUS, IOMMU, DRAM, HSTIMER0-5, and R-TIMER0-2 would be appreciated.

---

Enzo Adriano (2):
clk: sunxi-ng: add feature-aware M divider helper
clk: sunxi-ng: a523: use single-divider clock helpers

 drivers/clk/sunxi-ng/ccu-sun55i-a523-r.c |  37 +++---
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c   | 136 +++++++++++------------
 drivers/clk/sunxi-ng/ccu_div.h           |  18 ++-
 3 files changed, 96 insertions(+), 95 deletions(-)

base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda

2.53.0

Allow single M-divider clocks to retain common CCU features such as the hardware update bit without using the two-divider MP class.

---
not tested on A523/T527 hardware; testing on actual hardware is needed.

 drivers/clk/sunxi-ng/ccu_div.h | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

index be00b3277e976f..fd222ef0b8af90 100644
@@ -212,10 +212,10 @@ struct ccu_div {
 	SUNXI_CCU_M_WITH_GATE(_struct, _name, _parent, _reg,		\
 			      _mshift, _mwidth, 0, _flags)
 
-#define SUNXI_CCU_M_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
-				       _mshift, _mwidth,		\
-				       _muxshift, _muxwidth,		\
-				       _gate, _flags)			\
+#define SUNXI_CCU_M_DATA_WITH_MUX_GATE_FEAT(_struct, _name, _parents, _reg, \
+					    _mshift, _mwidth,		\
+					    _muxshift, _muxwidth,	\
+					    _gate, _flags, _features)	\
 	struct ccu_div _struct = {					\
 		.enable	= _gate,					\
 		.div	= _SUNXI_CCU_DIV(_mshift, _mwidth),		\

@@ -226,9 +226,19 @@ struct ccu_div {
 								   _parents, \
 								   &ccu_div_ops, \
 								   _flags), \
+			.features	= _features,			\
 		},							\
 	}
 
+#define SUNXI_CCU_M_DATA_WITH_MUX_GATE(_struct, _name, _parents, _reg,	\
+				       _mshift, _mwidth,		\
+				       _muxshift, _muxwidth,		\
+				       _gate, _flags)			\
+	SUNXI_CCU_M_DATA_WITH_MUX_GATE_FEAT(_struct, _name, _parents,	\
+					    _reg, _mshift, _mwidth,	\
+					    _muxshift, _muxwidth,	\
+					    _gate, _flags, 0)
+
 #define SUNXI_CCU_M_DATA_WITH_MUX(_struct, _name, _parents, _reg,	\
 				  _mshift, _mwidth,			\
 				  _muxshift, _muxwidth,			\

2.53.0

The A523 MBUS, IOMMU, and DRAM clocks have an M divider but no P divider, while HSTIMER0-5 and R-TIMER0-2 have a P divider but no M divider. They currently use the two-divider ccu_mp class by declaring the missing field with width zero.

ccu_mp_set_rate() unconditionally masks both divider fields, so a zero width forms an invalid GENMASK() range and can clear unrelated register bits. The M-only clocks also require the hardware update bit, which ccu_mp operations do not handle.

Use the single-divider M helper for MBUS, IOMMU, and DRAM, and the power-of-two P helper for the timer clocks. Preserve the parents, registers, real divider fields, muxes, gates, clock flags, and update-bit features.

---
not tested on A523/T527 hardware; testing on actual hardware is needed.

 drivers/clk/sunxi-ng/ccu-sun55i-a523-r.c |  37 +++---
 drivers/clk/sunxi-ng/ccu-sun55i-a523.c   | 136 +++++++++++------------
 2 files changed, 82 insertions(+), 91 deletions(-)

index db0e36d8838e74..349230e28c1c3a 100644
@@ -43,27 +43,24 @@ static SUNXI_CCU_M_DATA_WITH_MUX(r_apb1_clk, "r-apb1",
 				 24, 3,	/* mux */
 				 0);
 
-static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(r_cpu_timer0, "r-timer0",
-				       r_ahb_apb_parents, 0x100,
-				       0, 0,	/* no M */
-				       1, 3,	/* P */
-				       4, 3,	/* mux */
-				       BIT(0),
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer0, "r-timer0",
+				      r_ahb_apb_parents, 0x100,
+				      1, 3,	/* P */
+				      4, 3,	/* mux */
+				      BIT(0),
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer1, "r-timer1",
+				      r_ahb_apb_parents, 0x104,
+				      1, 3,	/* P */
+				      4, 3,	/* mux */
+				      BIT(0),
+				      0);
+static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer2, "r-timer2",
+				      r_ahb_apb_parents, 0x108,
+				      1, 3,	/* P */
+				      4, 3,	/* mux */
+				      BIT(0),
 				      0);
-static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(r_cpu_timer1, "r-timer1",
-				       r_ahb_apb_parents, 0x104,
-				       0, 0,	/* no M */
-				       1, 3,	/* P */
-				       4, 3,	/* mux */
-				       BIT(0),
-				       0);
-static SUNXI_CCU_MP_DATA_WITH_MUX_GATE(r_cpu_timer2, "r-timer2",
-				       r_ahb_apb_parents, 0x108,
-				       0, 0,	/* no M */
-				       1, 3,	/* P */
-				       4, 3,	/* mux */
-				       BIT(0),
-				       0);
 
 static SUNXI_CCU_GATE_HW(bus_r_timer_clk, "bus-r-timer", &r_ahb_clk.common.hw,
 			 0x11c, BIT(0), 0);

2.53.0

Hi all,

This RFT series has been superseded by Junhui Liu's tested four-patch series:

https://lore.kernel.org/r/[email protected]

That series implements the M-only and P-only clock conversions, was tested on the Radxa Cubie A5E, and has been applied to sunxi/clk-for-7.4. Please disregard this RFT series.

Thanks to Junhui for completing and testing the fix.

I reviewed this reply and take responsibility for its contents.

Regards,
Enzo

Hi Enzo,

I just realized that you had already posted an equivalent fix before I sent mine. I have been quite busy recently and missed your series, so I ended up posting a duplicate. I am sorry about that.

I will make sure to check the mailing list more carefully before posting patches in the future.

Best regards,
Junhui Liu