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

On Wed, Jul 22, 2026 at 8:56 AM Enzo Adriano
<[email protected]> 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 <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]
> Fixes: 6702d17f54a8 ("clk: sunxi-ng: a523: add video mod clocks")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Enzo Adriano <[email protected]>
> ---
> 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