[PATCH v3 03/15] clk: sunxi-ng: Add support for update bit

Andre Przywara posted 15 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v3 03/15] clk: sunxi-ng: Add support for update bit
Posted by Andre Przywara 11 months, 1 week ago
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@arm.com>
---
 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)
 
 /* 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);
-- 
2.46.3
Re: [PATCH v3 03/15] clk: sunxi-ng: Add support for update bit
Posted by Jernej Škrabec 11 months, 1 week ago
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@arm.com>
> ---
>  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.

With that fixed:
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

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);
> 
Re: [PATCH v3 03/15] clk: sunxi-ng: Add support for update bit
Posted by Andre Przywara 11 months, 1 week ago
On Tue, 04 Mar 2025 16:28:48 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> 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@arm.com>
> > ---
> >  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@gmail.com>

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