[PATCH] clk: sunxi-ng: ccu_mp: fix clocks without P dividers

[PATCH] clk: sunxi-ng: ccu_mp: fix clocks without P dividers

[Enzo Adriano]

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.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/20260712081341.9D1431F00A3D@smtp.kernel.org
Fixes: 6702d17f54a8 ("clk: sunxi-ng: a523: add video mod clocks")
Assisted-by: Codex:gpt-5
Signed-off-by: Enzo Adriano <enzo.adriano.code@gmail.com>
---
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(-)

diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
index 7cdb0eedc69b..aa6cb20447f1 100644
--- a/drivers/clk/sunxi-ng/ccu_mp.c
+++ b/drivers/clk/sunxi-ng/ccu_mp.c
@@ -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

Re: [PATCH] clk: sunxi-ng: ccu_mp: fix clocks without P dividers

[Andre Przywara]

Hi,

On 7/22/26 02:56, Enzo Adriano wrote:
> 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.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/20260712081341.9D1431F00A3D@smtp.kernel.org
> Fixes: 6702d17f54a8 ("clk: sunxi-ng: a523: add video mod clocks")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Enzo Adriano <enzo.adriano.code@gmail.com>
> ---
> 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.

Does that mean it's not tested on hardware?

> 
>   drivers/clk/sunxi-ng/ccu_mp.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
> index 7cdb0eedc69b..aa6cb20447f1 100644
> --- a/drivers/clk/sunxi-ng/ccu_mp.c
> +++ b/drivers/clk/sunxi-ng/ccu_mp.c
> @@ -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) {

Can you merge that into the upper conditional branch? So that it's just 
one if statement?
And then apply the same treatment to the M divider, which is set to 0 by 
some A523 clocks (hstimer and r-timer).

Cheers,
Andre

> +		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);
>   

Re: [PATCH] clk: sunxi-ng: ccu_mp: fix clocks without P dividers

[Chen-Yu Tsai]

On Wed, Jul 22, 2026 at 8:56 AM Enzo Adriano
<enzo.adriano.code@gmail.com> wrote:
>
> 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.

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

> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/20260712081341.9D1431F00A3D@smtp.kernel.org
> Fixes: 6702d17f54a8 ("clk: sunxi-ng: a523: add video mod clocks")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Enzo Adriano <enzo.adriano.code@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
> index 7cdb0eedc69b..aa6cb20447f1 100644
> --- a/drivers/clk/sunxi-ng/ccu_mp.c
> +++ b/drivers/clk/sunxi-ng/ccu_mp.c
> @@ -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

Re: [PATCH] clk: sunxi-ng: ccu_mp: fix clocks without P dividers

[Enzo Adriano]

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

Re: [PATCH] clk: sunxi-ng: ccu_mp: fix clocks without P dividers

[Chen-Yu Tsai]

On Thu, Jul 23, 2026 at 1:37 AM Enzo Adriano
<enzo.adriano.code@gmail.com> wrote:
>
> Hi Andre, Chen-Yu,
>
> Correct, this was not tested on hardware; it was build-tested only.

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

> 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