[PATCH 3/5] clk: sunxi-ng: mp: support clocks with just a shift register

[Chen-Yu Tsai]

On Wed, Sep 3, 2025 at 6:21 PM Andre Przywara <andre.przywara at arm.com> wrote:
>
> On Wed, 3 Sep 2025 12:20:55 +0800
> Chen-Yu Tsai <wens at csie.org> wrote:
>
> > On Wed, Sep 3, 2025 at 8:09 AM Andre Przywara <andre.przywara at arm.com> wrote:
> > >
> > > The "mp" clock models a mod clock with divider and a shift field. At
> > > least one clock in the Allwinner A523 features just a power-of-2 divider
> > > field, so support an initialisation of the clock without providing an
> > > actual divider field.
> > >
> > > Add a check whether the "width" field is 0, and skip the divider
> > > handling in this case, as the GENMASK macro will not work with a zero
> > > length.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > ---
> >
> > In my series I have a patch that adds this to the divider clocks,
> > thus adding a P-clock type to the M-clock bits.
>
> Yeah, I saw that, but wasn't convinced this would be better. Hence wanted
> to post my version as an alternative.
>
> > Maybe use that instead? I prefer we use actual matching types instead
> > of disabling one part of a complex clock type.
>
> Good that you bring up that topic: when looking for matching clocks I saw
> we have a lot of them, though often one is just a subset of some others,
> with some code duplication. And we use the pattern of "use type A, but
> without feature X" already, for instance for "NKMP without the K".
>
> So I was wondering if we should revisit this and clean this up. IIUC those
> clocks were all modelled after the H3 and earlier generation, and the
> clocks have changed since then. For instance I don't see PLLs with two
> multipliers (NK) after the A64 anymore.
>
> So what about we consolidate the various types into just a few distinct
> ones, like NKMP for all PLLs, for instance, and provides macros that
> disable fields as needed? This could ideally be done under the hood,
> leaving the per-SoC drivers mostly alone, hopefully.
>
> What do people think about that?

I guess we could combine the NK* types, since those are relatively few.

I would like to keep the basic ones, those with just one multiplier or
one divider, possibly combined with a mux and/or a gate, alone. These
are based on core clk helpers. We only add features like fixed post
dividers or update bit support on top of them.


ChenYu

> Cheers,
> Andre
>
> > >  drivers/clk/sunxi-ng/ccu_mp.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c
> > > index 354c981943b6f..a03dac294d048 100644
> > > --- a/drivers/clk/sunxi-ng/ccu_mp.c
> > > +++ b/drivers/clk/sunxi-ng/ccu_mp.c
> > > @@ -236,9 +236,11 @@ static int ccu_mp_set_rate(struct clk_hw *hw, unsigned long rate,
> > >         spin_lock_irqsave(cmp->common.lock, flags);
> > >
> > >         reg = readl(cmp->common.base + cmp->common.reg);
> > > -       reg &= ~GENMASK(cmp->m.width + cmp->m.shift - 1, cmp->m.shift);
> > > +       if (cmp->m.width)
> > > +               reg &= ~GENMASK(cmp->m.width + cmp->m.shift - 1, cmp->m.shift);
> > >         reg &= ~GENMASK(cmp->p.width + cmp->p.shift - 1, cmp->p.shift);
> > > -       reg |= (m - cmp->m.offset) << cmp->m.shift;
> > > +       if (cmp->m.width)
> > > +               reg |= (m - cmp->m.offset) << cmp->m.shift;
> > >         if (shift)
> > >                 reg |= ilog2(p) << cmp->p.shift;
> > >         else
> > > --
> > > 2.46.3
> > >
>