On Tue, 04 Mar 2025 16:28:48 +0100
Jernej Škrabec <jernej.skrabec@xxxxxxxxx> wrote:
Hi,
> Dne torek, 4. marec 2025 ob 02:27:53 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> > Some clocks in the Allwinner A523 SoC contain an "update bit" (bit 27),
> > which must be set to apply any register changes, namely the mux
> > selector, the divider and the gate bit.
> >
> > Add a new CCU feature bit to mark those clocks, and set bit 27 whenever
> > we are applying any changes.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> > ---
> > drivers/clk/sunxi-ng/ccu_common.h | 4 ++++
> > drivers/clk/sunxi-ng/ccu_div.c | 2 ++
> > drivers/clk/sunxi-ng/ccu_gate.c | 4 ++++
> > drivers/clk/sunxi-ng/ccu_mux.c | 2 ++
> > 4 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
> > index 50fd268329671..d41d33bdff470 100644
> > --- a/drivers/clk/sunxi-ng/ccu_common.h
> > +++ b/drivers/clk/sunxi-ng/ccu_common.h
> > @@ -20,10 +20,14 @@
> > #define CCU_FEATURE_KEY_FIELD BIT(8)
> > #define CCU_FEATURE_CLOSEST_RATE BIT(9)
> > #define CCU_FEATURE_DUAL_DIV BIT(10)
> > +#define CCU_FEATURE_UPDATE_BIT27 BIT(11)
>
> There is no reason to have "BIT27" in the name of the macro. This is similar
> to KEY_FIELD, which is generic name and doesn't specify either key or position
> of this key field. Maybe just CCU_FEATURE_UPDATE_BIT or something equaly
> generic.
Sure, done. This was mostly in anticipation of the typical Allwinner
behaviour of introducing another update bit at a different location in
the future. But I guess we use a bitmask should that happen.
> With that fixed:
> Reviewed-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
Many thanks!
Cheers,
Andre
>
> Best regards,
> Jernej
>
> >
> > /* MMC timing mode switch bit */
> > #define CCU_MMC_NEW_TIMING_MODE BIT(30)
> >
> > +/* Some clocks need this bit to actually apply register changes */
> > +#define CCU_SUNXI_UPDATE_BIT BIT(27)
> > +
> > struct device_node;
> >
> > struct ccu_common {
> > diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> > index 7f4691f09e01f..2d8b98fe4b13a 100644
> > --- a/drivers/clk/sunxi-ng/ccu_div.c
> > +++ b/drivers/clk/sunxi-ng/ccu_div.c
> > @@ -106,6 +106,8 @@ static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> >
> > reg = readl(cd->common.base + cd->common.reg);
> > reg &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd->div.shift);
> > + if (cd->common.features & CCU_FEATURE_UPDATE_BIT27)
> > + reg |= CCU_SUNXI_UPDATE_BIT;
> >
> > writel(reg | (val << cd->div.shift),
> > cd->common.base + cd->common.reg);
> > diff --git a/drivers/clk/sunxi-ng/ccu_gate.c b/drivers/clk/sunxi-ng/ccu_gate.c
> > index ac52fd6bff677..0490f95781361 100644
> > --- a/drivers/clk/sunxi-ng/ccu_gate.c
> > +++ b/drivers/clk/sunxi-ng/ccu_gate.c
> > @@ -20,6 +20,8 @@ void ccu_gate_helper_disable(struct ccu_common *common, u32 gate)
> > spin_lock_irqsave(common->lock, flags);
> >
> > reg = readl(common->base + common->reg);
> > + if (common->features & CCU_FEATURE_UPDATE_BIT27)
> > + reg |= CCU_SUNXI_UPDATE_BIT;
> > writel(reg & ~gate, common->base + common->reg);
> >
> > spin_unlock_irqrestore(common->lock, flags);
> > @@ -44,6 +46,8 @@ int ccu_gate_helper_enable(struct ccu_common *common, u32 gate)
> > spin_lock_irqsave(common->lock, flags);
> >
> > reg = readl(common->base + common->reg);
> > + if (common->features & CCU_FEATURE_UPDATE_BIT27)
> > + reg |= CCU_SUNXI_UPDATE_BIT;
> > writel(reg | gate, common->base + common->reg);
> >
> > spin_unlock_irqrestore(common->lock, flags);
> > diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
> > index d7ffbdeee9e04..82ee21e0d3a68 100644
> > --- a/drivers/clk/sunxi-ng/ccu_mux.c
> > +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> > @@ -197,6 +197,8 @@ int ccu_mux_helper_set_parent(struct ccu_common *common,
> > /* The key field always reads as zero. */
> > if (common->features & CCU_FEATURE_KEY_FIELD)
> > reg |= CCU_MUX_KEY_VALUE;
> > + if (common->features & CCU_FEATURE_UPDATE_BIT27)
> > + reg |= CCU_SUNXI_UPDATE_BIT;
> >
> > reg &= ~GENMASK(cm->width + cm->shift - 1, cm->shift);
> > writel(reg | (index << cm->shift), common->base + common->reg);
> >
>
>
>
>
>
